Re: [PATCH 1/6 v2] gst-plugins-bad: Use new PACKAGECONFIG feature

[Denys Dmytriyenko <denys@ti.com>]

On Fri, Aug 30, 2013 at 03:44:26PM +0000, Cooper Jr., Franklin wrote:
> 
> 
> > -----Original Message-----
> > From: Maupin, Chase
> > Sent: Friday, August 30, 2013 10:04 AM
> > To: Cooper Jr., Franklin; meta-arago@arago-project.org
> > Subject: RE: [meta-arago] [PATCH 1/6 v2] gst-plugins-bad: Use new
> > PACKAGECONFIG feature
> > 
> > >-----Original Message-----
> > >From: meta-arago-bounces@arago-project.org [mailto:meta-arago-
> > >bounces@arago-project.org] On Behalf Of Cooper Jr., Franklin
> > >Sent: Tuesday, August 27, 2013 2:51 PM
> > >To: meta-arago@arago-project.org
> > >Cc: Cooper Jr., Franklin
> > >Subject: [meta-arago] [PATCH 1/6 v2] gst-plugins-bad: Use new
> > >PACKAGECONFIG feature
> > >
> > >* Recent patch in oe-core allows certain features to be enabled or
> > >disabled
> > >  based on the PACKAGECONFIG variable.
> > >* Use this variable which will obsolete alot of the code in this
> > >bbappend while
> > >  achieving the same result.
> > >
> > >Signed-off-by: Franklin S. Cooper Jr <fcooper@ti.com>
> > >---
> > >Version 2 changes:
> > >Update patch tailored to recent change to base recipe in oe-core.
> > >
> > >
> > > .../gstreamer/gst-plugins-bad_0.10.23.bbappend     |   14 ++-----
> > >-------
> > > 1 files changed, 2 insertions(+), 12 deletions(-)
> > >
> > >diff --git a/meta-arago-distro/recipes-multimedia/gstreamer/gst-
> > >plugins-bad_0.10.23.bbappend b/meta-arago-distro/recipes-
> > >multimedia/gstreamer/gst-plugins-bad_0.10.23.bbappend
> > >index 8ae8050..8cee785 100644
> > >--- a/meta-arago-distro/recipes-multimedia/gstreamer/gst-plugins-
> > >bad_0.10.23.bbappend
> > >+++ b/meta-arago-distro/recipes-multimedia/gstreamer/gst-plugins-
> > >bad_0.10.23.bbappend
> > >@@ -1,21 +1,11 @@
> > >-PR_append = "-arago3"
> > >+PR_append = "-arago4"
> > >
> > > # look for files in this layer first
> > > FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
> > >
> > >-# Disable rsvg which caused gtk+ to be pulled in -DEPENDS :=
> > >"${@oe_filter_out('librsvg','${DEPENDS}', d)}"
> > >-EXTRA_OECONF += "--disable-rsvg"
> > >-
> > >-# Remove the restriction of which plugins are built. Previously
> > >fbdevsink -# was not built which is needed by the gstreamer demos
> > >included in the SDK.
> > >-EXTRA_OECONF := "${@oe_filter_out('--with-
> > >plugins=musicbrainz,wavpack,ivorbis,mpegvideoparse','${EXTRA_OECON
> > >F}', d)}"
> > 
> > I'm assuming that these are removed because of the COMMERCIAL filter setting
> > (I don't remember the full variable off the top of my head)
> > 
> > Should this be called out in the commit message?

> [Franklin] So this oe_filter_out wasn't due to COMMERCIAL filter it was due 
> to with-plugins will only build plugins in the list. So by filtering out 
> that string gst-plugins-bad will attempt to build all plugins. Further down 
> in the recipe you can see where certain plugins are explicitly removed 
> because of license reasoning. There is even a comment about it addressing 
> that.

> > 
> > >-
> > > SRC_URI += "file://0001-gstfbdevsink-Fix-depth-value-for-
> > >GST_VIDEO_CAPS_RGB_.patch"
> > >
> > >-# Add faad has a dependency to insure gst-plugins-bad-faad is built.
> > >-DEPENDS += "faad2"
> > >-RDEPENDS_${PN}-faad += "libfaad"
> > >+PACKAGECONFIG = "bzip curl faad"
> > 
> > I have some question here.  You changed the DEPENDS to faad instead of faad2.
> > I see in base.bbclass the DEPENDS magic based on PACKAGECONFIG, but the
> > RDEPENDS setting there didn't look like it would add libfaad.  Maybe I am just
> > not reading it right but did you check that?
> [Franklin] Ah your right.
> 
> Comment from base.bbclass
> PACKAGECONFIG[foo] = "--enable-foo,--disable-foo,foo_depends,foo_runtime_depends"
> 
> Snippet from gst-plugins-bad
> PACKAGECONFIG[faad] = "--enable-faad,--disable-faad,faad2"
> 
> So I probably need to change the override the above line with
> PACKAGECONFIG[faad] = "--enable-faad,--disable-faad,faad2,libfaad"
> 
> Let me double check this.

That is the correct use of PACKAGECONFIG, although handling shlib dependencies 
is usually done automatically. In other words, if you have DEPENDS on faad 
already and it installs libfaad that this package uses, it will figure out 
RDEPENDS automatically...


> > Also, why add curl and bzip too?  If needed should that be a separate patch or at
> > least updated in the commit message?

> [Franklin] In the base recipe PACKAGECONFIG is set to "bzip curl". Since I 
> am adding faad I needed to override that variable but I didn't want to lose 
> the default. Probably a better approach is

> PACKAGECONFIG = "${PACKAGECONFIG} faad"
> Although I don't know what ??= means so I will have to see if that works.

It is just a "lazy" variant of a regular ?= conditional assignment, i.e. it 
just wait until the end of the parsing to decide upon it:

http://docs.openembedded.org/bitbake/html/ch02.html#id398548


> > >
> > > # Disable features that have potential commercial licensing
> > >restrictions
> > > EXTRA_OECONF += "\
> > >--
> > >1.7.0.4
> > >
> > >_______________________________________________
> > >meta-arago mailing list
> > >meta-arago@arago-project.org
> > >http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago
> _______________________________________________
> meta-arago mailing list
> meta-arago@arago-project.org
> http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago