From 2f2930bbf6fd76dc8aaf19ca84f425c69126c9fc Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Mon, 7 Apr 2014 15:21:06 -0700 Subject: [PATCH] Bug 993141 - Fix bug where we were assuming DataChannel::mStream corresponded to the level. r=jib --- .../src/peerconnection/PeerConnectionImpl.cpp | 61 ++++++------------- .../src/peerconnection/PeerConnectionMedia.h | 4 ++ 2 files changed, 22 insertions(+), 43 deletions(-) diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index aec455f4a70..4eac0fdb12c 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -1993,57 +1993,32 @@ PeerConnectionImpl::BuildStatsQuery_m( // From the list of MediaPipelines, determine the set of NrIceMediaStreams // we are interested in. - std::set streamsGrabbed; - for (size_t p = 0; p < query->pipelines.Length(); ++p) { - - size_t level = query->pipelines[p]->level(); - MOZ_ASSERT(level); - - // Don't grab the same stream twice, since that causes duplication - // of the ICE stats. - if (streamsGrabbed.count(level)) { - continue; + std::set 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 + } + } - streamsGrabbed.insert(level); + for (auto s = levelsToGrab.begin(); s != levelsToGrab.end(); ++s) { // TODO(bcampen@mozilla.com): I may need to revisit this for bundle. // (Bug 786234) - RefPtr temp(mMedia->ice_media_stream(level - 1)); - if (temp) { + RefPtr temp(mMedia->ice_media_stream(*s - 1)); + RefPtr flow(mMedia->GetTransportFlow(*s, false)); + // flow can be null for unused levels, such as unused DataChannels + if (temp && flow) { query->streams.AppendElement(temp); - } else { - CSFLogError(logTag, "Failed to get NrIceMediaStream for level %zu " - "in %s: %s", - static_cast(level), - __FUNCTION__, - mHandle.c_str()); - MOZ_CRASH(); } } - // If the selector is null, we want to get ICE stats for the DataChannel - if (!aSelector && mDataConnection) { - std::vector streamIds; - mDataConnection->GetStreamIds(&streamIds); - - for (auto s = streamIds.begin(); s!= streamIds.end(); ++s) { - MOZ_ASSERT(*s); - - if (streamsGrabbed.count(*s) || *s == INVALID_STREAM) { - continue; - } - - streamsGrabbed.insert(*s); - - RefPtr temp(mMedia->ice_media_stream(*s - 1)); - - // This will be null if DataChannel is not in use - RefPtr flow(mMedia->GetTransportFlow(*s, false)); - if (temp && flow) { - query->streams.AppendElement(temp); - } - } - } return rv; } diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h index fbbff9977be..5a50c2525ec 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h @@ -279,6 +279,10 @@ class PeerConnectionMedia : public sigslot::has_slots<> { return mIceStreams[i]; } + size_t num_ice_media_streams() const { + return mIceStreams.size(); + } + // Add a stream nsresult AddStream(nsIDOMMediaStream* aMediaStream, uint32_t *stream_id);