bug 1161903 ensure pending DrainComplete is not run after Flush() r=cpearce
authorKarl Tomlinson <karlt+@karlt.net>
Thu, 07 May 2015 14:15:24 +1200
changeset 274260 7b11fc3efb145d63e7d2ab7931a0f0282ae54063
parent 274259 9a8248d7999ec8e68a9fcf2051d6c83c780c77fd
child 274261 3c8a74e0291d62da8d5a6017baa5eed33addb3a8
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1161903
milestone40.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 1161903 ensure pending DrainComplete is not run after Flush() r=cpearce Using a Decode task already in the queue had the potential for out of order flushing. This is similar in behavior to the implementation prior to f550eb7809b6, but keeps mDecoder->Flush() on the platform decoder's task queue, in the hope of avoiding any ordering problems from calling in the middle of decoding input, or races with MFTDecoder::mDiscontinuity. The contract is not clear on whether DrainComplete() should be run during Flush(), but I've kept it in this version.
dom/media/fmp4/wmf/WMFMediaDataDecoder.cpp
dom/media/fmp4/wmf/WMFMediaDataDecoder.h
--- a/dom/media/fmp4/wmf/WMFMediaDataDecoder.cpp
+++ b/dom/media/fmp4/wmf/WMFMediaDataDecoder.cpp
@@ -23,17 +23,16 @@ namespace mozilla {
 
 WMFMediaDataDecoder::WMFMediaDataDecoder(MFTManager* aMFTManager,
                                          FlushableMediaTaskQueue* aTaskQueue,
                                          MediaDataDecoderCallback* aCallback)
   : mTaskQueue(aTaskQueue)
   , mCallback(aCallback)
   , mMFTManager(aMFTManager)
   , mMonitor("WMFMediaDataDecoder")
-  , mIsDecodeTaskDispatched(false)
   , mIsFlushing(false)
   , mIsShutDown(false)
 {
 }
 
 WMFMediaDataDecoder::~WMFMediaDataDecoder()
 {
 }
@@ -56,100 +55,67 @@ WMFMediaDataDecoder::Shutdown()
   MOZ_DIAGNOSTIC_ASSERT(!mIsShutDown);
 
   if (mTaskQueue) {
     mTaskQueue->Dispatch(
       NS_NewRunnableMethod(this, &WMFMediaDataDecoder::ProcessShutdown));
   } else {
     ProcessShutdown();
   }
-#ifdef DEBUG
-  {
-    MonitorAutoLock mon(mMonitor);
-    // The MP4Reader should have flushed before calling Shutdown().
-    MOZ_ASSERT(!mIsDecodeTaskDispatched);
-  }
-#endif
   mIsShutDown = true;
   return NS_OK;
 }
 
 void
 WMFMediaDataDecoder::ProcessShutdown()
 {
   if (mMFTManager) {
     mMFTManager->Shutdown();
     mMFTManager = nullptr;
   }
   mDecoder = nullptr;
 }
 
-void
-WMFMediaDataDecoder::EnsureDecodeTaskDispatched()
-{
-  mMonitor.AssertCurrentThreadOwns();
-  if (!mIsDecodeTaskDispatched) {
-    mTaskQueue->Dispatch(
-      NS_NewRunnableMethod(this,
-      &WMFMediaDataDecoder::Decode));
-    mIsDecodeTaskDispatched = true;
-  }
-}
-
 // Inserts data into the decoder's pipeline.
 nsresult
 WMFMediaDataDecoder::Input(MediaRawData* aSample)
 {
   MOZ_ASSERT(mCallback->OnReaderTaskQueue());
   MOZ_DIAGNOSTIC_ASSERT(!mIsShutDown);
 
-  MonitorAutoLock mon(mMonitor);
-  mInput.push(aSample);
-  EnsureDecodeTaskDispatched();
+  nsCOMPtr<nsIRunnable> runnable =
+    NS_NewRunnableMethodWithArg<nsRefPtr<MediaRawData>>(
+      this,
+      &WMFMediaDataDecoder::ProcessDecode,
+      nsRefPtr<MediaRawData>(aSample));
+  mTaskQueue->Dispatch(runnable.forget());
   return NS_OK;
 }
 
 void
-WMFMediaDataDecoder::Decode()
+WMFMediaDataDecoder::ProcessDecode(MediaRawData* aSample)
 {
-  while (true) {
-    nsRefPtr<MediaRawData> input;
-    {
-      MonitorAutoLock mon(mMonitor);
-      MOZ_ASSERT(mIsDecodeTaskDispatched);
-      if (mInput.empty()) {
-        if (mIsFlushing) {
-          if (mDecoder) {
-            mDecoder->Flush();
-          }
-          mIsFlushing = false;
-        }
-        mIsDecodeTaskDispatched = false;
-        mon.NotifyAll();
-        return;
-      }
-      input = mInput.front();
-      mInput.pop();
+  {
+    MonitorAutoLock mon(mMonitor);
+    if (mIsFlushing) {
+      // Skip sample, to be released by runnable.
+      return;
     }
+  }
 
-    HRESULT hr = mMFTManager->Input(input);
-    if (FAILED(hr)) {
-      NS_WARNING("MFTManager rejected sample");
-      {
-        MonitorAutoLock mon(mMonitor);
-        PurgeInputQueue();
-      }
-      mCallback->Error();
-      continue; // complete flush if flushing
-    }
+  HRESULT hr = mMFTManager->Input(aSample);
+  if (FAILED(hr)) {
+    NS_WARNING("MFTManager rejected sample");
+    mCallback->Error();
+    return;
+  }
 
-    mLastStreamOffset = input->mOffset;
+  mLastStreamOffset = aSample->mOffset;
 
-    ProcessOutput();
-  }
+  ProcessOutput();
 }
 
 void
 WMFMediaDataDecoder::ProcessOutput()
 {
   nsRefPtr<MediaData> output;
   HRESULT hr = S_OK;
   while (SUCCEEDED(hr = mMFTManager->Output(mLastStreamOffset, output)) &&
@@ -157,53 +123,57 @@ WMFMediaDataDecoder::ProcessOutput()
     mCallback->Output(output);
   }
   if (hr == MF_E_TRANSFORM_NEED_MORE_INPUT) {
     if (mTaskQueue->IsEmpty()) {
       mCallback->InputExhausted();
     }
   } else if (FAILED(hr)) {
     NS_WARNING("WMFMediaDataDecoder failed to output data");
-    {
-      MonitorAutoLock mon(mMonitor);
-      PurgeInputQueue();
-    }
     mCallback->Error();
   }
 }
 
 void
-WMFMediaDataDecoder::PurgeInputQueue()
+WMFMediaDataDecoder::ProcessFlush()
 {
-  mMonitor.AssertCurrentThreadOwns();
-  while (!mInput.empty()) {
-    mInput.pop();
+  if (mDecoder) {
+    mDecoder->Flush();
   }
+  MonitorAutoLock mon(mMonitor);
+  mIsFlushing = false;
+  mon.NotifyAll();
 }
 
 nsresult
 WMFMediaDataDecoder::Flush()
 {
   MOZ_ASSERT(mCallback->OnReaderTaskQueue());
   MOZ_DIAGNOSTIC_ASSERT(!mIsShutDown);
 
+  nsCOMPtr<nsIRunnable> runnable =
+    NS_NewRunnableMethod(this, &WMFMediaDataDecoder::ProcessFlush);
   MonitorAutoLock mon(mMonitor);
-  PurgeInputQueue();
   mIsFlushing = true;
-  EnsureDecodeTaskDispatched();
-  while (mIsDecodeTaskDispatched || mIsFlushing) {
+  mTaskQueue->Dispatch(runnable.forget());
+  while (mIsFlushing) {
     mon.Wait();
   }
   return NS_OK;
 }
 
 void
 WMFMediaDataDecoder::ProcessDrain()
 {
-  if (mDecoder) {
+  bool isFlushing;
+  {
+    MonitorAutoLock mon(mMonitor);
+    isFlushing = mIsFlushing;
+  }
+  if (!isFlushing && mDecoder) {
     // Order the decoder to drain...
     if (FAILED(mDecoder->SendMFTMessage(MFT_MESSAGE_COMMAND_DRAIN, 0))) {
       NS_WARNING("Failed to send DRAIN command to MFT");
     }
     // Then extract all available output.
     ProcessOutput();
   }
   mCallback->DrainComplete();
--- a/dom/media/fmp4/wmf/WMFMediaDataDecoder.h
+++ b/dom/media/fmp4/wmf/WMFMediaDataDecoder.h
@@ -68,42 +68,49 @@ public:
   virtual nsresult Drain() override;
 
   virtual nsresult Shutdown() override;
 
   virtual bool IsHardwareAccelerated() const override;
 
 private:
 
-  void Decode();
-  void EnsureDecodeTaskDispatched();
-  void PurgeInputQueue();
+  // Called on the task queue. Inserts the sample into the decoder, and
+  // extracts output if available.
+  void ProcessDecode(MediaRawData* aSample);
 
   // Called on the task queue. Extracts output if available, and delivers
   // it to the reader. Called after ProcessDecode() and ProcessDrain().
   void ProcessOutput();
 
+  // Called on the task queue. Orders the MFT to flush.  There is no output to
+  // extract.
+  void ProcessFlush();
+
   // Called on the task queue. Orders the MFT to drain, and then extracts
   // all available output.
   void ProcessDrain();
 
   void ProcessShutdown();
 
   RefPtr<FlushableMediaTaskQueue> mTaskQueue;
   MediaDataDecoderCallback* mCallback;
 
   RefPtr<MFTDecoder> mDecoder;
   nsAutoPtr<MFTManager> mMFTManager;
 
   // The last offset into the media resource that was passed into Input().
   // This is used to approximate the decoder's position in the media resource.
   int64_t mLastStreamOffset;
 
+  // For access to and waiting on mIsFlushing
   Monitor mMonitor;
-  std::queue<nsRefPtr<MediaRawData>> mInput;
-  bool mIsDecodeTaskDispatched;
+  // Set on reader/decode thread calling Flush() to indicate that output is
+  // not required and so input samples on mTaskQueue need not be processed.
+  // Cleared on mTaskQueue.
   bool mIsFlushing;
+
   bool mIsShutDown;
 };
 
 } // namespace mozilla
 
 #endif // WMFMediaDataDecoder_h_