[libcamera-devel] [PATCH] android: Split HAL to it's own shared library

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:
> The libcamera Android HAL implementation should not be an integral part
> of libcamera, but a support library that utilises the libcamera public
> API.
> 
> Move the implementation to it's own distinct library.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 
> Please note that this no longer requires ChromeOS to manually symlink
> into the camera_hal directory.
> 
> A modification similar to the following should be applied to the Chrome
> ebuilds if this is applied:
> 
> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild
> index 433357f1ef74..cf90e438cfb3 100644
> --- a/media-libs/libcamera/libcamera-9999.ebuild
> +++ b/media-libs/libcamera/libcamera-9999.ebuild
> @@ -57,6 +57,4 @@ src_compile() {
>  
>  src_install() {
>         meson_src_install
> -
> -       dosym ../libcamera.so "/usr/$(get_libdir)/camera_hal/libcamera.so"
>  }

Could you submit a patch to CrOS ?

>  src/android/meson.build   | 10 ++++++++++
>  src/libcamera/meson.build | 11 -----------
>  src/meson.build           |  7 +++----
>  3 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/src/android/meson.build b/src/android/meson.build
> index 2be20c97e118..6170448a73ce 100644
> --- a/src/android/meson.build
> +++ b/src/android/meson.build
> @@ -3,6 +3,7 @@
>  android_deps = [
>      dependency('libexif', required : get_option('android')),
>      dependency('libjpeg', required : get_option('android')),
> +    libcamera_dep,
>  ]
>  
>  android_enabled = true
> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',
>                                           android_camera_metadata_sources,
>                                           c_args : '-Wno-shadow',
>                                           include_directories : android_includes)
> +
> +android_hal = shared_library('android-hal',

android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so
?

> +                             android_hal_sources,
> +                             name_prefix : '',
> +                             link_with : android_camera_metadata,
> +                             install : true,
> +                             cpp_args : libcamera_cpp_args,
> +                             include_directories : android_includes,
> +                             dependencies : android_deps)

Without install_dir, this will get installed in /usr/lib/, which isn't
where the CrOS camera service will look for it. We probably need a
configuration option to set the path, as it will differ between CrOS and
Android.

> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index e0a48aa23ea5..f77b80b878d8 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -129,16 +129,6 @@ libcamera_deps = [
>      dependency('threads'),
>  ]
>  
> -libcamera_link_with = []
> -
> -if android_enabled
> -    libcamera_sources += android_hal_sources
> -    includes += android_includes
> -    libcamera_link_with += android_camera_metadata
> -
> -    libcamera_deps += android_deps
> -endif
> -
>  # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.
>  # The build_rpath is stripped at install time by meson, so we determine at
>  # runtime if the library is running from an installed location by checking
> @@ -147,7 +137,6 @@ endif
>  libcamera = shared_library('camera',
>                             libcamera_sources,
>                             install : true,
> -                           link_with : libcamera_link_with,
>                             cpp_args : libcamera_cpp_args,
>                             include_directories : includes,
>                             objects : libcamera_objects,
> diff --git a/src/meson.build b/src/meson.build
> index 64d22b9ce5b6..9dc32751bbb1 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -31,11 +31,10 @@ endif
>  libcamera_cpp_args = []
>  libcamera_objects = []
>  
> -# The 'android' subdir must be processed first, and the build targets
> -# are included directly into the libcamera library when this is enabled.
> -subdir('android')
> -
> +# libcamera must be built first as a dependency to the other components.
>  subdir('libcamera')

This is really nice, having android first has been bugging me for a
while.

> +
> +subdir('android')
>  subdir('ipa')
>  
>  subdir('lc-compliance')

-- 
Regards,

Laurent Pinchart