Bug 1686696 - part2 : rework how does decoder handle owner's events when the logical position changes. r=bryce

Decoder needs to handle owner's events differently based on different situations, this patch makes the process clearer.

Differential Revision: https://phabricator.services.mozilla.com/D102674

dom/html/HTMLMediaElement.cpp

@@ -5381,6 +5381,9 @@ void HTMLMediaElement::SeekCompleted() {
   if (mTextTrackManager) {
     mTextTrackManager->DidSeek();
   }
+  // https://html.spec.whatwg.org/multipage/media.html#seeking:dom-media-seek
+  // (Step 16)
+  // TODO (bug 1688131): run these steps in a stable state.
   FireTimeUpdate(TimeupdateType::eMandatory);
   DispatchAsyncEvent(u"seeked"_ns);
   // We changed whether we're seeking so we need to AddRemoveSelfReference

dom/media/MediaDecoder.cpp

@@ -149,35 +149,6 @@ class MediaMemoryTracker : public nsIMemoryReporter {
   }
 };
 
-// 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;
 
 RefPtr<MediaMemoryPromise> GetMediaMemorySizes() {
@@ -897,10 +868,17 @@ void MediaDecoder::UpdateTelemetryHelperBasedOnPlayState(
   }
 }
 
-bool MediaDecoder::IsLoopingBack(double aPrevPos, double aCurPos) 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() && aCurPos < aPrevPos;
+MediaDecoder::PositionUpdate MediaDecoder::GetPositionUpdateReason(
+    double aPrevPos, double aCurPos) const {
+  MOZ_ASSERT(NS_IsMainThread());
+  // If current position is earlier than previous position and we didn't do
+  // seek, that means we looped back to the start position, which currently
+  // happens on audio only.
+  if (mLooping && !mSeekRequest.Exists() && aCurPos < aPrevPos) {
+    return PositionUpdate::eSeamlessLoopingSeeking;
+  }
+  return aPrevPos != aCurPos ? PositionUpdate::ePeriodicUpdate
+                             : PositionUpdate::eOther;
 }
 
 void MediaDecoder::UpdateLogicalPositionInternal() {
@@ -911,21 +889,46 @@ void MediaDecoder::UpdateLogicalPositionInternal() {
   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);
+
+  const PositionUpdate reason =
+      GetPositionUpdateReason(mLogicalPosition, currentPosition);
+  switch (reason) {
+    case PositionUpdate::ePeriodicUpdate:
+      SetLogicalPosition(currentPosition);
+      // This is actually defined in `TimeMarchesOn`, but we do that in decoder.
+      // https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:event-media-timeupdate-7
+      // TODO (bug 1688137): should we move it back to `TimeMarchesOn`?
+      GetOwner()->MaybeQueueTimeupdateEvent();
+      break;
+    case PositionUpdate::eSeamlessLoopingSeeking:
+      // When seamless seeking occurs, seeking was performed on the demuxer so
+      // the decoder doesn't know. That means decoder still thinks it's in
+      // playing. Therefore, we have to manually call those methods to notify
+      // the owner about seeking.
+      GetOwner()->SeekStarted();
+      SetLogicalPosition(currentPosition);
+      GetOwner()->SeekCompleted();
+      break;
+    default:
+      MOZ_ASSERT(reason == PositionUpdate::eOther);
+      SetLogicalPosition(currentPosition);
+      break;
+  }
 
   // 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();
+}
 
-  if (logicalPositionChanged) {
-    FireTimeUpdate();
+void MediaDecoder::SetLogicalPosition(double aNewPosition) {
+  MOZ_ASSERT(NS_IsMainThread());
+  if (mLogicalPosition == aNewPosition) {
+    return;
   }
+  mLogicalPosition = aNewPosition;
+  DDLOG(DDLogCategory::Property, "currentTime", mLogicalPosition);
 }
 
 void MediaDecoder::DurationChanged() {
@@ -1257,12 +1260,6 @@ MediaDecoderStateMachine* MediaDecoder::GetStateMachine() const {
   return mDecoderStateMachine;
 }
 
-void MediaDecoder::FireTimeUpdate() {
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
-  GetOwner()->MaybeQueueTimeupdateEvent();
-}
-
 bool MediaDecoder::CanPlayThrough() {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());

dom/media/MediaDecoder.h

@@ -294,14 +294,6 @@ class MediaDecoder : public DecoderDoctorLifeLogger<MediaDecoder> {
 
   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 aPrevPos, double aCurPos) const;
-
   // Returns true if we can play the entire media through without stopping
   // to buffer, given the current download and playback rates.
   bool CanPlayThrough();
@@ -450,6 +442,8 @@ class MediaDecoder : public DecoderDoctorLifeLogger<MediaDecoder> {
                               UniquePtr<MetadataTags> aTags,
                               MediaDecoderEventVisibility aEventVisibility);
 
+  void SetLogicalPosition(double aNewPosition);
+
   /******
    * The following members should be accessed with the decoder lock held.
    ******/
@@ -723,6 +717,19 @@ class MediaDecoder : public DecoderDoctorLifeLogger<MediaDecoder> {
   double GetVideoDecodeSuspendedTimeInSeconds() const;
 
  private:
+  /**
+   * This enum describes the reason why we need to update the logical position.
+   * ePeriodicUpdate : the position grows periodically during playback
+   * eSeamlessLoopingSeeking : the position changes due to demuxer level seek.
+   * eOther : due to normal seeking or other attributes changes, eg. playstate
+   */
+  enum class PositionUpdate {
+    ePeriodicUpdate,
+    eSeamlessLoopingSeeking,
+    eOther,
+  };
+  PositionUpdate GetPositionUpdateReason(double aPrevPos, double aCurPos) const;
+
   // Notify owner when the audible state changed
   void NotifyAudibleStateChanged();