diff mbox series

[1/2] package/bayer2rgb-neon: new package

Message ID 20190314153435.6317-1-eloi.bail@savoirfairelinux.com
State Accepted
Headers show
Series [1/2] package/bayer2rgb-neon: new package | expand

Commit Message

Éloi Bail March 14, 2019, 3:34 p.m. UTC
bayer2rgb-neon[1] is a library which allows decoding raw camera bayer
to RGB using NEON hardware acceleration.

[1]: https://git.phytec.de/bayer2rgb-neon/

Signed-off-by: Eloi Bail <eloi.bail@savoirfairelinux.com>
---
 package/Config.in                        |  1 +
 package/bayer2rgb-neon/Config.in         | 12 +++++++++
 package/bayer2rgb-neon/bayer2rgb-neon.mk | 32 ++++++++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 package/bayer2rgb-neon/Config.in
 create mode 100644 package/bayer2rgb-neon/bayer2rgb-neon.mk

Comments

Thomas Petazzoni March 29, 2019, 8:16 p.m. UTC | #1
Hello Eloi,

On Thu, 14 Mar 2019 16:34:34 +0100
Eloi Bail <eloi.bail@savoirfairelinux.com> wrote:

> bayer2rgb-neon[1] is a library which allows decoding raw camera bayer
> to RGB using NEON hardware acceleration.
> 
> [1]: https://git.phytec.de/bayer2rgb-neon/
> 
> Signed-off-by: Eloi Bail <eloi.bail@savoirfairelinux.com>

I have applied your patch, but after a fair number of changes, see
below for the details.

> ---
>  package/Config.in                        |  1 +
>  package/bayer2rgb-neon/Config.in         | 12 +++++++++
>  package/bayer2rgb-neon/bayer2rgb-neon.mk | 32 ++++++++++++++++++++++++
>  3 files changed, 45 insertions(+)

When adding a new package, an entry in the DEVELOPERS file must be
added. All new packages must also have a .hash file containing the hash
of the tarball and the license file.

> diff --git a/package/Config.in b/package/Config.in
> index 420e6e95a3..259bd50224 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -12,6 +12,7 @@ menu "Audio and video applications"

bayer2rgb-neon is not really an application, but more a library, so I
moved it to Library -> Graphics.

> diff --git a/package/bayer2rgb-neon/Config.in b/package/bayer2rgb-neon/Config.in
> new file mode 100644
> index 0000000000..702e2e316d
> --- /dev/null
> +++ b/package/bayer2rgb-neon/Config.in
> @@ -0,0 +1,12 @@
> +menuconfig BR2_PACKAGE_BAYER2RGB_NEON

A "menuconfig" is not appropriate for a single package, we use a simple
"config" option.

> +	bool "bayer2rgb-neon"
> +	depends on BR2_arm
> +	depends on !BR2_STATIC_LIBS
> +	depends on BR2_ARM_ENABLE_NEON

Having Neon enabled is not needed: we only need the CPU to suport Neon,
so I changed this to BR2_ARM_CPU_HAS_NEON.

> +	depends on BR2_TOOLCHAIN_BUILDROOT_CXX

This is not the correct option to depend on C++: this option only
exists when using an internal toolchain, so this dependency prevents
selecting this package when an external toolchain is used. The correct
dependency is BR2_INSTALL_LIBSTDCPP.

Another dependency that was missing is on the gcc version: because it
uses C++11, we need gcc >= 4.9.

> +	help
> +	  bayer2rgb-neon is a library which allows
> +	  to decode raw camera bayer to RGB using
> +	  NEON hardware acceleration.
> +
> +	  https://git.phytec.de/bayer2rgb-neon/

When a package has dependencies, we add a Config.in comment to document
those dependencies and help the user, so I've done that.

> diff --git a/package/bayer2rgb-neon/bayer2rgb-neon.mk b/package/bayer2rgb-neon/bayer2rgb-neon.mk
> new file mode 100644
> index 0000000000..964563d152
> --- /dev/null
> +++ b/package/bayer2rgb-neon/bayer2rgb-neon.mk
> @@ -0,0 +1,32 @@
> +################################################################################
> +#
> +# bayer2rgb-neon
> +#
> +################################################################################
> +
> +BAYER2RGB_NEON_VERSION = v0.4
> +BAYER2RGB_NEON_SOURCE = bayer2rgb-neon-$(BAYER2RGB_NEON_VERSION).tar.bz2
> +BAYER2RGB_NEON_SITE = https://git.phytec.de/bayer2rgb-neon/snapshot
> +
> +BAYER2RGB_NEON_LICENSE = GPL-3.0
> +BAYER2RGB_NEON_LICENSE_FILES = COPYING
> +
> +BAYER2RGB_NEON_INSTALL_STAGING = YES
> +
> +BAYER2RGB_NEON_DEPENDENCIES += host-pkgconf host-gengetopt

+= was not needed, a simple = is sufficient here

> +
> +BAYER2RGB_NEON_CONF_OPTS = --prefix="/usr"

This is not needed, as the autotools-package infrastructure already
passes the --prefix option.

> +BAYER2RGB_NEON_AUTORECONF = YES
> +
> +BAYER2RGB_NEON_CFLAGS = $(TARGET_CFLAGS)
> +BAYER2RGB_NEON_CFLAGS += -mfpu=neon
> +
> +BAYER2RGB_NEON_CONF_ENV = CFLAGS=" $(BAYER2RGB_NEON_CFLAGS)"

This was unnecessarily complicated:

BAYER2RGB_NEON_CONF_ENV = CFLAGS="$(TARGET_CFLAGS) -mfpu=neon"

is sufficient.

I've applied with those changes. Thanks!

Thomas
Éloi Bail April 2, 2019, 12:46 p.m. UTC | #2
> Hello Eloi,

> I have applied your patch, but after a fair number of changes, see
> below for the details.
Hi Thomas, 

Thanks for adding those changes before merging my patches ! 

Eloi
<html><body><div style="font-family: arial, helvetica, sans-serif; font-size: 12pt; color: #000000"><div data-marker="__SIG_PRE__"><div><div><div></div></div></div></div><div data-marker="__QUOTED_TEXT__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">Hello Eloi,<br><br>I have applied your patch, but after a fair number of changes, see<br>below for the details.</blockquote>Hi Thomas,<br><div><br></div></div><div data-marker="__QUOTED_TEXT__">Thanks for adding those changes before merging my patches !<br data-mce-bogus="1"></div><div data-marker="__QUOTED_TEXT__"><br data-mce-bogus="1"></div><div data-marker="__QUOTED_TEXT__">Eloi<br data-mce-bogus="1"></div><div data-marker="__QUOTED_TEXT__"><br data-mce-bogus="1"></div></div></body></html>
diff mbox series

Patch

diff --git a/package/Config.in b/package/Config.in
index 420e6e95a3..259bd50224 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -12,6 +12,7 @@  menu "Audio and video applications"
 	source "package/alsa-utils/Config.in"
 	source "package/atest/Config.in"
 	source "package/aumix/Config.in"
+	source "package/bayer2rgb-neon/Config.in"
 	source "package/bellagio/Config.in"
 	source "package/bluez-alsa/Config.in"
 	source "package/dvblast/Config.in"
diff --git a/package/bayer2rgb-neon/Config.in b/package/bayer2rgb-neon/Config.in
new file mode 100644
index 0000000000..702e2e316d
--- /dev/null
+++ b/package/bayer2rgb-neon/Config.in
@@ -0,0 +1,12 @@ 
+menuconfig BR2_PACKAGE_BAYER2RGB_NEON
+	bool "bayer2rgb-neon"
+	depends on BR2_arm
+	depends on !BR2_STATIC_LIBS
+	depends on BR2_ARM_ENABLE_NEON
+	depends on BR2_TOOLCHAIN_BUILDROOT_CXX
+	help
+	  bayer2rgb-neon is a library which allows
+	  to decode raw camera bayer to RGB using
+	  NEON hardware acceleration.
+
+	  https://git.phytec.de/bayer2rgb-neon/
diff --git a/package/bayer2rgb-neon/bayer2rgb-neon.mk b/package/bayer2rgb-neon/bayer2rgb-neon.mk
new file mode 100644
index 0000000000..964563d152
--- /dev/null
+++ b/package/bayer2rgb-neon/bayer2rgb-neon.mk
@@ -0,0 +1,32 @@ 
+################################################################################
+#
+# bayer2rgb-neon
+#
+################################################################################
+
+BAYER2RGB_NEON_VERSION = v0.4
+BAYER2RGB_NEON_SOURCE = bayer2rgb-neon-$(BAYER2RGB_NEON_VERSION).tar.bz2
+BAYER2RGB_NEON_SITE = https://git.phytec.de/bayer2rgb-neon/snapshot
+
+BAYER2RGB_NEON_LICENSE = GPL-3.0
+BAYER2RGB_NEON_LICENSE_FILES = COPYING
+
+BAYER2RGB_NEON_INSTALL_STAGING = YES
+
+BAYER2RGB_NEON_DEPENDENCIES += host-pkgconf host-gengetopt
+
+BAYER2RGB_NEON_CONF_OPTS = --prefix="/usr"
+BAYER2RGB_NEON_AUTORECONF = YES
+
+BAYER2RGB_NEON_CFLAGS = $(TARGET_CFLAGS)
+BAYER2RGB_NEON_CFLAGS += -mfpu=neon
+
+BAYER2RGB_NEON_CONF_ENV = CFLAGS=" $(BAYER2RGB_NEON_CFLAGS)"
+
+define BAYER2RGB_NEON_PRE_CONFIGURE_FIXUP
+	mkdir -p $(@D)/m4
+endef
+
+BAYER2RGB_NEON_PRE_CONFIGURE_HOOKS += BAYER2RGB_NEON_PRE_CONFIGURE_FIXUP
+
+$(eval $(autotools-package))