| Message ID | 1360016773-8691-2-git-send-email-alexandre.belloni@piout.net |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Alexandre Belloni, On Mon, 4 Feb 2013 23:26:13 +0100, Alexandre Belloni wrote: > --- /dev/null > +++ b/package/multimedia/gst-plugin-x170/Config.in > @@ -0,0 +1,10 @@ > +config BR2_PACKAGE_GST_PLUGIN_X170 > + bool "gst-plugin-x170" > + depends on BR2_PACKAGE_GSTREAMER A "depends on BR2_arm" should be added here. We generally try to hide such architecture-specific packages when they really don't make sense. Of course, technically speaking, it should only appear if the SoC is an AT91 SoC that has this specific video unit, but we are not able to do that with the existing Buildroot options. But we still try to make such options depend on BR2_arm. > diff --git a/package/multimedia/gst-plugin-x170/gst-plugin-x170.mk b/package/multimedia/gst-plugin-x170/gst-plugin-x170.mk > new file mode 100644 > index 0000000..0e14185 > --- /dev/null > +++ b/package/multimedia/gst-plugin-x170/gst-plugin-x170.mk > @@ -0,0 +1,11 @@ Missing comment header. > +GST_PLUGIN_X170_VERSION=1.0 > +GST_PLUGIN_X170_SOURCE=gst-plugin-x170-$(GST_PLUGIN_X170_VERSION).tar.gz > +GST_PLUGIN_X170_SITE=ftp://ftp.linux4sam.org/pub/demo/linux4sam_1.9/codec/ Spaces before and after the equal sign. <foo>_SOURCE line not beeded since it's the default value. > + > +GST_PLUGIN_X170_AUTORECONF = YES > +GST_PLUGIN_X170_AUTORECONF_OPT = -Im4/ Please add a justification here in a short comment before the _AUTORECONF option to explain why it's needed. Normally, it's not needed for tarballs when no patches are applied, so we try to give a short justification through a comment. > +GST_PLUGIN_X170_DEPENDENCIES = gstreamer libglib2 on2-8170-libs > + > +GST_PLUGIN_X170_CONF_ENV = CFLAGS="$(TARGET_CFLAGS) -I$(STAGING_DIR)/usr/include/glib-2.0 -I$(STAGING_DIR)/usr/lib/glib-2.0/include -I$(STAGING_DIR)/usr/include/gstreamer-0.10 -I$(STAGING_DIR)/usr/include/libxml2/" Isn't the configure script capable it detecting those libraries? What fails if you don't add all those -I values? > + > +$(eval $(autotools-package)) > diff --git a/package/multimedia/on2-8170-libs/Config.in b/package/multimedia/on2-8170-libs/Config.in > new file mode 100644 > index 0000000..35eb628 > --- /dev/null > +++ b/package/multimedia/on2-8170-libs/Config.in > @@ -0,0 +1,5 @@ > +config BR2_PACKAGE_ON2_8170_LIBS > + bool "on2-8170-libs" depends on BR2_arm. > + help > + Libraries for Hantro X170 video decoder > + Link to an upstream URL. Could probably be the same as the other package. > diff --git a/package/multimedia/on2-8170-libs/on2-8170-libs.mk b/package/multimedia/on2-8170-libs/on2-8170-libs.mk > new file mode 100644 > index 0000000..edb1c5d > --- /dev/null > +++ b/package/multimedia/on2-8170-libs/on2-8170-libs.mk > @@ -0,0 +1,20 @@ Comment header. > +ON2_8170_LIBS_VERSION=1.0 > +ON2_8170_LIBS_SOURCE=on2-8170-libs-$(ON2_8170_LIBS_VERSION).tar.gz > +ON2_8170_LIBS_SITE=ftp://ftp.linux4sam.org/pub/demo/linux4sam_1.9/codec/ Spaces before and after =. The _SOURCE value is the default, so you can get rid of it. > + > +ON2_8170_LIBS_INSTALL_STAGING = YES > + > +define ON2_8170_LIBS_BUILD_CMDS > +endef If it's empty, then it's not needed. The default _BUILD_CMDS for a generic-package is empty anyway. > +define ON2_8170_LIBS_INSTALL_STAGING_CMDS > + $(INSTALL) -D -m 0755 $(@D)/*.a $(STAGING_DIR)/usr/lib > + $(INSTALL) -D -m 0644 $(@D)/*.h $(STAGING_DIR)/usr/include > + $(INSTALL) -D -m 0755 $(@D)/*.so $(STAGING_DIR)/usr/lib > +endef > + > +define ON2_8170_LIBS_INSTALL_TARGET_CMDS > + $(INSTALL) -D -m 0755 $(@D)/*.so $(TARGET_DIR)/usr/lib > +endef If you use -D, then you must path a complete path as a second argument. Otherwise, if the destination directory doesn't exist, then your file will be copied as the directory name. I.e, you "libblabla.so" would be a file named "lib" under the usr/ directory. If we want to do a wildcard-based copy of files, I don't think install plays really well. We generally do a 'cp' instead in this case, but maybe others could comment on this specific point. Best regards, Thomas
Hi, On Mon, Feb 04, 2013 at 11:51:09PM +0100, Thomas Petazzoni wrote : > > + > > +GST_PLUGIN_X170_AUTORECONF = YES > > +GST_PLUGIN_X170_AUTORECONF_OPT = -Im4/ > > Please add a justification here in a short comment before the > _AUTORECONF option to explain why it's needed. Normally, it's not > needed for tarballs when no patches are applied, so we try to give a > short justification through a comment. > There is no configure script in the archive so you basically have to run autoreconf to be able to build. > > +GST_PLUGIN_X170_DEPENDENCIES = gstreamer libglib2 on2-8170-libs > > + > > +GST_PLUGIN_X170_CONF_ENV = CFLAGS="$(TARGET_CFLAGS) -I$(STAGING_DIR)/usr/include/glib-2.0 -I$(STAGING_DIR)/usr/lib/glib-2.0/include -I$(STAGING_DIR)/usr/include/gstreamer-0.10 -I$(STAGING_DIR)/usr/include/libxml2/" > > Isn't the configure script capable it detecting those libraries? What > fails if you don't add all those -I values? > The configure script itself seems to be able to find the includes correctly but the include path is limited to . and .. when compiling so I had to force the -I at configure. It may be related to the way I'm using autoreconf. I think that package is expectig you to use gst-autogen.sh to do that but I could'nt get that to work.
>>>>> "Alexandre" == Alexandre Belloni <alexandre.belloni@piout.net> writes:
Alexandre> Signed-off-by: Alexandre Belloni <alexandre.belloni@piout.net>
Hi,
Will you send a v2 fixing the comments Thomas had?
For the on2-8170-libs package, do you know what toolchain the binaries
have been built with? Do we need to limit the C library / arch variant
to ensure it is only available on compatible configs?
On 4 February 2013 22:51, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Alexandre Belloni, > > On Mon, 4 Feb 2013 23:26:13 +0100, Alexandre Belloni wrote: >> --- /dev/null >> +++ b/package/multimedia/gst-plugin-x170/Config.in >> @@ -0,0 +1,10 @@ >> +config BR2_PACKAGE_GST_PLUGIN_X170 >> + bool "gst-plugin-x170" >> + depends on BR2_PACKAGE_GSTREAMER > > A "depends on BR2_arm" should be added here. We generally try to hide > such architecture-specific packages when they really don't make sense. > Of course, technically speaking, it should only appear if the SoC is an > AT91 SoC that has this specific video unit, but we are not able to do > that with the existing Buildroot options. But we still try to make such > options depend on BR2_arm. I think it's ARM9 only, so we can probably be slightly more specific: depends on (BR2_arm920t || BR2_arm922t || BR2_arm926t) Simon
>>>>> "Simon" == Simon Dawson <spdawson@gmail.com> writes: >>> +config BR2_PACKAGE_GST_PLUGIN_X170 >>> + bool "gst-plugin-x170" >>> + depends on BR2_PACKAGE_GSTREAMER >> >> A "depends on BR2_arm" should be added here. We generally try to hide >> such architecture-specific packages when they really don't make sense. >> Of course, technically speaking, it should only appear if the SoC is an >> AT91 SoC that has this specific video unit, but we are not able to do >> that with the existing Buildroot options. But we still try to make such >> options depend on BR2_arm. Simon> I think it's ARM9 only, so we can probably be slightly more specific: Simon> depends on (BR2_arm920t || BR2_arm922t || BR2_arm926t) And the only device I've ever seen with this IP block integrated are at91sam9m10/11, both of which are arm926t.
Hi, On Sun, Apr 28, 2013 at 09:00:05PM +0200, Peter Korsgaard wrote : > Will you send a v2 fixing the comments Thomas had? > I had a look again today, I addressed almost all the comments but I'm still try to figure what is happening with the configure script. It is actually find glib and the other one correctly but it is not using the correct environment variable in the Makefile. I'll try to send something next week. > For the on2-8170-libs package, do you know what toolchain the binaries > have been built with? Do we need to limit the C library / arch variant > to ensure it is only available on compatible configs? > The symbol that are coming from the libc are limited and if I recall correctly, it was working with a toolchain built by buildroot (that is using uClibc). Regards
diff --git a/package/multimedia/Config.in b/package/multimedia/Config.in index 931e6d3..e13e9da 100644 --- a/package/multimedia/Config.in +++ b/package/multimedia/Config.in @@ -14,12 +14,14 @@ source "package/multimedia/gst-plugins-base/Config.in" source "package/multimedia/gst-plugins-good/Config.in" source "package/multimedia/gst-plugins-bad/Config.in" source "package/multimedia/gst-plugins-ugly/Config.in" +source "package/multimedia/gst-plugin-x170/Config.in" source "package/multimedia/lame/Config.in" source "package/multimedia/madplay/Config.in" source "package/multimedia/mpd/Config.in" source "package/multimedia/mpg123/Config.in" source "package/multimedia/mplayer/Config.in" source "package/multimedia/musepack/Config.in" +source "package/multimedia/on2-8170-libs/Config.in" source "package/opus-tools/Config.in" source "package/multimedia/pulseaudio/Config.in" source "package/multimedia/tidsp-binaries/Config.in" diff --git a/package/multimedia/gst-plugin-x170/Config.in b/package/multimedia/gst-plugin-x170/Config.in new file mode 100644 index 0000000..abbfa5e --- /dev/null +++ b/package/multimedia/gst-plugin-x170/Config.in @@ -0,0 +1,10 @@ +config BR2_PACKAGE_GST_PLUGIN_X170 + bool "gst-plugin-x170" + depends on BR2_PACKAGE_GSTREAMER + select BR2_PACKAGE_ON2_8170_LIBS + help + GStreamer plug-in to use the Hantro X170 video decoder present on + ATMEL AT91SAM9M10 SoC. + + http://www.at91.com/linux4sam/bin/view/Linux4SAM/SAM9M10Gstreamer + diff --git a/package/multimedia/gst-plugin-x170/gst-plugin-x170.mk b/package/multimedia/gst-plugin-x170/gst-plugin-x170.mk new file mode 100644 index 0000000..0e14185 --- /dev/null +++ b/package/multimedia/gst-plugin-x170/gst-plugin-x170.mk @@ -0,0 +1,11 @@ +GST_PLUGIN_X170_VERSION=1.0 +GST_PLUGIN_X170_SOURCE=gst-plugin-x170-$(GST_PLUGIN_X170_VERSION).tar.gz +GST_PLUGIN_X170_SITE=ftp://ftp.linux4sam.org/pub/demo/linux4sam_1.9/codec/ + +GST_PLUGIN_X170_AUTORECONF = YES +GST_PLUGIN_X170_AUTORECONF_OPT = -Im4/ +GST_PLUGIN_X170_DEPENDENCIES = gstreamer libglib2 on2-8170-libs + +GST_PLUGIN_X170_CONF_ENV = CFLAGS="$(TARGET_CFLAGS) -I$(STAGING_DIR)/usr/include/glib-2.0 -I$(STAGING_DIR)/usr/lib/glib-2.0/include -I$(STAGING_DIR)/usr/include/gstreamer-0.10 -I$(STAGING_DIR)/usr/include/libxml2/" + +$(eval $(autotools-package)) diff --git a/package/multimedia/on2-8170-libs/Config.in b/package/multimedia/on2-8170-libs/Config.in new file mode 100644 index 0000000..35eb628 --- /dev/null +++ b/package/multimedia/on2-8170-libs/Config.in @@ -0,0 +1,5 @@ +config BR2_PACKAGE_ON2_8170_LIBS + bool "on2-8170-libs" + help + Libraries for Hantro X170 video decoder + diff --git a/package/multimedia/on2-8170-libs/on2-8170-libs.mk b/package/multimedia/on2-8170-libs/on2-8170-libs.mk new file mode 100644 index 0000000..edb1c5d --- /dev/null +++ b/package/multimedia/on2-8170-libs/on2-8170-libs.mk @@ -0,0 +1,20 @@ +ON2_8170_LIBS_VERSION=1.0 +ON2_8170_LIBS_SOURCE=on2-8170-libs-$(ON2_8170_LIBS_VERSION).tar.gz +ON2_8170_LIBS_SITE=ftp://ftp.linux4sam.org/pub/demo/linux4sam_1.9/codec/ + +ON2_8170_LIBS_INSTALL_STAGING = YES + +define ON2_8170_LIBS_BUILD_CMDS +endef + +define ON2_8170_LIBS_INSTALL_STAGING_CMDS + $(INSTALL) -D -m 0755 $(@D)/*.a $(STAGING_DIR)/usr/lib + $(INSTALL) -D -m 0644 $(@D)/*.h $(STAGING_DIR)/usr/include + $(INSTALL) -D -m 0755 $(@D)/*.so $(STAGING_DIR)/usr/lib +endef + +define ON2_8170_LIBS_INSTALL_TARGET_CMDS + $(INSTALL) -D -m 0755 $(@D)/*.so $(TARGET_DIR)/usr/lib +endef + +$(eval $(generic-package))
Signed-off-by: Alexandre Belloni <alexandre.belloni@piout.net> --- package/multimedia/Config.in | 2 ++ package/multimedia/gst-plugin-x170/Config.in | 10 ++++++++++ .../multimedia/gst-plugin-x170/gst-plugin-x170.mk | 11 +++++++++++ package/multimedia/on2-8170-libs/Config.in | 5 +++++ package/multimedia/on2-8170-libs/on2-8170-libs.mk | 20 ++++++++++++++++++++ 5 files changed, 48 insertions(+) create mode 100644 package/multimedia/gst-plugin-x170/Config.in create mode 100644 package/multimedia/gst-plugin-x170/gst-plugin-x170.mk create mode 100644 package/multimedia/on2-8170-libs/Config.in create mode 100644 package/multimedia/on2-8170-libs/on2-8170-libs.mk