Skip to content

Commit

Permalink
REGRESSION (274442@main): [ MacOS wk2 ] media/media-source/media-sour…
Browse files Browse the repository at this point in the history
…ce-seek-complete.html is a flaky timeout

https://bugs.webkit.org/show_bug.cgi?id=269814
rdar://123338097

Reviewed by Jer Noble.

If a `currentTimeChanged` message between the GPUP and WP was mid-flight when a new seek operation was
started it would have set the current time to a stale value, so the time would appear to go backward
and the seek would never complete.

Prior 274442@main it *usually* didn't matter as the HTMLMediaElement cached the seeked time and would
partially hide the time going backward. Following this change however, the MediaPlayerPrivate became
the time reference, and so issues with the time being wrong became more problematic.
This issue could explain a series of intermittent failures we have seen ever since the MediaPlayer
was moved to the GPUP, including several changes made to prevent the time from ever going backward.

The explanation above likely explains on why we needed such workaround in the first place.

Also, before 274442@main if a seek was pending, it would have always returned the seek time when
querying MediaSource::currentTime; we changed it to return 0 whenever the MediaSource was closed.
We return to the original behaviour.

* LayoutTests/platform/mac-wk2/TestExpectations:
* Source/WebCore/Modules/mediasource/MediaSource.cpp:
(WebCore::MediaSource::currentTime const):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::playbackStateChanged): Add log.
(WebKit::MediaPlayerPrivateRemote::currentTimeChanged): return early if currently seeking. Add log

Canonical link: https://commits.webkit.org/275231@main
  • Loading branch information
jyavenard committed Feb 23, 2024
1 parent 4b665f0 commit 53463ea
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 6 deletions.
2 changes: 0 additions & 2 deletions LayoutTests/platform/mac-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2010,6 +2010,4 @@ webkit.org/b/269581 http/tests/site-isolation/mouse-events/context-menu-event-tw
[ Monterey+ ] http/tests/navigation/ping-attribute/area-cross-origin-from-https.html [ Pass Failure ]
[ Monterey+ ] http/tests/navigation/ping-attribute/area-cross-origin-from-https-UpgradeMixedContent.html [ Pass Failure ]

webkit.org/b/269814 [ Monterey+ ] media/media-source/media-source-seek-complete.html [ Pass Timeout ]

webkit.org/b/123404489 [ Monterey+ Debug ] imported/w3c/web-platform-tests/media-source/dedicated-worker/mediasource-worker-detach-element.html [ Skip ]
5 changes: 1 addition & 4 deletions Source/WebCore/Modules/mediasource/MediaSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,10 @@ MediaTime MediaSource::duration() const

MediaTime MediaSource::currentTime() const
{
if (isClosed())
return MediaTime::zeroTime();

if (m_pendingSeekTarget)
return m_pendingSeekTarget->time;

return m_private->currentTime();
return m_private ? m_private->currentTime() : MediaTime::zeroTime();
}

PlatformTimeRanges MediaSource::buffered() const
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ void MediaPlayerPrivateRemote::rateChanged(double rate)

void MediaPlayerPrivateRemote::playbackStateChanged(bool paused, MediaTime&& mediaTime, MonotonicTime&& wallTime)
{
INFO_LOG(LOGIDENTIFIER, mediaTime);
m_cachedState.paused = paused;
m_currentTimeEstimator.setTime(mediaTime, wallTime);
if (auto player = m_player.get())
Expand Down Expand Up @@ -517,6 +518,9 @@ void MediaPlayerPrivateRemote::sizeChanged(WebCore::FloatSize naturalSize)

void MediaPlayerPrivateRemote::currentTimeChanged(const MediaTime& mediaTime, const MonotonicTime& queryTime, bool timeIsProgressing)
{
INFO_LOG(LOGIDENTIFIER, mediaTime, " seeking:", bool(m_seeking));
if (m_seeking)
return;
auto oldCachedTime = m_currentTimeEstimator.cachedTime();
auto oldTimeIsProgressing = m_currentTimeEstimator.timeIsProgressing();
auto reverseJump = mediaTime < oldCachedTime;
Expand Down

0 comments on commit 53463ea

Please sign in to comment.