Bug 1370164: Properly handle flushing during ongoing operations. r=jwwang
authorJean-Yves Avenard <jyavenard@mozilla.com>
Mon, 05 Jun 2017 20:56:22 +0200
changeset 412971 78dcfa118bddbd18b987ae124552c00826a3826f
parent 412970 7d7a141c99e72ca7b486f5408c7d926803a0a7a7
child 412972 e39dfa768be99729f407caa406cfdcc961ce417c
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang
bugs1370164
milestone55.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 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;