[libcamera-devel] [PATCH 02/16] android: cros: Simplify integration

[Kieran Bingham]

Hi Laurent,

On 25/06/2021 04:20, Laurent Pinchart wrote:
> On Fri, Jun 25, 2021 at 06:18:31AM +0300, Laurent Pinchart wrote:
>> Hi Kieran,
>>
>> Thank you for the patch.
>>
>> On Fri, Jun 25, 2021 at 02:35:25AM +0100, Kieran Bingham wrote:
>>> The ChromeOS specific Android HAL support was overly-complex to support
>>> linking an additional library, and then passing the built object into
>>> the libcamera.so build.
>>>
>>> Now that it has its own distinct libcamera-hal.so, simplify the CrOS
>>> integration to build directly into that library.
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>
>>> ---
>>>  src/android/cros/meson.build | 11 +++--------
>>>  src/android/meson.build      | 13 +++----------
>>>  2 files changed, 6 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build
>>> index c2bda70838e2..35995dd814a2 100644
>>> --- a/src/android/cros/meson.build
>>> +++ b/src/android/cros/meson.build
>>> @@ -4,15 +4,10 @@ if get_option('android_platform') != 'cros'
>>>     subdir_done()
>>>  endif
>>>  
>>> -cros_hal_info_sources = files([
>>> +android_hal_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',
> 
> BTW, we're losing -Wno-shadow. I suppose that's fine as there's no .c
> file, but have you compiled this on CrOS to ensure there's no breakage ?

Yes, Sorry  I should have added that to the commit message, that's
intentional.

I'll add:

> The removal of the -Wno-shadow here is intentional, as it is not
> required for compilation of the camera3_hal.o object.



I believe the -Wno-shadow is cargo-culted from a copy paste of the
android_camera_metadata = static_library() definition (where I expect it
/is/ required)

The camea3_hal.cpp does not require -Wno-shadow to compile.



> 
>>> -                               include_directories : [android_includes,
>>> -                                                      libcamera_includes])
>>> +android_deps += dependency('libcros_camera')
>>>  
>>> -android_objects += cros_hal_info.extract_objects('camera3_hal.cpp')
>>> +android_cpp_args += ['-DOS_CHROMEOS']
>>> diff --git a/src/android/meson.build b/src/android/meson.build
>>> index feda3e0998bd..bd58ef964c2a 100644
>>> --- a/src/android/meson.build
>>> +++ b/src/android/meson.build
>>> @@ -37,15 +37,6 @@ endif
>>>  
>>>  android_deps += [libyuv_dep]
>>>  
>>> -android_cpp_args = []
>>> -android_objects = []
>>> -
>>> -if get_option('android_platform') == 'cros'
>>> -    android_cpp_args += ['-DOS_CHROMEOS']
>>> -endif
>>> -
>>> -subdir('cros')
>>> -
>>>  android_hal_sources = files([
>>>      'camera3_hal.cpp',
>>>      'camera_capabilities.cpp',
>>> @@ -63,6 +54,9 @@ android_hal_sources = files([
>>>      'yuv/post_processor_yuv.cpp'
>>>  ])
>>>  
>>> +android_cpp_args = []
>>> +
>>> +subdir('cros')
>>>  subdir('mm')
>>>  
>>>  android_camera_metadata_sources = files([
>>> @@ -80,6 +74,5 @@ libcamera_hal = shared_library('libcamera-hal',
>>>                                 link_with : android_camera_metadata,
>>>                                 install : true,
>>>                                 cpp_args : android_cpp_args,
>>> -                               objects : android_objects,
>>>                                 include_directories : android_includes,
>>>                                 dependencies : android_deps)
>