<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 5:26 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">Hi Hiro,<br>
<br>
On Wed, May 26, 2021 at 05:10:55PM +0900, Hirokazu Honda wrote:<br>
> Hi Paul,<br>
> <br>
> On Wed, May 26, 2021 at 4:40 PM <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>> wrote:<br>
> <br>
> 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<br>
> 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>
> *------------------------------------------------------------------------------<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>
> <br>
> <br>
> Thanks. I think this is correctly exported. I confirmed that tear_down and<br>
> set_up both were called by cros_camera_service.<br>
> <br>
<br>
Ah, okay.<br>
<br>
> <br>
> > diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/<br>
> 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<br>
> 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>
> <br>
> <br>
> Correct.<br>
> <br>
> <br>
> 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>
> <br>
> <br>
> I see.<br>
> Since cros_hal_info is static library, if we include that, I think we have to<br>
> add that to libcamera_link_with like android_camera_metadata?<br>
<br>
I don't think so, because it's linked back into libcamera:<br>
<br>
libcamera_objects += cros_hal_info.extract_objects('camera3_hal.cpp')<br>
<br></blockquote><div><br></div><div>Ack. I am not familiar enough with meson. :p</div><div>I will upload the next version shortly.</div><div><br></div><div>Thanks,</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<br>
> ('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>
> <br>
</blockquote></div></div>