Bug 1269178: P2. Ensure that no skip to next keyframe logic is activated while there's a pending seek. r?gerald draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Mon, 02 May 2016 10:53:31 +1000
changeset 358185 86d8f7449993934539bec175a834081bd8b0ca97
parent 358173 35d2ec553b47ffb2ec220fd88c2e14138a7b47d0
child 358186 db5d1b8cf1eabb38015df57d9c5d21f0efda47af
push id16959
push userbmo:jyavenard@mozilla.com
push dateMon, 02 May 2016 06:10:08 +0000
reviewersgerald
bugs1269178
milestone49.0a1
Bug 1269178: P2. Ensure that no skip to next keyframe logic is activated while there's a pending seek. r?gerald While a seek is pending, the demuxer has been reset meaning that ShouldSkip() would almost always incorrectly return true and would corrupt the seeking state. MozReview-Commit-ID: 6R912Fu6Nul
dom/media/MediaFormatReader.cpp
dom/media/MediaFormatReader.h
--- a/dom/media/MediaFormatReader.cpp
+++ b/dom/media/MediaFormatReader.cpp
@@ -515,17 +515,20 @@ MediaFormatReader::RequestVideoData(bool
   }
 
   if (mShutdown) {
     NS_WARNING("RequestVideoData on shutdown MediaFormatReader!");
     return MediaDataPromise::CreateAndReject(CANCELED, __func__);
   }
 
   media::TimeUnit timeThreshold{media::TimeUnit::FromMicroseconds(aTimeThreshold)};
-  if (ShouldSkip(aSkipToNextKeyframe, timeThreshold)) {
+  // Ensure we have no pending seek going as ShouldSkip could return out of date
+  // information.
+  if (!mVideo.HasInternalSeekPending() &&
+      ShouldSkip(aSkipToNextKeyframe, timeThreshold)) {
     // Cancel any pending demux request.
     mVideo.mDemuxRequest.DisconnectIfExists();
 
     // 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.
@@ -993,16 +996,19 @@ MediaFormatReader::HandleDemuxedSamples(
   // We have serviced the decoder's request for more data.
   decoder.mInputExhausted = false;
 }
 
 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.ResetDemuxer();
   decoder.mSeekRequest.Begin(decoder.mTrackDemuxer->Seek(decoder.mTimeThreshold.ref().mTime)
              ->Then(OwnerThread(), __func__,
                     [self, aTrack] (media::TimeUnit aTime) {
                       auto& decoder = self->GetDecoderData(aTrack);
@@ -1270,18 +1276,16 @@ MediaFormatReader::WaitForData(MediaData
 }
 
 nsresult
 MediaFormatReader::ResetDecode()
 {
   MOZ_ASSERT(OnTaskQueue());
   LOGV("");
 
-  mAudio.mSeekRequest.DisconnectIfExists();
-  mVideo.mSeekRequest.DisconnectIfExists();
   mSeekPromise.RejectIfExists(NS_OK, __func__);
   mSkipRequest.DisconnectIfExists();
 
   // Do the same for any data wait promises.
   mAudio.mWaitingPromise.RejectIfExists(WaitForDataRejectValue(MediaData::AUDIO_DATA, WaitForDataRejectValue::CANCELED), __func__);
   mVideo.mWaitingPromise.RejectIfExists(WaitForDataRejectValue(MediaData::VIDEO_DATA, WaitForDataRejectValue::CANCELED), __func__);
 
   // Reset miscellaneous seeking state.
--- a/dom/media/MediaFormatReader.h
+++ b/dom/media/MediaFormatReader.h
@@ -325,16 +325,17 @@ private:
     virtual bool HasPromise() = 0;
     virtual void RejectPromise(MediaDecoderReader::NotDecodedReason aReason,
                                const char* aMethodName) = 0;
 
     void ResetDemuxer()
     {
       // Clear demuxer related data.
       mDemuxRequest.DisconnectIfExists();
+      mSeekRequest.DisconnectIfExists();
       mTrackDemuxer->Reset();
     }
 
     void ResetState()
     {
       MOZ_ASSERT(mOwner->OnTaskQueue());
       mDemuxEOS = false;
       mWaitingForData = false;
@@ -351,16 +352,21 @@ private:
       mLastSampleTime.reset();
       mOutput.Clear();
       mNumSamplesInput = 0;
       mNumSamplesOutput = 0;
       mSizeOfQueue = 0;
       mNextStreamSourceID.reset();
     }
 
+    bool HasInternalSeekPending() const
+    {
+      return mTimeThreshold && !mTimeThreshold.ref().mHasSeeked;
+    }
+
     // Used by the MDSM for logging purposes.
     Atomic<size_t> mSizeOfQueue;
     // Used by the MDSM to determine if video decoding is hardware accelerated.
     // This value is updated after a frame is successfully decoded.
     Atomic<bool> mIsHardwareAccelerated;
     // Sample format monitoring.
     uint32_t mLastStreamSourceID;
     Maybe<uint32_t> mNextStreamSourceID;