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

Jacopo Mondi jacopo at jmondi.org
Tue Aug 6 16:26:09 CEST 2019


Hi Laurent,

On Tue, Aug 06, 2019 at 04:22:40PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tue, Aug 06, 2019 at 12:38:04PM +0200, Jacopo Mondi wrote:
> > On Mon, Aug 05, 2019 at 02:49:36PM +0300, Laurent Pinchart wrote:
> > > 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.
> >
> > Wouldn't this open door to mis-alignements between the metadata
> > library version we compile libcamera against and the one the camera
> > framework on the target system provides ? In the same way we might
> > have a mis-alignemnt of headers version when linking dynamically, we
> > would have the same if we statically link our version of the metadata
> > library.. This is tricky to solve, as we might then need to
> > ship/include different version of this library depending which is the
> > target system the HAL is compiled for... As of now, as we only target CrOS,
> > keeping in sync headers and the library from the cros sources and here
> > is not hard, but what when Android support will be added?
>
> Are there differences in the data layout between different Android
> versions ? I thought the layout was stable (but I could be wrong).

I can't tell, seems unlikely, but still... I was more concerned about
using tags defined in a later library versions than the one running on
the system more than the layout itself...
>
> > For now, with a single system supported, if we make sure headers and
> > the metadata library implementation are in sync between libcamera and
> > the cros sources, I guess statically linking would only give us a tad
> > fatter library without any additional guarantees.
>
> Making sure they're in sync is the issue. You should either copy both
> the header and sources to the libcamera tree, and compile statically, or
> copy none. Especially copying the sources to link dynamically and then
> run against a different binary is a bad idea. I have a preference for
> copying both for now, but if you prefer copying neither, I'm OK with
> that too. We would then need another compilation option to point to the
> headers and library to be compiled and linked against.
>

Ok, I indeed prefer to copy both for now instead of requiring users to
have their own copies on the system and point to them. We'll might
want to change this at a later stage or let the integration in
android/cros system handle this.


> > > 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 ?
> >
> > Eh, how fun is to deal with legal stuff..
> >
> > So, not need to say but IANAL and none of us is, so the best source I
> > could find is this stackoverflow post
> > https://opensource.stackexchange.com/questions/5664/linking-from-lgpl-2-1-software-to-apache-2-0-library
> >
> > The general question we are trying to answer is:
> >
> >     Can copyleft-licensed code depend on non-copyleft-licensed code that
> >     is using a license that is deemed incompatible with a given copyleft
> >     license?
> >
> > Going through the answers and the FAQs on the GPL license website, it
> > seems the me the one that applies the most to our use case is:
> >
> >
> >         What legal issues come up if I use GPL-incompatible libraries with
> >         GPL software? If you want your program to link against a library
> >         not covered by the system library exception, you need to provide
> >         permission to do that.
> >
> >         So even though the LGPL is not the GPL, I would say that the best way
> >         to do this would be to follow the guidelines provided at the FAQ and
> >         license your LGPL library with an exception stating that this does not
> >         extend to the Apache-licensed dependencies. Something similar to
> >         OpenSSL exceptions commonly found in several places such as here.
> >
> > That's because I'm not sure I found a proper definition of "system
> > library exception" for LGPL-2.1. If the metadata library falls under
> > that term, we should be good the way we are, otherwise we would need
> > to add a notice.
>
> The system library exception mostly covers glibc, and possibly a few
> other similar system libraries from the GNU project. The camera metadata
> library isn't included.

I have guessed so.
>
> The GPL and LGPL apply to binary distribution time, so linking
> statically against the metadata library is probably an issue, while
> linking dynamically is probably safer. I wonder how much this calls for
> a custom implementation...
>

Won't an adding an exception cover the static linking case as well?
I agree that an internal implementation might be desirable and would
save us some license headaches, but it should be implemented and I'm
not sure it is top priority at the moment...


> > > > +/*
> > > > + * 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190806/062c5025/attachment.sig>


More information about the libcamera-devel mailing list