[Buildroot] [PATCH 1/3] libamcodec: new Package
daggs
daggs at gmx.com
Wed Jun 1 20:58:21 UTC 2016
Greetings,
> Hello Dagg,
>
> thanks for your contributions. Below are some comments I would like you
> to read. Please keep scrolling down.
>
> On 01/06/16 11:27, Dagg wrote:
> > Signed-off-by: Dagg <daggs at gmx.com>
> > ---
> > package/Config.in | 1 +
> > package/libamcodec/Config.in | 5 +++++
> > package/libamcodec/libamcodec.mk | 33 +++++++++++++++++++++++++++++++++
>
> You also need to add a package/libamcodec/libamcodec.hash file.
>
> > 3 files changed, 39 insertions(+)
> > create mode 100644 package/libamcodec/Config.in
> > create mode 100644 package/libamcodec/libamcodec.mk
> >
>
> [...]
>
> > diff --git a/package/libamcodec/Config.in b/package/libamcodec/Config.in
> > new file mode 100644
> > index 0000000..bb2d924
> > --- /dev/null
> > +++ b/package/libamcodec/Config.in
> > @@ -0,0 +1,5 @@
> > +config BR2_PACKAGE_LIBAMCODEC
> > + bool "libamcodec"
> > + depends on BR2_arm || BR2_aarch64
>
> Doesn't work for other architectures? Why?
>
> > + help
> > + Interface library for Amlogic media codecs
>
> We usually add the URL here.
>
> > diff --git a/package/libamcodec/libamcodec.mk b/package/libamcodec/libamcodec.mk
> > new file mode 100644
> > index 0000000..e983192
> > --- /dev/null
> > +++ b/package/libamcodec/libamcodec.mk
> > @@ -0,0 +1,33 @@
> > +################################################################################
> > +#
> > +# libamcodec
> > +#
> > +################################################################################
> > +
> > +LIBAMCODEC_VERSION = ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d
> > +LIBAMCODEC_SITE = https://github.com/mdrjr/c2_aml_libs.git
>
>
> You can use out GitHub helper here:
>
> LIBAMCODEC_SITE = $(call github,mdrjr,c2_aml_libs,$(LIBAMCODEC_VERSION))
>
> > +LIBAMCODEC_SITE_METHOD = git
>
> And this line will not be needed since you are using the GitHub helper.
>
> > +LIBAMCODEC_LICENSE = GPL
>
> Which version of GPL? Also, no LIBAMCODEC_LICENSE_FILES?
>
> > +
> > +define LIBAMCODEC_BUILD_CMDS
> > + $(MAKE) -C $(@D)/amavutils CC="$(TARGET_CC)" LD="$(TARGET_LD)" PREFIX="$(STAGING_DIR)/usr"
> > + $(MAKE) -C $(@D)/amadec CC="$(TARGET_CC)" LD="$(TARGET_LD)" PREFIX="$(STAGING_DIR)/usr"
> > + $(MAKE) -C $(@D)/amcodec CC="$(TARGET_CC)" LD="$(TARGET_LD)" PREFIX="$(STAGING_DIR)/usr"
>
> You have an extra white space before this line. Remove it.
>
> Some comments regarding the whole LIBAMCODEC_BUILD_CMDS:
>
> - Always put $(TARGET_MAKE_ENV) before $(MAKE).
> - Instead of defining CC and LD you could just pass
> $(TARGET_CONFIGURE_OPTS).
> - amavutils and amadec use M_PREFIX instead of PREFIX.
>
> Something like this would work fine:
>
> $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/amavutils
> M_PREFIX=$(STAGING_DIR)/usr
> $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/amadec
> M_PREFIX=$(STAGING_DIR)/usr
> $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/amcodec
> PREFIX=$(STAGING_DIR)
>
> > +endef
> > +
> > +define LIBAMCODEC_INSTALL_STAGING_CMDS
> > + $(INSTALL) -D -m 0755 $(@D)/amavutils/libamavutils.so $(STAGING_DIR)/usr/lib
> > + $(INSTALL) -D -m 0555 $(@D)/amadec/libamadec.so $(STAGING_DIR)/usr/lib
> > + $(INSTALL) -D -m 0555 $(@D)/amcodec/libamcodec.so $(STAGING_DIR)/usr/lib
> > +
> > + mkdir -p $(STAGING_DIR)/usr/include/amcodec
> > + cp -rf $(@D)/amcodec/include/* $(STAGING_DIR)/usr/include/amcodec
> > +endef
> > +
> > +define LIBAMCODEC_INSTALL_TARGET_CMDS
> > + $(INSTALL) -D -m 0755 $(@D)/amavutils/libamavutils.so $(TARGET_DIR)/usr/lib
> > + $(INSTALL) -D -m 0555 $(@D)/amadec/libamadec.so $(TARGET_DIR)/usr/lib
> > + $(INSTALL) -D -m 0555 $(@D)/amcodec/libamcodec.so $(TARGET_DIR)/usr/lib
> > +endef
> > +
>
> You have an extra white space/tab here. Remove it.
>
> > +$(eval $(generic-package))
> >
>
> Now some final comments. You package fails to build with this error:
>
> /br/output/host/usr/bin/arm-linux-gnueabihf-gcc -O2 -fPIC -g
> -I/br/output/build/libamcodec-ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d/amavutils
> -I/br/output/build/libamcodec-ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d/amavutils/include
> -I/br/output/build/libamcodec-ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d/amavutils/../amcodec/include
> -L/usr/lib -I/usr/include -c -o amaudioutils.o amaudioutils.c
> arm-linux-gnueabihf-gcc: ERROR: unsafe header/library path used in
> cross-compilation: '/usr/lib'
>
> This is because your package is using "-L/usr/lib -I/usr/include" which
> are considered unsafe header/library paths (it will use libs and
> includes from the host machine, not from the staging area). And
> Buildroot treats this as an error because
> BR2_COMPILER_PARANOID_UNSAFE_PATH is enabled.
>
> If you disable BR2_COMPILER_PARANOID_UNSAFE_PATH it will not fail at
> that point, but it will fail later like this:
>
> /br/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-linux-gnueabihf/5.3.1/../../../../arm-linux-gnueabihf/bin/ld:
> skipping incompatible /usr/lib/libpthread.so when searching for -lpthread
> /br/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-linux-gnueabihf/5.3.1/../../../../arm-linux-gnueabihf/bin/ld:
> skipping incompatible /usr/lib/libm.so when searching for -lm
> /usr/lib/librt.so: file not recognized: File format not recognized
>
> This is because is trying to use /usr/lib/librt.so from the host machine
> which has a different architecture than the target. And it's using the
> /usr/lib/librt.so from the host machine because of the explained before
> (the unsafe header/library paths).
>
> However this problem will be solved when you amend the commands in the
> LIBAMCODEC_BUILD_CMDS as I told you above.
>
> Now, after this, it will fail with an error like this one:
>
> audio_out/alsa-out.c:18:28: fatal error: alsa/asoundlib.h: No such file
> or directory
>
> This is because your package needs alsa-lib in order to be built, so you
> have depend on it.
>
> Add "select BR2_PACKAGE_ALSA_LIB" to the Config.in, and don't forget to
> propagate alsa-lib' dependencies, so also add "depends on
> BR2_TOOLCHAIN_HAS_THREADS # alsa-lib". And this will make necessary to
> add a comment section to the Config.in as well, like this:
>
>
> comment "libamcodec needs a toolchain w/ threads"
> depends on !BR2_TOOLCHAIN_HAS_THREADS
>
> And in the libamcodec.mk you need to add the alsa-lib dependency as well
> to make sure it will be built before your package. That way the alsa-lib
> headers and libs will be ready to use in the $(STAGING_DIR):
>
> LIBAMCODEC_DEPENDENCIES = alsa-lib
>
> After all of that your package will fail again to build with an error
> like this one:
>
> make[1]: Entering directory
> '/br/output/build/libamcodec-ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d/amcodec'
> MAKE audio_ctl
> make[2]: Entering directory
> '/br/output/build/libamcodec-ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d/amcodec/audio_ctl'
> make[2]: Leaving directory
> '/vr/output/build/libamcodec-ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d/amcodec/audio_ctl'
> make[2]: Entering directory
> '/vr/output/build/libamcodec-ed1d8b8c54b05c1a02a8ee44c59744e4cbed3d8d/amcodec/audio_ctl'
> CC audio_ctrl.c
> audio_ctrl.c: In function 'audio_basic_init':
> audio_ctrl.c:26:5: warning: implicit declaration of function
> 'audio_decode_basic_init' [-Wimplicit-function-declaration]
> audio_decode_basic_init();
> ^
> LD build-in.o
> ld: audio_ctrl.o: Relocations in generic ELF (EM: 40)
> ld: audio_ctrl.o: Relocations in generic ELF (EM: 40)
> ld: audio_ctrl.o: Relocations in generic ELF (EM: 40)
> ld: audio_ctrl.o: Relocations in generic ELF (EM: 40)
> ld: audio_ctrl.o: Relocations in generic ELF (EM: 40)
> audio_ctrl.o: error adding symbols: File in wrong format
>
> I let you fix this one and the others that may appear.
>
> Regards,
>
> Vincent.
>
most of the rejects were addressed in the mail to Thomas.
I did compiled it before and it exhibited no failures, while some I understand why I didn't saw issues with it (alsa-lib), others I don't (last issue for example).
thanks for the comments, they were helpful.
I'll post a new patch soon.
Dagg.
More information about the buildroot
mailing list