[libcamera-devel] [PATCH RFC 1/2] android: Move ChromeOS specific Camera HAL calls to camera3_hal.cpp
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Wed May 26 10:26:12 CEST 2021
Hi Hiro,
On Wed, May 26, 2021 at 05:10:55PM +0900, Hirokazu Honda wrote:
> 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.
>
Ah, okay.
>
> > 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?
I don't think so, because it's linked back into libcamera:
libcamera_objects += cros_hal_info.extract_objects('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
>
More information about the libcamera-devel
mailing list