Bug 1577505 - Progress currentTime for a MediaStream only while potentially playing. r=jib
authorAndreas Pehrson <apehrson@mozilla.com>
Mon, 04 Nov 2019 13:43:01 +0000
changeset 500373 a81df2957c097309ea213774fe83adfa629b0a6f
parent 500372 8529cff7ba28966df1cb83213576659a06dd0a6f
child 500374 f88700daff3a2eaeadeab2b5bc2cf5b1cbc3fe57
push id114164
push useraiakab@mozilla.com
push dateTue, 05 Nov 2019 10:06:15 +0000
treeherdermozilla-inbound@4d585c7edc76 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib
bugs1577505
milestone72.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 1577505 - Progress currentTime for a MediaStream only while potentially playing. r=jib It started progressing as soon as we set up the rendering of the tracks in the stream, which is too early according to the HTMLMediaElement spec. Now it starts progressing when we're potentially playing. The difference being that we now wait for mReadyState to go beyond HAVE_CURRENT_DATA before hooking up the time progression mechanism. Differential Revision: https://phabricator.services.mozilla.com/D49354
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -434,37 +434,62 @@ class HTMLMediaElement::MediaStreamRende
 
   MediaStreamRenderer(AbstractThread* aMainThread,
                       VideoFrameContainer* aVideoContainer,
                       void* aAudioOutputKey)
       : mVideoContainer(aVideoContainer),
         mAudioOutputKey(aAudioOutputKey),
         mWatchManager(this, aMainThread) {}
 
+  void Shutdown() {
+    for (const auto& t : nsTArray<WeakPtr<MediaStreamTrack>>(mAudioTracks)) {
+      if (t) {
+        RemoveTrack(t->AsAudioStreamTrack());
+      }
+    }
+    if (mVideoTrack) {
+      RemoveTrack(mVideoTrack->AsVideoStreamTrack());
+    }
+    mWatchManager.Shutdown();
+  }
+
   void UpdateGraphTime() {
     mGraphTime =
         mGraphTimeDummy->mTrack->Graph()->CurrentTime() - *mGraphTimeOffset;
   }
 
+  void SetProgressingCurrentTime(bool aProgress) {
+    if (aProgress == mProgressingCurrentTime) {
+      return;
+    }
+
+    MOZ_DIAGNOSTIC_ASSERT(mGraphTimeDummy);
+    mProgressingCurrentTime = aProgress;
+    MediaTrackGraph* graph = mGraphTimeDummy->mTrack->Graph();
+    if (mProgressingCurrentTime) {
+      mGraphTimeOffset = Some(graph->CurrentTime().Ref() - mGraphTime);
+      mWatchManager.Watch(graph->CurrentTime(),
+                          &MediaStreamRenderer::UpdateGraphTime);
+    } else {
+      mWatchManager.Unwatch(graph->CurrentTime(),
+                            &MediaStreamRenderer::UpdateGraphTime);
+    }
+  }
+
   void Start() {
     if (mRendering) {
       return;
     }
 
     mRendering = true;
 
     if (!mGraphTimeDummy) {
       return;
     }
 
-    MediaTrackGraph* graph = mGraphTimeDummy->mTrack->Graph();
-    mGraphTimeOffset = Some(graph->CurrentTime().Ref() - mGraphTime);
-    mWatchManager.Watch(graph->CurrentTime(),
-                        &MediaStreamRenderer::UpdateGraphTime);
-
     for (const auto& t : mAudioTracks) {
       if (t) {
         t->AsAudioStreamTrack()->AddAudioOutput(mAudioOutputKey);
         t->AsAudioStreamTrack()->SetAudioOutputVolume(mAudioOutputKey,
                                                       mAudioOutputVolume);
       }
     }
 
@@ -479,19 +504,16 @@ class HTMLMediaElement::MediaStreamRende
     }
 
     mRendering = false;
 
     if (!mGraphTimeDummy) {
       return;
     }
 
-    mWatchManager.Unwatch(mGraphTimeDummy->mTrack->Graph()->CurrentTime(),
-                          &MediaStreamRenderer::UpdateGraphTime);
-
     for (const auto& t : mAudioTracks) {
       if (t) {
         t->AsAudioStreamTrack()->RemoveAudioOutput(mAudioOutputKey);
       }
     }
 
     if (mVideoTrack) {
       mVideoTrack->AsVideoStreamTrack()->RemoveVideoOutput(mVideoContainer);
@@ -566,27 +588,19 @@ class HTMLMediaElement::MediaStreamRende
   // Set if we're rendering video.
   const RefPtr<VideoFrameContainer> mVideoContainer;
 
   // Set if we're rendering audio, nullptr otherwise.
   void* const mAudioOutputKey;
 
  private:
   ~MediaStreamRenderer() {
-    for (const auto& t : nsTArray<WeakPtr<MediaStreamTrack>>(mAudioTracks)) {
-      if (t) {
-        RemoveTrack(t->AsAudioStreamTrack());
-      }
-    }
-    if (mVideoTrack) {
-      RemoveTrack(mVideoTrack->AsVideoStreamTrack());
-    }
-
     MOZ_DIAGNOSTIC_ASSERT(mAudioTracks.IsEmpty());
     MOZ_DIAGNOSTIC_ASSERT(!mVideoTrack);
+    MOZ_DIAGNOSTIC_ASSERT(mWatchManager.IsShutdown());
   }
 
   void EnsureGraphTimeDummy() {
     if (mGraphTimeDummy) {
       return;
     }
 
     MediaTrackGraph* graph = nullptr;
@@ -603,28 +617,25 @@ class HTMLMediaElement::MediaStreamRende
 
     if (!graph) {
       return;
     }
 
     // This dummy keeps `graph` alive and ensures access to it.
     mGraphTimeDummy = MakeRefPtr<SharedDummyTrack>(
         graph->CreateSourceTrack(MediaSegment::AUDIO));
-
-    if (mRendering) {
-      mGraphTimeOffset = Some(graph->CurrentTime() - mGraphTime);
-      mWatchManager.Watch(graph->CurrentTime(),
-                          &MediaStreamRenderer::UpdateGraphTime);
-    }
   }
 
   // True when all tracks are being rendered, i.e., when the media element is
   // playing.
   bool mRendering = false;
 
+  // True while we're progressing mGraphTime. False otherwise.
+  bool mProgressingCurrentTime = false;
+
   // The audio output volume for all audio tracks.
   float mAudioOutputVolume = 1.0f;
 
   // WatchManager for mGraphTime.
   WatchManager<MediaStreamRenderer> mWatchManager;
 
   // A dummy MediaTrack to guarantee a MediaTrackGraph is kept alive while
   // we're actively rendering, so we can track the graph's current time. Set
@@ -2167,18 +2178,21 @@ void HTMLMediaElement::ResetState() {
   // There might be a pending MediaDecoder::PlaybackPositionChanged() which
   // will overwrite |mMediaInfo.mVideo.mDisplay| in UpdateMediaSize() to give
   // staled videoWidth and videoHeight. We have to call ForgetElement() here
   // such that the staled callbacks won't reach us.
   if (mVideoFrameContainer) {
     mVideoFrameContainer->ForgetElement();
     mVideoFrameContainer = nullptr;
   }
-  // mMediaStreamRenderer, has a strong reference to mVideoFrameContainer.
-  mMediaStreamRenderer = nullptr;
+  if (mMediaStreamRenderer) {
+    // mMediaStreamRenderer, has a strong reference to mVideoFrameContainer.
+    mMediaStreamRenderer->Shutdown();
+    mMediaStreamRenderer = nullptr;
+  }
 }
 
 void HTMLMediaElement::SelectResourceWrapper() {
   SelectResource();
   MaybeBeginCloningVisually();
   mIsRunningSelectResource = false;
   mHaveQueuedSelectResource = false;
   mIsDoingExplicitLoad = false;
@@ -4862,16 +4876,25 @@ void HTMLMediaElement::UpdateSrcMediaStr
         mSelectedVideoStreamTrack->AddVideoOutput(mFirstFrameListener);
       }
     }
 
     SetCapturedOutputStreamsEnabled(false);  // Mute
   }
 }
 
+void HTMLMediaElement::UpdateSrcStreamPotentiallyPlaying() {
+  if (!mMediaStreamRenderer) {
+    // Notifications are async, the renderer could have been cleared.
+    return;
+  }
+
+  mMediaStreamRenderer->SetProgressingCurrentTime(IsPotentiallyPlaying());
+}
+
 void HTMLMediaElement::UpdateSrcStreamTime() {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mSrcStreamPlaybackEnded) {
     // We do a separate FireTimeUpdate() when this is set.
     return;
   }
 
@@ -4881,21 +4904,28 @@ void HTMLMediaElement::UpdateSrcStreamTi
 void HTMLMediaElement::SetupSrcMediaStreamPlayback(DOMMediaStream* aStream) {
   NS_ASSERTION(!mSrcStream && !mFirstFrameListener,
                "Should have been ended already");
 
   mSrcStream = aStream;
 
   mMediaStreamRenderer = MakeAndAddRef<MediaStreamRenderer>(
       mAbstractMainThread, GetVideoFrameContainer(), this);
+  mWatchManager.Watch(mPaused,
+                      &HTMLMediaElement::UpdateSrcStreamPotentiallyPlaying);
+  mWatchManager.Watch(mReadyState,
+                      &HTMLMediaElement::UpdateSrcStreamPotentiallyPlaying);
+  mWatchManager.Watch(mSrcStreamPlaybackEnded,
+                      &HTMLMediaElement::UpdateSrcStreamPotentiallyPlaying);
   mWatchManager.Watch(mMediaStreamRenderer->CurrentGraphTime(),
                       &HTMLMediaElement::UpdateSrcStreamTime);
   SetVolumeInternal();
 
   UpdateSrcMediaStreamPlaying();
+  UpdateSrcStreamPotentiallyPlaying();
   mSrcStreamVideoPrincipal = NodePrincipal();
 
   // If we pause this media element, track changes in the underlying stream
   // will continue to fire events at this element and alter its track list.
   // That's simpler than delaying the events, but probably confusing...
   nsTArray<RefPtr<MediaStreamTrack>> tracks;
   mSrcStream->GetTracks(tracks);
   for (const RefPtr<MediaStreamTrack>& track : tracks) {
@@ -4921,18 +4951,25 @@ void HTMLMediaElement::EndSrcMediaStream
   }
   if (mFirstFrameListener) {
     mSelectedVideoStreamTrack->RemoveVideoOutput(mFirstFrameListener);
   }
   mSelectedVideoStreamTrack = nullptr;
   mFirstFrameListener = nullptr;
 
   if (mMediaStreamRenderer) {
+    mWatchManager.Unwatch(mPaused,
+                          &HTMLMediaElement::UpdateSrcStreamPotentiallyPlaying);
+    mWatchManager.Unwatch(mReadyState,
+                          &HTMLMediaElement::UpdateSrcStreamPotentiallyPlaying);
+    mWatchManager.Unwatch(mSrcStreamPlaybackEnded,
+                          &HTMLMediaElement::UpdateSrcStreamPotentiallyPlaying);
     mWatchManager.Unwatch(mMediaStreamRenderer->CurrentGraphTime(),
                           &HTMLMediaElement::UpdateSrcStreamTime);
+    mMediaStreamRenderer->Shutdown();
     mMediaStreamRenderer = nullptr;
   }
 
   mSrcStream->UnregisterTrackListener(mMediaStreamTrackListener.get());
   mMediaStreamTrackListener = nullptr;
   mSrcStreamTracksAvailable = false;
   mSrcStreamPlaybackEnded = false;
   mSrcStreamVideoPrincipal = nullptr;
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -817,16 +817,23 @@ class HTMLMediaElement : public nsGeneri
   void EndSrcMediaStreamPlayback();
   /**
    * Ensure we're playing mSrcStream if and only if we're not paused.
    */
   enum { REMOVING_SRC_STREAM = 0x1 };
   void UpdateSrcMediaStreamPlaying(uint32_t aFlags = 0);
 
   /**
+   * Ensure currentTime progresses if and only if we're potentially playing
+   * mSrcStream. Called by the watch manager while we're playing mSrcStream, and
+   * one of the inputs to the potentially playing algorithm changes.
+   */
+  void UpdateSrcStreamPotentiallyPlaying();
+
+  /**
    * mSrcStream's graph's CurrentTime() has been updated. It might be time to
    * fire "timeupdate".
    */
   void UpdateSrcStreamTime();
 
   /**
    * Called by our DOMMediaStream::TrackListener when a new MediaStreamTrack has
    * been added to the playback stream of |mSrcStream|.