Bug 1426486 - P2. Make mTrackId a base member. r?bwc draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Thu, 21 Dec 2017 17:57:24 +0100
changeset 714091 a4b712c8a9eb2a21e101cb8ddfdef45f153d24cd
parent 714060 3f34c94dac865ca902b7316b15e694d2b7815902
child 744520 aa9e5d5e732232f35328718421695e3ab227d827
push id93847
push userbmo:jyavenard@mozilla.com
push dateThu, 21 Dec 2017 17:40:00 +0000
reviewersbwc
bugs1426486
milestone59.0a1
Bug 1426486 - P2. Make mTrackId a base member. r?bwc Also remove unused code. mTrackId / mTrackIdexternal were protected/private members and only ever written. MozReview-Commit-ID: C3wMhxSCA2H
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -1321,40 +1321,31 @@ MediaPipeline::PacketReceived(TransportL
 
 class MediaPipelineTransmit::PipelineListener : public MediaStreamVideoSink
 {
   friend class MediaPipelineTransmit;
 
 public:
   explicit PipelineListener(const RefPtr<MediaSessionConduit>& aConduit)
     : mConduit(aConduit)
-    , mTrackId(TRACK_INVALID)
-    , mMutex("MediaPipelineTransmit::PipelineListener")
-    , mTrackIdexternal(TRACK_INVALID)
     , mActive(false)
     , mEnabled(false)
     , mDirectConnect(false)
   {
   }
 
   ~PipelineListener()
   {
     NS_ReleaseOnMainThreadSystemGroup("MediaPipeline::mConduit",
                                       mConduit.forget());
     if (mConverter) {
       mConverter->Shutdown();
     }
   }
 
-  // Dispatches setting the internal TrackID to TRACK_INVALID to the media
-  // graph thread to keep it in sync with other MediaStreamGraph operations
-  // like RemoveListener() and AddListener(). The TrackID will be updated on
-  // the next NewData() callback.
-  void UnsetTrackId(MediaStreamGraphImpl* aGraph);
-
   void SetActive(bool aActive) { mActive = aActive; }
   void SetEnabled(bool aEnabled) { mEnabled = aEnabled; }
 
   // These are needed since nested classes don't have access to any particular
   // instance of the parent
   void SetAudioProxy(const RefPtr<AudioProxyThread>& aProxy)
   {
     mAudioProcessing = aProxy;
@@ -1401,35 +1392,22 @@ public:
   void NotifyDirectListenerInstalled(InstallationResult aResult) override;
   void NotifyDirectListenerUninstalled() override;
 
   // Implement MediaStreamVideoSink
   void SetCurrentFrames(const VideoSegment& aSegment) override;
   void ClearFrames() override {}
 
 private:
-  void UnsetTrackIdImpl()
-  {
-    MutexAutoLock lock(mMutex);
-    mTrackId = mTrackIdexternal = TRACK_INVALID;
-  }
-
   void NewData(const MediaSegment& aMedia, TrackRate aRate = 0);
 
   RefPtr<MediaSessionConduit> mConduit;
   RefPtr<AudioProxyThread> mAudioProcessing;
   RefPtr<VideoFrameConverter> mConverter;
 
-  // May be TRACK_INVALID until we see data from the track
-  TrackID mTrackId; // this is the current TrackID this listener is attached to
-  Mutex mMutex;
-  // protected by mMutex
-  // May be TRACK_INVALID until we see data from the track
-  TrackID mTrackIdexternal; // this is queried from other threads
-
   // active is true if there is a transport to send on
   mozilla::Atomic<bool> mActive;
   // enabled is true if the media access control permits sending
   // actual content; when false you get black/silence
   mozilla::Atomic<bool> mEnabled;
 
   // Written and read on the MediaStreamGraph thread
   bool mDirectConnect;
@@ -1440,18 +1418,18 @@ private:
 // We pass converted frames on to MediaPipelineTransmit::PipelineListener
 // where they are further forwarded to VideoConduit.
 // MediaPipelineTransmit calls Detach() during shutdown to ensure there is
 // no cyclic dependencies between us and PipelineListener.
 class MediaPipelineTransmit::VideoFrameFeeder : public VideoConverterListener
 {
 public:
   explicit VideoFrameFeeder(const RefPtr<PipelineListener>& aListener)
-    : mListener(aListener)
-    , mMutex("VideoFrameFeeder")
+    : mMutex("VideoFrameFeeder")
+    , mListener(aListener)
   {
     MOZ_COUNT_CTOR(VideoFrameFeeder);
   }
 
   void Detach()
   {
     MutexAutoLock lock(mMutex);
 
@@ -1488,18 +1466,18 @@ public:
     }
 
     mListener->OnVideoFrameConverted(aVideoFrame);
   }
 
 protected:
   virtual ~VideoFrameFeeder() { MOZ_COUNT_DTOR(VideoFrameFeeder); }
 
+  Mutex mMutex; // Protects the member below.
   RefPtr<PipelineListener> mListener;
-  Mutex mMutex;
 };
 
 MediaPipelineTransmit::MediaPipelineTransmit(
   const std::string& aPc,
   nsCOMPtr<nsIEventTarget> aMainThread,
   nsCOMPtr<nsIEventTarget> aStsThread,
   bool aIsVideo,
   dom::MediaStreamTrack* aDomTrack,
@@ -1705,21 +1683,16 @@ MediaPipelineTransmit::ReplaceTrack(RefP
   }
 
   RefPtr<dom::MediaStreamTrack> oldTrack = mDomTrack;
   bool wasTransmitting = oldTrack && mTransmitting;
   Stop();
   mDomTrack = aDomTrack;
   SetDescription();
 
-  if (oldTrack) {
-    // Unsets the track id after RemoveListener() takes effect.
-    mListener->UnsetTrackId(oldTrack->GraphImpl());
-  }
-
   if (wasTransmitting) {
     Start();
   }
   return NS_OK;
 }
 
 nsresult
 MediaPipeline::ConnectTransport_s(TransportInfo& aInfo)
@@ -1886,33 +1859,16 @@ MediaPipeline::PipelineTransport::SendRt
                  &MediaPipeline::PipelineTransport::SendRtpRtcpPacket_s,
                  buf,
                  false),
     NS_DISPATCH_NORMAL);
 
   return NS_OK;
 }
 
-void
-MediaPipelineTransmit::PipelineListener::UnsetTrackId(
-  MediaStreamGraphImpl* aGraph)
-{
-  class Message : public ControlMessage
-  {
-  public:
-    explicit Message(PipelineListener* listener)
-      : ControlMessage(nullptr)
-      , mListener(listener)
-    {
-    }
-    virtual void Run() override { mListener->UnsetTrackIdImpl(); }
-    RefPtr<PipelineListener> mListener;
-  };
-  aGraph->AppendMessage(MakeUnique<Message>(this));
-}
 // Called if we're attached with AddDirectListener()
 void
 MediaPipelineTransmit::PipelineListener::NotifyRealtimeTrackData(
   MediaStreamGraph* aGraph,
   StreamTime aOffset,
   const MediaSegment& aMedia)
 {
   CSFLogDebug(
@@ -2029,16 +1985,17 @@ MediaPipelineTransmit::PipelineListener:
   NewData(aSegment);
 }
 
 class GenericReceiveListener : public MediaStreamListener
 {
 public:
   explicit GenericReceiveListener(dom::MediaStreamTrack* aTrack)
     : mTrack(aTrack)
+    , 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");
   }
@@ -2087,28 +2044,29 @@ public:
   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(dom::MediaStreamTrack* aTrack)
-        : ControlMessage(aTrack->GetInputStream())
-        , mTrackId(aTrack->GetInputTrackId())
+      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>(mTrack));
+    mTrack->GraphImpl()->AppendMessage(MakeUnique<Message>(mSource, mTrackId));
     // This breaks the cycle with the SourceMediaStream
     mSource->RemoveListener(this);
   }
 
   // Must be called on the main thread
   void SetPrincipalHandle_m(const PrincipalHandle& aPrincipalHandle)
   {
     class Message : public ControlMessage
@@ -2138,16 +2096,17 @@ public:
   // Must be called on the MediaStreamGraph thread
   void SetPrincipalHandle_msg(const PrincipalHandle& aPrincipalHandle)
   {
     mPrincipalHandle = aPrincipalHandle;
   }
 
 protected:
   RefPtr<dom::MediaStreamTrack> mTrack;
+  const TrackID mTrackId;
   const RefPtr<SourceMediaStream> mSource;
   TrackTicks mPlayedTicks;
   PrincipalHandle mPrincipalHandle;
   bool mListening;
   Atomic<bool> mMaybeTrackNeedsUnmute;
 };
 
 MediaPipelineReceive::MediaPipelineReceive(const std::string& aPc,
@@ -2167,17 +2126,16 @@ MediaPipelineReceive::~MediaPipelineRece
 class MediaPipelineReceiveAudio::PipelineListener
   : public GenericReceiveListener
 {
 public:
   PipelineListener(dom::MediaStreamTrack* aTrack,
                    const RefPtr<MediaSessionConduit>& aConduit)
     : GenericReceiveListener(aTrack)
     , mConduit(aConduit)
-    , mTrackId(mTrack->GetInputTrackId())
     // AudioSession conduit only supports 16, 32, 44.1 and 48kHz
     // This is an artificial limitation, it would however require more changes
     // to support any rates.
     // If the sampling rate is not-supported, we will use 48kHz instead.
     , mRate(static_cast<AudioSessionConduit*>(mConduit.get())
                 ->IsSamplingFreqSupported(mSource->GraphRate())
               ? mSource->GraphRate()
               : WEBRTC_MAX_SAMPLE_RATE)
@@ -2306,17 +2264,16 @@ private:
         // we can't un-read the data, but that's ok since we don't want to
         // buffer - but don't i-loop!
         break;
       }
     }
   }
 
   RefPtr<MediaSessionConduit> mConduit;
-  const TrackID mTrackId;
   const TrackRate mRate;
   const RefPtr<AutoTaskQueue> mTaskQueue;
   // Graph's current sampling rate
   TrackTicks mLastLog = 0; // mPlayedTicks when we last logged
 };
 
 MediaPipelineReceiveAudio::MediaPipelineReceiveAudio(
   const std::string& aPc,