<div dir="ltr"><div dir="ltr">Hi Paul,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 26, 2021 at 4:40 PM <<a href="mailto:paul.elder@ideasonboard.com">paul.elder@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Hiro,<br>
<br>
On Mon, May 24, 2021 at 08:56:39PM +0900, Hirokazu Honda wrote:<br>
> ChromeOS specific Camera HAL calls are in android/cros directory.<br>
> Moves them to android/camera3_hal.cpp by enclosing them with<br>
> OS_CHROMEOS macro.<br>
<br>
I'm not too fond of this movement...<br>
<br>
> <br>
> Signed-off-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org" target="_blank">hiroh@chromium.org</a>><br>
> <br>
> ---<br>
> Found LOG macro conflict issues in libchrome and libcamera.<br>
> See the error message. <a href="https://paste.debian.net/1198594/" rel="noreferrer" target="_blank">https://paste.debian.net/1198594/</a><br>
> I ask for comments, while I hack the conflict by undefining LOG of<br>
> libcamera.<br>
<br>
I remember running into this conflict.<br>
<br>
> ---<br>
> src/android/camera3_hal.cpp | 27 +++++++++++++++++++++++++++<br>
> src/android/cros/camera3_hal.cpp | 21 ---------------------<br>
> src/android/cros/meson.build | 17 -----------------<br>
> src/android/meson.build | 3 +--<br>
> 4 files changed, 28 insertions(+), 40 deletions(-)<br>
> delete mode 100644 src/android/cros/camera3_hal.cpp<br>
> delete mode 100644 src/android/cros/meson.build<br>
> <br>
> diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp<br>
> index 08773d33..f2d4799f 100644<br>
> --- a/src/android/camera3_hal.cpp<br>
> +++ b/src/android/camera3_hal.cpp<br>
> @@ -5,6 +5,14 @@<br>
> * camera3_hal.cpp - Android Camera HALv3 module<br>
> */<br>
> <br>
> +#if defined(OS_CHROMEOS)<br>
> +#include <cros-camera/cros_camera_hal.h><br>
> +/* HACK. LOG is defined in logging.h in chrome. It conflicts LOG macro in<br>
> + * libcamera.<br>
> + */<br>
> +#undef LOG<br>
> +#endif<br>
> +<br>
> #include <hardware/camera_common.h><br>
> <br>
> #include "libcamera/internal/log.h"<br>
> @@ -115,3 +123,22 @@ camera_module_t HAL_MODULE_INFO_SYM = {<br>
> .init = hal_init,<br>
> .reserved = {},<br>
> };<br>
> +<br>
> +#if defined(OS_CHROMEOS)<br>
> +/*------------------------------------------------------------------------------<br>
> + * ChromeOS specific Camera HAL callbacks<br>
> + */<br>
> +<br>
> +static void set_up(cros::CameraMojoChannelManagerToken *token)<br>
> +{<br>
> +}<br>
> +<br>
> +static void tear_down()<br>
> +{<br>
> +}<br>
> +<br>
> +cros::cros_camera_hal_t CROS_CAMERA_EXPORT CROS_CAMERA_HAL_INFO_SYM = {<br>
> + .set_up = set_up,<br>
> + .tear_down = tear_down<br>
> +};<br>
> +#endif<br>
<br>
Does this work? Are these symbols exported?<br>
<br>
When I wrote this (well, the hunk below) originally, it wasn't being<br>
exported, that's why in the meson file I have a static_library().<br>
<br></blockquote><div><br></div><div>Thanks. I think this is correctly exported. I confirmed that tear_down and set_up both were called by cros_camera_service.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp<br>
> deleted file mode 100644<br>
> index 31ad36ac..00000000<br>
> --- a/src/android/cros/camera3_hal.cpp<br>
> +++ /dev/null<br>
> @@ -1,21 +0,0 @@<br>
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> -/*<br>
> - * Copyright (C) 2021, Google Inc.<br>
> - *<br>
> - * camera3_hal.cpp - cros-specific components of Android Camera HALv3 module<br>
> - */<br>
> -<br>
> -#include <cros-camera/cros_camera_hal.h><br>
<br>
If I understand correctly, what you want to do here is:<br>
<br>
#include "../camera_hal_manager.h"<br>
<br>
Which fails because:<br>
<br>
src/android/cros/../camera_hal_manager.h:20:10: fatal error:<br>
'libcamera/camera_manager.h' file not found:<br>
#include <libcamera/camera_manager.h><br>
<br>
Is this correct?<br>
<br></blockquote><div><br></div><div>Correct.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
In this case, adding libcamera_includes to the include_directories list<br>
in src/android/cros/meson.build would fix this. Which does include<br>
private and android headers, but I think it's better than merging this<br>
into android/camera3_hal.cpp.<br>
<br></blockquote><div><br></div><div>I see.</div><div>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?</div><div> </div><div>-Hiro</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Paul<br>
<br>
> -<br>
> -static void set_up(cros::CameraMojoChannelManagerToken *token)<br>
> -{<br>
> -}<br>
> -<br>
> -static void tear_down()<br>
> -{<br>
> -}<br>
> -<br>
> -cros::cros_camera_hal_t CROS_CAMERA_EXPORT CROS_CAMERA_HAL_INFO_SYM = {<br>
> - .set_up = set_up,<br>
> - .tear_down = tear_down<br>
> -};<br>
> diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build<br>
> deleted file mode 100644<br>
> index 4aab0f20..00000000<br>
> --- a/src/android/cros/meson.build<br>
> +++ /dev/null<br>
> @@ -1,17 +0,0 @@<br>
> -# SPDX-License-Identifier: CC0-1.0<br>
> -<br>
> -if get_option('android_platform') != 'cros'<br>
> - subdir_done()<br>
> -endif<br>
> -<br>
> -cros_hal_info_sources = files([<br>
> - 'camera3_hal.cpp',<br>
> -])<br>
> -<br>
> -cros_hal_info = static_library('cros_hal_info',<br>
> - cros_hal_info_sources,<br>
> - dependencies : dependency('libcros_camera'),<br>
> - c_args : '-Wno-shadow',<br>
> - include_directories : android_includes)<br>
> -<br>
> -libcamera_objects += cros_hal_info.extract_objects('camera3_hal.cpp')<br>
> diff --git a/src/android/meson.build b/src/android/meson.build<br>
> index 2be20c97..84144f33 100644<br>
> --- a/src/android/meson.build<br>
> +++ b/src/android/meson.build<br>
> @@ -37,10 +37,9 @@ android_deps += [libyuv_dep]<br>
> <br>
> if get_option('android_platform') == 'cros'<br>
> libcamera_cpp_args += [ '-DOS_CHROMEOS']<br>
> + android_deps += [dependency('libcros_camera')]<br>
> endif<br>
> <br>
> -subdir('cros')<br>
> -<br>
> android_hal_sources = files([<br>
> 'camera3_hal.cpp',<br>
> 'camera_hal_manager.cpp',<br>
> --<br>
> 2.31.1.818.g46aad6cb9e-goog<br>
</blockquote></div></div>