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

Message ID 20210524115640.2334778-2-hiroh@chromium.org
State Superseded
Headers show
Series
  • Fix SIGSEGV in ChromeOS camera3 test
Related show

Commit Message

Hirokazu Honda May 24, 2021, 11:56 a.m. UTC 
ChromeOS specific Camera HAL calls are in android/cros directory.
Moves them to android/camera3_hal.cpp by enclosing them with
OS_CHROMEOS macro.

Signed-off-by: Hirokazu Honda <hiroh@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.
---
 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

--
2.31.1.818.g46aad6cb9e-goog

Comments

Paul Elder May 26, 2021, 7:40 a.m. UTC | #1 
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@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
Hirokazu Honda May 26, 2021, 8:10 a.m. UTC | #2 
Hi Paul,

On Wed, May 26, 2021 at 4:40 PM <paul.elder@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@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
>
Paul Elder May 26, 2021, 8:26 a.m. UTC | #3 
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@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@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
>
Hirokazu Honda May 26, 2021, 8:30 a.m. UTC | #4 
Hi Paul,

On Wed, May 26, 2021 at 5:26 PM <paul.elder@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@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@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
> >
>

Patch
diff mbox series 

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
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>
-
-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',