Bug 236884 – [GStreamer] Switch media player to playbin3

• Component: Platform
• Status: REOPENED

[Philippe Normand]

MSE and Mediastream require playbin3. Regular playback uses playbin3 if GStreamer >= 1.20 is detected, unless the WEBKIT_GST_USE_PLAYBIN2 environment variable is set to 1.

[Philippe Normand]

Created attachment 452642 [details]
Patch

[Philippe Normand]

Created attachment 452643 [details]
Patch

[EWS]

Committed r290325 (247647@main): <https://commits.webkit.org/247647@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452643 [details].

[Radar WebKit Bug Importer]

<rdar://problem/89307315>

[Philippe Normand]

Comment on attachment 452643 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452643&action=review

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2671
> +    auto usePlaybin2 = usePlaybin2Override ? parseInteger<unsigned>(usePlaybin2Override) : 0;

Clearly I didn't test this properly, nice auto footgun. This is actually an optional<unsigned>.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2672
> +    if ((usePlaybin2 || !webkitGstCheckVersion(1, 20, 0)) && !isMediaSource() && !url.protocolIs("mediastream"))

so the usePlaybin2 test here always returns true. I'll follow-up in a new patch, there are layout test updates needed.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 237326

[Philippe Normand]

Pull request: https://github.com/WebKit/WebKit/pull/4007

[Philippe Normand]

(In reply to Philippe Normand from comment #7)
> Pull request: https://github.com/WebKit/WebKit/pull/4007

I've found a couple more playbin3 issues which make me think this is not production-ready yet, at least until 1.22:

- https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1419 which was closed actually. Download buffering is not runtime toggle-able in playbin3
- https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1562 URI redirects are not supported yet
- https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1418 some tests failing due to this one, though I think it's a wpt-specific corner case...

So I think we shouldn't flip the switch yet, it's too early.

[Philippe Normand]

(In reply to Philippe Normand from comment #8)
> (In reply to Philippe Normand from comment #7)
> > Pull request: https://github.com/WebKit/WebKit/pull/4007
> 
> I've found a couple more playbin3 issues which make me think this is not
> production-ready yet, at least until 1.22:
> 
> - https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1419 which was
> closed actually. Download buffering is not runtime toggle-able in playbin3

Nothing we can do about this.

> - https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1562 URI
> redirects are not supported yet

Fixed upstream (scheduled to ship in 1.24).

> - https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1418 some
> tests failing due to this one, though I think it's a wpt-specific corner
> case...
> 

MR fixing this: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4725

> So I think we shouldn't flip the switch yet, it's too early.

Playbin3 support for decoder/sinks is going to be another requirement.

[Philippe Normand]

AFAICT the last big blocker is https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4391

playbin3 in 1.24 is no longer advertised as experimental.

[Philippe Normand]

Pull request: https://github.com/WebKit/WebKit/pull/25429

[Xabier Rodríguez Calvar]

(In reply to Philippe Normand from comment #10)
> AFAICT the last big blocker is
> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4391
> 
> playbin3 in 1.24 is no longer advertised as experimental.

That PR is not landing. I witnessed Edward's review in person and he is not accepting that. So we'll have to live with the current custom solutions for the corresponding platforms.

[Philippe Normand]

(In reply to Xabier Rodríguez Calvar from comment #12)
> (In reply to Philippe Normand from comment #10)
> > AFAICT the last big blocker is
> > https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4391
> > 
> > playbin3 in 1.24 is no longer advertised as experimental.
> 
> That PR is not landing. I witnessed Edward's review in person and he is not
> accepting that. So we'll have to live with the current custom solutions for
> the corresponding platforms.

We could provide a playbin2 quirk for those :)