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

[Hirokazu Honda]

Hi Paul,

On Wed, May 26, 2021 at 4:40 PM <paul.elder at ideasonboard.com> wrote:

> 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().
>
>
Thanks. I think this is correctly exported. I confirmed that tear_down and
set_up both were called by cros_camera_service.


> > 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?
>
>
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.
>
>
I see.
Since cros_hal_info is static library, if we include that, I think we have
to add that to libcamera_link_with like android_camera_metadata?

-Hiro

>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210526/beb85b9d/attachment-0001.htm>