Bug 236884

Created attachment 452642 [details]
Patch

Created attachment 452643 [details]
Patch

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].

<rdar://problem/89307315>

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.

Re-opened since this is blocked by bug 237326

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

(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.

(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.

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.

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

(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.

(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 :)