Bug 1269408: P4. Ensure the decoders are flushed prior performing an internal seek. r?gerald draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Thu, 05 May 2016 15:06:40 +1000
changeset 364201 6fa970aefeb4360ec493302c5e5ae0c446c44ba1
parent 364191 bcb643ca881e2ce0befa3c5140d7cd4934af3b79
child 364202 4c7b1824bd51ca566a112d81b7509a011ba2bd46
push id17383
push userbmo:jyavenard@mozilla.com
push dateFri, 06 May 2016 06:16:15 +0000
reviewersgerald
bugs1269408
milestone49.0a1
Bug 1269408: P4. Ensure the decoders are flushed prior performing an internal seek. r?gerald Some decoders (WMF) keep some internal counters on how many frames have been output and use this to calculate the time of the decoded audio frame. As such, when internally seeking, the next frame decoded would have always been past the seek target. MozReview-Commit-ID: puzs6ecqbD Amend P4 MozReview-Commit-ID: HdOe9RxcOFD
dom/media/MediaFormatReader.cpp
dom/media/MediaFormatReader.h
--- a/dom/media/MediaFormatReader.cpp
+++ b/dom/media/MediaFormatReader.cpp
@@ -92,17 +92,17 @@ MediaFormatReader::Shutdown()
   }
 
   mDemuxerInitRequest.DisconnectIfExists();
   mMetadataPromise.RejectIfExists(ReadMetadataFailureReason::METADATA_ERROR, __func__);
   mSeekPromise.RejectIfExists(NS_ERROR_FAILURE, __func__);
   mSkipRequest.DisconnectIfExists();
 
   if (mAudio.mDecoder) {
-    Flush(TrackInfo::kAudioTrack);
+    Reset(TrackInfo::kAudioTrack);
     if (mAudio.HasPromise()) {
       mAudio.RejectPromise(CANCELED, __func__);
     }
     mAudio.mInitPromise.DisconnectIfExists();
     mAudio.ShutdownDecoder();
   }
   if (mAudio.mTrackDemuxer) {
     mAudio.ResetDemuxer();
@@ -112,17 +112,17 @@ MediaFormatReader::Shutdown()
   if (mAudio.mTaskQueue) {
     mAudio.mTaskQueue->BeginShutdown();
     mAudio.mTaskQueue->AwaitShutdownAndIdle();
     mAudio.mTaskQueue = nullptr;
   }
   MOZ_ASSERT(mAudio.mPromise.IsEmpty());
 
   if (mVideo.mDecoder) {
-    Flush(TrackInfo::kVideoTrack);
+    Reset(TrackInfo::kVideoTrack);
     if (mVideo.HasPromise()) {
       mVideo.RejectPromise(CANCELED, __func__);
     }
     mVideo.mInitPromise.DisconnectIfExists();
     mVideo.ShutdownDecoder();
   }
   if (mVideo.mTrackDemuxer) {
     mVideo.ResetDemuxer();
@@ -530,18 +530,18 @@ MediaFormatReader::RequestVideoData(bool
 
     // I think it's still possible for an output to have been sent from the decoder
     // and is currently sitting in our event queue waiting to be processed. The following
     // flush won't clear it, and when we return to the event loop it'll be added to our
     // output queue and be used.
     // This code will count that as dropped, which was the intent, but not quite true.
     mDecoder->NotifyDecodedFrames(0, 0, SizeOfVideoQueueInFrames());
 
-    Flush(TrackInfo::kVideoTrack);
-    RefPtr<MediaDataPromise> p = mVideo.mPromise.Ensure(__func__);
+    Reset(TrackInfo::kVideoTrack);
+    RefPtr<MediaDataPromise> p = mVideo.EnsurePromise(__func__);
     SkipVideoDemuxToNextKeyFrame(timeThreshold);
     return p;
   }
 
   RefPtr<MediaDataPromise> p = mVideo.mPromise.Ensure(__func__);
   NotifyDecodingRequested(TrackInfo::kVideoTrack);
 
   return p;
@@ -949,19 +949,19 @@ MediaFormatReader::HandleDemuxedSamples(
       }
 
       LOG("%s stream id has changed from:%d to:%d, recreating decoder.",
           TrackTypeToStr(aTrack), decoder.mLastStreamSourceID,
           info->GetID());
       decoder.mInfo = info;
       decoder.mLastStreamSourceID = info->GetID();
       decoder.mNextStreamSourceID.reset();
-      // Flush will clear our array of queued samples. So make a copy now.
+      // Reset will clear our array of queued samples. So make a copy now.
       nsTArray<RefPtr<MediaRawData>> samples{decoder.mQueuedSamples};
-      Flush(aTrack);
+      Reset(aTrack);
       decoder.ShutdownDecoder();
       if (sample->mKeyframe) {
         decoder.mQueuedSamples.AppendElements(Move(samples));
         NotifyDecodingRequested(aTrack);
       } else {
         InternalSeekTarget seekTarget =
           decoder.mTimeThreshold.refOr(InternalSeekTarget(TimeUnit::FromMicroseconds(sample->mTime), false));
         LOG("Stream change occurred on a non-keyframe. Seeking to:%lld",
@@ -1003,18 +1003,17 @@ MediaFormatReader::HandleDemuxedSamples(
 void
 MediaFormatReader::InternalSeek(TrackType aTrack, const InternalSeekTarget& aTarget)
 {
   MOZ_ASSERT(OnTaskQueue());
   LOG("%s internal seek to %f",
       TrackTypeToStr(aTrack), aTarget.mTime.ToSeconds());
 
   auto& decoder = GetDecoderData(aTrack);
-  decoder.mTimeThreshold = Some(aTarget);
-  RefPtr<MediaFormatReader> self = this;
+  decoder.Flush();
   decoder.ResetDemuxer();
   decoder.mTimeThreshold = Some(aTarget);
   RefPtr<MediaFormatReader> self = this;
   decoder.mSeekRequest.Begin(decoder.mTrackDemuxer->Seek(decoder.mTimeThreshold.ref().mTime)
              ->Then(OwnerThread(), __func__,
                     [self, aTrack] (media::TimeUnit aTime) {
                       auto& decoder = self->GetDecoderData(aTrack);
                       decoder.mSeekRequest.Complete();
@@ -1304,23 +1303,23 @@ MediaFormatReader::ResetDecode()
   mVideo.mWaitingPromise.RejectIfExists(WaitForDataRejectValue(MediaData::VIDEO_DATA, WaitForDataRejectValue::CANCELED), __func__);
 
   // Reset miscellaneous seeking state.
   mPendingSeekTime.reset();
 
   ResetDemuxers();
 
   if (HasVideo()) {
-    Flush(TrackInfo::kVideoTrack);
+    Reset(TrackInfo::kVideoTrack);
     if (mVideo.HasPromise()) {
       mVideo.RejectPromise(CANCELED, __func__);
     }
   }
   if (HasAudio()) {
-    Flush(TrackInfo::kAudioTrack);
+    Reset(TrackInfo::kAudioTrack);
     if (mAudio.HasPromise()) {
       mAudio.RejectPromise(CANCELED, __func__);
     }
   }
   return MediaDecoderReader::ResetDecode();
 }
 
 void
@@ -1378,33 +1377,27 @@ MediaFormatReader::Error(TrackType aTrac
 {
   RefPtr<nsIRunnable> task =
     NewRunnableMethod<TrackType>(
       this, &MediaFormatReader::NotifyError, aTrack);
   OwnerThread()->Dispatch(task.forget());
 }
 
 void
-MediaFormatReader::Flush(TrackType aTrack)
+MediaFormatReader::Reset(TrackType aTrack)
 {
   MOZ_ASSERT(OnTaskQueue());
-  LOG("Flush(%s) BEGIN", TrackTypeToStr(aTrack));
+  LOG("Reset(%s) BEGIN", TrackTypeToStr(aTrack));
 
   auto& decoder = GetDecoderData(aTrack);
-  if (!decoder.mDecoder) {
-    decoder.ResetState();
-    return;
-  }
 
-  decoder.mDecoder->Flush();
-  // Purge the current decoder's state.
-  // ResetState clears mOutputRequested flag so that we ignore all output until
-  // the next request for more data.
   decoder.ResetState();
-  LOG("Flush(%s) END", TrackTypeToStr(aTrack));
+  decoder.Flush();
+
+  LOG("Reset(%s) END", TrackTypeToStr(aTrack));
 }
 
 void
 MediaFormatReader::SkipVideoDemuxToNextKeyFrame(media::TimeUnit aTimeThreshold)
 {
   MOZ_ASSERT(OnTaskQueue());
 
   MOZ_ASSERT(mVideo.mDecoder);
--- a/dom/media/MediaFormatReader.h
+++ b/dom/media/MediaFormatReader.h
@@ -165,17 +165,17 @@ private:
   // Initializes mLayersBackendType if possible.
   void InitLayersBackendType();
 
   // DecoderCallback proxies the MediaDataDecoderCallback calls to these
   // functions.
   void Output(TrackType aType, MediaData* aSample);
   void InputExhausted(TrackType aTrack);
   void Error(TrackType aTrack);
-  void Flush(TrackType aTrack);
+  void Reset(TrackType aTrack);
   void DrainComplete(TrackType aTrack);
 
   bool ShouldSkip(bool aSkipToNextKeyframe, media::TimeUnit aTimeThreshold);
   void ResetDemuxers();
 
   size_t SizeOfQueue(TrackType aTrack);
 
   RefPtr<PDMFactory> mPlatform;
@@ -324,24 +324,48 @@ private:
 
     // These get overriden in the templated concrete class.
     // Indicate if we have a pending promise for decoded frame.
     // Rejecting the promise will stop the reader from decoding ahead.
     virtual bool HasPromise() = 0;
     virtual void RejectPromise(MediaDecoderReader::NotDecodedReason aReason,
                                const char* aMethodName) = 0;
 
+    // Clear track demuxer related data.
     void ResetDemuxer()
     {
-      // Clear demuxer related data.
       mDemuxRequest.DisconnectIfExists();
       mSeekRequest.DisconnectIfExists();
       mTrackDemuxer->Reset();
+      mQueuedSamples.Clear();
     }
 
+    // Flush the decoder if any and reset decoding related data.
+    // Decoding will be suspended until mInputRequested is set again.
+    // Following a flush, the decoder is ready to accept any new data.
+    void Flush()
+    {
+      if (mDecoder) {
+        mDecoder->Flush();
+      }
+      mDecodingRequested = false;
+      mOutputRequested = false;
+      mInputExhausted = false;
+      mOutput.Clear();
+      mNumSamplesInput = 0;
+      mNumSamplesOutput = 0;
+      mSizeOfQueue = 0;
+      mDraining = false;
+      mDrainComplete = false;
+    }
+
+    // Reset the state of the DecoderData, clearing all queued frames
+    // (pending demuxed and decoded).
+    // Decoding will be suspended until mInputRequested is set again.
+    // The track demuxer is *not* reset.
     void ResetState()
     {
       MOZ_ASSERT(mOwner->OnTaskQueue());
       mDemuxEOS = false;
       mWaitingForData = false;
       mReceivedNewData = false;
       mDiscontinuity = true;
       mQueuedSamples.Clear();