[Buildroot] [PATCH 1/1] package/wpewebkit: add option to enable media-stream

[Buildroot] [PATCH 1/1] package/wpewebkit: add option to enable media-stream

[James Hilliard]

This has a compile time dependency on gst1-plugins-bad due to
the codecparsers dependency.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 package/wpewebkit/Config.in    | 6 ++++++
 package/wpewebkit/wpewebkit.mk | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/package/wpewebkit/Config.in b/package/wpewebkit/Config.in
index 04b2bf3beb..df1c900d9f 100644
--- a/package/wpewebkit/Config.in
+++ b/package/wpewebkit/Config.in
@@ -105,6 +105,12 @@ config BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
 	  This option pulls in all of the required dependencies
 	  to enable multimedia (video/audio) support.
 
+config BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM
+	bool "media-stream support"
+	depends on BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
+	help
+	  This option enables media-stream support.
+
 if BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
 
 config BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL
diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
index 5c5e6625f8..9ad569a472 100644
--- a/package/wpewebkit/wpewebkit.mk
+++ b/package/wpewebkit/wpewebkit.mk
@@ -46,6 +46,13 @@ WPEWEBKIT_CONF_OPTS += \
 	-DENABLE_WEB_AUDIO=OFF
 endif
 
+ifeq ($(BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM),y)
+WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=ON
+WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
+else
+WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=OFF
+endif
+
 ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
 WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
 else
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/wpewebkit: add option to enable media-stream

[Adrian Perez de Castro]

[-- Attachment #1.1: Type: text/plain, Size: 2583 bytes --]

Hi James, all,

On Fri, 24 Sep 2021 04:53:40 -0600 James Hilliard <james.hilliard1@gmail.com> wrote:
> This has a compile time dependency on gst1-plugins-bad due to
> the codecparsers dependency.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  package/wpewebkit/Config.in    | 6 ++++++
>  package/wpewebkit/wpewebkit.mk | 7 +++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/package/wpewebkit/Config.in b/package/wpewebkit/Config.in
> index 04b2bf3beb..df1c900d9f 100644
> --- a/package/wpewebkit/Config.in
> +++ b/package/wpewebkit/Config.in
> @@ -105,6 +105,12 @@ config BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
>  	  This option pulls in all of the required dependencies
>  	  to enable multimedia (video/audio) support.
>  
> +config BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM
> +	bool "media-stream support"
> +	depends on BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
> +	help
> +	  This option enables media-stream support.
> +
>  if BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
>  
>  config BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL
> diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
> index 5c5e6625f8..9ad569a472 100644
> --- a/package/wpewebkit/wpewebkit.mk
> +++ b/package/wpewebkit/wpewebkit.mk
> @@ -46,6 +46,13 @@ WPEWEBKIT_CONF_OPTS += \
>  	-DENABLE_WEB_AUDIO=OFF
>  endif
>  
> +ifeq ($(BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM),y)
> +WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=ON
> +WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
> +else
> +WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=OFF

The problem I see here is that we are forcing to disable a feature which
wildly popular web sites (e.g. YouTube) use more and more and being able
to disable it is likely to do more harm than good. Sites will break and
people will wrongly blame WPE WebKit :-\

The right approach here would be to break the circular dependency is doing
two passes on the gst1-plugins-bad build: one basic build which has the
things that WebKit needs (i.e. codecparsers), then use that to build WebKit,
then rebuild with all the options enabled. This second gst1-plugins-bad
build would be the one to install to the target image.

Unfortunately, I think the “good” solution it is not trivial to achieve
with Buildroot. What comes to mind is having a new gst1-plugins-bad-minimal
package which is used only as a throwaway dependency to build wpewebkit.
If somebody has a better suggestion, I would love to know :)

> +endif
> +
>  ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
>  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
>  else
> -- 
> 2.25.1
> 

Cheers,
—Adrián

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/wpewebkit: add option to enable media-stream

[James Hilliard]

On Fri, Oct 1, 2021 at 4:58 AM Adrian Perez de Castro <aperez@igalia.com> wrote:
>
> Hi James, all,
>
> On Fri, 24 Sep 2021 04:53:40 -0600 James Hilliard <james.hilliard1@gmail.com> wrote:
> > This has a compile time dependency on gst1-plugins-bad due to
> > the codecparsers dependency.
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >  package/wpewebkit/Config.in    | 6 ++++++
> >  package/wpewebkit/wpewebkit.mk | 7 +++++++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/package/wpewebkit/Config.in b/package/wpewebkit/Config.in
> > index 04b2bf3beb..df1c900d9f 100644
> > --- a/package/wpewebkit/Config.in
> > +++ b/package/wpewebkit/Config.in
> > @@ -105,6 +105,12 @@ config BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
> >         This option pulls in all of the required dependencies
> >         to enable multimedia (video/audio) support.
> >
> > +config BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM
> > +     bool "media-stream support"
> > +     depends on BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
> > +     help
> > +       This option enables media-stream support.
> > +
> >  if BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
> >
> >  config BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL
> > diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
> > index 5c5e6625f8..9ad569a472 100644
> > --- a/package/wpewebkit/wpewebkit.mk
> > +++ b/package/wpewebkit/wpewebkit.mk
> > @@ -46,6 +46,13 @@ WPEWEBKIT_CONF_OPTS += \
> >       -DENABLE_WEB_AUDIO=OFF
> >  endif
> >
> > +ifeq ($(BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM),y)
> > +WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=ON
> > +WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
> > +else
> > +WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=OFF
>
> The problem I see here is that we are forcing to disable a feature which
> wildly popular web sites (e.g. YouTube) use more and more and being able
> to disable it is likely to do more harm than good. Sites will break and
> people will wrongly blame WPE WebKit :-\

But this feature is effectively unconditionally disabled right now
since it can not be
built without wpewebkit depending on gst1-plugins-bad at build time, so at least
this is an improvement as it would now be possible to enable media
stream support.

>
> The right approach here would be to break the circular dependency is doing
> two passes on the gst1-plugins-bad build: one basic build which has the
> things that WebKit needs (i.e. codecparsers), then use that to build WebKit,
> then rebuild with all the options enabled. This second gst1-plugins-bad
> build would be the one to install to the target image.
>
> Unfortunately, I think the “good” solution it is not trivial to achieve
> with Buildroot. What comes to mind is having a new gst1-plugins-bad-minimal
> package which is used only as a throwaway dependency to build wpewebkit.
> If somebody has a better suggestion, I would love to know :)

Well apparently if the wpe plugin in  gst1-plugins-bad is moved to
gst1-plugins-good
that would also solve the issue since wpewebkit does not depend on
gst1-plugins-good
at build time, although not sure what's needed upstream to do so.

>
> > +endif
> > +
> >  ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
> >  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
> >  else
> > --
> > 2.25.1
> >
>
> Cheers,
> —Adrián
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/wpewebkit: add option to enable media-stream

[Arnout Vandecappelle]

On 24/09/2021 12:53, James Hilliard wrote:
> This has a compile time dependency on gst1-plugins-bad due to
> the codecparsers dependency.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

  Applied to master, thanks.

  As James mentioned, the current situation already is that wpewebkit is 
compiled without mediastream support. So although this is not a perfect 
solution, it's at least a step forward.

  Regards,
  Arnout

> ---
>   package/wpewebkit/Config.in    | 6 ++++++
>   package/wpewebkit/wpewebkit.mk | 7 +++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/package/wpewebkit/Config.in b/package/wpewebkit/Config.in
> index 04b2bf3beb..df1c900d9f 100644
> --- a/package/wpewebkit/Config.in
> +++ b/package/wpewebkit/Config.in
> @@ -105,6 +105,12 @@ config BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
>   	  This option pulls in all of the required dependencies
>   	  to enable multimedia (video/audio) support.
>   
> +config BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM
> +	bool "media-stream support"
> +	depends on BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
> +	help
> +	  This option enables media-stream support.
> +
>   if BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
>   
>   config BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL
> diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
> index 5c5e6625f8..9ad569a472 100644
> --- a/package/wpewebkit/wpewebkit.mk
> +++ b/package/wpewebkit/wpewebkit.mk
> @@ -46,6 +46,13 @@ WPEWEBKIT_CONF_OPTS += \
>   	-DENABLE_WEB_AUDIO=OFF
>   endif
>   
> +ifeq ($(BR2_PACKAGE_WPEWEBKIT_MEDIA_STREAM),y)
> +WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=ON
> +WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
> +else
> +WPEWEBKIT_CONF_OPTS += -DENABLE_MEDIA_STREAM=OFF
> +endif
> +
>   ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
>   WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
>   else
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/wpewebkit: add option to enable media-stream

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 > On 24/09/2021 12:53, James Hilliard wrote:
 >> This has a compile time dependency on gst1-plugins-bad due to
 >> the codecparsers dependency.
 >> 
 >> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

 >  Applied to master, thanks.

 >  As James mentioned, the current situation already is that wpewebkit
 > is compiled without mediastream support. So although this is not a
 > perfect solution, it's at least a step forward.

Committed to 2021.08.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/wpewebkit: add option to enable media-stream

[Adrian Perez de Castro]

[-- Attachment #1.1: Type: text/plain, Size: 840 bytes --]

Hi,

On Wed, 06 Oct 2021 21:12:37 +0200 Arnout Vandecappelle <arnout@mind.be> wrote:
 
> On 24/09/2021 12:53, James Hilliard wrote:
> > This has a compile time dependency on gst1-plugins-bad due to
> > the codecparsers dependency.
> > 
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> 
>   Applied to master, thanks.
> 
>   As James mentioned, the current situation already is that wpewebkit is 
> compiled without mediastream support. So although this is not a perfect 
> solution, it's at least a step forward.

Right, I was under the (erroneous) impression that we were enabling
media-stream by default in the build system. Turns out, that is only
true for developer builds--I should have double checked instead of
relying on my sleepy brain :]

Thanks James for the patch, and Arnout for merging it!


Cheers,
—Adrián

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot