[Buildroot] [PATCH 1/3] libamcodec: new Package

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Dagg]

Signed-off-by: Dagg <daggs@gmx.com>
---
 package/Config.in                |  1 +
 package/libamcodec/Config.in     |  5 +++++
 package/libamcodec/libamcodec.mk | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+)
 create mode 100644 package/libamcodec/Config.in
 create mode 100644 package/libamcodec/libamcodec.mk

diff --git a/package/Config.in b/package/Config.in
index 6c6a562..0a56070 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1074,6 +1074,7 @@ endmenu
 
 menu "Multimedia"
 	source "package/bitstream/Config.in"
+	source "package/libamcodec/Config.in"
 	source "package/libass/Config.in"
 	source "package/libbluray/Config.in"
 	source "package/libdcadec/Config.in"
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
+	help
+	    Interface library for Amlogic media codecs
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
+LIBAMCODEC_SITE_METHOD = git
+LIBAMCODEC_LICENSE = GPL
+
+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"
+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
+									    
+$(eval $(generic-package))
-- 
2.8.3

[Buildroot] [PATCH 2/3] select libamlogic for odroidc2 boards

[Dagg]

Signed-off-by: Dagg <daggs@gmx.com>
---
 configs/odroidc2_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/odroidc2_defconfig b/configs/odroidc2_defconfig
index 1945983..960b2d1 100644
--- a/configs/odroidc2_defconfig
+++ b/configs/odroidc2_defconfig
@@ -22,6 +22,9 @@ BR2_LINUX_KERNEL_USE_INTREE_DTS=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="meson64_odroidc2"
 BR2_LINUX_KERNEL_IMAGE=y
 
+# Media
+BR2_PACKAGE_LIBAMCODEC=y
+
 # U-Boot
 BR2_TARGET_UBOOT=y
 BR2_TARGET_UBOOT_CUSTOM_GIT=y
-- 
2.8.3

[Buildroot] [PATCH 3/3] allow kodi to select libamlogic if set

[Dagg]

this change is  based on the libreelec implementation

Signed-off-by: Dagg <daggs@gmx.com>
---
 package/kodi/kodi.mk | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/package/kodi/kodi.mk b/package/kodi/kodi.mk
index e163c24..d0b2c22 100644
--- a/package/kodi/kodi.mk
+++ b/package/kodi/kodi.mk
@@ -56,6 +56,9 @@ endif
 ifeq ($(BR2_PACKAGE_LIBFSLVPUWRAP),y)
 KODI_DEPENDENCIES += libfslvpuwrap
 KODI_CONF_OPTS += --enable-codec=imxvpu
+else ifeq ($(BR2_PACKAGE_LIBAMCODEC),y)
+KODI_DEPENDENCIES += libamcodec
+KODI_CONF_OPTS += --enable-codec=amcodec
 endif
 
 ifeq ($(BR2_PACKAGE_LIBCAP),y)
-- 
2.8.3

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Thomas Petazzoni]

Hello,

Thanks for your contributions! It generally looks good, but I have a
few comments, see below.

On Wed,  1 Jun 2016 13:27:18 +0300, Dagg wrote:
> Signed-off-by: Dagg <daggs@gmx.com>

Would it be possible to use your complete name for your contributions?

> diff --git a/package/Config.in b/package/Config.in
> index 6c6a562..0a56070 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -1074,6 +1074,7 @@ endmenu
>  
>  menu "Multimedia"
>  	source "package/bitstream/Config.in"
> +	source "package/libamcodec/Config.in"
>  	source "package/libass/Config.in"
>  	source "package/libbluray/Config.in"
>  	source "package/libdcadec/Config.in"
> 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
> +	help
> +	    Interface library for Amlogic media codecs

Please add an empty line here, followed by the URL of the upstrealm
project. It can be just a link to the homepage of the Github project.

> 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
> +LIBAMCODEC_SITE_METHOD = git

Please use the "github" function available in Buildroot. See the
documentation and the numerous other examples in Buildroot (you can
grep for "call,github").

Also, please add a hash file for this package.

> +LIBAMCODEC_LICENSE = GPL

This should be more specific: GPLv2, GPLv2+, GPLv3, GPLv3+.

Also, please add a value for <pkg>_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"

Can you use $(TARGET_CONFIGURE_OPTS) instead of CC=... LD=... ?

PREFIX=$(STAGING_DIR)/usr is not correct, it should be PREFIX=/usr,
because once on the target, things will be in /usr.

Also, why is the PREFIX needed at build time?

> +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
> +									    
> +$(eval $(generic-package))

Other than that, looks good.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Vicente Olivert Riera]

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@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.

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Thomas Petazzoni]

Hello,

On Wed, 1 Jun 2016 13:53:17 +0100, Vicente Olivert Riera wrote:

> > +config BR2_PACKAGE_LIBAMCODEC
> > +	bool "libamcodec"
> > +	depends on BR2_arm || BR2_aarch64  
> 
> Doesn't work for other architectures? Why?

Because this library is specific to Amlogic SoCs, so showing it on ARM
and AArch64 only makes sense.

> $(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)

No, PREFIX=$(STAGING_DIR) is wrong, as I stated in my review. Unless
PREFIX is used in a non-standard way by this package, which is possible
(but in this case it should be explained by a comment).

> 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.

Thanks for doing some testing, I only did a review and did not go all
the way to testing, so this is definitely appreciated. Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Vicente Olivert Riera]

Regards,

Vincent.

On 01/06/16 14:20, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 1 Jun 2016 13:53:17 +0100, Vicente Olivert Riera wrote:
> 
>>> +config BR2_PACKAGE_LIBAMCODEC
>>> +	bool "libamcodec"
>>> +	depends on BR2_arm || BR2_aarch64  
>>
>> Doesn't work for other architectures? Why?
> 
> Because this library is specific to Amlogic SoCs, so showing it on ARM
> and AArch64 only makes sense.

Oh, I see.

>> $(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)
> 
> No, PREFIX=$(STAGING_DIR) is wrong, as I stated in my review. Unless
> PREFIX is used in a non-standard way by this package, which is possible
> (but in this case it should be explained by a comment).

The fist time I saw the PREFIX stuff I thought that it was wrong,
because that's something you use when you are going to install the
package, like DESTDIR. But looking at the Makefiles I've seen this
package uses the PREFIX (and M_PREFIX) vars to build the includes and
library paths. So if we don't pass PREFIX and M_PREFIX then the Makefile
will do "-L/usr/lib and -I/usr/include" instead of
"-L/br/output/staging/usr/lib -I/br/output/staging/usr/include".

Regards,

Vincent.

>> 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.
> 
> Thanks for doing some testing, I only did a review and did not go all
> the way to testing, so this is definitely appreciated. Thanks a lot!
> 
> Thomas
> 

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Thomas Petazzoni]

Hello,

On Wed, 1 Jun 2016 14:26:04 +0100, Vicente Olivert Riera wrote:

> The fist time I saw the PREFIX stuff I thought that it was wrong,
> because that's something you use when you are going to install the
> package, like DESTDIR. But looking at the Makefiles I've seen this
> package uses the PREFIX (and M_PREFIX) vars to build the includes and
> library paths. So if we don't pass PREFIX and M_PREFIX then the Makefile
> will do "-L/usr/lib and -I/usr/include" instead of
> "-L/br/output/staging/usr/lib -I/br/output/staging/usr/include".

Alright, thanks for the explanation. It's indeed a non-standard use of
PREFIX, so it should simply be explained by a short comment above the
<pkg>_BUILD_CMDS variable.

Thanks again for the review!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] libamcodec: new Package

[daggs]

Greetings,
> Hello,
> 
> Thanks for your contributions! It generally looks good, but I have a
> few comments, see below.
> 
> On Wed,  1 Jun 2016 13:27:18 +0300, Dagg wrote:
> > Signed-off-by: Dagg <daggs@gmx.com>
> 
> Would it be possible to use your complete name for your contributions?
> 
I'd rather not, is this a limitation?

> > diff --git a/package/Config.in b/package/Config.in
> > index 6c6a562..0a56070 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -1074,6 +1074,7 @@ endmenu
> >  
> >  menu "Multimedia"
> >  	source "package/bitstream/Config.in"
> > +	source "package/libamcodec/Config.in"
> >  	source "package/libass/Config.in"
> >  	source "package/libbluray/Config.in"
> >  	source "package/libdcadec/Config.in"
> > 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
> > +	help
> > +	    Interface library for Amlogic media codecs
> 
> Please add an empty line here, followed by the URL of the upstrealm
> project. It can be just a link to the homepage of the Github project.
> 
> > 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
> > +LIBAMCODEC_SITE_METHOD = git
> 
> Please use the "github" function available in Buildroot. See the
> documentation and the numerous other examples in Buildroot (you can
> grep for "call,github").
> 
> Also, please add a hash file for this package.
> 
> > +LIBAMCODEC_LICENSE = GPL
> 
> This should be more specific: GPLv2, GPLv2+, GPLv3, GPLv3+.
> 
> Also, please add a value for <pkg>_LICENSE_FILES.
> 
there is no license files or label whatsoever, the only header I saw is like this:
/**
 * \file adec-external-ctrl.c
 * \brief  Audio Dec Lib Functions
 * \version 1.0.0
 * \date 2011-03-08
 */
/* Copyright (C) 2007-2011, Amlogic Inc.
 * All right reserved
 *
 */

> > +
> > +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"
> 
> Can you use $(TARGET_CONFIGURE_OPTS) instead of CC=... LD=... ?
> 
> PREFIX=$(STAGING_DIR)/usr is not correct, it should be PREFIX=/usr,
> because once on the target, things will be in /usr.
> 
> Also, why is the PREFIX needed at build time?
> 
> > +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
> > +									    
> > +$(eval $(generic-package))
> 
> Other than that, looks good.
> 
> Thanks!
> 
> Thomas
done, will suggest a new patch soon.
thanks for the comments.

Dagg.

[Buildroot] [PATCH 1/3] libamcodec: new Package

[daggs]

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@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.

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Thomas Petazzoni]

Hello,

On Wed, 1 Jun 2016 22:52:50 +0200, daggs wrote:

> > Would it be possible to use your complete name for your contributions?
> >   
> I'd rather not, is this a limitation?

Well, like the Linux kernel, we use the Developer Certificate of Origin
(DCO), which is why we have this Signed-off-by line. And this requires
having the real name. From Documentation/SubmittingPatches in the Linux
kernel:

"""
The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.  The rules are pretty simple: if you
can certify the below:

        Developer's Certificate of Origin 1.1

        By making a contribution to this project, I certify that:

        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or

        (b) The contribution is based upon previous work that, to the best
            of my knowledge, is covered under an appropriate open source
            license and I have the right under that license to submit that
            work with modifications, whether created in whole or in part
            by me, under the same open source license (unless I am
            permitted to submit under a different license), as indicated
            in the file; or

        (c) The contribution was provided directly to me by some other
            person who certified (a), (b) or (c) and I have not modified
            it.

        (d) I understand and agree that this project and the contribution
            are public and that a record of the contribution (including all
            personal information I submit with it, including my sign-off) is
            maintained indefinitely and may be redistributed consistent with
            this project or the open source license(s) involved.

then you just add a line saying

        Signed-off-by: Random J Developer <random@developer.example.org>

using your real name (sorry, no pseudonyms or anonymous contributions.)
"""

See the last line, which is the one important here.

> > Also, please add a hash file for this package.
> >   
> > > +LIBAMCODEC_LICENSE = GPL  
> > 
> > This should be more specific: GPLv2, GPLv2+, GPLv3, GPLv3+.
> > 
> > Also, please add a value for <pkg>_LICENSE_FILES.
> >   
> there is no license files or label whatsoever, the only header I saw is like this:
> /**
>  * \file adec-external-ctrl.c
>  * \brief  Audio Dec Lib Functions
>  * \version 1.0.0
>  * \date 2011-03-08
>  */
> /* Copyright (C) 2007-2011, Amlogic Inc.
>  * All right reserved
>  *
>  */

Then, how did you conclude that it was licensed under the GPL ?

If what you show here is all what you have in terms of licensing, then
this package is not under the GPL, and is not even freely
redistributable.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] libamcodec: new Package

[daggs]

Greetings,

> Hello,
> 
> On Wed, 1 Jun 2016 22:52:50 +0200, daggs wrote:
> 
> > > Would it be possible to use your complete name for your contributions?
> > >   
> > I'd rather not, is this a limitation?
> 
> Well, like the Linux kernel, we use the Developer Certificate of Origin
> (DCO), which is why we have this Signed-off-by line. And this requires
> having the real name. From Documentation/SubmittingPatches in the Linux
> kernel:
> 
> """
> The sign-off is a simple line at the end of the explanation for the
> patch, which certifies that you wrote it or otherwise have the right to
> pass it on as an open-source patch.  The rules are pretty simple: if you
> can certify the below:
> 
>         Developer's Certificate of Origin 1.1
> 
>         By making a contribution to this project, I certify that:
> 
>         (a) The contribution was created in whole or in part by me and I
>             have the right to submit it under the open source license
>             indicated in the file; or
> 
>         (b) The contribution is based upon previous work that, to the best
>             of my knowledge, is covered under an appropriate open source
>             license and I have the right under that license to submit that
>             work with modifications, whether created in whole or in part
>             by me, under the same open source license (unless I am
>             permitted to submit under a different license), as indicated
>             in the file; or
> 
>         (c) The contribution was provided directly to me by some other
>             person who certified (a), (b) or (c) and I have not modified
>             it.
> 
>         (d) I understand and agree that this project and the contribution
>             are public and that a record of the contribution (including all
>             personal information I submit with it, including my sign-off) is
>             maintained indefinitely and may be redistributed consistent with
>             this project or the open source license(s) involved.
> 
> then you just add a line saying
> 
>         Signed-off-by: Random J Developer <random@developer.example.org>
> 
> using your real name (sorry, no pseudonyms or anonymous contributions.)
> """
> 
> See the last line, which is the one important here.
> 
I see, I'll keep posting patches, if they are rejected because I'm using only my first name, then let it be.
I value my privacy more than I value my support for open-sourcing.
I don't mind if someone comes along and resends them as his own, I want to contribute but there is a limitation to what I'm willing sacrifice.

> > > Also, please add a hash file for this package.
> > >   
> > > > +LIBAMCODEC_LICENSE = GPL  
> > > 
> > > This should be more specific: GPLv2, GPLv2+, GPLv3, GPLv3+.
> > > 
> > > Also, please add a value for <pkg>_LICENSE_FILES.
> > >   
> > there is no license files or label whatsoever, the only header I saw is like this:
> > /**
> >  * \file adec-external-ctrl.c
> >  * \brief  Audio Dec Lib Functions
> >  * \version 1.0.0
> >  * \date 2011-03-08
> >  */
> > /* Copyright (C) 2007-2011, Amlogic Inc.
> >  * All right reserved
> >  *
> >  */
> 
> Then, how did you conclude that it was licensed under the GPL ?
> 
> If what you show here is all what you have in terms of licensing, then
> this package is not under the GPL, and is not even freely
> redistributable.
> 
> Thomas
the previous source I used was gpl I think but wasn't of the correct origin.
I'll send a question to the owner of the repo.

Thanks,

Dagg.

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Thomas Petazzoni]

Hello,

On Wed, 1 Jun 2016 23:16:36 +0200, daggs wrote:

> I see, I'll keep posting patches, if they are rejected because I'm
> using only my first name, then let it be. I value my privacy more
> than I value my support for open-sourcing. I don't mind if someone
> comes along and resends them as his own, I want to contribute but
> there is a limitation to what I'm willing sacrifice.

I fully respect this choice. There are however some legal aspects to
take care of as well, so I guess it should be a decision of the
Buildroot community to accept or not patches that are submitted under a
pseudonym or anonymously.

> the previous source I used was gpl I think but wasn't of the correct
> origin. I'll send a question to the owner of the repo.

OK, thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] libamcodec: new Package

[daggs]

Greetings,
>
> Hello,
> 
> On Wed, 1 Jun 2016 23:16:36 +0200, daggs wrote:
> 
> > I see, I'll keep posting patches, if they are rejected because I'm
> > using only my first name, then let it be. I value my privacy more
> > than I value my support for open-sourcing. I don't mind if someone
> > comes along and resends them as his own, I want to contribute but
> > there is a limitation to what I'm willing sacrifice.
> 
> I fully respect this choice. There are however some legal aspects to
> take care of as well, so I guess it should be a decision of the
> Buildroot community to accept or not patches that are submitted under a
> pseudonym or anonymously.
sorry for making this harder.
is there another way to verify with without placing such info?
what prevents me for example from sending patches under your name?
isn't it just a text?

> 
> > the previous source I used was gpl I think but wasn't of the correct
> > origin. I'll send a question to the owner of the repo.
> 
> OK, thanks!
got a replay from the maintainer, he will ask amlogic and will let me know.

> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> 

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Thomas Petazzoni]

Hello,

On Wed, 1 Jun 2016 23:16:36 +0200, daggs wrote:

> I see, I'll keep posting patches, if they are rejected because I'm using only my first name, then let it be.
> I value my privacy more than I value my support for open-sourcing.
> I don't mind if someone comes along and resends them as his own, I want to contribute but there is a limitation to what I'm willing sacrifice.

On the other hand, a very simple Google/DuckDuckGo search shows up your
real name. If you really wanted to be anonymous, you should have never
used your real name, or alternatively change your e-mail address when
going anonymous.

See for example http://comments.gmane.org/gmane.linux.alsa.devel/114494.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] libamcodec: new Package

[daggs]

Greetings,
> Hello,
> 
> On Wed, 1 Jun 2016 23:16:36 +0200, daggs wrote:
> 
> > I see, I'll keep posting patches, if they are rejected because I'm using only my first name, then let it be.
> > I value my privacy more than I value my support for open-sourcing.
> > I don't mind if someone comes along and resends them as his own, I want to contribute but there is a limitation to what I'm willing sacrifice.
> 
> On the other hand, a very simple Google/DuckDuckGo search shows up your
> real name. If you really wanted to be anonymous, you should have never
> used your real name, or alternatively change your e-mail address when
> going anonymous.
> 
> See for example http://comments.gmane.org/gmane.linux.alsa.devel/114494.
> 
> Best regards,
> 
> Thomas

I didn't said I wanted to be anonymous, I just didn't wanted to share my full name.
I'm well aware it can be found by searching the net. I cannot change the past. I'm however entitled to change the way I tick when I think it is needed.
e.g. the fact I used to do something in the past, doesn't means I need to continue do it forever.

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Yann E. MORIN]

Daggs, All,

On 2016-06-04 17:02 +0200, daggs spake thusly:
> > On Wed, 1 Jun 2016 23:16:36 +0200, daggs wrote:
> > > I see, I'll keep posting patches, if they are rejected because I'm using only my first name, then let it be.
> > > I value my privacy more than I value my support for open-sourcing.
> > > I don't mind if someone comes along and resends them as his own, I want to contribute but there is a limitation to what I'm willing sacrifice.
> > 
> > On the other hand, a very simple Google/DuckDuckGo search shows up your
> > real name. If you really wanted to be anonymous, you should have never
> > used your real name, or alternatively change your e-mail address when
> > going anonymous.
> > 
> > See for example http://comments.gmane.org/gmane.linux.alsa.devel/114494.
> 
> I didn't said I wanted to be anonymous, I just didn't wanted to share my full name.
> I'm well aware it can be found by searching the net. I cannot change the past. I'm however entitled to change the way I tick when I think it is needed.
> e.g. the fact I used to do something in the past, doesn't means I need to continue do it forever.

The problem is that we can't take patches from anonymous or pseudonyms.
We can't take patches from people that do not use their real name, or
only use part of their name.

The reason for the SoB lines are that we need to be able to identify
who submits a patch. We need traceability.

For a brief (and incomplete) history about the SoB lines, remember the
SCO vs. Linux dispute [0]. So, Linus proposed [1] that every author of
a patch actually "signs off" the patch, to prove the origin of the code,
by introducing the DCO.

The goal of the DCO, and the SoB lines defined therein, is to ensure
that the person that submits code is authorised to do so, accept that
the code is contributed under the same license (or a compatible license)
as that of the project.

[0] https://en.wikipedia.org/wiki/SCO/Linux_controversies
[1] https://lwn.net/Articles/86436/

Sp, please use your real name when you submit patches. Otherwise, we
won't be able to apply them. And, as you said, "if someone comes along
and resends them as his own" is not possible either.

Note that I entirely understand your concern about the way you want to
identify yourself. This is a totally understandable situation. I do not
ask you to change the way you identify yourself.

However, if the way you identify yourself is in contradiction with our
requirements, either you change your identity, or we can't use your
changes.

In the meantime, I'll mark your patches as "Rejected" in our patchwork.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 3/3] allow kodi to select libamlogic if set

[Bernd Kuhls]

Am Wed, 01 Jun 2016 13:27:20 +0300 schrieb Dagg:

> +else ifeq ($(BR2_PACKAGE_LIBAMCODEC),y)
> +KODI_DEPENDENCIES += libamcodec
> +KODI_CONF_OPTS += --enable-codec=amcodec

Hi,

afaics Kodi 17.0-Krypton will remove support for amcodec:
https://github.com/xbmc/xbmc/pull/9688

Is your patch still useful after the version bump, in other words,
will libamcodec still be usable by Kodi on a Linux-based system?

Regards, Bernd

[Buildroot] [PATCH 1/3] libamcodec: new Package

[daggs]

Yann,

> Daggs, All,
> 
> On 2016-06-04 17:02 +0200, daggs spake thusly:
> > > On Wed, 1 Jun 2016 23:16:36 +0200, daggs wrote:
> > > > I see, I'll keep posting patches, if they are rejected because I'm using only my first name, then let it be.
> > > > I value my privacy more than I value my support for open-sourcing.
> > > > I don't mind if someone comes along and resends them as his own, I want to contribute but there is a limitation to what I'm willing sacrifice.
> > > 
> > > On the other hand, a very simple Google/DuckDuckGo search shows up your
> > > real name. If you really wanted to be anonymous, you should have never
> > > used your real name, or alternatively change your e-mail address when
> > > going anonymous.
> > > 
> > > See for example http://comments.gmane.org/gmane.linux.alsa.devel/114494.
> > 
> > I didn't said I wanted to be anonymous, I just didn't wanted to share my full name.
> > I'm well aware it can be found by searching the net. I cannot change the past. I'm however entitled to change the way I tick when I think it is needed.
> > e.g. the fact I used to do something in the past, doesn't means I need to continue do it forever.
> 
> The problem is that we can't take patches from anonymous or pseudonyms.
> We can't take patches from people that do not use their real name, or
> only use part of their name.
> 
> The reason for the SoB lines are that we need to be able to identify
> who submits a patch. We need traceability.
> 
> For a brief (and incomplete) history about the SoB lines, remember the
> SCO vs. Linux dispute [0]. So, Linus proposed [1] that every author of
> a patch actually "signs off" the patch, to prove the origin of the code,
> by introducing the DCO.
> 
> The goal of the DCO, and the SoB lines defined therein, is to ensure
> that the person that submits code is authorised to do so, accept that
> the code is contributed under the same license (or a compatible license)
> as that of the project.
> 
> [0] https://en.wikipedia.org/wiki/SCO/Linux_controversies
> [1] https://lwn.net/Articles/86436/
> 
> Sp, please use your real name when you submit patches. Otherwise, we
> won't be able to apply them. And, as you said, "if someone comes along
> and resends them as his own" is not possible either.
> 
> Note that I entirely understand your concern about the way you want to
> identify yourself. This is a totally understandable situation. I do not
> ask you to change the way you identify yourself.
> 
> However, if the way you identify yourself is in contradiction with our
> requirements, either you change your identity, or we can't use your
> changes.
> 
> In the meantime, I'll mark your patches as "Rejected" in our patchwork.
> 
> Regards,
> Yann E. MORIN.
> 

I'll ask the following question if I may, if I send patches to the mailing list labeled as RFC, will I get any response or I'll be ignored?

Dagg.

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 > Hello,
 > On Wed, 1 Jun 2016 23:16:36 +0200, daggs wrote:

 >> I see, I'll keep posting patches, if they are rejected because I'm
 >> using only my first name, then let it be. I value my privacy more
 >> than I value my support for open-sourcing. I don't mind if someone
 >> comes along and resends them as his own, I want to contribute but
 >> there is a limitation to what I'm willing sacrifice.

 > I fully respect this choice. There are however some legal aspects to
 > take care of as well, so I guess it should be a decision of the
 > Buildroot community to accept or not patches that are submitted under a
 > pseudonym or anonymously.

So far we have not accepted pseudonyms (on purpose at least, perhaps
something has sneaked in), and I would prefer to keep it like that.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/3] libamcodec: new Package

[Peter Korsgaard]

>>>>> "daggs" == daggs  <daggs@gmx.com> writes:

Hi,

>> I fully respect this choice. There are however some legal aspects to
 >> take care of as well, so I guess it should be a decision of the
 >> Buildroot community to accept or not patches that are submitted under a
 >> pseudonym or anonymously.
 > sorry for making this harder.
 > is there another way to verify with without placing such info?
 > what prevents me for example from sending patches under your name?
 > isn't it just a text?

It is "just text", just like a signature.

What is the problem with using your full name? I see you did so in your
From on the v2 patches any way and there's plenty of google hits on your
email address.

-- 
Bye, Peter Korsgaard