Bug 201824 - [MSE][GStreamer] Allow MSE on builds with gst>=1.14
Summary: [MSE][GStreamer] Allow MSE on builds with gst>=1.14
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alicia Boya García
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-16 09:53 PDT by Alicia Boya García
Modified: 2019-10-24 02:00 PDT (History)
14 users (show)

See Also:


Attachments
Patch (1.88 KB, patch)
2019-09-16 09:54 PDT, Alicia Boya García
mcatanzaro: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alicia Boya García 2019-09-16 09:53:13 PDT
The WebKitMediaSrc rework introduced usage of playbin3 for the
MSE player, which is not working as-is in 1.14, and therefore the
GStreamer requirement was bumped to 1.16.

playbin3 still has been found to be able to work in 1.14 with a simple
patch in gst-plugins-base, and being able to do this is important for
LTS distros, so we're lowering the requirement to 1.14 again.

This is the gst-plugins-base patch needed for playbin3 MSE to work in
WebKit: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/434
Comment 1 Alicia Boya García 2019-09-16 09:54:44 PDT
Created attachment 378866 [details]
Patch
Comment 2 Carlos Garcia Campos 2019-09-17 00:49:35 PDT
So, do we need to backport that patch to gst 1.14 branch? or should we notify distros to use that patch to make MSE work in webkit-2.26 with gst 1.14?
Comment 3 Alberto Garcia 2019-09-17 01:32:12 PDT
(In reply to Carlos Garcia Campos from comment #2)
> So, do we need to backport that patch to gst 1.14 branch? or should we
> notify distros to use that patch to make MSE work in webkit-2.26 with gst
> 1.14?

I'm not sure if having gst patched in order to be able to use webkit 2.26 is an option...

Or did I understand it incorrectly?
Comment 4 Carlos Garcia Campos 2019-09-17 01:36:31 PDT
(In reply to Alberto Garcia from comment #3)
> (In reply to Carlos Garcia Campos from comment #2)
> > So, do we need to backport that patch to gst 1.14 branch? or should we
> > notify distros to use that patch to make MSE work in webkit-2.26 with gst
> > 1.14?
> 
> I'm not sure if having gst patched in order to be able to use webkit 2.26 is
> an option...
> 
> Or did I understand it incorrectly?

Doesn't debian update gst in stable version? The patch is already in gst, we only need to backport it to 1.14 branch upstream
Comment 5 Alberto Garcia 2019-09-17 02:26:07 PDT
(In reply to Carlos Garcia Campos from comment #4)
> (In reply to Alberto Garcia from comment #3)
> Doesn't debian update gst in stable version? The patch is already in gst, we
> only need to backport it to 1.14 branch upstream

Yes, we can ask the patch to be backported, but I'm not sure if
requiring changes in another package for a new update to work is
something acceptable, e.g. someone could get webkitgtk 2.26.x via
security updates and not have a patched gst installed.
Comment 6 Adrian Perez 2019-09-17 03:07:27 PDT
(In reply to Alberto Garcia from comment #5)
> (In reply to Carlos Garcia Campos from comment #4)
> > (In reply to Alberto Garcia from comment #3)
> > Doesn't debian update gst in stable version? The patch is already in gst, we
> > only need to backport it to 1.14 branch upstream
> 
> Yes, we can ask the patch to be backported, but I'm not sure if
> requiring changes in another package for a new update to work is
> something acceptable, e.g. someone could get webkitgtk 2.26.x via
> security updates and not have a patched gst installed.

If we know that the distribution is willing to update the GStreamer
package, cannot the update to the WebKitGTK package depend on a
version equal or newer of the GStreamer package than the one that
included the fix?
Comment 7 Michael Catanzaro 2019-09-17 07:24:19 PDT
Comment on attachment 378866 [details]
Patch

This will break distros. You can't expect distros to know that a special patch has to be applied. You should require the three-digit version number that's needed for this to work. E.g. require 1.14.6. Sadly, GStreamer developers say 1.14 is dead and there will be no 1.14.6, so can't do that.

I guess stable distros will just have to disable MSE. :/ This is not only important for LTS distros. Fedora 29 is still supported for another two months, and it will have to disable MSE to update to 2.26.
Comment 8 Michael Catanzaro 2019-09-17 07:28:22 PDT
One option would be to revert the MSE rewrite for 2.26 and leave it only in trunk, that will avoid the problem except for LTS distros. LTS distros don't expect all features to be enabled anyway. And if we defer to 2.28, the timing will work out nicely such that 18.04 users will be able to upgrade to 20.04 around the same time that 18.04 loses support for MSE.
Comment 9 Alberto Garcia 2019-09-17 07:32:57 PDT
(In reply to Michael Catanzaro from comment #8)
> One option would be to revert the MSE rewrite for 2.26 and leave it
> only in trunk, that will avoid the problem except for LTS
> distros. LTS distros don't expect all features to be enabled anyway.

But isn't MSE a requirement for YouTube? Surely LTE users don't expect
that YouTube starts to break after they install the latest security
upgrade.
Comment 10 Michael Catanzaro 2019-09-17 07:47:04 PDT
(In reply to Alberto Garcia from comment #9)
> But isn't MSE a requirement for YouTube? Surely LTE users don't expect
> that YouTube starts to break after they install the latest security
> upgrade.

Without H.264, yes. With H.264: not to my knowledge, so Ubuntu should be OK... hopefully.

At any rate, that's why I suggested the delay, so the new LTS will be out by the time the older LTS loses the MSE support.

Really, the best solution by far is to just do a GStreamer 1.14.6. That would make this a lot easier.
Comment 11 Alberto Garcia 2019-09-17 08:01:05 PDT
(In reply to Michael Catanzaro from comment #10)
> Really, the best solution by far is to just do a GStreamer 1.14.6. That
> would make this a lot easier.

It this the only patch that needs to be backported?

https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/commit/89d0e9cc92a86aa0227ee87406737b6d31670aea
Comment 12 Michael Catanzaro 2019-09-17 08:20:23 PDT
That's what Alicia says. So if you patch that downstream, you can also patch WebKit downstream to remove the requirement for gst 1.16 and all should be good.
Comment 13 Philippe Normand 2019-09-17 09:20:36 PDT
(In reply to Alberto Garcia from comment #9)
> (In reply to Michael Catanzaro from comment #8)
> > One option would be to revert the MSE rewrite for 2.26 and leave it
> > only in trunk, that will avoid the problem except for LTS
> > distros. LTS distros don't expect all features to be enabled anyway.
> 
> But isn't MSE a requirement for YouTube? 

It is.

Michael suggests to revert the MSE backend rewrite. Which means the old, slightly broken backend would be back in 2.26.

(In reply to Michael Catanzaro from comment #10)
> (In reply to Alberto Garcia from comment #9)
> > But isn't MSE a requirement for YouTube? Surely LTE users don't expect
> > that YouTube starts to break after they install the latest security
> > upgrade.
> 
> Without H.264, yes. With H.264: not to my knowledge, so Ubuntu should be
> OK... hopefully.
> 

Youtube desktop now relies on MSE, this has nothing to do with the codec, AFAIK.
Comment 14 Konstantin Tokarev 2019-09-17 09:24:03 PDT
(In reply to Philippe Normand from comment #13)
> Youtube desktop now relies on MSE, this has nothing to do with the codec,
> AFAIK.

FWIW, it works fine here with H.264 and disabled MSE
Comment 15 Carlos Garcia Campos 2019-09-18 00:41:04 PDT
We can still go with the approach of supporting both. It's a pain to maintain, but it's the only way qwe can fix it without disabling MSE and without requiring any additional action from distros.
Comment 16 Alberto Garcia 2019-09-18 00:47:11 PDT
(In reply to Michael Catanzaro from comment #12)
> That's what Alicia says. So if you patch that downstream, you can also patch
> WebKit downstream to remove the requirement for gst 1.16 and all should be
> good.

Note that e.g. Debian has gst 1.14.4 (not 1.14.5). Would it still be enough to backport just that patch?
Comment 17 Alicia Boya García 2019-09-18 03:44:06 PDT
(In reply to Carlos Garcia Campos from comment #15)
> We can still go with the approach of supporting both. It's a pain to
> maintain, but it's the only way qwe can fix it without disabling MSE and
> without requiring any additional action from distros.

Please, no. I think it's unreasonable to keep two MSE backends in WebKit trunk just because some downstream distros refuse to update or patch broken GStreamer versions.

If you want to keep running old versions some patches will be needed here and there, and in the case of Debian stable they have already A LOT of GStreamer patches, I don't think we're such a special case.
Comment 18 Alicia Boya García 2019-09-18 03:45:40 PDT
(In reply to Alberto Garcia from comment #16)
> (In reply to Michael Catanzaro from comment #12)
> > That's what Alicia says. So if you patch that downstream, you can also patch
> > WebKit downstream to remove the requirement for gst 1.16 and all should be
> > good.
> 
> Note that e.g. Debian has gst 1.14.4 (not 1.14.5). Would it still be enough
> to backport just that patch?

Unless there is an unforeseen issue, yes.
Comment 19 Carlos Garcia Campos 2019-09-19 00:48:00 PDT
(In reply to Alicia Boya García from comment #17)
> (In reply to Carlos Garcia Campos from comment #15)
> > We can still go with the approach of supporting both. It's a pain to
> > maintain, but it's the only way qwe can fix it without disabling MSE and
> > without requiring any additional action from distros.
> 
> Please, no. I think it's unreasonable to keep two MSE backends in WebKit
> trunk just because some downstream distros refuse to update or patch broken
> GStreamer versions.

It's a huge pain, but it's reasonable to keep backwards compatibility. We could start by applying the patch in the stable branch only, to ensure everybody can update to 2.26.1 without having to do anything else. Then we notify distros that a patch in gst 1.14 is needed, and once the patch si applied, we revert the patch in stable, land this one in trunk and backport it to stable.

> If you want to keep running old versions some patches will be needed here
> and there, and in the case of Debian stable they have already A LOT of
> GStreamer patches, I don't think we're such a special case.

WebKitGTK provides backwards compatibility in API and ABI, so in theory, you shouldn't need any patch or anything else to use new versions of the lib in old apps/distros. I agree that sometimes that's not so easy and patches here and there are required, but that's the exception. We need to ensure that all distros that update to WebKitGTK 2.26 include the gst patch, not just Debian.
Comment 20 Michael Catanzaro 2019-09-19 05:48:36 PDT
We need to tell distros to patch https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/merge_requests/427 anyway so we can do both at the same time. I'm still seeing regular web process deadlocks in Ephy Tech Preview, despairing, and surprise! it's still this old bug because the fix was never released.

Would be much better to get GStreamer upstream to make new releases, of course.
Comment 21 Michael Catanzaro 2019-09-19 07:46:30 PDT
(In reply to Carlos Garcia Campos from comment #19)
> It's a huge pain, but it's reasonable to keep backwards compatibility. We
> could start by applying the patch in the stable branch only, to ensure
> everybody can update to 2.26.1 without having to do anything else. Then we
> notify distros that a patch in gst 1.14 is needed, and once the patch si
> applied, we revert the patch in stable, land this one in trunk and backport
> it to stable.

We agreed to revert Alicia's work in 2.26 branch, so no changes will be needed for stable.

For trunk, I think the simplest solution is to just close this bug. We tell distros they should apply the necessary gstreamer patch and also give them a patch to drop the build requirement back to 1.14, which they can apply if they also use the gstreamer patch. But upstream we need to require 1.16, because GStreamer developers have EOLed 1.14 and will not accept the patch. And I don't think it's reasonable to ask Alicia to keep the old codepath around in trunk. This way we can save Alicia from doing a bunch of extra work.
Comment 22 Philippe Normand 2019-10-24 02:00:05 PDT
This isn't an issue anymore because the old MSE backend is back in trunk and in 2.26.