Bug 1151139 - Simplify how we choose which streams to gather stats from. r=mt, a=abillings
authorByron Campen [:bwc] <docfaraday@gmail.com>
Mon, 06 Apr 2015 11:52:28 -0700
changeset 258411 e62ca3da49e1
parent 258410 c821f76bf302
child 258412 d4ee3499fe0d
push id4661
push userryanvm@gmail.com
push date2015-04-09 18:39 +0000
treeherdermozilla-beta@3f5e298cb641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt, abillings
bugs1151139
milestone38.0
Bug 1151139 - Simplify how we choose which streams to gather stats from. r=mt, a=abillings
media/mtransport/nricectx.h
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
--- a/media/mtransport/nricectx.h
+++ b/media/mtransport/nricectx.h
@@ -234,16 +234,22 @@ class NrIceCtx {
     return nullptr;
   }
 
   void RemoveStream(size_t index)
   {
     streams_[index] = nullptr;
   }
 
+  // Some might be null
+  size_t GetStreamCount() const
+  {
+    return streams_.size();
+  }
+
   // The name of the ctx
   const std::string& name() const { return name_; }
 
   // Get ufrag and password.
   std::string ufrag() const;
   std::string pwd() const;
 
   // Current state
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -318,17 +318,18 @@ struct nsISupportsWeakReference::COMType
 };
 const nsIID nsISupportsWeakReference::COMTypeInfo<nsSupportsWeakReference, void>::kIID = NS_ISUPPORTSWEAKREFERENCE_IID;
 
 namespace mozilla {
 
 #ifdef MOZILLA_INTERNAL_API
 RTCStatsQuery::RTCStatsQuery(bool internal) :
   failed(false),
-  internalStats(internal) {
+  internalStats(internal),
+  grabAllLevels(false) {
 }
 
 RTCStatsQuery::~RTCStatsQuery() {
   MOZ_ASSERT(NS_IsMainThread());
 }
 
 #endif
 
@@ -2877,41 +2878,18 @@ PeerConnectionImpl::BuildStatsQuery_m(
       }
     } else {
       for (auto it = pipelines.begin(); it != pipelines.end(); ++it) {
         query->pipelines.AppendElement(it->second);
       }
     }
   }
 
-  // From the list of MediaPipelines, determine the set of NrIceMediaStreams
-  // we are interested in.
-  std::set<size_t> levelsToGrab;
-  if (aSelector) {
-    for (size_t p = 0; p < query->pipelines.Length(); ++p) {
-      size_t level = query->pipelines[p]->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);
-    }
-  }
-
-  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));
-    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 (!aSelector) {
+    query->grabAllLevels = true;
   }
 
   return rv;
 }
 
 static void ToRTCIceCandidateStats(
     const std::vector<NrIceCandidate>& candidates,
     RTCStatsType candidateType,
@@ -3176,30 +3154,42 @@ PeerConnectionImpl::ExecuteStatsQuery_s(
             s.mBitrateStdDev.Construct(bitrateStdDev);
             s.mDiscardedPackets.Construct(discardedPackets);
           }
         }
         query->report->mInboundRTPStreamStats.Value().AppendElement(s);
         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)) {
+        RecordIceStats_s(*query->iceCtx->GetStream(p),
+                         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;
 }
 
 void PeerConnectionImpl::GetStatsForPCObserver_s(
     const std::string& pcHandle, // The Runnable holds the memory
     nsAutoPtr<RTCStatsQuery> query) {
 
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -201,17 +201,17 @@ class RTCStatsQuery {
     bool failed;
 
   private:
     friend class PeerConnectionImpl;
     std::string pcName;
     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
 
 // Enter an API call and check that the state is OK,
 // the PC isn't closed, etc.
 #define PC_AUTO_ENTER_API_CALL(assert_ice_ready) \
     do { \