Bug 254399 - [MSE] video's readyState keeps incorrectly switching between HAVE_CURRENT_DATA and HAVE_METADATA
Summary: [MSE] video's readyState keeps incorrectly switching between HAVE_CURRENT_DAT...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-03-24 02:09 PDT by Jean-Yves Avenard [:jya]
Modified: 2023-03-25 04:55 PDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Yves Avenard [:jya] 2023-03-24 02:09:06 PDT
Problem exposed with media/media-source/media-source-monitor-source-buffers.html test with MockMediaSource running in the GPU Process (bug 225367).

The test performs an appendBuffer and wait for the video element to fire `waiting` and then make sure that readyState is HTMLMediaElement.HAVE_CURRENT_DATA

Per spec, when the readyState moves from HAVE_FUTURE_DATA to HAVE_CURRENT_DATA and lower, the video element must fire the waiting event [1]
```
If the previous ready state was HAVE_FUTURE_DATA or more, and the new ready state is HAVE_CURRENT_DATA or less

    If the media element was potentially playing before its readyState attribute changed to a value lower than HAVE_FUTURE_DATA, and the element has not ended playback, and playback has not stopped due to errors, paused for user interaction, or paused for in-band content, the user agent must queue a media element task given the media element to fire an event named timeupdate at the element, and queue a media element task given the media element to fire an event named waiting at the element.
```

The test fails because video.readyState is HAVE_METADATA only.

Tracing the code, and placing a breakpoint on the `waiting` event handler, we can see that the readyState value keeps oscillating between HAVE_CURRENT_DATA and HAVE_METADATA.

The issue is a consequence to the changes made in bug 225396.
In order to reduce how often the HTMLMediaElement running in the content process queries the MediaPlayerPrivate for the current media time, it simulate the progress of currentTime by using the wall clock instead.
At regular time, the MediaPlayerPrivate in the GPU Process will update the time position through a call to `mediaplayerprivateremote::currentTimeChanged(const MediaTime& mediaTime, const MonotonicTime& queryTime, bool timeIsProgressing)`

the value `timeIsProgressing` is based on the readySate [3]
```
bool RemoteMediaPlayerProxy::mediaPlayerPausedOrStalled() const
{
    return m_player->paused() || m_player->readyState() < MediaPlayer::ReadyState::HaveFutureData;
}
```

However, due the to current architecture on how the HTMLMediaElement [CP] communicate with the MediaPlayerPrivate [GPUP] the time is dissociated from the readySate and the readyState in the GPUP is updated asynchronously.
When the player reaches the end of the buffered range, it sends an update to the CP via `currentTimeChanged` at this time readyState is HAVE_FUTURE_DATA as it hasn't been updated yet. So timeIsProgressing is set to true.

The HTMLMediaElement then pause the player [4].
The MediaPlayerPrivateRemote::pause() will adjust the time as if it had progressed since the last time the time was read.
```
m_cachedMediaTime += MediaTime::createWithDouble(m_rate * (now - m_cachedMediaTimeQueryTime).value());
```

Depending on how long it took for the HTMLMediaElement to process the previous time change, the delay will always be > 0 and so m_cachedMediaTime is now greater than the buffered range.

Upon the next run of the Media Source sourceBuffer monitoring [5]:
```
        If HTMLMediaElement.buffered does not contain a TimeRanges for the current playback position:

        Set the HTMLMediaElement.readyState attribute to HAVE_METADATA.
        Note

        Per HTMLMediaElement ready states [HTML] logic, HTMLMediaElement.readyState changes may trigger events on the HTMLMediaElement.
```

the readyState is now set to HAVE_METADATA and sent to the MediaPlayer in the GPUP. Which will send again the time back to the content process where as the time appear to have progressed (even though it went backward).
the HTMLMediaElement unpause the MediaPlayer and in the sourceBuffer monitoring we have:
```
        If HTMLMediaElement.buffered contains a TimeRanges that ends at the current playback position and does not have a range covering the time immediately after the current position:

        Set the HTMLMediaElement.readyState attribute to HAVE_CURRENT_DATA.
```

and this is send back to the GPUP.
And we repeat from the steps above.
readyState moves between HAVE_METADATA and HAVE_CURRENT_DATA forever.


[1] https://html.spec.whatwg.org/multipage/media.html#concept-media-load-resource
[2] https://searchfox.org/wubkat/rev/c54bd36a7abe6dc4dc91898fbec13f659ad010ba/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp#416
[3] https://searchfox.org/wubkat/rev/c54bd36a7abe6dc4dc91898fbec13f659ad010ba/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp#914-922
[4] https://searchfox.org/wubkat/rev/c54bd36a7abe6dc4dc91898fbec13f659ad010ba/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp#221-228
[5] https://w3c.github.io/media-source/#buffer-monitoring
Comment 1 Radar WebKit Bug Importer 2023-03-24 02:09:59 PDT
<rdar://problem/107178961>
Comment 2 Radar WebKit Bug Importer 2023-03-24 02:10:33 PDT
<rdar://problem/107178976>
Comment 3 Jean-Yves Avenard [:jya] 2023-03-24 02:11:30 PDT
In addition; currentTime will keep moving past the end of the buffered range and back
Comment 4 Jean-Yves Avenard [:jya] 2023-03-24 03:13:25 PDT
Pull request: https://github.com/WebKit/WebKit/pull/11912
Comment 5 Jean-Yves Avenard [:jya] 2023-03-25 02:57:07 PDT
Pull request: https://github.com/WebKit/WebKit/pull/11965
Comment 6 EWS 2023-03-25 04:55:53 PDT
Committed 262112@main (f78ee0bea723): <https://commits.webkit.org/262112@main>

Reviewed commits have been landed. Closing PR #11912 and removing active labels.