Bug 1248314. Part 1 - Since OutputStreamManager::Connect/Disconnect is tightly coupled with the constructor/destructor of DecodedStreamData, it would improve cohesion to let DecodedStreamData manage an OutputStreamManager and know when to call OutputStreamManager::Connect/Disconnect. r=roc. draft
authorJW Wang <jwwang@mozilla.com>
Mon, 15 Feb 2016 11:39:13 +0800
changeset 330951 b8da2293574d00031e937279d323be467fef6921
parent 330910 e355cacefc881ba360d412853b57e8e060e966f4
child 330952 9565342e22f3163086ca3427e7a9989450dd7f89
push id10865
push userjwwang@mozilla.com
push dateMon, 15 Feb 2016 03:44:00 +0000
reviewersroc
bugs1248314
milestone47.0a1
Bug 1248314. Part 1 - Since OutputStreamManager::Connect/Disconnect is tightly coupled with the constructor/destructor of DecodedStreamData, it would improve cohesion to let DecodedStreamData manage an OutputStreamManager and know when to call OutputStreamManager::Connect/Disconnect. r=roc. MozReview-Commit-ID: Gg6WxMzAXSp
dom/media/mediasink/DecodedStream.cpp
--- a/dom/media/mediasink/DecodedStream.cpp
+++ b/dom/media/mediasink/DecodedStream.cpp
@@ -102,17 +102,17 @@ UpdateStreamSuspended(MediaStream* aStre
  * We have at most one DecodedStreamDaata per MediaDecoder. Its stream
  * is used as the input for each ProcessedMediaStream created by calls to
  * captureStream(UntilEnded). Seeking creates a new source stream, as does
  * replaying after the input as ended. In the latter case, the new source is
  * not connected to streams created by captureStreamUntilEnded.
  */
 class DecodedStreamData {
 public:
-  DecodedStreamData(SourceMediaStream* aStream,
+  DecodedStreamData(OutputStreamManager* aOutputStreamManager,
                     MozPromiseHolder<GenericPromise>&& aPromise);
   ~DecodedStreamData();
   int64_t GetPosition() const;
   void SetPlaying(bool aPlaying);
 
   /* The following group of fields are protected by the decoder's monitor
    * and can be read or written on any thread.
    */
@@ -131,46 +131,50 @@ public:
   // video tracks added).
   bool mStreamInitialized;
   bool mHaveSentFinish;
   bool mHaveSentFinishAudio;
   bool mHaveSentFinishVideo;
 
   // The decoder is responsible for calling Destroy() on this stream.
   const RefPtr<SourceMediaStream> mStream;
-  RefPtr<DecodedStreamGraphListener> mListener;
+  const RefPtr<DecodedStreamGraphListener> mListener;
   bool mPlaying;
   // True if we need to send a compensation video frame to ensure the
   // StreamTime going forward.
   bool mEOSVideoCompensation;
+
+  const RefPtr<OutputStreamManager> mOutputStreamManager;
 };
 
-DecodedStreamData::DecodedStreamData(SourceMediaStream* aStream,
+DecodedStreamData::DecodedStreamData(OutputStreamManager* aOutputStreamManager,
                                      MozPromiseHolder<GenericPromise>&& aPromise)
   : mAudioFramesWritten(0)
   , mNextVideoTime(-1)
   , mNextAudioTime(-1)
   , mStreamInitialized(false)
   , mHaveSentFinish(false)
   , mHaveSentFinishAudio(false)
   , mHaveSentFinishVideo(false)
-  , mStream(aStream)
+  , mStream(aOutputStreamManager->Graph()->CreateSourceStream(nullptr))
+  // DecodedStreamGraphListener will resolve this promise.
+  , mListener(new DecodedStreamGraphListener(mStream, Move(aPromise)))
+  // mPlaying is initially true because MDSM won't start playback until playing
+  // becomes true. This is consistent with the settings of AudioSink.
   , mPlaying(true)
   , mEOSVideoCompensation(false)
+  , mOutputStreamManager(aOutputStreamManager)
 {
-  // DecodedStreamGraphListener will resolve this promise.
-  mListener = new DecodedStreamGraphListener(mStream, Move(aPromise));
   mStream->AddListener(mListener);
-
-  // mPlaying is initially true because MDSM won't start playback until playing
-  // becomes true. This is consistent with the settings of AudioSink.
+  mOutputStreamManager->Connect(mStream);
 }
 
 DecodedStreamData::~DecodedStreamData()
 {
+  mOutputStreamManager->Disconnect();
   mListener->Forget();
   mStream->Destroy();
 }
 
 int64_t
 DecodedStreamData::GetPosition() const
 {
   return mListener->GetLastOutputTime();
@@ -306,19 +310,17 @@ DecodedStream::DestroyData(UniquePtr<Dec
 {
   AssertOwnerThread();
 
   if (!aData) {
     return;
   }
 
   DecodedStreamData* data = aData.release();
-  RefPtr<DecodedStream> self = this;
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
-    self->mOutputStreamManager->Disconnect();
     delete data;
   });
   AbstractThread::MainThread()->Dispatch(r.forget());
 }
 
 void
 DecodedStream::CreateData(MozPromiseHolder<GenericPromise>&& aPromise)
 {
@@ -327,19 +329,17 @@ DecodedStream::CreateData(MozPromiseHold
   // No need to create a source stream when there are no output streams. This
   // happens when RemoveOutput() is called immediately after StartPlayback().
   if (!mOutputStreamManager->Graph()) {
     // Resolve the promise to indicate the end of playback.
     aPromise.Resolve(true, __func__);
     return;
   }
 
-  auto source = mOutputStreamManager->Graph()->CreateSourceStream(nullptr);
-  auto data = new DecodedStreamData(source, Move(aPromise));
-  mOutputStreamManager->Connect(data->mStream);
+  auto data = new DecodedStreamData(mOutputStreamManager, Move(aPromise));
 
   class R : public nsRunnable {
     typedef void(DecodedStream::*Method)(UniquePtr<DecodedStreamData>);
   public:
     R(DecodedStream* aThis, Method aMethod, DecodedStreamData* aData)
       : mThis(aThis), mMethod(aMethod), mData(aData) {}
     NS_IMETHOD Run() override
     {
@@ -349,19 +349,17 @@ DecodedStream::CreateData(MozPromiseHold
   private:
     virtual ~R()
     {
       // mData is not transferred when dispatch fails and Run() is not called.
       // We need to dispatch a task to ensure DecodedStreamData is destroyed
       // properly on the main thread.
       if (mData) {
         DecodedStreamData* data = mData.release();
-        RefPtr<DecodedStream> self = mThis.forget();
         nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
-          self->mOutputStreamManager->Disconnect();
           delete data;
         });
         // We are in tail dispatching phase. Don't call
         // AbstractThread::MainThread()->Dispatch() to avoid reentrant
         // AutoTaskDispatcher.
         NS_DispatchToMainThread(r.forget());
       }
     }