Bug 1140089 - Call SetPullEnabled on all streams in PCMedia when offer/answer concludes. r=jesup, a=sledru
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 06 Mar 2015 14:37:11 -0800
changeset 257681 6129d46fedce5ad6488ae104fa25f26e5c7aba42
parent 257680 fcc4b9a7a8c1240a4007ee60591f4c366a88de32
child 257682 f3b4c80c999e3f83aaf01d43877d21af4798cdc0
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup, sledru
bugs1140089
milestone38.0a2
Bug 1140089 - Call SetPullEnabled on all streams in PCMedia when offer/answer concludes. r=jesup, a=sledru
media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
--- a/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
@@ -422,17 +422,17 @@ MediaPipelineFactory::CreateMediaPipelin
   nsRefPtr<RemoteSourceStreamInfo> stream =
       mPCMedia->GetRemoteStreamById(aTrack.GetStreamId());
 
   RefPtr<MediaPipelineReceive> pipeline;
 
   TrackID numericTrackId = stream->GetNumericTrackId(aTrack.GetTrackId());
   MOZ_ASSERT(numericTrackId != TRACK_INVALID);
 
-  bool queue_track = stream->QueueTracks();
+  bool queue_track = stream->ShouldQueueTracks();
 
   MOZ_MTLOG(ML_DEBUG, __FUNCTION__ << ": Creating pipeline for "
             << numericTrackId << " -> " << aTrack.GetTrackId());
 
   if (aTrack.GetMediaType() == SdpMediaSection::kAudio) {
     pipeline = new MediaPipelineReceiveAudio(
         mPC->GetHandle(),
         mPC->GetMainThread().get(),
@@ -477,19 +477,16 @@ MediaPipelineFactory::CreateMediaPipelin
   if (NS_FAILED(rv)) {
     MOZ_MTLOG(ML_ERROR, "Couldn't store receiving pipeline " <<
                         static_cast<unsigned>(rv));
     return rv;
   }
 
   stream->SyncPipeline(pipeline);
 
-  if (queue_track) {
-    stream->TrackQueued(aTrack.GetTrackId());
-  }
   return NS_OK;
 }
 
 nsresult
 MediaPipelineFactory::CreateMediaPipelineSending(
     const JsepTrackPair& aTrackPair,
     const JsepTrack& aTrack,
     size_t aLevel,
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -349,16 +349,20 @@ nsresult PeerConnectionMedia::UpdateMedi
     if (pair.mSending) {
       rv = factory.CreateOrUpdateMediaPipeline(pair, *pair.mSending);
       if (NS_FAILED(rv)) {
         return rv;
       }
     }
   }
 
+  for (auto& stream : mRemoteSourceStreams) {
+    stream->StartReceiving();
+  }
+
   return NS_OK;
 }
 
 void
 PeerConnectionMedia::StartIceChecks(const JsepSession& session) {
 
   std::vector<size_t> numComponentsByLevel;
   auto transports = session.GetTransports();
@@ -1106,43 +1110,34 @@ RemoteSourceStreamInfo::SyncPipeline(
       CSFLogDebug(logTag, "Syncing %p to %p, %s to %s",
                           video_conduit, audio_conduit,
                           i->first.c_str(), aPipeline->trackid().c_str());
     }
   }
 }
 
 void
-RemoteSourceStreamInfo::TrackQueued(const std::string& trackId)
+RemoteSourceStreamInfo::StartReceiving()
 {
-  // When tracks start being queued we know the pipelines have been created.
-  mPipelinesCreated = true;
+  if (mReceiving || mPipelines.empty()) {
+    return;
+  }
 
-  MOZ_ASSERT(mTracksToQueue.count(trackId) > 0);
-  mTracksToQueue.erase(trackId);
-
-  CSFLogDebug(logTag, "Queued adding of track id %d to MediaStream %p. "
-                      "%zu more tracks to queue.",
-                      GetNumericTrackId(trackId),
-                      GetMediaStream()->GetStream(),
-                      mTracksToQueue.size());
+  mReceiving = true;
 
-  // If all tracks have been queued for this stream, finish adding them.
-  if (mTracksToQueue.empty()) {
-    SourceMediaStream* source = GetMediaStream()->GetStream()->AsSourceStream();
-    source->FinishAddTracks();
-    source->SetPullEnabled(true);
-    // AdvanceKnownTracksTicksTime(HEAT_DEATH_OF_UNIVERSE) means that in
-    // theory per the API, we can't add more tracks before that
-    // time. However, the impl actually allows it, and it avoids a whole
-    // bunch of locking that would be required (and potential blocking)
-    // if we used smaller values and updated them on each NotifyPull.
-    source->AdvanceKnownTracksTime(STREAM_TIME_MAX);
-    CSFLogDebug(logTag, "Finished adding tracks to MediaStream %p", source);
-  }
+  SourceMediaStream* source = GetMediaStream()->GetStream()->AsSourceStream();
+  source->FinishAddTracks();
+  source->SetPullEnabled(true);
+  // AdvanceKnownTracksTicksTime(HEAT_DEATH_OF_UNIVERSE) means that in
+  // theory per the API, we can't add more tracks before that
+  // time. However, the impl actually allows it, and it avoids a whole
+  // bunch of locking that would be required (and potential blocking)
+  // if we used smaller values and updated them on each NotifyPull.
+  source->AdvanceKnownTracksTime(STREAM_TIME_MAX);
+  CSFLogDebug(logTag, "Finished adding tracks to MediaStream %p", source);
 }
 
 RefPtr<MediaPipeline> SourceStreamInfo::GetPipelineByTrackId_m(
     const std::string& trackId) {
   ASSERT_ON_THREAD(mParent->GetMainThread());
 
   // Refuse to hand out references if we're tearing down.
   // (Since teardown involves a dispatch to and from STS before MediaPipelines
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
@@ -143,36 +143,31 @@ public:
 
 class RemoteSourceStreamInfo : public SourceStreamInfo {
   ~RemoteSourceStreamInfo() {}
  public:
   RemoteSourceStreamInfo(already_AddRefed<DOMMediaStream> aMediaStream,
                          PeerConnectionMedia *aParent,
                          const std::string& aId)
     : SourceStreamInfo(aMediaStream, aParent, aId),
-      mPipelinesCreated(false)
+      mReceiving(false)
   {
   }
 
   void SyncPipeline(RefPtr<MediaPipelineReceive> aPipeline);
 
 #ifdef MOZILLA_INTERNAL_API
   void UpdatePrincipal_m(nsIPrincipal* aPrincipal);
 #endif
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(RemoteSourceStreamInfo)
 
   virtual void AddTrack(const std::string& track) MOZ_OVERRIDE
   {
     mTrackIdMap.push_back(track);
-    MOZ_ASSERT(!mPipelinesCreated || mTracksToQueue.empty(),
-               "Track added while waiting for existing tracks to be queued.");
-    if (!mPipelinesCreated) {
-      mTracksToQueue.insert(track);
-    }
     SourceStreamInfo::AddTrack(track);
   }
 
   TrackID GetNumericTrackId(const std::string& trackId) const
   {
     for (size_t i = 0; i < mTrackIdMap.size(); ++i) {
       if (mTrackIdMap[i] == trackId) {
         return static_cast<TrackID>(i + 1);
@@ -187,44 +182,40 @@ class RemoteSourceStreamInfo : public So
         static_cast<size_t>(numericTrackId) > mTrackIdMap.size()) {
       return NS_ERROR_INVALID_ARG;;
     }
 
     *trackId = mTrackIdMap[numericTrackId - 1];
     return NS_OK;
   }
 
+  void StartReceiving();
+
   /**
    * Returns true if a |MediaPipeline| should be queueing its track instead of
    * adding it to the |SourceMediaStream| directly.
    */
-  bool QueueTracks() const
+  bool ShouldQueueTracks() const
   {
-    return !mPipelinesCreated || !mTracksToQueue.empty();
+    return !mReceiving;
   }
 
-  void TrackQueued(const std::string& trackId);
-
  private:
   // For remote streams, the MediaStreamGraph API forces us to select a
   // numeric track id before creation of the MediaStreamTrack, and does not
   // allow us to specify a string-based id until later. We cannot simply use
   // something based on mline index, since renegotiation can move tracks
   // around. Hopefully someday we'll be able to specify the string id up-front,
   // and have the numeric track id selected for us, in which case this variable
   // and its dependencies can go away.
   std::vector<std::string> mTrackIdMap;
 
-  // When a remote stream gets created we need to add its initial set of tracks
-  // atomically. Here we track which tracks we have created Pipelines for and
-  // that will be queued later on.
-  std::set<std::string> mTracksToQueue;
-
-  // True if we have finished creating the initial set of pipelines
-  bool mPipelinesCreated;
+  // True iff SetPullEnabled(true) has been called on the DOMMediaStream. This
+  // happens when offer/answer concludes.
+  bool mReceiving;
 };
 
 class PeerConnectionMedia : public sigslot::has_slots<> {
   ~PeerConnectionMedia() {}
 
  public:
   explicit PeerConnectionMedia(PeerConnectionImpl *parent);