241 lines
8.9 KiB
Diff
241 lines
8.9 KiB
Diff
|
From 5f61ae17ec82d288a3fe4892ec999c0e20c486c0 Mon Sep 17 00:00:00 2001
|
||
|
From: "Byron Campen [:bwc]" <docfaraday@gmail.com>
|
||
|
Date: Mon, 6 Apr 2015 11:52:28 -0700
|
||
|
Subject: [PATCH] Bug 1151139 - Simplify how we choose which streams to gather
|
||
|
stats from. r=mt, a=abillings
|
||
|
|
||
|
---
|
||
|
...t_peerConnection_offerRequiresReceiveAudio.html | 2 +
|
||
|
...t_peerConnection_offerRequiresReceiveVideo.html | 2 +
|
||
|
...rConnection_offerRequiresReceiveVideoAudio.html | 2 +
|
||
|
media/mtransport/nricectx.h | 13 +++++
|
||
|
media/mtransport/nricemediastream.cpp | 1 +
|
||
|
media/mtransport/nricemediastream.h | 5 +-
|
||
|
.../src/peerconnection/PeerConnectionImpl.cpp | 66 ++++++++++------------
|
||
|
.../src/peerconnection/PeerConnectionImpl.h | 2 +-
|
||
|
8 files changed, 54 insertions(+), 39 deletions(-)
|
||
|
|
||
|
diff --git a/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveAudio.html b/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveAudio.html
|
||
|
index 69d7e49..d68c078 100644
|
||
|
--- a/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveAudio.html
|
||
|
+++ b/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveAudio.html
|
||
|
@@ -17,6 +17,8 @@
|
||
|
|
||
|
runTest(function() {
|
||
|
var test = new PeerConnectionTest();
|
||
|
+ test.chain.remove('PC_LOCAL_CHECK_STATS');
|
||
|
+ test.chain.remove('PC_REMOTE_CHECK_STATS');
|
||
|
test.setOfferConstraints({ mandatory: { OfferToReceiveAudio: true } });
|
||
|
test.run();
|
||
|
});
|
||
|
diff --git a/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideo.html b/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideo.html
|
||
|
index 5f1d0e5..0ecb0b7 100644
|
||
|
--- a/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideo.html
|
||
|
+++ b/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideo.html
|
||
|
@@ -17,6 +17,8 @@
|
||
|
|
||
|
runTest(function() {
|
||
|
var test = new PeerConnectionTest();
|
||
|
+ test.chain.remove('PC_LOCAL_CHECK_STATS');
|
||
|
+ test.chain.remove('PC_REMOTE_CHECK_STATS');
|
||
|
test.setOfferConstraints({ mandatory: { OfferToReceiveVideo: true } });
|
||
|
test.run();
|
||
|
});
|
||
|
diff --git a/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideoAudio.html b/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideoAudio.html
|
||
|
index c3dea10..78eb0d4 100644
|
||
|
--- a/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideoAudio.html
|
||
|
+++ b/dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideoAudio.html
|
||
|
@@ -17,6 +17,8 @@
|
||
|
|
||
|
runTest(function() {
|
||
|
var test = new PeerConnectionTest();
|
||
|
+ test.chain.remove('PC_LOCAL_CHECK_STATS');
|
||
|
+ test.chain.remove('PC_REMOTE_CHECK_STATS');
|
||
|
test.setOfferConstraints({ mandatory: {
|
||
|
OfferToReceiveVideo: true,
|
||
|
OfferToReceiveAudio: true
|
||
|
diff --git a/media/mtransport/nricectx.h b/media/mtransport/nricectx.h
|
||
|
index d1209a7..7350666 100644
|
||
|
--- a/media/mtransport/nricectx.h
|
||
|
+++ b/media/mtransport/nricectx.h
|
||
|
@@ -196,6 +196,19 @@ class NrIceCtx {
|
||
|
RefPtr<NrIceMediaStream> CreateStream(const std::string& name,
|
||
|
int components);
|
||
|
|
||
|
+ RefPtr<NrIceMediaStream> GetStream(size_t index) {
|
||
|
+ if (index < streams_.size()) {
|
||
|
+ return streams_[index];
|
||
|
+ }
|
||
|
+ return nullptr;
|
||
|
+ }
|
||
|
+
|
||
|
+ // Some might be null
|
||
|
+ size_t GetStreamCount() const
|
||
|
+ {
|
||
|
+ return streams_.size();
|
||
|
+ }
|
||
|
+
|
||
|
// The name of the ctx
|
||
|
const std::string& name() const { return name_; }
|
||
|
|
||
|
diff --git a/media/mtransport/nricemediastream.cpp b/media/mtransport/nricemediastream.cpp
|
||
|
index 9e96cb5..d2b6429 100644
|
||
|
--- a/media/mtransport/nricemediastream.cpp
|
||
|
+++ b/media/mtransport/nricemediastream.cpp
|
||
|
@@ -209,6 +209,7 @@ nsresult NrIceMediaStream::ParseAttributes(std::vector<std::string>&
|
||
|
return NS_ERROR_FAILURE;
|
||
|
}
|
||
|
|
||
|
+ has_parsed_attrs_ = true;
|
||
|
return NS_OK;
|
||
|
}
|
||
|
|
||
|
diff --git a/media/mtransport/nricemediastream.h b/media/mtransport/nricemediastream.h
|
||
|
index aba5fc3..2494ecf 100644
|
||
|
--- a/media/mtransport/nricemediastream.h
|
||
|
+++ b/media/mtransport/nricemediastream.h
|
||
|
@@ -149,6 +149,7 @@ class NrIceMediaStream {
|
||
|
|
||
|
// Parse remote attributes
|
||
|
nsresult ParseAttributes(std::vector<std::string>& candidates);
|
||
|
+ bool HasParsedAttributes() const { return has_parsed_attrs_; }
|
||
|
|
||
|
// Parse trickle ICE candidate
|
||
|
nsresult ParseTrickleCandidate(const std::string& candidate);
|
||
|
@@ -204,7 +205,8 @@ class NrIceMediaStream {
|
||
|
name_(name),
|
||
|
components_(components),
|
||
|
stream_(nullptr),
|
||
|
- opaque_(nullptr) {}
|
||
|
+ opaque_(nullptr),
|
||
|
+ has_parsed_attrs_(false) {}
|
||
|
|
||
|
DISALLOW_COPY_ASSIGN(NrIceMediaStream);
|
||
|
|
||
|
@@ -214,6 +216,7 @@ class NrIceMediaStream {
|
||
|
const int components_;
|
||
|
nr_ice_media_stream *stream_;
|
||
|
ScopedDeletePtr<NrIceOpaque> opaque_;
|
||
|
+ bool has_parsed_attrs_;
|
||
|
};
|
||
|
|
||
|
|
||
|
diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
|
||
|
index ebcc17d..c70e3e4 100644
|
||
|
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
|
||
|
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
|
||
|
@@ -149,7 +149,8 @@ PRLogModuleInfo *signalingLogInfo() {
|
||
|
namespace sipcc {
|
||
|
|
||
|
#ifdef MOZILLA_INTERNAL_API
|
||
|
-RTCStatsQuery::RTCStatsQuery(bool internal) : internalStats(internal) {
|
||
|
+RTCStatsQuery::RTCStatsQuery(bool internal) : internalStats(internal),
|
||
|
+ grabAllLevels(false) {
|
||
|
}
|
||
|
|
||
|
RTCStatsQuery::~RTCStatsQuery() {
|
||
|
@@ -2037,32 +2038,8 @@ PeerConnectionImpl::BuildStatsQuery_m(
|
||
|
|
||
|
query->iceCtx = mMedia->ice_ctx();
|
||
|
|
||
|
- // From the list of MediaPipelines, determine the set of NrIceMediaStreams
|
||
|
- // we are interested in.
|
||
|
- std::set<size_t> levelsToGrab;
|
||
|
- if (trackId) {
|
||
|
- for (size_t p = 0; p < query->pipelines.Length(); ++p) {
|
||
|
- size_t level = query->pipelines[p]->level();
|
||
|
- MOZ_ASSERT(level);
|
||
|
- levelsToGrab.insert(level);
|
||
|
- }
|
||
|
- } else {
|
||
|
- // We want to grab all streams, so ignore the pipelines (this also ends up
|
||
|
- // grabbing DataChannel streams, which is what we want)
|
||
|
- for (size_t s = 0; s < mMedia->num_ice_media_streams(); ++s) {
|
||
|
- levelsToGrab.insert(s + 1); // mIceStreams is 0-indexed
|
||
|
- }
|
||
|
- }
|
||
|
-
|
||
|
- for (auto s = levelsToGrab.begin(); s != levelsToGrab.end(); ++s) {
|
||
|
- // TODO(bcampen@mozilla.com): I may need to revisit this for bundle.
|
||
|
- // (Bug 786234)
|
||
|
- RefPtr<NrIceMediaStream> temp(mMedia->ice_media_stream(*s - 1));
|
||
|
- RefPtr<TransportFlow> flow(mMedia->GetTransportFlow(*s, false));
|
||
|
- // flow can be null for unused levels, such as unused DataChannels
|
||
|
- if (temp && flow) {
|
||
|
- query->streams.AppendElement(temp);
|
||
|
- }
|
||
|
+ if (!trackId) {
|
||
|
+ query->grabAllLevels = true;
|
||
|
}
|
||
|
|
||
|
return rv;
|
||
|
@@ -2103,6 +2080,9 @@ static void RecordIceStats_s(
|
||
|
bool internalStats,
|
||
|
DOMHighResTimeStamp now,
|
||
|
RTCStatsReportInternal* report) {
|
||
|
+ if (!mediaStream.HasParsedAttributes()) {
|
||
|
+ return;
|
||
|
+ }
|
||
|
|
||
|
NS_ConvertASCIItoUTF16 componentId(mediaStream.name().c_str());
|
||
|
if (internalStats) {
|
||
|
@@ -2292,20 +2272,32 @@ PeerConnectionImpl::ExecuteStatsQuery_s(RTCStatsQuery *query) {
|
||
|
break;
|
||
|
}
|
||
|
}
|
||
|
+
|
||
|
+ if (!query->grabAllLevels) {
|
||
|
+ // If we're grabbing all levels, that means we want datachannels too,
|
||
|
+ // which don't have pipelines.
|
||
|
+ if (query->iceCtx->GetStream(p - 1)) {
|
||
|
+ RecordIceStats_s(*query->iceCtx->GetStream(p - 1),
|
||
|
+ query->internalStats,
|
||
|
+ query->now,
|
||
|
+ &(query->report));
|
||
|
+ }
|
||
|
+ }
|
||
|
}
|
||
|
|
||
|
- // Gather stats from ICE
|
||
|
- for (size_t s = 0; s != query->streams.Length(); ++s) {
|
||
|
- RecordIceStats_s(*query->streams[s],
|
||
|
- query->internalStats,
|
||
|
- query->now,
|
||
|
- &(query->report));
|
||
|
+ if (query->grabAllLevels) {
|
||
|
+ for (size_t i = 0; i < query->iceCtx->GetStreamCount(); ++i) {
|
||
|
+ if (query->iceCtx->GetStream(i)) {
|
||
|
+ RecordIceStats_s(*query->iceCtx->GetStream(i),
|
||
|
+ query->internalStats,
|
||
|
+ query->now,
|
||
|
+ &(query->report));
|
||
|
+ }
|
||
|
+ }
|
||
|
}
|
||
|
|
||
|
- // NrIceCtx and NrIceMediaStream must be destroyed on STS, so it is not safe
|
||
|
- // to dispatch them back to main.
|
||
|
- // We clear streams first to maintain destruction order
|
||
|
- query->streams.Clear();
|
||
|
+ // NrIceCtx must be destroyed on STS, so it is not safe
|
||
|
+ // to dispatch it back to main.
|
||
|
query->iceCtx = nullptr;
|
||
|
return NS_OK;
|
||
|
}
|
||
|
diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
|
||
|
index 847085c..497230a 100644
|
||
|
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
|
||
|
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
|
||
|
@@ -174,7 +174,7 @@ class RTCStatsQuery {
|
||
|
bool internalStats;
|
||
|
nsTArray<mozilla::RefPtr<mozilla::MediaPipeline>> pipelines;
|
||
|
mozilla::RefPtr<NrIceCtx> iceCtx;
|
||
|
- nsTArray<mozilla::RefPtr<NrIceMediaStream>> streams;
|
||
|
+ bool grabAllLevels;
|
||
|
DOMHighResTimeStamp now;
|
||
|
};
|
||
|
#endif // MOZILLA_INTERNAL_API
|
||
|
--
|
||
|
2.2.1
|
||
|
|