Patchwork [v6,09/10] libopenmax: Add libopenmax virtual package

login
register
mail settings
Submitter Spenser Gilliland
Date May 15, 2013, 9:59 p.m.
Message ID <1368655178-19176-10-git-send-email-spenser@gillilanding.com>
Download mbox | patch
Permalink /patch/244194/
State Superseded
Headers show

Comments

Spenser Gilliland - May 15, 2013, 9:59 p.m.
this adds the libopenmax virtual package for hardware based video acceleration

Signed-off-by: Spenser Gilliland <spenser@gillilanding.com>
---
 package/opengl/Config.in                |    3 +++
 package/opengl/libopenmax/libopenmax.mk |   24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+)
 create mode 100644 package/opengl/libopenmax/libopenmax.mk
Thomas Petazzoni - May 16, 2013, 7:58 a.m.
Dear Spenser Gilliland,

On Wed, 15 May 2013 16:59:37 -0500, Spenser Gilliland wrote:

> diff --git a/package/opengl/Config.in b/package/opengl/Config.in
> index 81616f9..1636807 100644
> --- a/package/opengl/Config.in
> +++ b/package/opengl/Config.in
> @@ -6,3 +6,6 @@ config BR2_PACKAGE_HAS_OPENGL_ES
>  
>  config BR2_PACKAGE_HAS_OPENVG
>  	bool
> +
> +config BR2_PACKAGE_HAS_OPENMAX
> +	bool

I'm starting to wonder if those virtual packages shouldn't be separated
from each other, and named virtual-<something>, i.e:

	package/virtual-opengl/
	package/virtual-openvg/
	package/virtual-egl/
	package/virtual-openmax/

but ok, that's certainly a different work. This current patch series is
already long enough.

> +ifeq ($(BR2_PACKAGE_BELLAGIO),y)
> +LIBOPENMAX_DEPENDENCIES += bellagio
> +endif

Unless I missed something, there is no bellagio package in Buildroot
for now, so this chunk should only be added once bellagio is added.

Thanks,

Thomas
Spenser Gilliland - May 16, 2013, 2:18 p.m.
Thomas,

> I'm starting to wonder if those virtual packages shouldn't be separated
> from each other, and named virtual-<something>, i.e:
>
>         package/virtual-opengl/
>         package/virtual-openvg/
>         package/virtual-egl/
>         package/virtual-openmax/
>
> but ok, that's certainly a different work. This current patch series is
> already long enough.

I agree that a change like you suggested would be better but it's
probably best left for another patch series due to the required
changes being quite large.

>> +ifeq ($(BR2_PACKAGE_BELLAGIO),y)
>> +LIBOPENMAX_DEPENDENCIES += bellagio
>> +endif
>
> Unless I missed something, there is no bellagio package in Buildroot
> for now, so this chunk should only be added once bellagio is added.

It's in package/multimedia/bellagio.


Thanks,
Spenser



--
Spenser Gilliland
Computer Engineer
Doctoral Candidate
Thomas Petazzoni - May 16, 2013, 2:22 p.m.
Dear Spenser Gilliland,

On Thu, 16 May 2013 09:18:38 -0500, Spenser Gilliland wrote:

> > but ok, that's certainly a different work. This current patch series is
> > already long enough.
> 
> I agree that a change like you suggested would be better but it's
> probably best left for another patch series due to the required
> changes being quite large.

Agreed, I'm fine with getting this part of your patch as is, and later
on, do some followup improvements. Especially since all those packages
and options have no effect on the configurations people may be
creating, some it doesn't cause any compatibility issue if we move
things around or rename those hidden kconfig options.

> >> +ifeq ($(BR2_PACKAGE_BELLAGIO),y)
> >> +LIBOPENMAX_DEPENDENCIES += bellagio
> >> +endif
> >
> > Unless I missed something, there is no bellagio package in Buildroot
> > for now, so this chunk should only be added once bellagio is added.
> 
> It's in package/multimedia/bellagio.

Argh, my bad. Stupid packages in subdirectories. I didn't remember
having bellagio, so I checked by doing package/bell<tab><tab>, and
nothing showed up, making me -incorrectly- conclude that this package
didn't exist.

Sorry for the noise on this one.

Thomas
Spenser Gilliland - May 16, 2013, 2:35 p.m.
Thomas,

> Argh, my bad. Stupid packages in subdirectories. I didn't remember
> having bellagio, so I checked by doing package/bell<tab><tab>, and
> nothing showed up, making me -incorrectly- conclude that this package
> didn't exist.

If we were to do a cleanup. I'd actually argue that the entire
multimedia directory needs to go away. Gstreamer/gstreamer1 would need
their own directories but everything else could be in the root.
However, this is not really in scope for the patch series at hand.

Spenser

--
Spenser Gilliland
Computer Engineer
Doctoral Candidate
Thomas Petazzoni - May 16, 2013, 2:38 p.m.
Dear Spenser Gilliland,

On Thu, 16 May 2013 09:35:55 -0500, Spenser Gilliland wrote:

> If we were to do a cleanup. I'd actually argue that the entire
> multimedia directory needs to go away. Gstreamer/gstreamer1 would need
> their own directories but everything else could be in the root.

Right. Peter also like to have everything at the root, so I believe he
will most likely not object to such a move.

> However, this is not really in scope for the patch series at hand.

Definitely.

Thomas
Peter Korsgaard - May 26, 2013, 8:48 p.m.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 >> If we were to do a cleanup. I'd actually argue that the entire
 >> multimedia directory needs to go away. Gstreamer/gstreamer1 would need
 >> their own directories but everything else could be in the root.

 Thomas> Right. Peter also like to have everything at the root, so I
 Thomas> believe he will most likely not object to such a move.

Indeed. I tend to make the same mistake as Thomas and only look directly
under package/ ;)

Patch

diff --git a/package/opengl/Config.in b/package/opengl/Config.in
index 81616f9..1636807 100644
--- a/package/opengl/Config.in
+++ b/package/opengl/Config.in
@@ -6,3 +6,6 @@  config BR2_PACKAGE_HAS_OPENGL_ES
 
 config BR2_PACKAGE_HAS_OPENVG
 	bool
+
+config BR2_PACKAGE_HAS_OPENMAX
+	bool
diff --git a/package/opengl/libopenmax/libopenmax.mk b/package/opengl/libopenmax/libopenmax.mk
new file mode 100644
index 0000000..dc29370
--- /dev/null
+++ b/package/opengl/libopenmax/libopenmax.mk
@@ -0,0 +1,24 @@ 
+#############################################################
+#
+# Virtual package for libopenmax
+#
+#############################################################
+
+LIBOPENMAX_SOURCE =
+
+ifeq ($(BR2_PACKAGE_RPI_USERLAND),y)
+LIBOPENMAX_DEPENDENCIES += rpi-userland
+endif
+
+ifeq ($(BR2_PACKAGE_BELLAGIO),y)
+LIBOPENMAX_DEPENDENCIES += bellagio
+endif
+
+ifeq ($(LIBOPENMAX_DEPENDENCIES),y)
+define LIBOPENMAX_CONFIGURE_CMDS
+	echo "No libopenmax implementation selected. Configuration error."
+	exit 1
+endef
+endif
+
+$(eval $(generic-package))