Bug 1499903 - part1 : correct the events order when we're in the seamless looping. r=chunmin
authoralwu <alwu@mozilla.com>
Fri, 26 Oct 2018 23:21:00 +0000
changeset 443225 3f6d3c425b42d0b916cc6b05514bde45faf46753
parent 443224 b5e9f8e954d67e039597495016dc8a54fc38452f
child 443226 106752b77131ea619aa9ef1cba4e48ba1177fc28
push id34944
push userncsoregi@mozilla.com
push dateSat, 27 Oct 2018 09:49:55 +0000
treeherdermozilla-central@49d47a692ca4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschunmin
bugs1499903
milestone65.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 1499903 - part1 : correct the events order when we're in the seamless looping. r=chunmin When the media which has `loop` attribute is playing to the end, the spec mentions that media should do seek to the start position [1]. During seeking, the dispatched events order [2] for MediaElement should be 1. seeking 2. timeupdate 3. seeked [1] https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:attr-media-loop-2 [2] https://html.spec.whatwg.org/multipage/media.html#seeking:dom-media-seek Differential Revision: https://phabricator.services.mozilla.com/D9324
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -114,16 +114,47 @@ public:
     DecodersArray& decoders = Decoders();
     decoders.RemoveElement(aDecoder);
     if (decoders.IsEmpty()) {
       sUniqueInstance = nullptr;
     }
   }
 };
 
+// When media is looping back to the head position, the spec [1] mentions that
+// MediaElement should dispatch `seeking` first, `timeupdate`, and `seeked` in
+// the end. This guard should be created before we fire `timeupdate` so that it
+// can ensure the event order.
+// [1]
+// https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:attr-media-loop-2
+// https://html.spec.whatwg.org/multipage/media.html#seeking:dom-media-seek
+class MOZ_RAII SeekEventsGuard
+{
+public:
+  explicit SeekEventsGuard(MediaDecoderOwner* aOwner, bool aIsLoopingBack)
+    : mOwner(aOwner)
+    , mIsLoopingBack(aIsLoopingBack)
+  {
+    MOZ_ASSERT(mOwner);
+    if (mIsLoopingBack) {
+      mOwner->SeekStarted();
+    }
+  }
+
+  ~SeekEventsGuard() {
+    MOZ_ASSERT(mOwner);
+    if (mIsLoopingBack) {
+      mOwner->SeekCompleted();
+    }
+  }
+private:
+  MediaDecoderOwner* mOwner;
+  bool mIsLoopingBack;
+};
+
 StaticRefPtr<MediaMemoryTracker> MediaMemoryTracker::sUniqueInstance;
 
 LazyLogModule gMediaTimerLog("MediaTimer");
 
 constexpr TimeUnit MediaDecoder::DEFAULT_NEXT_FRAME_AVAILABLE_BUFFERED;
 
 void
 MediaDecoder::InitStatics()
@@ -373,20 +404,16 @@ MediaDecoder::OnPlaybackEvent(MediaPlayb
 {
   switch (aEvent.mType) {
     case MediaPlaybackEvent::PlaybackEnded:
       PlaybackEnded();
       break;
     case MediaPlaybackEvent::SeekStarted:
       SeekingStarted();
       break;
-    case MediaPlaybackEvent::Loop:
-      GetOwner()->DispatchAsyncEvent(NS_LITERAL_STRING("seeking"));
-      GetOwner()->DispatchAsyncEvent(NS_LITERAL_STRING("seeked"));
-      break;
     case MediaPlaybackEvent::Invalidate:
       Invalidate();
       break;
     case MediaPlaybackEvent::EnterVideoSuspend:
       GetOwner()->DispatchAsyncEvent(NS_LITERAL_STRING("mozentervideosuspend"));
       break;
     case MediaPlaybackEvent::ExitVideoSuspend:
       GetOwner()->DispatchAsyncEvent(NS_LITERAL_STRING("mozexitvideosuspend"));
@@ -799,22 +826,21 @@ MediaDecoder::NotifyPrincipalChanged()
 }
 
 void
 MediaDecoder::OnSeekResolved()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
   AbstractThread::AutoEnter context(AbstractMainThread());
-  mSeekRequest.Complete();
-
   mLogicallySeeking = false;
 
   // Ensure logical position is updated after seek.
   UpdateLogicalPositionInternal();
+  mSeekRequest.Complete();
 
   GetOwner()->SeekCompleted();
   GetOwner()->AsyncResolveSeekDOMPromiseIfExists();
 }
 
 void
 MediaDecoder::OnSeekRejected()
 {
@@ -850,27 +876,39 @@ MediaDecoder::ChangeState(PlayState aSta
 
   if (mPlayState == PLAY_STATE_PLAYING) {
     GetOwner()->ConstructMediaTracks(mInfo);
   } else if (IsEnded()) {
     GetOwner()->RemoveMediaTracks();
   }
 }
 
+bool
+MediaDecoder::IsLoopingBack(double aPrevPosition, double aCurPosition) const
+{
+  // If current position is early than previous position and we didn't do seek,
+  // that means we looped back to the start position.
+  return mLooping &&
+         !mSeekRequest.Exists() &&
+         aCurPosition < aPrevPosition;
+}
+
 void
 MediaDecoder::UpdateLogicalPositionInternal()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
 
   double currentPosition = CurrentPosition().ToSeconds();
   if (mPlayState == PLAY_STATE_ENDED) {
     currentPosition = std::max(currentPosition, mDuration);
   }
   bool logicalPositionChanged = mLogicalPosition != currentPosition;
+  SeekEventsGuard guard(GetOwner(),
+                        IsLoopingBack(mLogicalPosition, currentPosition));
   mLogicalPosition = currentPosition;
   DDLOG(DDLogCategory::Property, "currentTime", mLogicalPosition);
 
   // Invalidate the frame so any video data is displayed.
   // Do this before the timeupdate event so that if that
   // event runs JavaScript that queries the media size, the
   // frame has reflowed and the size updated beforehand.
   Invalidate();
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -289,16 +289,19 @@ private:
   }
 
   layers::ImageContainer* GetImageContainer();
 
   // Fire timeupdate events if needed according to the time constraints
   // outlined in the specification.
   void FireTimeUpdate();
 
+  // True if we're going to loop back to the head position when media is in looping.
+  bool IsLoopingBack(double aPrevPosition, double aCurPosition) const;
+
   // Returns true if we can play the entire media through without stopping
   // to buffer, given the current download and playback rates.
   bool CanPlayThrough();
 
   // Called from HTMLMediaElement when owner document activity changes
   virtual void SetElementVisibility(bool aIsDocumentVisible,
                                     Visibility aElementVisibility,
                                     bool aIsElementInTree);
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -2350,30 +2350,17 @@ DecodingState::Step()
   // Start playback if necessary so that the clock can be properly queried.
   if (!mIsPrerolling) {
     mMaster->MaybeStartPlayback();
   }
 
   TimeUnit before = mMaster->GetMediaTime();
   mMaster->UpdatePlaybackPositionPeriodically();
 
-  // Fire the `seeking` and `seeked` events to meet the HTML spec
-  // when the media is looped back from the end to the beginning.
-  if (before > mMaster->GetMediaTime()) {
-    MOZ_ASSERT(mMaster->mLooping);
-    mMaster->mOnPlaybackEvent.Notify(MediaPlaybackEvent::Loop);
-  // After looping is cancelled, the time won't be corrected, and therefore we
-  // can check it to see if the end of the media track is reached. Make sure
-  // the media is started before comparing the time, or it's meaningless.
-  // Without checking IsStarted(), the media will be terminated immediately
-  // after seeking forward. When the state is just transited from seeking state,
-  // GetClock() is smaller than GetMediaTime(), since GetMediaTime() is updated
-  // upon seek is completed while GetClock() will be updated after the media is
-  // started again.
-  } else if (mMaster->mMediaSink->IsStarted() && !mMaster->mLooping) {
+  if (mMaster->mMediaSink->IsStarted() && !mMaster->mLooping) {
     TimeUnit adjusted = mMaster->GetClock();
     Reader()->AdjustByLooping(adjusted);
     if (adjusted < before) {
       mMaster->StopPlayback();
       mMaster->mAudioDataRequest.DisconnectIfExists();
       AudioQueue().Finish();
       mMaster->mAudioCompleted = true;
       SetState<CompletedState>();
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -118,17 +118,16 @@ struct MediaPlaybackEvent
 {
   enum EventType
   {
     PlaybackStarted,
     PlaybackStopped,
     PlaybackProgressed,
     PlaybackEnded,
     SeekStarted,
-    Loop,
     Invalidate,
     EnterVideoSuspend,
     ExitVideoSuspend,
     StartVideoSuspendTimer,
     CancelVideoSuspendTimer,
     VideoOnlySeekBegin,
     VideoOnlySeekCompleted,
   } mType;