Bug 180233

Summary: [GTK] Turn on ENABLE_MEDIA_SOURCE
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: aboya, bugs-noreply, clopez, mcatanzaro
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch clopez: review-

Description Michael Catanzaro 2017-11-30 19:39:54 PST
Turn on ENABLE_MEDIA_SOURCE

It's still disabled at runtime. We had previously agreed to enable media source ASAP so that it can get as much testing as possible during the development cycle, but this stalled due to $CONCERNS. Now, after discussing with Alicia, I think we are close to being able to toggle the runtime default, but not quite yet as there are a few essential patches that are not yet upstream. So we can consider the runtime flag in a separate bug. But there is no reason to leave the build flag turned off. The build flag does not seem to break anything. This will make it possible to test MSE in MiniBrowser without having to rebuild WebKit.
Comment 1 Michael Catanzaro 2017-11-30 19:40:28 PST
Created attachment 328067 [details]
Patch
Comment 2 Michael Catanzaro 2017-11-30 19:43:11 PST
Note: this will break the Debian stable bot again, since its GStreamer is not new enough. It will need to be taught to build with -DENABLE_MEDIA_SOURCE=OFF. I don't know how to do that.

That reminds me, this is not ready for cq+ because the GStreamer dependency is not defined anywhere in CMake. The build needs to fail at the CMake stage if GStreamer is not new enough for MSE. I don't know what version of GStreamer is required; we'll need Alicia or Enrique to comment on that.
Comment 3 Carlos Alberto Lopez Perez 2017-12-01 04:18:50 PST
(In reply to Michael Catanzaro from comment #2)
> Note: this will break the Debian stable bot again, since its GStreamer is
> not new enough. It will need to be taught to build with
> -DENABLE_MEDIA_SOURCE=OFF. I don't know how to do that.
> 

I will do that

> That reminds me, this is not ready for cq+ because the GStreamer dependency
> is not defined anywhere in CMake. The build needs to fail at the CMake stage
> if GStreamer is not new enough for MSE. I don't know what version of
> GStreamer is required; we'll need Alicia or Enrique to comment on that.


"Thanks" to the slow restart of the master in https://bugs.webkit.org/show_bug.cgi?id=177338#c45 we know that this feature built fine on Ubuntu 16.04 but not on Debian jessie (oldstable).

Debian jessie has gstreamer 1.4 and Ubuntu LTS has 1.8.

Therefore the minimum version is either 1.6 or 1.8


But from the above link we know also that this depends on GST_BUFFER_DTS_OR_PTS definition that appeared on GStreamer 1.8.

Therefore: this is your number: 1.8 is the minimum version, please upload a new patch making this depend on gst >= 1.8
Comment 4 Carlos Alberto Lopez Perez 2017-12-01 04:19:15 PST
Comment on attachment 328067 [details]
Patch

This feature should depend on gst => 1.8
Comment 5 Carlos Alberto Lopez Perez 2017-12-01 04:26:32 PST
(In reply to Carlos Alberto Lopez Perez from comment #3)
> (In reply to Michael Catanzaro from comment #2)
> > Note: this will break the Debian stable bot again, since its GStreamer is
> > not new enough. It will need to be taught to build with
> > -DENABLE_MEDIA_SOURCE=OFF. I don't know how to do that.
> > 
> 
> I will do that
> 

Done: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Debian%20Stable%20%28Build%29/builds/7950/steps/compile-webkit/logs/stdio

This was done by simply setting on the machine:

export BUILD_WEBKIT_ARGS="--cmakeargs=-DENABLE_MEDIA_SOURCE=OFF --cmakeargs=-DUSE_WOFF2=OFF"
Comment 6 Carlos Alberto Lopez Perez 2017-12-01 05:30:37 PST
Also please use the previous bug for this

*** This bug has been marked as a duplicate of bug 167107 ***
Comment 7 Michael Catanzaro 2017-12-01 07:23:32 PST
Maybe we should use this bug for enabling the build option, but keep the previous bug around for enabling the runtime option? Because I am not turning on MSE by default: it will still be off at runtime.
Comment 8 Alicia Boya GarcĂ­a 2017-12-04 04:32:29 PST
(In reply to Carlos Alberto Lopez Perez from comment #3)
> But from the above link we know also that this depends on
> GST_BUFFER_DTS_OR_PTS definition that appeared on GStreamer 1.8.

GST_BUFFER_DTS_OR_PTS() is a trivial utility macro, it could be recreated in WebKit easily.

I'm more worried about matroskademux patches.