[v6,10/10] gst-omx: add gst-omx package

gst-omx enables OpenMAX based accelerated video decode.

Signed-off-by: Spenser Gilliland <spenser@gillilanding.com>
---
 package/multimedia/Config.in          |    1 +
 package/multimedia/gst-omx/Config.in  |   11 +++++++++++
 package/multimedia/gst-omx/gst-omx.mk |    9 +++++++++
 3 files changed, 21 insertions(+)
 create mode 100644 package/multimedia/gst-omx/Config.in
 create mode 100644 package/multimedia/gst-omx/gst-omx.mk

Dear Spenser Gilliland,

On Wed, 15 May 2013 16:59:38 -0500, Spenser Gilliland wrote:
> gst-omx enables OpenMAX based accelerated video decode.
> 
> Signed-off-by: Spenser Gilliland <spenser@gillilanding.com>

One question: it would be good to mention, in your cover letter, on
which platform this was tested. Was it tested on RPi? With what command
line/tool?

> +comment "gst-omx requires a OpenMAX implementation please select rpi-userland or bellagio"
> +	depends on BR2_PACKAGE_GSTREAMER1 && ! BR2_PACKAGE_HAS_OPENMAX

I'm not sure it's reasonable to list the available OpenMAX
implementations. So I'd just stick with "gst-omx requires an OpenMAX
implementation".

No space between ! and BR2_PACKAGE_HAS_OPENMAX.

> diff --git a/package/multimedia/gst-omx/gst-omx.mk b/package/multimedia/gst-omx/gst-omx.mk
> new file mode 100644
> index 0000000..7f54900
> --- /dev/null
> +++ b/package/multimedia/gst-omx/gst-omx.mk
> @@ -0,0 +1,9 @@

The usual (and useless, but conventional) comment header is missing.

> +GST_OMX_VERSION=1.0.0
> +GST_OMX_SOURCE=gst-omx-$(GST_OMX_VERSION).tar.xz
> +GST_OMX_SITE=http://gstreamer.freedesktop.org/src/gst-omx/

Spaces before and after =.

> +GST_OMX_DEPENDENCIES = gstreamer1 gst1-plugins-base libopenmax
> +GST_OMX_LICENSE = LGPLv2.1
> +GST_OMX_LICENSE_FILE = COPYING
> +GST_OMX_INSTALL_STAGING = YES
> +
> +$(eval $(autotools-package))

Not sure here: do we have a reason to install this to the staging/
space? I thought it was only installing a plugin, and not a library
against which other things would be linked with.

Apparently, the other gst1-plugins-* packages that you submitted don't
install to staging, unless I missed it (which may very be possible, I
only had a quick look).

Thanks!

Thomas

Thomas,

> One question: it would be good to mention, in your cover letter, on
> which platform this was tested. Was it tested on RPi? With what command
> line/tool?

I'll add this info to the cover letter.  It was tested with gst-launch.

>> +comment "gst-omx requires a OpenMAX implementation please select rpi-userland or bellagio"
>> +     depends on BR2_PACKAGE_GSTREAMER1 && ! BR2_PACKAGE_HAS_OPENMAX
>
> I'm not sure it's reasonable to list the available OpenMAX
> implementations. So I'd just stick with "gst-omx requires an OpenMAX
> implementation".
>
> No space between ! and BR2_PACKAGE_HAS_OPENMAX.

Fixed.

>> diff --git a/package/multimedia/gst-omx/gst-omx.mk b/package/multimedia/gst-omx/gst-omx.mk
>> new file mode 100644
>> index 0000000..7f54900
>> --- /dev/null
>> +++ b/package/multimedia/gst-omx/gst-omx.mk
>> @@ -0,0 +1,9 @@
>
> The usual (and useless, but conventional) comment header is missing.
>
>> +GST_OMX_VERSION=1.0.0
>> +GST_OMX_SOURCE=gst-omx-$(GST_OMX_VERSION).tar.xz
>> +GST_OMX_SITE=http://gstreamer.freedesktop.org/src/gst-omx/
>
> Spaces before and after =.

Fixed.

>> +GST_OMX_DEPENDENCIES = gstreamer1 gst1-plugins-base libopenmax
>> +GST_OMX_LICENSE = LGPLv2.1
>> +GST_OMX_LICENSE_FILE = COPYING
>> +GST_OMX_INSTALL_STAGING = YES
>> +
>> +$(eval $(autotools-package))
>
> Not sure here: do we have a reason to install this to the staging/
> space? I thought it was only installing a plugin, and not a library
> against which other things would be linked with.

Not required in staging dir.  I've removed the line from the latest version.

I'm going to wait another couple days before I sent another version
just in case more reviews come in.  If you want to see the changes in
the meantime take a look at the rpi branch on my github buildroot
clone at https://github.com/Spenser309/buildroot/commits/rpi .

Thanks,
Spenser

--
Spenser Gilliland
Computer Engineer
Doctoral Candidate

Dear Spenser Gilliland,

On Thu, 16 May 2013 09:29:53 -0500, Spenser Gilliland wrote:

> > One question: it would be good to mention, in your cover letter, on
> > which platform this was tested. Was it tested on RPi? With what command
> > line/tool?
> 
> I'll add this info to the cover letter.  It was tested with gst-launch.

Ok. Since your branch is named rpi, I suppose it has been tested on the
Rasberry Pi. Have you build tested the thing against Bellagio? Can you
clarify for me which hardware platforms does Bellagio support? Or maybe
it's a purely software implementation of OpenMAX, a little bit like the
software rasterizer of Mesa is a pure software implementation of
OpenGL ?

And, yes, having the details on how to test this will be useful for
other users of the Rasberry Pi who may want to test your patches (and
hopefully add their Tested-by).

Also, I'm pretty sure we should find a way to collect this information
of how a package can be used/tested. Should it be in the main
documentation? A per-package readme.txt when it makes sense? In the
help text of the package kconfig option?

> I'm going to wait another couple days before I sent another version
> just in case more reviews come in.  If you want to see the changes in
> the meantime take a look at the rpi branch on my github buildroot
> clone at https://github.com/Spenser309/buildroot/commits/rpi .

Ok, thanks, sounds good.

Thomas