[1/1] gst1-vaapi: new package

Signed-off-by: Jesper Bækdahl <jbb@gamblify.com>
---
 package/gstreamer1/Config.in                  |  1 +
 package/gstreamer1/gst1-vaapi/Config.in       | 11 +++++++++++
 package/gstreamer1/gst1-vaapi/gst1-vaapi.hash |  2 ++
 package/gstreamer1/gst1-vaapi/gst1-vaapi.mk   | 14 ++++++++++++++
 4 files changed, 28 insertions(+)
 create mode 100644 package/gstreamer1/gst1-vaapi/Config.in
 create mode 100644 package/gstreamer1/gst1-vaapi/gst1-vaapi.hash
 create mode 100644 package/gstreamer1/gst1-vaapi/gst1-vaapi.mk

Hello,

Thanks for this contribution!

On Thu, 15 Dec 2016 15:53:51 +0100, Jesper Bækdahl wrote:
> --- /dev/null
> +++ b/package/gstreamer1/gst1-vaapi/Config.in
> @@ -0,0 +1,11 @@
> +config BR2_PACKAGE_GST1_VAAPI
> +	bool "gst1-vaapi"
> +	select BR2_PACKAGE_GST1_PLUGINS_BASE
> +	select BR2_PACKAGE_GST1_PLUGINS_BAD

Are you sure you need those set of plugins to be built to build the
gst1-vaapi plugin? I'm not too familiar with GStreamer, so maybe this
is correct, but it looks strange.

> +	select BR2_PACKAGE_GST1_PLUGINS_LIBVA
> +	select BR2_PACKAGE_GST1_PLUGINS_LIBDRM

These two lines however are clearly wrong: those symbols don't exist in
Buildroot. And your .mk file has a dependency on libva and libdrm
instead. So these should be:

	select BR2_PACKAGE_LIBVA
	select BR2_PACKAGE_LIBDRM

However, you need to inherit the dependencies of those packages:

        depends on BR2_TOOLCHAIN_HAS_THREADS # libdrm/libpthread-stubs
        depends on !BR2_STATIC_LIBS

Thanks,

Thomas

>>>>> "Jesper" == Jesper Bækdahl <jbb@gamblify.com> writes:

 > Signed-off-by: Jesper Bækdahl <jbb@gamblify.com>

Thanks for the patch. Besides the feedback Thomas already provided:

> +################################################################################
 > +#
 > +# gst1-vaapi
 > +#
 > +################################################################################
 > +
 > +GST1_VAAPI_VERSION = 1.10.2
 > +GST1_VAAPI_SOURCE = gstreamer-vaapi-$(GST1_VAAPI_VERSION).tar.xz
 > +GST1_VAAPI_SITE = https://gstreamer.freedesktop.org/src/gstreamer-vaapi
 > +GST1_VAAPI_DEPENDENCIES = gstreamer1 gst1-plugins-base gst1-plugins-bad libva libdrm
 > +GST1_VAAPI_LICENSE = GPLv2+
 > +GST1_VAAPI_LICENSE_FILES = COPYING

That is not correct. The tarball doesn't even have a COPYING file. It
should be:

GST1_VAAPI_LICENSE = LGPLv2.1+
GST1_VAAPI_LICENSE_FILES = COPYING.LIB

On 15 December 2016 at 21:47, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
>> --- /dev/null
>> +++ b/package/gstreamer1/gst1-vaapi/Config.in
>> @@ -0,0 +1,11 @@
>> +config BR2_PACKAGE_GST1_VAAPI
>> +     bool "gst1-vaapi"
>> +     select BR2_PACKAGE_GST1_PLUGINS_BASE
>> +     select BR2_PACKAGE_GST1_PLUGINS_BAD
>
> Are you sure you need those set of plugins to be built to build the
> gst1-vaapi plugin? I'm not too familiar with GStreamer, so maybe this
> is correct, but it looks strange.

I added them based on the requirements section found here:
https://github.com/01org/gstreamer-vaapi

>> +     select BR2_PACKAGE_GST1_PLUGINS_LIBVA
>> +     select BR2_PACKAGE_GST1_PLUGINS_LIBDRM
>
> These two lines however are clearly wrong: those symbols don't exist in
> Buildroot. And your .mk file has a dependency on libva and libdrm
> instead. So these should be:
>
>         select BR2_PACKAGE_LIBVA
>         select BR2_PACKAGE_LIBDRM

Will fix this.

> However, you need to inherit the dependencies of those packages:
>
>         depends on BR2_TOOLCHAIN_HAS_THREADS # libdrm/libpthread-stubs
>         depends on !BR2_STATIC_LIBS

I will add those as dependencies too.


Best Regards,

Jesper Bækdahl
Hardware/Software Developer
Gamblify

Hello,


> > Are you sure you need those set of plugins to be built to build the
> > gst1-vaapi plugin? I'm not too familiar with GStreamer, so maybe this
> > is correct, but it looks strange.  
> 
> I added them based on the requirements section found here:
> https://github.com/01org/gstreamer-vaapi

OK. The configure.ac script indeed checks for plugins-base, but not for
plugins-bad.

Could you have a look at the configure.ac script? It seems like it
supports multiple renderers, not only DRM. If you don't want to support
all of them because you don't use them, that's fine. But you should
then use the appropriate --enable/--disable options to enable the right
renderer, and disable the other ones.

Thanks!

Thomas

> OK. The configure.ac script indeed checks for plugins-base, but not for
> plugins-bad.

In configure.ac line 245 we have:
AG_GST_CHECK_MODULES([GST_CODEC_PARSERS],
  [gstreamer-codecparsers-$GST_API_VERSION], [$GST_PBADREQ], [yes])

which is checking for gstreamer-codecparsers that is part of plugins-bad?

> Could you have a look at the configure.ac script? It seems like it
> supports multiple renderers, not only DRM. If you don't want to support
> all of them because you don't use them, that's fine. But you should
> then use the appropriate --enable/--disable options to enable the right
> renderer, and disable the other ones.

I can add conditional CONF_OPTS that adds --enable/--disable based on
which renderers can be built e.g.:
ifeq ($(BR2_PACKAGE_LIBDRM),y)
GST1_VAAPI_CONF_OPTS += --enable-drm
else
GST1_VAAPI_CONF_OPTS += --disable-drm
endif


Best Regards,

Jesper Bækdahl
Hardware/Software Developer
Gamblify

Hello,

On Tue, 3 Jan 2017 10:56:45 +0100, Jesper Bækdahl wrote:
> > OK. The configure.ac script indeed checks for plugins-base, but not for
> > plugins-bad.  
> 
> In configure.ac line 245 we have:
> AG_GST_CHECK_MODULES([GST_CODEC_PARSERS],
>   [gstreamer-codecparsers-$GST_API_VERSION], [$GST_PBADREQ], [yes])
> 
> which is checking for gstreamer-codecparsers that is part of plugins-bad?

Hum, ok. I haven't checked myself, but if you say so, fine :-)

> > Could you have a look at the configure.ac script? It seems like it
> > supports multiple renderers, not only DRM. If you don't want to support
> > all of them because you don't use them, that's fine. But you should
> > then use the appropriate --enable/--disable options to enable the right
> > renderer, and disable the other ones.  
> 
> I can add conditional CONF_OPTS that adds --enable/--disable based on
> which renderers can be built e.g.:
> ifeq ($(BR2_PACKAGE_LIBDRM),y)
> GST1_VAAPI_CONF_OPTS += --enable-drm
> else
> GST1_VAAPI_CONF_OPTS += --disable-drm
> endif

You can do that, *or* if you don't need those renderers, simply disable
them unconditionally for now. The next user interested in one renderer
or the other can tweak the .mk file and send a follow-up patch.

Thanks!

Thomas

Hello,

On Thu, 15 Dec 2016 15:53:51 +0100, Jesper Bækdahl wrote:
> Signed-off-by: Jesper Bækdahl <jbb@gamblify.com>

I have applied a similar patch from Adam Duskett:

  https://git.buildroot.org/buildroot/commit/?id=c1e4e8545dad4d587c6739d7f9942c5e21e7490d
  https://git.buildroot.org/buildroot/commit/?id=d5c7f009875683a2c310478f846ddeb12d862d0c

Best regards,

Thomas