[libcamera-devel] [PATCH RFC 1/2] android: Move ChromeOS specific Camera HAL calls to camera3_hal.cpp

[paul.elder at ideasonboard.com]

Hello Hiro,

On Mon, May 24, 2021 at 08:56:39PM +0900, Hirokazu Honda wrote:
> ChromeOS specific Camera HAL calls are in android/cros directory.
> Moves them to android/camera3_hal.cpp by enclosing them with
> OS_CHROMEOS macro.

I'm not too fond of this movement...

> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> 
> ---
> Found LOG macro conflict issues in libchrome and libcamera.
> See the error message.  https://paste.debian.net/1198594/
> I ask for comments, while I hack the conflict by undefining LOG of
> libcamera.

I remember running into this conflict.

> ---
>  src/android/camera3_hal.cpp      | 27 +++++++++++++++++++++++++++
>  src/android/cros/camera3_hal.cpp | 21 ---------------------
>  src/android/cros/meson.build     | 17 -----------------
>  src/android/meson.build          |  3 +--
>  4 files changed, 28 insertions(+), 40 deletions(-)
>  delete mode 100644 src/android/cros/camera3_hal.cpp
>  delete mode 100644 src/android/cros/meson.build
> 
> diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp
> index 08773d33..f2d4799f 100644
> --- a/src/android/camera3_hal.cpp
> +++ b/src/android/camera3_hal.cpp
> @@ -5,6 +5,14 @@
>   * camera3_hal.cpp - Android Camera HALv3 module
>   */
> 
> +#if defined(OS_CHROMEOS)
> +#include <cros-camera/cros_camera_hal.h>
> +/* HACK. LOG is defined in logging.h in chrome. It conflicts LOG macro in
> + * libcamera.
> + */
> +#undef LOG
> +#endif
> +
>  #include <hardware/camera_common.h>
> 
>  #include "libcamera/internal/log.h"
> @@ -115,3 +123,22 @@ camera_module_t HAL_MODULE_INFO_SYM = {
>  	.init = hal_init,
>  	.reserved = {},
>  };
> +
> +#if defined(OS_CHROMEOS)
> +/*------------------------------------------------------------------------------
> + * ChromeOS specific Camera HAL callbacks
> + */
> +
> +static void set_up(cros::CameraMojoChannelManagerToken *token)
> +{
> +}
> +
> +static void tear_down()
> +{
> +}
> +
> +cros::cros_camera_hal_t CROS_CAMERA_EXPORT CROS_CAMERA_HAL_INFO_SYM = {
> +	.set_up = set_up,
> +	.tear_down = tear_down
> +};
> +#endif

Does this work? Are these symbols exported?

When I wrote this (well, the hunk below) originally, it wasn't being
exported, that's why in the meson file I have a static_library().

> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp
> deleted file mode 100644
> index 31ad36ac..00000000
> --- a/src/android/cros/camera3_hal.cpp
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2021, Google Inc.
> - *
> - * camera3_hal.cpp - cros-specific components of Android Camera HALv3 module
> - */
> -
> -#include <cros-camera/cros_camera_hal.h>

If I understand correctly, what you want to do here is:

#include "../camera_hal_manager.h"

Which fails because:

src/android/cros/../camera_hal_manager.h:20:10: fatal error:
'libcamera/camera_manager.h' file not found:
#include <libcamera/camera_manager.h>
 
Is this correct?

In this case, adding libcamera_includes to the include_directories list
in src/android/cros/meson.build would fix this. Which does include
private and android headers, but I think it's better than merging this
into android/camera3_hal.cpp.


Paul

> -
> -static void set_up(cros::CameraMojoChannelManagerToken *token)
> -{
> -}
> -
> -static void tear_down()
> -{
> -}
> -
> -cros::cros_camera_hal_t CROS_CAMERA_EXPORT CROS_CAMERA_HAL_INFO_SYM = {
> -	.set_up = set_up,
> -	.tear_down = tear_down
> -};
> diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build
> deleted file mode 100644
> index 4aab0f20..00000000
> --- a/src/android/cros/meson.build
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -# SPDX-License-Identifier: CC0-1.0
> -
> -if get_option('android_platform') != 'cros'
> -   subdir_done()
> -endif
> -
> -cros_hal_info_sources = files([
> -    'camera3_hal.cpp',
> -])
> -
> -cros_hal_info = static_library('cros_hal_info',
> -                               cros_hal_info_sources,
> -                               dependencies : dependency('libcros_camera'),
> -                               c_args : '-Wno-shadow',
> -                               include_directories : android_includes)
> -
> -libcamera_objects += cros_hal_info.extract_objects('camera3_hal.cpp')
> diff --git a/src/android/meson.build b/src/android/meson.build
> index 2be20c97..84144f33 100644
> --- a/src/android/meson.build
> +++ b/src/android/meson.build
> @@ -37,10 +37,9 @@ android_deps += [libyuv_dep]
> 
>  if get_option('android_platform') == 'cros'
>     libcamera_cpp_args += [ '-DOS_CHROMEOS']
> +   android_deps += [dependency('libcros_camera')]
>  endif
> 
> -subdir('cros')
> -
>  android_hal_sources = files([
>      'camera3_hal.cpp',
>      'camera_hal_manager.cpp',
> --
> 2.31.1.818.g46aad6cb9e-goog