[libcamera-devel] [PATCH 4/5] android: Add camera metadata library

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Aug 01, 2019 at 05:54:19PM +0200, Jacopo Mondi wrote:
> Import the Android camera metadata library from the ChromiumOS build
> system.
> 
> The camera metadata library has been copied from
> https://chromium.googlesource.com/chromiumos/platform2
> at revision ceb477360a8012ee38e80746258f4828aad6b4c7.
> 
> The original path in the Cros platform2/ repository is:
> camera/android/libcamera_metadata/src
> 
> Create a new build target for the camera metadata library to
> compile the libcamera Android HAL implementation against it.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/meson.build                       |   10 +
>  src/android/metadata/camera_metadata.c        | 1204 +++++++
>  .../metadata/camera_metadata_tag_info.c       | 2811 +++++++++++++++++
>  3 files changed, 4025 insertions(+)
>  create mode 100644 src/android/meson.build
>  create mode 100644 src/android/metadata/camera_metadata.c
>  create mode 100644 src/android/metadata/camera_metadata_tag_info.c
> 
> diff --git a/src/android/meson.build b/src/android/meson.build
> new file mode 100644
> index 000000000000..e988dfa9ee63
> --- /dev/null
> +++ b/src/android/meson.build
> @@ -0,0 +1,10 @@
> +android_camera_metadata_sources = files([
> +    'metadata/camera_metadata.c',
> +])
> +
> +# Do not install by default as the target systems (Android, ChromeOS) already
> +# ship a libcamera_metadata.so library.

Shouldn't you thus compile it statically (or at least as a static
library) ? If we want to link at runtime against the shared library
provided by the system then we should compile against the corresponding
headers, not against our own copy of the headers. That wouldn't be
practical, so statically linking is likely best.

Longer term I think we should replace this with a custom C++
implementation.

> +android_camera_metadata = shared_library('camera_metadata',
> +                                         android_camera_metadata_sources,
> +                                         install : false,
> +                                         include_directories : android_includes)
> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c
> new file mode 100644
> index 000000000000..6bfd02da29c7
> --- /dev/null
> +++ b/src/android/metadata/camera_metadata.c
> @@ -0,0 +1,1204 @@

SPDX here too please (in a separate patch).

What are the implications of linking LGPL-2.1+ and Apache-2.0 code
together ?

> +/*
> + * Copyright (C) 2012 The Android Open Source Project
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */

[snip]

-- 
Regards,

Laurent Pinchart