[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Peter Seiderer]

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Notes:
  - just compile tested yet
  - named webrtcbin option (instead of webrtc), because of the
    BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC entry in Config.in.legacy
    (see [1] for history)

[1] https://git.buildroot.net/buildroot/commit/?id=4c06d2490a07f0b88f42c56c7409899fd2f5608a
---
 package/gstreamer1/gst1-plugins-bad/Config.in         | 11 +++++++++++
 .../gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk   |  7 +++++++
 2 files changed, 18 insertions(+)

diff --git a/package/gstreamer1/gst1-plugins-bad/Config.in b/package/gstreamer1/gst1-plugins-bad/Config.in
index 869f0a9d45..5d0480cfb0 100644
--- a/package/gstreamer1/gst1-plugins-bad/Config.in
+++ b/package/gstreamer1/gst1-plugins-bad/Config.in
@@ -558,6 +558,17 @@ config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBP
 	help
 	  Webp image format plugin
 
+config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTCBIN
+	bool "webrtcbin"
+	depends on !BR2_STATIC_LIBS # libnice -> gnutls
+	select BR2_PACKAGE_GST1_PLUGINS_BASE # libgstsdp
+	select BR2_PACKAGE_LIBNICE
+	help
+	  WebRTC plugins (webrtcbin - a bin for webrtc connections)
+
+comment "webrtcbin needs a toolchain w/ dynamic library"
+	depends on BR2_STATIC_LIBS
+
 config BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTCDSP
 	bool "webrtcdsp"
 	# All depends from webrtc-audio-processing
diff --git a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
index f5b081f972..2220554a2d 100644
--- a/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
+++ b/package/gstreamer1/gst1-plugins-bad/gst1-plugins-bad.mk
@@ -703,6 +703,13 @@ else
 GST1_PLUGINS_BAD_CONF_OPTS += --disable-webp
 endif
 
+ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTCBIN),y)
+GST1_PLUGINS_BAD_CONF_OPTS += --enable-webrtc
+GST1_PLUGINS_BAD_DEPENDENCIES += gst1-plugins-base libnice
+else
+GST1_PLUGINS_BAD_CONF_OPTS += --disable-webrtc
+endif
+
 ifeq ($(BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTCDSP),y)
 GST1_PLUGINS_BAD_CONF_OPTS += --enable-webrtcdsp
 GST1_PLUGINS_BAD_DEPENDENCIES += webrtc-audio-processing
-- 
2.21.0

[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Thomas Petazzoni]

Hello,

+Arnout in Cc, our Config.in.legacy/backward-compatibility guru.

On Sun, 10 Mar 2019 18:51:09 +0100
Peter Seiderer <ps.report@gmx.net> wrote:

> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
> Notes:
>   - just compile tested yet
>   - named webrtcbin option (instead of webrtc), because of the
>     BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC entry in Config.in.legacy
>     (see [1] for history)

Would it be a big problem to reuse the
BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC option ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Arnout Vandecappelle]

On 12/03/2019 22:27, Thomas Petazzoni wrote:
> Hello,
> 
> +Arnout in Cc, our Config.in.legacy/backward-compatibility guru.
> 
> On Sun, 10 Mar 2019 18:51:09 +0100
> Peter Seiderer <ps.report@gmx.net> wrote:
> 
>> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
>> ---
>> Notes:
>>   - just compile tested yet
>>   - named webrtcbin option (instead of webrtc), because of the
>>     BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC entry in Config.in.legacy
>>     (see [1] for history)
> 
> Would it be a big problem to reuse the
> BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC option ?

 I believe the plugin got a different name, and is used in a different way. So
reusing the same option could lead to things that stop working - if it's a
different name, at least there is still the legacy warning.

 That said, in -bad API changes sometimes happen anyway, so maybe we shouldn't
worry too much about that.

 Regards,
 Arnout

[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Peter Seiderer]

Hello Arnout,

On Wed, 13 Mar 2019 10:05:19 +0100, Arnout Vandecappelle <arnout@mind.be> wrote:

> On 12/03/2019 22:27, Thomas Petazzoni wrote:
> > Hello,
> > 
> > +Arnout in Cc, our Config.in.legacy/backward-compatibility guru.
> > 
> > On Sun, 10 Mar 2019 18:51:09 +0100
> > Peter Seiderer <ps.report@gmx.net> wrote:
> >   
> >> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> >> ---
> >> Notes:
> >>   - just compile tested yet
> >>   - named webrtcbin option (instead of webrtc), because of the
> >>     BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC entry in Config.in.legacy
> >>     (see [1] for history)  
> > 
> > Would it be a big problem to reuse the
> > BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC option ?  
> 
>  I believe the plugin got a different name, and is used in a different way. So
> reusing the same option could lead to things that stop working - if it's a
> different name, at least there is still the legacy warning.
> 
>  That said, in -bad API changes sometimes happen anyway, so maybe we shouldn't
> worry too much about that.

The history is there is a webrtcdsp plugin in gst1-plugins-bad which
was (false) named simple webrtc in buildroot and was renamed (with a legacy
entry) to webrtcdsp. Since gstreamer-1.12.x there is an 'real' additional/new webrtc
plugin in gst1-plugins-bad (nothing to do with -bad API changes, just a
buildroot history problem)...

Two possibilities:

A) add a new buildroot option with the right name (and delete the legacy entry?)

B) add a new buildroot option with the wrong name webrtcbin instead of webrtc,
   but avoid the legacy entry problem (as the suggested RFC patch does)


Regards,
Peter

> 
>  Regards,
>  Arnout
> 

[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Arnout Vandecappelle]

On 13/03/2019 20:17, Peter Seiderer wrote:
> Hello Arnout,
> 
> On Wed, 13 Mar 2019 10:05:19 +0100, Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>> On 12/03/2019 22:27, Thomas Petazzoni wrote:
>>> Hello,
>>>
>>> +Arnout in Cc, our Config.in.legacy/backward-compatibility guru.
>>>
>>> On Sun, 10 Mar 2019 18:51:09 +0100
>>> Peter Seiderer <ps.report@gmx.net> wrote:
>>>   
>>>> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
>>>> ---
>>>> Notes:
>>>>   - just compile tested yet
>>>>   - named webrtcbin option (instead of webrtc), because of the
>>>>     BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC entry in Config.in.legacy
>>>>     (see [1] for history)  
>>>
>>> Would it be a big problem to reuse the
>>> BR2_PACKAGE_GST1_PLUGINS_BAD_PLUGIN_WEBRTC option ?  
>>
>>  I believe the plugin got a different name, and is used in a different way. So
>> reusing the same option could lead to things that stop working - if it's a
>> different name, at least there is still the legacy warning.
>>
>>  That said, in -bad API changes sometimes happen anyway, so maybe we shouldn't
>> worry too much about that.
> 
> The history is there is a webrtcdsp plugin in gst1-plugins-bad which
> was (false) named simple webrtc in buildroot and was renamed (with a legacy
> entry) to webrtcdsp. Since gstreamer-1.12.x there is an 'real' additional/new webrtc
> plugin in gst1-plugins-bad (nothing to do with -bad API changes, just a
> buildroot history problem)...

 OK, I minsunderstood...

 The Config.in option was introduced in 2017.02, and renamed for 2017.08.

 However, the rename was also backported to 2017.02.4.

 In other words, the only people who could be hit by this issue are people that
used 2017.02 and never upgraded even to 2017.02.4.

 I think it's OK if we risk breaking something for those people. Especially
because the problem is very obvious and very easy to fix.


> 
> Two possibilities:
> 
> A) add a new buildroot option with the right name (and delete the legacy entry?)

 In other words, go for this option (and indeed delete the legacy entry).

 Regards,
 Arnout

> 
> B) add a new buildroot option with the wrong name webrtcbin instead of webrtc,
>    but avoid the legacy entry problem (as the suggested RFC patch does)
> 
> 
> Regards,
> Peter
> 
>>
>>  Regards,
>>  Arnout
>>
> 

[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Thomas Petazzoni]

On Thu, 14 Mar 2019 08:27:56 +0100
Arnout Vandecappelle <arnout@mind.be> wrote:

>  The Config.in option was introduced in 2017.02, and renamed for 2017.08.
> 
>  However, the rename was also backported to 2017.02.4.
> 
>  In other words, the only people who could be hit by this issue are people that
> used 2017.02 and never upgraded even to 2017.02.4.
> 
>  I think it's OK if we risk breaking something for those people. Especially
> because the problem is very obvious and very easy to fix.

Yes, I agree: I believe it's better to have the right name for the
option moving forward, and the problem with re-using the name of the
previous option is not too bad.

So, I'm happy to see that both of us agree on the direction to take
with this.

Thanks

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [RFC v1] package/gst1-plugins-bad: add webrtcbin option

[Peter Seiderer]

Hello Arnout, Thomas,

On Thu, 14 Mar 2019 08:54:45 +0100, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> On Thu, 14 Mar 2019 08:27:56 +0100
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
> >  The Config.in option was introduced in 2017.02, and renamed for 2017.08.
> > 
> >  However, the rename was also backported to 2017.02.4.
> > 
> >  In other words, the only people who could be hit by this issue are people that
> > used 2017.02 and never upgraded even to 2017.02.4.
> > 
> >  I think it's OK if we risk breaking something for those people. Especially
> > because the problem is very obvious and very easy to fix.  
> 
> Yes, I agree: I believe it's better to have the right name for the
> option moving forward, and the problem with re-using the name of the
> previous option is not too bad.
> 
> So, I'm happy to see that both of us agree on the direction to take
> with this.

To both: thanks for review and advice, updated patch follows...

Regards,
Peter

> 
> Thanks
> 
> Thomas