Bug 1450845 - MediaDecoderStateMachine now ignores SeekToNextFrame if already seeking. r?jya draft
authorBryce Van Dyk <bvandyk@mozilla.com>
Wed, 06 Jun 2018 15:17:30 -0400
changeset 805274 6fe8aac5584e7bd04ae934e6987070f5e0356c16
parent 801403 5866d6685849311f057e7e229b9ace63a2641c29
child 805275 26e58eee1cb26075427b222ba1ccc61c0652a621
push id112612
push userbvandyk@mozilla.com
push dateThu, 07 Jun 2018 16:04:45 +0000
reviewersjya
bugs1450845, 1410225
milestone62.0a1
Bug 1450845 - MediaDecoderStateMachine now ignores SeekToNextFrame if already seeking. r?jya SeekToNextFrame is handled differently than other seeks by the MediaDecoderStateMachine, and should not take place while other seeks already are. Bug 1410225 implemented some changes in HTMLMediaElement to prevent this, but it's still possible to move to a seeking state in the MDSM and accept SeekToNextFrame (as in this bug). This changeset changes the MDSM to reject SeekToNextFrame if a seek is already happening. Since the MDSM now does this the changes from bug 1410225 can be removed. This has the functional change of the promise from SeekToNextFrame being rejected if the seek in not performed due to another seek. Previously the promise would succeed when the other seek completed. This seems sensible as the next frame seek does not actually take place. MozReview-Commit-ID: HD9WRFq3LZV
dom/html/HTMLMediaElement.cpp
dom/media/MediaDecoderStateMachine.cpp
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -2707,22 +2707,16 @@ HTMLMediaElement::FastSeek(double aTime,
   LOG(LogLevel::Debug, ("Reporting telemetry VIDEO_FASTSEEK_USED"));
   Telemetry::Accumulate(Telemetry::VIDEO_FASTSEEK_USED, 1);
   RefPtr<Promise> tobeDropped = Seek(aTime, SeekTarget::PrevSyncPoint, aRv);
 }
 
 already_AddRefed<Promise>
 HTMLMediaElement::SeekToNextFrame(ErrorResult& aRv)
 {
-  if (mSeekDOMPromise) {
-    // We can't perform NextFrameSeek while seek is already in action.
-    // Just return the pending seek promise.
-    return do_AddRef(mSeekDOMPromise);
-  }
-
   /* This will cause JIT code to be kept around longer, to help performance
    * when using SeekToNextFrame to iterate through every frame of a video.
    */
   nsPIDOMWindowInner* win = OwnerDoc()->GetInnerWindow();
 
   if (win) {
     if (JSObject* obj = win->AsGlobal()->GetGlobalJSObject()) {
       js::NotifyAnimationActivity(obj);
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -868,16 +868,31 @@ public:
     // Do nothing since we want a valid video frame to show when seek is done.
   }
 
   void HandleResumeVideoDecoding(const TimeUnit&) override
   {
     // Do nothing. We will resume video decoding in the decoding state.
   }
 
+  // We specially handle next frame seeks by ignoring them if we're already
+  // seeking.
+  RefPtr<MediaDecoder::SeekPromise> HandleSeek(SeekTarget aTarget) override
+  {
+    if (aTarget.IsNextFrame()) {
+      // We ignore next frame seeks if we already have a seek pending
+      SLOG("Already SEEKING, ignoring seekToNextFrame");
+      MOZ_ASSERT(!mSeekJob.mPromise.IsEmpty(), "Seek shouldn't be finished");
+      return MediaDecoder::SeekPromise::CreateAndReject(/* aIgnored = */ true,
+                                                        __func__);
+    }
+
+    return StateObject::HandleSeek(aTarget);
+  }
+
 protected:
   SeekJob mSeekJob;
   EventVisibility mVisibility;
 
   virtual void DoSeek() = 0;
   // Transition to the next state (defined by the subclass) when seek is completed.
   virtual void GoToNextState() { SetState<DecodingState>(); }
   void SeekCompleted();