Re: [Buildroot] [PATCH v6 1/1] package/libcamera-apps: new package

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Peter, All,

+Kieran

On 2022-02-11 16:06 +0100, Peter Seiderer spake thusly:
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>

Finally applied to master, thanks.

However, see below for somme important comments...

> ---
[--SNIP--]
> diff --git a/package/libcamera-apps/0001-cmake-remove-Werror-and-Wfatal-errors-compiler-comma.patch b/package/libcamera-apps/0001-cmake-remove-Werror-and-Wfatal-errors-compiler-comma.patch
> new file mode 100644
> index 0000000000..988d89cca3
> --- /dev/null
> +++ b/package/libcamera-apps/0001-cmake-remove-Werror-and-Wfatal-errors-compiler-comma.patch
> @@ -0,0 +1,37 @@
> +From 6bd733b52f0b4039407c676cd3126a9e2f79456b Mon Sep 17 00:00:00 2001
> +From: Peter Seiderer <ps.report@gmx.net>
> +Date: Fri, 5 Feb 2021 18:24:08 +0100
> +Subject: [PATCH] cmake: remove -Werror and -Wfatal-errors compiler command
> + line option
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Fixes (with bootlin-riscv64-musl toolchain):
> +
> +  .../build/libcamera-apps-2d1009e3badcc8047361ff81149ad6cba3b911b5/core/libcamera_app.cpp:30:22: error: overflow in conversion from ‘long unsigned int’ to ‘int’ changes value from ‘2154321408’ to ‘-2140645888’ [-Werror=overflow]
> +     30 |  int ret = ioctl(fd, VIDIOC_QUERYCAP, &caps);
> +        |                      ^~~~~~~~~~~~~~~
> +  cc1plus: all warnings being treated as errors

Kieran, I believe this really needs some investigation on your side.
Peter, can you provide a defconfig that exhibits the issue, please?

[--SNIP--]
> diff --git a/package/libcamera-apps/Config.in b/package/libcamera-apps/Config.in
> new file mode 100644
> index 0000000000..75807b9c01
> --- /dev/null
> +++ b/package/libcamera-apps/Config.in
> @@ -0,0 +1,30 @@
> +config BR2_PACKAGE_LIBCAMERA_APPS
> +	bool "libcamera-apps"
> +	depends on BR2_PACKAGE_LIBCAMERA_ARCH_SUPPORTS # libcamera
> +	depends on BR2_INSTALL_LIBSTDCPP # libcamera/boost
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # libcamera/boost
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_7 # C++17/libcamera
> +	depends on !BR2_STATIC_LIBS # gnutls/libcamera
> +	depends on BR2_USE_WCHAR # gnutls/libcamera/boost
> +	select BR2_PACKAGE_BOOST
> +	select BR2_PACKAGE_BOOST_PROGRAM_OPTIONS
> +	select BR2_PACKAGE_JPEG
> +	select BR2_PACKAGE_LIBCAMERA
> +	select BR2_PACKAGE_LIBEXIF
> +	select BR2_PACKAGE_LIBPNG
> +	select BR2_PACKAGE_TIFF
> +	select BR2_PACKAGE_XLIB_LIBX11 if BR2_PACKAGE_XORG7 && (BR2_PACKAGE_HAS_LIBEGL || BR2_PACKAGE_HAS_LIBGL)
> +	select BR2_PACKAGE_LIBEPOXY if BR2_PACKAGE_XORG7 && (BR2_PACKAGE_HAS_LIBEGL || BR2_PACKAGE_HAS_LIBGL)

Look closely those two select: if neither EGL nor GL are neabled, then
neither libeopxy nor xlib-libx11 are enabled, even when Xorg is. So, see
later [0]...

> +	select BR2_PACKAGE_QT5BASE_GUI if BR2_PACKAGE_QT5
> +	select BR2_PACKAGE_QT5BASE_WIDGETS if BR2_PACKAGE_QT5

When Qt5 is enabled, then Qt5GUI and Qt5Widgets are enabled, so [1]...

> +

Spurious empty line, I dropeed it.

[--SNIP--]
> diff --git a/package/libcamera-apps/libcamera-apps.mk b/package/libcamera-apps/libcamera-apps.mk
> new file mode 100644
> index 0000000000..23cc02793d
> --- /dev/null
> +++ b/package/libcamera-apps/libcamera-apps.mk
> @@ -0,0 +1,46 @@
> +################################################################################
> +#
> +# libcamera-apps
> +#
> +################################################################################
> +
> +LIBCAMERA_APPS_VERSION = 2d1009e3badcc8047361ff81149ad6cba3b911b5
> +LIBCAMERA_APPS_SITE = $(call github,raspberrypi,libcamera-apps,$(LIBCAMERA_APPS_VERSION))
> +LIBCAMERA_APPS_LICENSE = BSD-2-Clause
> +LIBCAMERA_APPS_LICENSE_FILES = license.txt
> +LIBCAMERA_APPS_DEPENDENCIES = \
> +	host-pkgconf \
> +	boost \
> +	jpeg \
> +	libcamera \
> +	libexif \
> +	libpng \
> +	tiff
> +
> +LIBCAMERA_APPS_CONF_OPTS = \
> +	-DENABLE_COMPILE_FLAGS_FOR_TARGET=disabled \
> +	-DENABLE_OPENCV=0 \
> +	-DENABLE_TFLITE=0
> +
> +ifeq ($(BR2_PACKAGE_LIBDRM),y)
> +LIBCAMERA_APPS_CONF_OPTS += -DENABLE_DRM=1
> +LIBCAMERA_APPS_DEPENDENCIES += libdrm
> +else
> +LIBCAMERA_APPS_CONF_OPTS += -DENABLE_DRM=0
> +endif
> +
> +ifeq ($(BR2_PACKAGE_XORG7)$(BR2_PACKAGE_LIBEPOXY),yy)
> +LIBCAMERA_APPS_CONF_OPTS += -DENABLE_X11=1
> +LIBCAMERA_APPS_DEPENDENCIES += libepoxy xlib_libX11

[0] so here, the condition is only on Xorg && linepoxy, and nothing
about xlib-libx11, but still a dependency is added against it, although
it may well be not enabled.

So, I changed the condition to:

    ifeq ($(BR2_PACKAGE_XORG7),y)
    LIBCAMERA_APPS_DEPENDENCIES += \
        $(if $(BR2_PACKAGE_LIBEPOXY),libepoxy) \
        $(if $(BR2_PACKAGE_XLIB_LIBX11),xlib_libX11)
    LIBCAMERA_APPS_CONF_OPTS += -DENABLE_X11=1
    else
    LIBCAMERA_APPS_CONF_OPTS += -DENABLE_X11=0
    endif

Argh, and now I see I missed something: indeed, libepoxy and xlib-libx11
are selected udner the same conditions, so if either is enabled, the
other is.... Oh, wait, no, they can either be enabled manually by the
user, and not 'selected' by libcanera-apps, so we still can have the
situation where they are not both enabled...

So, if both xlib-libx11 are required, then please send a patch to fix
the comndition. In any case, pelase review the current code, at least to
check if I did not mess too much...

> +else
> +LIBCAMERA_APPS_CONF_OPTS += -DENABLE_X11=0
> +endif
> +
> +ifeq ($(BR2_PACKAGE_QT5)$(BR2_PACKAGE_QT5BASE_GUI)$(BR2_PACKAGE_QT5BASE_WIDGETS),yyy) 

[1] so here, GUI and widgets will be enabled when Qt5 is, so checking
for them is redundant.

Also:

    # make check-package
    package/libcamera-apps/libcamera-apps.mk:39: line contains trailing whitespace

Applied to master with a buncha fixes for all the above, thanks.

Regards,
Yann E. MORIN.

> +LIBCAMERA_APPS_CONF_OPTS += -DENABLE_QT=1
> +LIBCAMERA_APPS_DEPENDENCIES += qt5base
> +else
> +LIBCAMERA_APPS_CONF_OPTS += -DENABLE_QT=0
> +endif
> +
> +$(eval $(cmake-package))
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot