Skip to content

[MSE][GStreamer] Make fake preroll asynchronous #47446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eocanha
Copy link
Contributor

@eocanha eocanha commented Jul 1, 2025

2a0710d

[MSE][GStreamer] Make fake preroll asynchronous
https://bugs.webkit.org/show_bug.cgi?id=295289

Reviewed by NOBODY (OOPS!).

With the current implementation the whole seek flow as executed in a single main loop cycle. As a result, the JS app had no chance to spot that video.seeking attribute is ever set to true. Also some apps listen to video.onseeking and expects that video.seeking attribute to be true in such case. That wasn't a valid assumption.

Also, the behaviour isn't the same in <video> elements that are audio-only as in <audio> elements.

This happens on the Amlogic platform, that doesn't have asynchronous state changes on its audio sink. Spotify is broken there because of this problem.

See:
WebPlatformForEmbedded/WPEWebKit#1232
WebPlatformForEmbedded/WPEWebKit#1527

This patch calls didPreroll() also for <video> elements that are audio-only. Also makes that call async by putting it on the HTML media element task queue to make sure it is executed after dispatching 'seeking' event to JS so the
app has a chance to notice that HTMLmedia.seeking attribute is 'true'.

Original author: Andrzej Surdej <[email protected]>

* Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:
(WebCore::MediaPlayerPrivateGStreamerMSE::doSeek): Find the actual final audio sink. Call didPreroll() asynchronously for any media element having only audio.

2a0710d

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@eocanha eocanha requested a review from philn as a code owner July 1, 2025 17:19
@eocanha eocanha self-assigned this Jul 1, 2025
@eocanha eocanha added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jul 1, 2025
https://bugs.webkit.org/show_bug.cgi?id=295289

Reviewed by NOBODY (OOPS!).

With the current implementation the whole seek flow as executed in a single main loop cycle. As a result, the JS app had no chance to spot that video.seeking attribute is ever set to true. Also some apps listen to video.onseeking and expects that video.seeking attribute to be true in such case. That wasn't a valid assumption.

Also, the behaviour isn't the same in <video> elements that are audio-only as in <audio> elements.

This happens on the Amlogic platform, that doesn't have asynchronous state changes on its audio sink. Spotify is broken there because of this problem.

See:
WebPlatformForEmbedded/WPEWebKit#1232
WebPlatformForEmbedded/WPEWebKit#1527

This patch calls didPreroll() also for <video> elements that are audio-only. Also makes that call async by putting it on the HTML media element task queue to make sure it is executed after dispatching 'seeking' event to JS so the
app has a chance to notice that HTMLmedia.seeking attribute is 'true'.

Original author: Andrzej Surdej <[email protected]>

* Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:
(WebCore::MediaPlayerPrivateGStreamerMSE::doSeek): Find the actual final audio sink. Call didPreroll() asynchronously for any media element having only audio.
@eocanha eocanha removed the merging-blocked Applied to prevent a change from being merged label Jul 3, 2025
@eocanha eocanha force-pushed the metro-issue-1527 branch from c446216 to 2a0710d Compare July 3, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants