Bug 1378085 P0 - update playback position only if event visibility is observable; r?jwwang draft
authorKaku Kuo <kaku@mozilla.com>
Thu, 06 Jul 2017 10:59:16 +0800
changeset 604561 4137f5141c407930aec9bdba93a701bc289cbec9
parent 603705 fef489e8c2a193dde885adc48deb74cc883a5881
child 604562 2dbd5366f3b01e5cf283917a3fb6b96323a9635b
push id67123
push userbmo:kaku@mozilla.com
push dateThu, 06 Jul 2017 03:57:15 +0000
reviewersjwwang
bugs1378085
milestone56.0a1
Bug 1378085 P0 - update playback position only if event visibility is observable; r?jwwang MozReview-Commit-ID: 4G436mptS1w
dom/media/MediaDecoderStateMachine.cpp
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -952,43 +952,44 @@ class MediaDecoderStateMachine::SeekingS
 {
 public:
   explicit SeekingState(Master* aPtr) : StateObject(aPtr) { }
 
   RefPtr<MediaDecoder::SeekPromise> Enter(SeekJob&& aSeekJob,
                                           EventVisibility aVisibility)
   {
     mSeekJob = Move(aSeekJob);
+    mVisibility = aVisibility;
 
     // Always switch off the blank decoder otherwise we might become visible
     // in the middle of seeking and won't have a valid video frame to show
     // when seek is done.
     if (mMaster->mVideoDecodeSuspended) {
       mMaster->mVideoDecodeSuspended = false;
       mMaster->mOnPlaybackEvent.Notify(MediaEventType::ExitVideoSuspend);
       Reader()->SetVideoBlankDecode(false);
     }
 
     // Dispatch a mozvideoonlyseekbegin event to indicate UI for corresponding
     // changes.
     if (mSeekJob.mTarget->IsVideoOnly()) {
       mMaster->mOnPlaybackEvent.Notify(MediaEventType::VideoOnlySeekBegin);
     }
 
-    // Don't stop playback for a video-only seek since audio is playing.
-    if (!mSeekJob.mTarget->IsVideoOnly()) {
+    // Suppressed visibility comes from two cases: (1) leaving dormant state,
+    // and (2) resuming suspended video decoder. We want both cases to be
+    // transparent to the user. So we only notify the change when the seek
+    // request is from the user.
+    if (mVisibility == EventVisibility::Observable) {
+      // Don't stop playback for a video-only seek since we want to keep playing
+      // audio and we don't need to stop playback while leaving dormant for the
+      // playback should has been stopped.
       mMaster->StopPlayback();
-    }
-
-    mMaster->UpdatePlaybackPositionInternal(mSeekJob.mTarget->GetTime());
-
-    if (aVisibility == EventVisibility::Observable) {
+      mMaster->UpdatePlaybackPositionInternal(mSeekJob.mTarget->GetTime());
       mMaster->mOnPlaybackEvent.Notify(MediaEventType::SeekStarted);
-      // We want dormant actions to be transparent to the user.
-      // So we only notify the change when the seek request is from the user.
       mMaster->UpdateNextFrameStatus(
         MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_SEEKING);
     }
 
     RefPtr<MediaDecoder::SeekPromise> p = mSeekJob.mPromise.Ensure(__func__);
 
     DoSeek();
 
@@ -1016,16 +1017,17 @@ public:
   void HandleResumeVideoDecoding(const TimeUnit&) override
   {
     // We set mVideoDecodeSuspended to false in Enter().
     MOZ_ASSERT(false, "Shouldn't have suspended video decoding.");
   }
 
 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();
   virtual TimeUnit CalculateNewCurrentTime() const = 0;
 };
 
@@ -2469,17 +2471,21 @@ SeekingState::SeekCompleted()
 
   // Notify FirstFrameLoaded now if we haven't since we've decoded some data
   // for readyState to transition to HAVE_CURRENT_DATA and fire 'loadeddata'.
   if (!mMaster->mSentFirstFrameLoadedEvent) {
     mMaster->FinishDecodeFirstFrame();
   }
 
   // Ensure timestamps are up to date.
-  if (!target.IsVideoOnly()) {
+  // Suppressed visibility comes from two cases: (1) leaving dormant state,
+  // and (2) resuming suspended video decoder. We want both cases to be
+  // transparent to the user. So we only notify the change when the seek
+  // request is from the user.
+  if (mVisibility == EventVisibility::Observable) {
     // Don't update playback position for video-only seek.
     // Otherwise we might have |newCurrentTime > mMediaSink->GetPosition()|
     // and fail the assertion in GetClock() since we didn't stop MediaSink.
     mMaster->UpdatePlaybackPositionInternal(newCurrentTime);
   }
 
   // Dispatch an event so that the UI can change in response to the end of
   // video-only seek