[libcamera-devel] [PATCH RFC 1/2] android: Move ChromeOS specific Camera HAL calls to camera3_hal.cpp
Hirokazu Honda
hiroh at chromium.org
Wed May 26 10:30:40 CEST 2021
Hi Paul,
On Wed, May 26, 2021 at 5:26 PM <paul.elder at ideasonboard.com> wrote:
> 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')
>
>
Ack. I am not familiar enough with meson. :p
I will upload the next version shortly.
Thanks,
-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/ae2e8cf1/attachment.htm>
More information about the libcamera-devel
mailing list