Bug 1408294 - Simplify MSG integration code for MediaPipelineReceive. r?bwc draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Thu, 21 Dec 2017 15:55:26 +0100
changeset 717222 e7f948cad029143d41e6ba5c003184afe978f95b
parent 717221 7d92e3c70daca5584a5ef5a4ed45f982841097ca
child 717223 13704ac1aa486a23551b28b75ec107d5f238321d
push id94604
push userbmo:apehrson@mozilla.com
push dateMon, 08 Jan 2018 14:02:37 +0000
reviewersbwc
bugs1408294
milestone59.0a1
Bug 1408294 - Simplify MSG integration code for MediaPipelineReceive. r?bwc Most importantly this avoids having a SourceMediaStream *with a track* but *without any listeners*. I'm adding asserts to ensure that all NotifyPull()s append enough data to all live tracks. MozReview-Commit-ID: InGj3n0f0y3
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp
media/webrtc/signaling/src/peerconnection/TransceiverImpl.h
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -1995,38 +1995,57 @@ public:
     , mTrackId(aTrack->GetInputTrackId())
     , mSource(mTrack->GetInputStream()->AsSourceStream())
     , mPlayedTicks(0)
     , mPrincipalHandle(PRINCIPAL_HANDLE_NONE)
     , mListening(false)
     , mMaybeTrackNeedsUnmute(true)
   {
     MOZ_RELEASE_ASSERT(mSource, "Must be used with a SourceMediaStream");
+
+    if (mTrack->AsAudioStreamTrack()) {
+      mSource->AddAudioTrack(
+          mTrackId, mSource->GraphRate(), 0, new AudioSegment());
+    } else if (mTrack->AsVideoStreamTrack()) {
+      mSource->AddTrack(mTrackId, 0, new VideoSegment());
+    } else {
+      MOZ_ASSERT_UNREACHABLE("Unknown track type");
+    }
+    CSFLogDebug(
+      LOGTAG,
+      "GenericReceiveListener added %s track %d (%p) to stream %p",
+      mTrack->AsAudioStreamTrack() ? "audio" : "video",
+      mTrackId,
+      mTrack.get(),
+      mSource.get());
+
+    mSource->AdvanceKnownTracksTime(STREAM_TIME_MAX);
+    mSource->AddListener(this);
   }
 
   virtual ~GenericReceiveListener()
   {
     NS_ReleaseOnMainThreadSystemGroup(
       "GenericReceiveListener::track_", mTrack.forget());
   }
 
   void AddSelf()
   {
     if (!mListening) {
       mListening = true;
-      mSource->AddListener(this);
+      mSource->SetPullEnabled(true);
       mMaybeTrackNeedsUnmute = true;
     }
   }
 
   void RemoveSelf()
   {
     if (mListening) {
       mListening = false;
-      mSource->RemoveListener(this);
+      mSource->SetPullEnabled(false);
     }
   }
 
   void OnRtpReceived()
   {
     if (mMaybeTrackNeedsUnmute) {
       mMaybeTrackNeedsUnmute = false;
       NS_DispatchToMainThread(
@@ -2042,35 +2061,20 @@ public:
       mTrack->MutedChanged(false);
     }
   }
 
   void EndTrack()
   {
     CSFLogDebug(LOGTAG, "GenericReceiveListener ending track");
 
-    // We do this on MSG to avoid it racing against StartTrack.
-    class Message : public ControlMessage
-    {
-    public:
-      explicit Message(SourceMediaStream* aStream, TrackID aTrackId)
-        : ControlMessage(aStream)
-        , mTrackId(aTrackId)
-      {
-      }
 
-      void Run() override { mStream->AsSourceStream()->EndTrack(mTrackId); }
-
-
-      const TrackID mTrackId;
-    };
-
-    mTrack->GraphImpl()->AppendMessage(MakeUnique<Message>(mSource, mTrackId));
     // This breaks the cycle with the SourceMediaStream
     mSource->RemoveListener(this);
+    mSource->EndTrack(mTrackId);
   }
 
   // Must be called on the main thread
   void SetPrincipalHandle_m(const PrincipalHandle& aPrincipalHandle)
   {
     class Message : public ControlMessage
     {
     public:
@@ -2357,17 +2361,17 @@ public:
     // Don't append if we've already provided a frame that supposedly
     // goes past the current aDesiredTime Doing so means a negative
     // delta and thus messes up handling of the graph
     if (delta > 0) {
       VideoSegment segment;
       IntSize size = image ? image->GetSize() : IntSize(mWidth, mHeight);
       segment.AppendFrame(image.forget(), delta, size, mPrincipalHandle);
       // Handle track not actually added yet or removed/finished
-      if (!mSource->AppendToTrack(mTrack->GetInputTrackId(), &segment)) {
+      if (!mSource->AppendToTrack(mTrackId, &segment)) {
         CSFLogError(LOGTAG, "AppendToTrack failed");
         return;
       }
       mPlayedTicks = aDesiredTime;
     }
   }
 
   // Accessors for external writes from the renderer
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -28,17 +28,16 @@
 #include "nsISocketTransportService.h"
 #include "nsIConsoleService.h"
 #include "nsThreadUtils.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsProxyRelease.h"
 #include "nsQueryObject.h"
 #include "prtime.h"
-#include "MediaEngine.h"
 
 #include "AudioConduit.h"
 #include "VideoConduit.h"
 #include "runnable_utils.h"
 #include "PeerConnectionCtx.h"
 #include "PeerConnectionImpl.h"
 #include "PeerConnectionMedia.h"
 #include "RemoteTrackSource.h"
@@ -2400,23 +2399,23 @@ PeerConnectionImpl::CreateReceiveTrack(S
     // we're either certain that we need isolation for the streams, OR
     // we're not sure and we can fix the stream in SetDtlsConnected
     principal =  NullPrincipal::CreateWithInheritedAttributes(doc->NodePrincipal());
   }
 
   RefPtr<MediaStreamTrack> track;
   if (audio) {
     track = stream->CreateDOMTrack(
-        kAudioTrack,
+        333, // Use a constant TrackID. Dependents read this from the DOM track.
         MediaSegment::AUDIO,
         new RemoteTrackSource(principal,
                               NS_ConvertASCIItoUTF16("remote audio")));
   } else {
     track = stream->CreateDOMTrack(
-        kVideoTrack,
+        666, // Use a constant TrackID. Dependents read this from the DOM track.
         MediaSegment::VIDEO,
         new RemoteTrackSource(principal,
                               NS_ConvertASCIItoUTF16("remote video")));
   }
 
   stream->AddTrackInternal(track);
   // Spec says remote tracks start out muted.
   track->MutedChanged(true);
--- a/media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp
@@ -57,18 +57,16 @@ TransceiverImpl::TransceiverImpl(
   }
 
   if (!IsValid()) {
     return;
   }
 
   mConduit->SetPCHandle(mPCHandle);
 
-  StartReceiveStream();
-
   mTransmitPipeline = new MediaPipelineTransmit(
       mPCHandle,
       mMainThread.get(),
       mStsThread.get(),
       IsVideo(),
       mSendTrack,
       mConduit);
 }
@@ -1007,83 +1005,16 @@ TransceiverImpl::UpdateVideoExtmap(const
   RefPtr<VideoSessionConduit> conduit = static_cast<VideoSessionConduit*>(
       mConduit.get());
 
   if (!extmaps.empty()) {
     conduit->SetLocalRTPExtensions(aSending, extmaps);
   }
 }
 
-static void StartTrack(MediaStream* aSource,
-                       nsAutoPtr<MediaSegment>&& aSegment)
-{
-  class Message : public ControlMessage {
-   public:
-    Message(MediaStream* aStream, nsAutoPtr<MediaSegment>&& aSegment)
-      : ControlMessage(aStream),
-        segment_(aSegment) {}
-
-    void Run() override {
-      TrackRate track_rate = mStream->GraphRate();
-      StreamTime current_end = mStream->GetTracksEnd();
-      MOZ_MTLOG(ML_DEBUG, "current_end = " << current_end);
-      TrackTicks current_ticks =
-        mStream->TimeToTicksRoundUp(track_rate, current_end);
-
-      // Add a track 'now' to avoid possible underrun, especially if we add
-      // a track "later".
-
-      if (current_end != 0L) {
-        MOZ_MTLOG(ML_DEBUG, "added track @ " << current_end << " -> "
-                  << mStream->StreamTimeToSeconds(current_end));
-      }
-
-      // To avoid assertions, we need to insert a dummy segment that covers up
-      // to the "start" time for the track
-      segment_->AppendNullData(current_ticks);
-      MOZ_MTLOG(ML_DEBUG, "segment_->GetDuration() = " << segment_->GetDuration());
-      if (segment_->GetType() == MediaSegment::AUDIO) {
-        MOZ_MTLOG(ML_DEBUG, "Calling AddAudioTrack");
-        mStream->AsSourceStream()->AddAudioTrack(
-            kAudioTrack,
-            track_rate,
-            0,
-            static_cast<AudioSegment*>(segment_.forget()));
-      } else {
-        mStream->AsSourceStream()->AddTrack(kVideoTrack, 0, segment_.forget());
-      }
-
-      mStream->AsSourceStream()->SetPullEnabled(true);
-      mStream->AsSourceStream()->AdvanceKnownTracksTime(STREAM_TIME_MAX);
-    }
-   private:
-    nsAutoPtr<MediaSegment> segment_;
-  };
-
-  aSource->GraphImpl()->AppendMessage(
-      MakeUnique<Message>(aSource, Move(aSegment)));
-  MOZ_MTLOG(ML_INFO, "Dispatched track-add on stream " << aSource);
-}
-
-void
-TransceiverImpl::StartReceiveStream()
-{
-  MOZ_MTLOG(ML_DEBUG, mPCHandle << "[" << mMid << "]: " << __FUNCTION__);
-  SourceMediaStream* source(mReceiveTrack->GetInputStream()->AsSourceStream());
-
-  nsAutoPtr<MediaSegment> segment;
-  if (IsVideo()) {
-    segment = new VideoSegment;
-  } else {
-    segment = new AudioSegment;
-  }
-
-  StartTrack(source, Move(segment));
-}
-
 void
 TransceiverImpl::Stop()
 {
   mTransmitPipeline->Stop();
   mTransmitPipeline->DetachMedia();
   mReceivePipeline->Stop();
   mReceivePipeline->DetachMedia();
   // Make sure that stats queries stop working on this transceiver.
--- a/media/webrtc/signaling/src/peerconnection/TransceiverImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/TransceiverImpl.h
@@ -130,17 +130,16 @@ private:
   void InitAudio();
   void InitVideo();
   nsresult UpdateAudioConduit();
   nsresult UpdateVideoConduit();
   nsresult ConfigureVideoCodecMode(VideoSessionConduit& aConduit);
   // This will eventually update audio extmap too
   void UpdateVideoExtmap(const JsepTrackNegotiatedDetails& aDetails,
                          bool aSending);
-  void StartReceiveStream();
   void Stop();
 
   const std::string mPCHandle;
   RefPtr<JsepTransceiver> mJsepTransceiver;
   std::string mMid;
   bool mHaveStartedReceiving;
   bool mHaveSetupTransport;
   nsCOMPtr<nsIEventTarget> mMainThread;