All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option
@ 2019-06-05  5:51 James Hilliard
  2019-06-05  8:51 ` Adrian Perez de Castro
  2019-06-08 20:54 ` Arnout Vandecappelle
  0 siblings, 2 replies; 6+ messages in thread
From: James Hilliard @ 2019-06-05  5:51 UTC (permalink / raw)
  To: buildroot

Removed gst1-plugins-bad dependency from wpewebkit gstreamer-gl option
so that there isn't a circular dependency with gst1-plugins-bad.

It appears that wpewebkit gstreamer-gl only has a runtime dependency on
gst1-plugins-bad.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 package/gstreamer1/gst1-plugins-bad/Config.in          | 10 ++++++++++
 .../gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk    |  7 +++++++
 package/wpewebkit/wpewebkit.mk                         |  1 -
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/package/gstreamer1/gst1-plugins-bad/Config.in b/package/gstreamer1/gst1-plugins-bad/Config.in
index e1e7b05a59..3d19012e95 100644
--- a/package/gstreamer1/gst1-plugins-bad/Config.in
+++ b/package/gstreamer1/gst1-plugins-bad/Config.in
@@ -583,6 +583,16 @@ comment "webrtcdsp needs a toolchain w/ C++, NPTL, gcc >= 4.8"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS_NPTL \
 		|| !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8
 
+config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE
+	bool "wpe"
+	default y
+	depends on BR2_PACKAGE_WPEWEBKIT
+	depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
+
+comment "wpe needs the gst1-plugins-base opengl library and wpewebkit"
+	depends on !BR2_PACKAGE_WPEWEBKIT \
+		|| !BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
+
 config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265
 	bool "x265"
 	depends on BR2_INSTALL_LIBSTDCPP
diff --git a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
index 32edde4901..c6e2877dde 100644
--- a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
+++ b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
@@ -698,6 +698,13 @@ else
 GST1_PLUGINS_BAD_CONF_OPTS += --disable-webrtcdsp
 endif
 
+ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE),y)
+GST1_PLUGINS_BAD_CONF_OPTS += --enable-wpe
+GST1_PLUGINS_BAD_DEPENDENCIES += wpewebkit
+else
+GST1_PLUGINS_BAD_CONF_OPTS += --disable-wpe
+endif
+
 ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265),y)
 GST1_PLUGINS_BAD_CONF_OPTS += --enable-x265
 GST1_PLUGINS_BAD_DEPENDENCIES += x265
diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
index b59a1f793a..4dc4a29064 100644
--- a/package/wpewebkit/wpewebkit.mk
+++ b/package/wpewebkit/wpewebkit.mk
@@ -34,7 +34,6 @@ endif
 
 ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
 WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
-WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
 else
 WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=OFF
 endif
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option
  2019-06-05  5:51 [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option James Hilliard
@ 2019-06-05  8:51 ` Adrian Perez de Castro
  2019-06-05  9:05   ` Adrian Perez de Castro
  2019-06-08 20:54 ` Arnout Vandecappelle
  1 sibling, 1 reply; 6+ messages in thread
From: Adrian Perez de Castro @ 2019-06-05  8:51 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue,  4 Jun 2019 23:51:25 -0600, James Hilliard <james.hilliard1@gmail.com> wrote:
> Removed gst1-plugins-bad dependency from wpewebkit gstreamer-gl option
> so that there isn't a circular dependency with gst1-plugins-bad.
> 
> It appears that wpewebkit gstreamer-gl only has a runtime dependency on
> gst1-plugins-bad.

That's correct.

By the way, thanks for submitting a patch for enabling the WPE GStreamer
element ? I completely forgot about it when making the update to WPE
WebKit 2.24.x  :-)

(Note to self: have an option to enable the WPEQt API when the needed
Qt dependencies are enabled.)

> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Reviewed-by: Adrian Perez de Castro <aperez@igalia.com>

> ---
>  package/gstreamer1/gst1-plugins-bad/Config.in          | 10 ++++++++++
>  .../gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk    |  7 +++++++
>  package/wpewebkit/wpewebkit.mk                         |  1 -
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/package/gstreamer1/gst1-plugins-bad/Config.in b/package/gstreamer1/gst1-plugins-bad/Config.in
> index e1e7b05a59..3d19012e95 100644
> --- a/package/gstreamer1/gst1-plugins-bad/Config.in
> +++ b/package/gstreamer1/gst1-plugins-bad/Config.in
> @@ -583,6 +583,16 @@ comment "webrtcdsp needs a toolchain w/ C++, NPTL, gcc >= 4.8"
>  	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS_NPTL \
>  		|| !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8
>  
> +config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE
> +	bool "wpe"
> +	default y
> +	depends on BR2_PACKAGE_WPEWEBKIT
> +	depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
> +
> +comment "wpe needs the gst1-plugins-base opengl library and wpewebkit"
> +	depends on !BR2_PACKAGE_WPEWEBKIT \
> +		|| !BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
> +
>  config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265
>  	bool "x265"
>  	depends on BR2_INSTALL_LIBSTDCPP
> diff --git a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> index 32edde4901..c6e2877dde 100644
> --- a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> +++ b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> @@ -698,6 +698,13 @@ else
>  GST1_PLUGINS_BAD_CONF_OPTS += --disable-webrtcdsp
>  endif
>  
> +ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE),y)
> +GST1_PLUGINS_BAD_CONF_OPTS += --enable-wpe
> +GST1_PLUGINS_BAD_DEPENDENCIES += wpewebkit
> +else
> +GST1_PLUGINS_BAD_CONF_OPTS += --disable-wpe
> +endif
> +
>  ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265),y)
>  GST1_PLUGINS_BAD_CONF_OPTS += --enable-x265
>  GST1_PLUGINS_BAD_DEPENDENCIES += x265
> diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
> index b59a1f793a..4dc4a29064 100644
> --- a/package/wpewebkit/wpewebkit.mk
> +++ b/package/wpewebkit/wpewebkit.mk
> @@ -34,7 +34,6 @@ endif
>  
>  ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
>  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
> -WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
>  else
>  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=OFF
>  endif
> -- 
> 2.20.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190605/1b0db9a8/attachment.asc>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option
  2019-06-05  8:51 ` Adrian Perez de Castro
@ 2019-06-05  9:05   ` Adrian Perez de Castro
  0 siblings, 0 replies; 6+ messages in thread
From: Adrian Perez de Castro @ 2019-06-05  9:05 UTC (permalink / raw)
  To: buildroot

Hi again,

Just one small note below...

On Wed, 5 Jun 2019 11:51:48 +0300, Adrian Perez de Castro <aperez@igalia.com> wrote:
 
> On Tue,  4 Jun 2019 23:51:25 -0600, James Hilliard <james.hilliard1@gmail.com> wrote:
> > Removed gst1-plugins-bad dependency from wpewebkit gstreamer-gl option
> > so that there isn't a circular dependency with gst1-plugins-bad.
> > 
> > It appears that wpewebkit gstreamer-gl only has a runtime dependency on
> > gst1-plugins-bad.
> 
> That's correct.
> 
> By the way, thanks for submitting a patch for enabling the WPE GStreamer
> element ? I completely forgot about it when making the update to WPE
> WebKit 2.24.x  :-)
> 
> (Note to self: have an option to enable the WPEQt API when the needed
> Qt dependencies are enabled.)
> 
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> 
> Reviewed-by: Adrian Perez de Castro <aperez@igalia.com>
> 
> > ---
> >  package/gstreamer1/gst1-plugins-bad/Config.in          | 10 ++++++++++
> >  .../gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk    |  7 +++++++
> >  package/wpewebkit/wpewebkit.mk                         |  1 -
> >  3 files changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/gstreamer1/gst1-plugins-bad/Config.in b/package/gstreamer1/gst1-plugins-bad/Config.in
> > index e1e7b05a59..3d19012e95 100644
> > --- a/package/gstreamer1/gst1-plugins-bad/Config.in
> > +++ b/package/gstreamer1/gst1-plugins-bad/Config.in
> > @@ -583,6 +583,16 @@ comment "webrtcdsp needs a toolchain w/ C++, NPTL, gcc >= 4.8"
> >  	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS_NPTL \
> >  		|| !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8
> >  
> > +config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE
> > +	bool "wpe"
> > +	default y
> > +	depends on BR2_PACKAGE_WPEWEBKIT
> > +	depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL

Strictly speaking, you would also need a ?select BR2_PACKAGE_WPEBACKEND_FDO?,
but the ?wpewebkit? package already selects it (which is not wrong, but not
100% correct either ? I have a pending mail+patches about this to be sent
later on).

Anyway, this patch can still be merged as-is, so this is mainly a reminder
that the dependency might need to be added later on.

> > +comment "wpe needs the gst1-plugins-base opengl library and wpewebkit"
> > +	depends on !BR2_PACKAGE_WPEWEBKIT \
> > +		|| !BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
> > +
> >  config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265
> >  	bool "x265"
> >  	depends on BR2_INSTALL_LIBSTDCPP
> > diff --git a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> > index 32edde4901..c6e2877dde 100644
> > --- a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> > +++ b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> > @@ -698,6 +698,13 @@ else
> >  GST1_PLUGINS_BAD_CONF_OPTS += --disable-webrtcdsp
> >  endif
> >  
> > +ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE),y)
> > +GST1_PLUGINS_BAD_CONF_OPTS += --enable-wpe
> > +GST1_PLUGINS_BAD_DEPENDENCIES += wpewebkit
> > +else
> > +GST1_PLUGINS_BAD_CONF_OPTS += --disable-wpe
> > +endif
> > +
> >  ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265),y)
> >  GST1_PLUGINS_BAD_CONF_OPTS += --enable-x265
> >  GST1_PLUGINS_BAD_DEPENDENCIES += x265
> > diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
> > index b59a1f793a..4dc4a29064 100644
> > --- a/package/wpewebkit/wpewebkit.mk
> > +++ b/package/wpewebkit/wpewebkit.mk
> > @@ -34,7 +34,6 @@ endif
> >  
> >  ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
> >  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
> > -WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
> >  else
> >  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=OFF
> >  endif

Cheers,
?Adri?n
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190605/e3c5c0e7/attachment.asc>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option
  2019-06-05  5:51 [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option James Hilliard
  2019-06-05  8:51 ` Adrian Perez de Castro
@ 2019-06-08 20:54 ` Arnout Vandecappelle
  2019-06-09 21:59   ` James Hilliard
  1 sibling, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2019-06-08 20:54 UTC (permalink / raw)
  To: buildroot



On 05/06/2019 07:51, James Hilliard wrote:
> Removed gst1-plugins-bad dependency from wpewebkit gstreamer-gl option
> so that there isn't a circular dependency with gst1-plugins-bad.

 This should be a separate patch, with a commit message that explains why the
dependency isn't needed. My guess is that it's a runtime dependency because the
GL plugin is loaded dynamically.

 That patch should also add a '# runtime' comment in the wpewebkit Config.in, i.e.:

config BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL
        bool "use gstreamer-gl"
        default y
        depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
        select BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL # runtime


 Also, I wonder if the same doesn't apply to the gst1-plugins-good, gst1-libav
and possibly even gst1-plugins-base dependencies.


> 
> It appears that wpewebkit gstreamer-gl only has a runtime dependency on
> gst1-plugins-bad.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  package/gstreamer1/gst1-plugins-bad/Config.in          | 10 ++++++++++
>  .../gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk    |  7 +++++++
>  package/wpewebkit/wpewebkit.mk                         |  1 -
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/package/gstreamer1/gst1-plugins-bad/Config.in b/package/gstreamer1/gst1-plugins-bad/Config.in
> index e1e7b05a59..3d19012e95 100644
> --- a/package/gstreamer1/gst1-plugins-bad/Config.in
> +++ b/package/gstreamer1/gst1-plugins-bad/Config.in
> @@ -583,6 +583,16 @@ comment "webrtcdsp needs a toolchain w/ C++, NPTL, gcc >= 4.8"
>  	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS_NPTL \
>  		|| !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8
>  
> +config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE
> +	bool "wpe"
> +	default y
> +	depends on BR2_PACKAGE_WPEWEBKIT

 We normally select packages instead of depending on them. Yes, that implies
copying all its dependencies.

 If there is a good reason to use a depends instead of select, please explain it
in the commit message.


 Also, just to be clear: there is no dependency on
BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA or BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL ?


 Regards,
 Arnout

> +	depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
> +
> +comment "wpe needs the gst1-plugins-base opengl library and wpewebkit"
> +	depends on !BR2_PACKAGE_WPEWEBKIT \
> +		|| !BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
> +
>  config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265
>  	bool "x265"
>  	depends on BR2_INSTALL_LIBSTDCPP
> diff --git a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> index 32edde4901..c6e2877dde 100644
> --- a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> +++ b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> @@ -698,6 +698,13 @@ else
>  GST1_PLUGINS_BAD_CONF_OPTS += --disable-webrtcdsp
>  endif
>  
> +ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE),y)
> +GST1_PLUGINS_BAD_CONF_OPTS += --enable-wpe
> +GST1_PLUGINS_BAD_DEPENDENCIES += wpewebkit
> +else
> +GST1_PLUGINS_BAD_CONF_OPTS += --disable-wpe
> +endif
> +
>  ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265),y)
>  GST1_PLUGINS_BAD_CONF_OPTS += --enable-x265
>  GST1_PLUGINS_BAD_DEPENDENCIES += x265
> diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
> index b59a1f793a..4dc4a29064 100644
> --- a/package/wpewebkit/wpewebkit.mk
> +++ b/package/wpewebkit/wpewebkit.mk
> @@ -34,7 +34,6 @@ endif
>  
>  ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
>  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
> -WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
>  else
>  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=OFF
>  endif
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option
  2019-06-08 20:54 ` Arnout Vandecappelle
@ 2019-06-09 21:59   ` James Hilliard
  2019-06-10 12:37     ` Arnout Vandecappelle
  0 siblings, 1 reply; 6+ messages in thread
From: James Hilliard @ 2019-06-09 21:59 UTC (permalink / raw)
  To: buildroot

On Sat, Jun 8, 2019 at 2:54 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 05/06/2019 07:51, James Hilliard wrote:
> > Removed gst1-plugins-bad dependency from wpewebkit gstreamer-gl option
> > so that there isn't a circular dependency with gst1-plugins-bad.
>
>  This should be a separate patch, with a commit message that explains why the
> dependency isn't needed. My guess is that it's a runtime dependency because the
> GL plugin is loaded dynamically.
Yeah, that's my assumption but I didn't extensively check that.
>
>  That patch should also add a '# runtime' comment in the wpewebkit Config.in, i.e.:
>
> config BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL
>         bool "use gstreamer-gl"
>         default y
>         depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
>         select BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL # runtime
>
>
>  Also, I wonder if the same doesn't apply to the gst1-plugins-good, gst1-libav
> and possibly even gst1-plugins-base dependencies.
>
>
> >
> > It appears that wpewebkit gstreamer-gl only has a runtime dependency on
> > gst1-plugins-bad.
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >  package/gstreamer1/gst1-plugins-bad/Config.in          | 10 ++++++++++
> >  .../gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk    |  7 +++++++
> >  package/wpewebkit/wpewebkit.mk                         |  1 -
> >  3 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/gstreamer1/gst1-plugins-bad/Config.in b/package/gstreamer1/gst1-plugins-bad/Config.in
> > index e1e7b05a59..3d19012e95 100644
> > --- a/package/gstreamer1/gst1-plugins-bad/Config.in
> > +++ b/package/gstreamer1/gst1-plugins-bad/Config.in
> > @@ -583,6 +583,16 @@ comment "webrtcdsp needs a toolchain w/ C++, NPTL, gcc >= 4.8"
> >       depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS_NPTL \
> >               || !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8
> >
> > +config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE
> > +     bool "wpe"
> > +     default y
> > +     depends on BR2_PACKAGE_WPEWEBKIT
>
>  We normally select packages instead of depending on them. Yes, that implies
> copying all its dependencies.
>
>  If there is a good reason to use a depends instead of select, please explain it
> in the commit message.
I basically just copied this:
https://github.com/buildroot/buildroot/blob/280149ce60774aab6a75e3f257185b0d6d2b144e/package/gstreamer1/gst1-plugins-bad/Config.in#L403
>
>
>  Also, just to be clear: there is no dependency on
> BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA or BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL ?
Not from what I could tell, at least there didn't appear to be in the
autoconf script here:
https://github.com/GStreamer/gst-plugins-bad/blob/1.16/configure.ac#L2301
Those all appear to be BR2_PACKAGE_WPEWEBKIT dependencies.
The configure script however doesn't check for gl headers even though
they are required for wpe,
hence the need to add BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL as a
buildroot dependency to ensure the build doesn't fail when wpewebkit
and gst1-plugins-bad
are enabled without opengl.
>
>
>  Regards,
>  Arnout
>
> > +     depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
> > +
> > +comment "wpe needs the gst1-plugins-base opengl library and wpewebkit"
> > +     depends on !BR2_PACKAGE_WPEWEBKIT \
> > +             || !BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
> > +
> >  config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265
> >       bool "x265"
> >       depends on BR2_INSTALL_LIBSTDCPP
> > diff --git a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> > index 32edde4901..c6e2877dde 100644
> > --- a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> > +++ b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
> > @@ -698,6 +698,13 @@ else
> >  GST1_PLUGINS_BAD_CONF_OPTS += --disable-webrtcdsp
> >  endif
> >
> > +ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE),y)
> > +GST1_PLUGINS_BAD_CONF_OPTS += --enable-wpe
> > +GST1_PLUGINS_BAD_DEPENDENCIES += wpewebkit
> > +else
> > +GST1_PLUGINS_BAD_CONF_OPTS += --disable-wpe
> > +endif
> > +
> >  ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_X265),y)
> >  GST1_PLUGINS_BAD_CONF_OPTS += --enable-x265
> >  GST1_PLUGINS_BAD_DEPENDENCIES += x265
> > diff --git a/package/wpewebkit/wpewebkit.mk b/package/wpewebkit/wpewebkit.mk
> > index b59a1f793a..4dc4a29064 100644
> > --- a/package/wpewebkit/wpewebkit.mk
> > +++ b/package/wpewebkit/wpewebkit.mk
> > @@ -34,7 +34,6 @@ endif
> >
> >  ifeq ($(BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL),y)
> >  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=ON
> > -WPEWEBKIT_DEPENDENCIES += gst1-plugins-bad
> >  else
> >  WPEWEBKIT_CONF_OPTS += -DUSE_GSTREAMER_GL=OFF
> >  endif
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option
  2019-06-09 21:59   ` James Hilliard
@ 2019-06-10 12:37     ` Arnout Vandecappelle
  0 siblings, 0 replies; 6+ messages in thread
From: Arnout Vandecappelle @ 2019-06-10 12:37 UTC (permalink / raw)
  To: buildroot



On 09/06/2019 23:59, James Hilliard wrote:
> On Sat, Jun 8, 2019 at 2:54 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>>
>>
>>
>> On 05/06/2019 07:51, James Hilliard wrote:
[snip]
>>> +config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE
>>> +     bool "wpe"
>>> +     default y
>>> +     depends on BR2_PACKAGE_WPEWEBKIT
>>
>>  We normally select packages instead of depending on them. Yes, that implies
>> copying all its dependencies.
>>
>>  If there is a good reason to use a depends instead of select, please explain it
>> in the commit message.
> I basically just copied this:
> https://github.com/buildroot/buildroot/blob/280149ce60774aab6a75e3f257185b0d6d2b144e/package/gstreamer1/gst1-plugins-bad/Config.in#L403

 That's different. An OpenGL provider is not something that can be select'ed,
because there are several providers possible and there's no way to know
automatically which one it should be. So the

	depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL

part is definitely correct.

 However, I was talking about the wpewebkit dependency. It is certainly possible
to select wpewebkit rather than depend on it. But then you do need to copy all
the dependencies, like this:

config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE
	bool "wpe"
	depends on BR2_PACKAGE_WPEWEBKIT_ARCH_SUPPORTS # wpewebkit
	depends on !BR2_BINFMT_FLAT # wpewebkit -> icu
	depends on !BR2_STATIC_LIBS # wpewebkit -> wayland
	depends on BR2_INSTALL_LIBSTDCPP # wpewebkit -> harfbuzz, icu
 	depends on BR2_TOOLCHAIN_HAS_THREADS # wpewebkit -> wayland, icu, libsoup
	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_6 # wpewebkit
	depends on BR2_HOST_GCC_AT_LEAST_4_8 # wpewebkit -> icu
	depends on BR2_USE_WCHAR # wpewebkit -> icu, libsoup
	depends on BR2_PACKAGE_HAS_LIBGLES # wpewebkit -> libepoxy
	depends on BR2_PACKAGE_HAS_LIBEGL # wpewebkit -> libepoxy
	depends on BR2_PACKAGE_HAS_LIBEGL_WAYLAND # wpewebkit -> wpebackend-fdo
	depends on BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL
	select BR2_PACKAGE_WPEWEBKIT
 	help
	  ...

comment "wpe plugin needs a toolchain w/ C++, wchar, threads, dynamic library,
gcc >= 6, host gcc >= 4.8"
	depends on BR2_PACKAGE_WPEWEBKIT_ARCH_SUPPORTS
	depends on !BR2_BINFMT_FLAT
	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR \
		|| !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS \
		|| !BR2_TOOLCHAIN_GCC_AT_LEAST_6 || !BR2_HOST_GCC_AT_LEAST_4_8

comment "wpe plugin needs an OpenGL ES w/ EGL-capable Wayland backend"
	depends on BR2_PACKAGE_WPEWEBKIT_ARCH_SUPPORTS
	depends on !BR2_BINFMT_FLAT
	depends on !BR2_PACKAGE_HAS_LIBGLES || !BR2_PACKAGE_HAS_LIBEGL \
		|| !BR2_PACKAGE_HAS_LIBEGL_WAYLAND


>>  Also, just to be clear: there is no dependency on
>> BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA or BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL ?
> Not from what I could tell, at least there didn't appear to be in the
> autoconf script here:
> https://github.com/GStreamer/gst-plugins-bad/blob/1.16/configure.ac#L2301
> Those all appear to be BR2_PACKAGE_WPEWEBKIT dependencies.
> The configure script however doesn't check for gl headers even though
> they are required for wpe,
> hence the need to add BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL as a
> buildroot dependency to ensure the build doesn't fail when wpewebkit
> and gst1-plugins-bad
> are enabled without opengl.

 See, that's where I'm confused. Even if
BR2_PACKAGE_GST1_PLUGINS_BASE_HAS_LIB_OPENGL is enabled, it is still possible
that BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA and BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL
are disabled. Does it still work in that case? I tested that it builds, but does
it actually work at runtime too?



 However, it turns out that I accidentally pushed my test branch two days ago,
so it has already been applied to master. I thought about reverting it, but
there haven't been any build failures, and my local tests did build fine.

 About the 'depends on' instead of 'select', we do have something like that for
a couple of other "big" plugins (e.g. directfb), and it does make the code a
whole lot simpler. So let's keep it like it is now.

 The only thing I'd still like you to do is to check if the following
configuration works correctly at runtime:

BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WPE=y
# BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_GL is not set
# BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WAYLAND is not set
BR2_PACKAGE_WPEWEBKIT=y
# BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA is not set

 If this doesn't work at runtime, we may need to add

	select BR2_PACKAGE_WPEWEBKIT_MULTIMEDIA
	select BR2_PACKAGE_WPEWEBKIT_USE_GSTREAMER_GL

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-10 12:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-05  5:51 [Buildroot] [PATCH 1/1] package/gstreamer1/gst1-plugins-bad: add wpe plugin option James Hilliard
2019-06-05  8:51 ` Adrian Perez de Castro
2019-06-05  9:05   ` Adrian Perez de Castro
2019-06-08 20:54 ` Arnout Vandecappelle
2019-06-09 21:59   ` James Hilliard
2019-06-10 12:37     ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.