Bug 1128420 - remove finished output streams from MediaDecoder::mOutputStreams. r=roc.
authorJW Wang <jwwang@mozilla.com>
Tue, 10 Feb 2015 10:45:41 +0800
changeset 228233 0f4fc6a418ed71564195cde9979d7588688515a5
parent 228232 eec32dc1a5394a2f3e07e1f66ea7480b9098be90
child 228234 9b3ea7966b91bdcdb8c547d45fedb6a0a1724a0b
push id55364
push userjwwang@mozilla.com
push dateTue, 10 Feb 2015 02:45:49 +0000
treeherdermozilla-inbound@0f4fc6a418ed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1128420
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1128420 - remove finished output streams from MediaDecoder::mOutputStreams. r=roc.
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -345,48 +345,111 @@ MediaDecoder::DecodedStreamGraphListener
 {
   if (event == EVENT_FINISHED) {
     nsCOMPtr<nsIRunnable> event =
       NS_NewRunnableMethod(this, &DecodedStreamGraphListener::DoNotifyFinished);
     aGraph->DispatchToMainThreadAfterStreamStateUpdate(event.forget());
   }
 }
 
+class MediaDecoder::OutputStreamListener : public MediaStreamListener {
+public:
+  OutputStreamListener(MediaDecoder* aDecoder, MediaStream* aStream)
+    : mDecoder(aDecoder), mStream(aStream) {}
+
+  virtual void NotifyEvent(
+      MediaStreamGraph* aGraph,
+      MediaStreamListener::MediaStreamGraphEvent event) MOZ_OVERRIDE {
+    if (event == EVENT_FINISHED) {
+      nsRefPtr<nsIRunnable> r = NS_NewRunnableMethod(
+          this, &OutputStreamListener::DoNotifyFinished);
+      aGraph->DispatchToMainThreadAfterStreamStateUpdate(r.forget());
+    }
+  }
+
+  void Forget() {
+    MOZ_ASSERT(NS_IsMainThread());
+    mDecoder = nullptr;
+  }
+
+private:
+  void DoNotifyFinished() {
+    MOZ_ASSERT(NS_IsMainThread());
+    if (!mDecoder) {
+      return;
+    }
+
+    // Remove the finished stream so it won't block the decoded stream.
+    ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+    auto& streams = mDecoder->OutputStreams();
+    // Don't read |mDecoder| in the loop since removing the element will lead
+    // to ~OutputStreamData() which will call Forget() to reset |mDecoder|.
+    for (int32_t i = streams.Length() - 1; i >= 0; --i) {
+      auto& os = streams[i];
+      MediaStream* p = os.mStream.get();
+      if (p == mStream.get()) {
+        if (os.mPort) {
+          os.mPort->Destroy();
+          os.mPort = nullptr;
+        }
+        streams.RemoveElementAt(i);
+        break;
+      }
+    }
+  }
+
+  // Main thread only
+  MediaDecoder* mDecoder;
+  nsRefPtr<MediaStream> mStream;
+};
+
+void
+MediaDecoder::OutputStreamData::Init(MediaDecoder* aDecoder,
+                                     ProcessedMediaStream* aStream)
+{
+  mStream = aStream;
+  mListener = new OutputStreamListener(aDecoder, aStream);
+  aStream->AddListener(mListener);
+}
+
+MediaDecoder::OutputStreamData::~OutputStreamData()
+{
+  mListener->Forget();
+}
+
 void MediaDecoder::DestroyDecodedStream()
 {
   MOZ_ASSERT(NS_IsMainThread());
   GetReentrantMonitor().AssertCurrentThreadIn();
 
   if (GetDecodedStream()) {
     GetStateMachine()->ResyncMediaStreamClock();
   } else {
     // Avoid the redundant blocking to output stream.
     return;
   }
 
   // All streams are having their SourceMediaStream disconnected, so they
   // need to be explicitly blocked again.
   for (int32_t i = mOutputStreams.Length() - 1; i >= 0; --i) {
     OutputStreamData& os = mOutputStreams[i];
+    // Explicitly remove all existing ports.
+    // This is not strictly necessary but it's good form.
+    MOZ_ASSERT(os.mPort, "Double-delete of the ports!");
+    os.mPort->Destroy();
+    os.mPort = nullptr;
     // During cycle collection, nsDOMMediaStream can be destroyed and send
     // its Destroy message before this decoder is destroyed. So we have to
     // be careful not to send any messages after the Destroy().
     if (os.mStream->IsDestroyed()) {
       // Probably the DOM MediaStream was GCed. Clean up.
-      MOZ_ASSERT(os.mPort, "Double-delete of the ports!");
-      os.mPort->Destroy();
       mOutputStreams.RemoveElementAt(i);
-      continue;
+    } else {
+      os.mStream->ChangeExplicitBlockerCount(1);
     }
-    os.mStream->ChangeExplicitBlockerCount(1);
-    // Explicitly remove all existing ports. This is not strictly necessary but it's
-    // good form.
-    MOZ_ASSERT(os.mPort, "Double-delete of the ports!");
-    os.mPort->Destroy();
-    os.mPort = nullptr;
   }
 
   mDecodedStream = nullptr;
 }
 
 void MediaDecoder::UpdateStreamBlockingForStateMachinePlaying()
 {
   GetReentrantMonitor().AssertCurrentThreadIn();
@@ -424,22 +487,18 @@ void MediaDecoder::RecreateDecodedStream
   mDecodedStream = new DecodedStreamData(this, aStartTimeUSecs,
     MediaStreamGraph::GetInstance()->CreateSourceStream(nullptr));
 
   // Note that the delay between removing ports in DestroyDecodedStream
   // and adding new ones won't cause a glitch since all graph operations
   // between main-thread stable states take effect atomically.
   for (int32_t i = mOutputStreams.Length() - 1; i >= 0; --i) {
     OutputStreamData& os = mOutputStreams[i];
-    if (os.mStream->IsDestroyed()) {
-      // Probably the DOM MediaStream was GCed. Clean up.
-      // No need to destroy the port; all ports have been destroyed here.
-      mOutputStreams.RemoveElementAt(i);
-      continue;
-    }
+    MOZ_ASSERT(!os.mStream->IsDestroyed(),
+        "Should've been removed in DestroyDecodedStream()");
     ConnectDecodedStreamToOutputStream(&os);
   }
   UpdateStreamBlockingForStateMachinePlaying();
 
   mDecodedStream->mHaveBlockedForPlayState = mPlayState != PLAY_STATE_PLAYING;
   if (mDecodedStream->mHaveBlockedForPlayState) {
     mDecodedStream->mStream->ChangeExplicitBlockerCount(1);
   }
@@ -457,17 +516,17 @@ void MediaDecoder::AddOutputStream(Proce
       mDecoderStateMachine->SetAudioCaptured();
     }
     if (!GetDecodedStream()) {
       int64_t t = mDecoderStateMachine ?
                   mDecoderStateMachine->GetCurrentTimeUs() : 0;
       RecreateDecodedStream(t);
     }
     OutputStreamData* os = mOutputStreams.AppendElement();
-    os->Init(aStream, aFinishWhenEnded);
+    os->Init(this, aStream);
     ConnectDecodedStreamToOutputStream(os);
     if (aFinishWhenEnded) {
       // Ensure that aStream finishes the moment mDecodedStream does.
       aStream->SetAutofinish(true);
     }
   }
 
   // This can be called before Load(), in which case our mDecoderStateMachine
@@ -940,42 +999,16 @@ void MediaDecoder::PlaybackEnded()
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mShuttingDown ||
       mPlayState == PLAY_STATE_SEEKING ||
       mPlayState == PLAY_STATE_LOADING) {
     return;
   }
 
-  {
-    ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
-
-    for (int32_t i = mOutputStreams.Length() - 1; i >= 0; --i) {
-      OutputStreamData& os = mOutputStreams[i];
-      if (os.mStream->IsDestroyed()) {
-        // Probably the DOM MediaStream was GCed. Clean up.
-        MOZ_ASSERT(os.mPort, "Double-delete of the ports!");
-        os.mPort->Destroy();
-        mOutputStreams.RemoveElementAt(i);
-        continue;
-      }
-      if (os.mFinishWhenEnded) {
-        // Shouldn't really be needed since mDecodedStream should already have
-        // finished, but doesn't hurt.
-        os.mStream->Finish();
-        MOZ_ASSERT(os.mPort, "Double-delete of the ports!");
-        os.mPort->Destroy();
-        // Not really needed but it keeps the invariant that a stream not
-        // connected to mDecodedStream is explicity blocked.
-        os.mStream->ChangeExplicitBlockerCount(1);
-        mOutputStreams.RemoveElementAt(i);
-      }
-    }
-  }
-
   PlaybackPositionChanged();
   ChangeState(PLAY_STATE_ENDED);
   InvalidateWithFlags(VideoFrameContainer::INVALIDATE_FORCE);
 
   UpdateReadyStateForData();
   if (mOwner)  {
     mOwner->PlaybackEnded();
   }
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -474,27 +474,27 @@ public:
     // Protected by mMutex
     nsRefPtr<MediaStream> mStream;
     // Protected by mMutex
     int64_t mLastOutputTime; // microseconds
     // Protected by mMutex
     bool mStreamFinishedOnMainThread;
   };
 
+  class OutputStreamListener;
+
   struct OutputStreamData {
-    void Init(ProcessedMediaStream* aStream, bool aFinishWhenEnded)
-    {
-      mStream = aStream;
-      mFinishWhenEnded = aFinishWhenEnded;
-    }
+    void Init(MediaDecoder* aDecoder, ProcessedMediaStream* aStream);
+    ~OutputStreamData();
     nsRefPtr<ProcessedMediaStream> mStream;
     // mPort connects mDecodedStream->mStream to our mStream.
     nsRefPtr<MediaInputPort> mPort;
-    bool mFinishWhenEnded;
+    nsRefPtr<OutputStreamListener> mListener;
   };
+
   /**
    * Connects mDecodedStream->mStream to aStream->mStream.
    */
   void ConnectDecodedStreamToOutputStream(OutputStreamData* aStream);
   /**
    * Disconnects mDecodedStream->mStream from all outputs and clears
    * mDecodedStream.
    */