Bug 1370164: Properly handle flushing during ongoing operations. r?jwwang draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Mon, 05 Jun 2017 20:56:22 +0200
changeset 589470 f5e799e21e60db692843b52f4c4d525f559a3406
parent 589175 955fac9359159d81f3f56af8eb3782afa697a42d
child 631901 eee2badc4cd96a8140bd4e9d1e8b23bf0dfd92dd
push id62395
push userbmo:jyavenard@mozilla.com
push dateTue, 06 Jun 2017 09:04:42 +0000
reviewersjwwang
bugs1370164
milestone55.0a1
Bug 1370164: Properly handle flushing during ongoing operations. r?jwwang MozReview-Commit-ID: 4eAHAuBqOtK
dom/media/platforms/wrappers/H264Converter.cpp
dom/media/platforms/wrappers/H264Converter.h
--- a/dom/media/platforms/wrappers/H264Converter.cpp
+++ b/dom/media/platforms/wrappers/H264Converter.cpp
@@ -127,24 +127,57 @@ H264Converter::Decode(MediaRawData* aSam
 
   return mDecoder->Decode(aSample);
 }
 
 RefPtr<MediaDataDecoder::FlushPromise>
 H264Converter::Flush()
 {
   mDecodePromiseRequest.DisconnectIfExists();
-  mFlushRequest.DisconnectIfExists();
-  mShutdownRequest.DisconnectIfExists();
   mDecodePromise.RejectIfExists(NS_ERROR_DOM_MEDIA_CANCELED, __func__);
   mNeedKeyframe = true;
+
+  /*
+      When we detect a change of content in the H264 stream, we first flush the
+    current decoder (1), then shut it down (2).
+    It is possible possible for H264Converter::Flush to be called during any of
+    those time.
+    If during (1):
+      - mFlushRequest and mFlushPromise will not be empty.
+      - The old decoder can still be used, with the current extradata as stored
+        in mCurrentConfig.mExtraData.
+
+    If during (2):
+      - mShutdownRequest and mShutdownPromise won't be empty.
+      - mDecoder is empty.
+      - The old decoder is no longer referenced by the H264Converter.
+  */
+  if (mFlushPromise) {
+    // Flush in progress, hijack that one.
+    mFlushRequest.Disconnect();
+    return mFlushPromise.forget();
+  }
   if (mDecoder) {
     return mDecoder->Flush();
   }
-  return FlushPromise::CreateAndResolve(true, __func__);
+  if (!mShutdownPromise) {
+    return FlushPromise::CreateAndResolve(true, __func__);
+  }
+
+  mShutdownRequest.Disconnect();
+  // Let's continue when the the current shutdown completes.
+  RefPtr<ShutdownPromise> shutdownPromise = mShutdownPromise.forget();
+  return shutdownPromise->Then(
+    AbstractThread::GetCurrent()->AsTaskQueue(),
+    __func__,
+    [](bool) { return FlushPromise::CreateAndResolve(true, __func__); },
+    [](bool) {
+      MOZ_ASSERT_UNREACHABLE("Shutdown promises are always resolved");
+      return FlushPromise::CreateAndResolve(true, __func__);
+    });
 }
 
 RefPtr<MediaDataDecoder::DecodePromise>
 H264Converter::Drain()
 {
   mNeedKeyframe = true;
   if (mDecoder) {
     return mDecoder->Drain();
@@ -153,18 +186,21 @@ H264Converter::Drain()
 }
 
 RefPtr<ShutdownPromise>
 H264Converter::Shutdown()
 {
   mInitPromiseRequest.DisconnectIfExists();
   mDecodePromiseRequest.DisconnectIfExists();
   mFlushRequest.DisconnectIfExists();
+  mFlushPromise = nullptr;
   mShutdownRequest.DisconnectIfExists();
   mPendingSample = nullptr;
+  mNeedAVCC.reset();
+
   if (mShutdownPromise) {
     // We have a shutdown in progress, return that promise instead as we can't
     // shutdown a decoder twice.
     return mShutdownPromise.forget();
   }
   if (mDecoder) {
     RefPtr<MediaDataDecoder> decoder = mDecoder.forget();
     return decoder->Shutdown();
@@ -342,16 +378,18 @@ H264Converter::CheckForSPSChange(MediaRa
     if (!*mCanRecycleDecoder) {
       // If the decoder can't be recycled, the out of band extradata will never
       // change as the H264Converter will be recreated by the MediaFormatReader
       // instead. So there's no point in testing for changes.
       return NS_OK;
     }
     // This sample doesn't contain inband SPS/PPS
     // We now check if the out of band one has changed.
+    // This scenario can only occur on Android with devices that can recycle a
+    // decoder.
     if (mp4_demuxer::AnnexB::HasSPS(aSample->mExtraData) &&
         !mp4_demuxer::AnnexB::CompareExtraData(aSample->mExtraData,
                                                mOriginalExtraData)) {
       extra_data = mOriginalExtraData = aSample->mExtraData;
     }
   }
   if (mp4_demuxer::AnnexB::CompareExtraData(extra_data,
                                             mCurrentConfig.mExtraData)) {
@@ -368,43 +406,44 @@ H264Converter::CheckForSPSChange(MediaRa
     }
     mNeedKeyframe = true;
     return NS_OK;
   }
 
   // The SPS has changed, signal to flush the current decoder and create a
   // new one.
   RefPtr<H264Converter> self = this;
-  mDecoder->Flush()
+  mFlushPromise = mDecoder->Flush();
+  mFlushPromise
     ->Then(AbstractThread::GetCurrent()->AsTaskQueue(),
            __func__,
            [self, sample, this]() {
              mFlushRequest.Complete();
              mShutdownPromise = Shutdown();
              mShutdownPromise
                ->Then(AbstractThread::GetCurrent()->AsTaskQueue(),
                       __func__,
                       [self, sample, this]() {
                         mShutdownRequest.Complete();
                         mShutdownPromise = nullptr;
-                        mNeedAVCC.reset();
                         nsresult rv = CreateDecoderAndInit(sample);
                         if (rv == NS_ERROR_DOM_MEDIA_INITIALIZING_DECODER) {
                           // All good so far, will continue later.
                           return;
                         }
                         MOZ_ASSERT(NS_FAILED(rv));
                         mDecodePromise.Reject(rv, __func__);
                         return;
                       },
                       [] { MOZ_CRASH("Can't reach here'"); })
                ->Track(mShutdownRequest);
            },
            [self, this](const MediaResult& aError) {
              mFlushRequest.Complete();
+             mFlushPromise = nullptr;
              mDecodePromise.Reject(aError, __func__);
            })
     ->Track(mFlushRequest);
   return NS_ERROR_DOM_MEDIA_INITIALIZING_DECODER;
 }
 
 void
 H264Converter::UpdateConfigFromExtraData(MediaByteBuffer* aExtraData)
--- a/dom/media/platforms/wrappers/H264Converter.h
+++ b/dom/media/platforms/wrappers/H264Converter.h
@@ -87,16 +87,17 @@ private:
   RefPtr<layers::ImageContainer> mImageContainer;
   const RefPtr<TaskQueue> mTaskQueue;
   RefPtr<MediaRawData> mPendingSample;
   RefPtr<MediaDataDecoder> mDecoder;
   MozPromiseRequestHolder<InitPromise> mInitPromiseRequest;
   MozPromiseRequestHolder<DecodePromise> mDecodePromiseRequest;
   MozPromiseHolder<DecodePromise> mDecodePromise;
   MozPromiseRequestHolder<FlushPromise> mFlushRequest;
+  RefPtr<FlushPromise> mFlushPromise;
   MozPromiseRequestHolder<ShutdownPromise> mShutdownRequest;
   RefPtr<ShutdownPromise> mShutdownPromise;
 
   RefPtr<GMPCrashHelper> mGMPCrashHelper;
   Maybe<bool> mNeedAVCC;
   nsresult mLastError;
   bool mNeedKeyframe = true;
   const TrackInfo::TrackType mType;