[{"id":2315,"web_url":"https://patchwork.libcamera.org/comment/2315/","msgid":"<20190805114936.GP29747@pendragon.ideasonboard.com>","date":"2019-08-05T11:49:36","subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Aug 01, 2019 at 05:54:19PM +0200, Jacopo Mondi wrote:\n> Import the Android camera metadata library from the ChromiumOS build\n> system.\n> \n> The camera metadata library has been copied from\n> https://chromium.googlesource.com/chromiumos/platform2\n> at revision ceb477360a8012ee38e80746258f4828aad6b4c7.\n> \n> The original path in the Cros platform2/ repository is:\n> camera/android/libcamera_metadata/src\n> \n> Create a new build target for the camera metadata library to\n> compile the libcamera Android HAL implementation against it.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/meson.build | 10 +\n> src/android/metadata/camera_metadata.c | 1204 +++++++\n> .../metadata/camera_metadata_tag_info.c | 2811 +++++++++++++++++\n> 3 files changed, 4025 insertions(+)\n> create mode 100644 src/android/meson.build\n> create mode 100644 src/android/metadata/camera_metadata.c\n> create mode 100644 src/android/metadata/camera_metadata_tag_info.c\n> \n> diff --git a/src/android/meson.build b/src/android/meson.build\n> new file mode 100644\n> index 000000000000..e988dfa9ee63\n> --- /dev/null\n> +++ b/src/android/meson.build\n> @@ -0,0 +1,10 @@\n> +android_camera_metadata_sources = files([\n> + 'metadata/camera_metadata.c',\n> +])\n> +\n> +# Do not install by default as the target systems (Android, ChromeOS) already\n> +# ship a libcamera_metadata.so library.\n\nShouldn't you thus compile it statically (or at least as a static\nlibrary) ? If we want to link at runtime against the shared library\nprovided by the system then we should compile against the corresponding\nheaders, not against our own copy of the headers. That wouldn't be\npractical, so statically linking is likely best.\n\nLonger term I think we should replace this with a custom C++\nimplementation.\n\n> +android_camera_metadata = shared_library('camera_metadata',\n> + android_camera_metadata_sources,\n> + install : false,\n> + include_directories : android_includes)\n> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> new file mode 100644\n> index 000000000000..6bfd02da29c7\n> --- /dev/null\n> +++ b/src/android/metadata/camera_metadata.c\n> @@ -0,0 +1,1204 @@\n\nSPDX here too please (in a separate patch).\n\nWhat are the implications of linking LGPL-2.1+ and Apache-2.0 code\ntogether ?\n\n> +/*\n> + * Copyright (C) 2012 The Android Open Source Project\n> + *\n> + * Licensed under the Apache License, Version 2.0 (the \"License\");\n> + * you may not use this file except in compliance with the License.\n> + * You may obtain a copy of the License at\n> + *\n> + * http://www.apache.org/licenses/LICENSE-2.0\n> + *\n> + * Unless required by applicable law or agreed to in writing, software\n> + * distributed under the License is distributed on an \"AS IS\" BASIS,\n> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n> + * See the License for the specific language governing permissions and\n> + * limitations under the License.\n> + */\n\n[snip]","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 37D1C60E33\n\tfor ;\n\tMon, 5 Aug 2019 13:49:39 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8A69C2F9;\n\tMon, 5 Aug 2019 13:49:38 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565005778;\n\tbh=WJCCrZgDkUtsLzje2QbNRbIy852LyDwTmHJqP6oPRNo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sMS0dXTkBPFJBeHewlveMPGxYsL5G4zbje5GcwXfCVZeflduBNzNbq7sdRMqEIYqx\n\tfLonYS/ZaPos7Xh8Tuj3awg6/VKeaqc8vCNlu5U1LYWecNJCwjmph/nSUY085rnAom\n\tMxDkUNKN8snXJgBKeSVuHhIEs7iP1EIi/mwI6+jE=","Date":"Mon, 5 Aug 2019 14:49:36 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190805114936.GP29747@pendragon.ideasonboard.com>","References":"<20190801155420.24694-1-jacopo@jmondi.org>\n\t<20190801155420.24694-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190801155420.24694-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 05 Aug 2019 11:49:39 -0000"}},{"id":2323,"web_url":"https://patchwork.libcamera.org/comment/2323/","msgid":"<20190806103804.p7ntcoa6kuitl4jt@uno.localdomain>","date":"2019-08-06T10:38:04","subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Aug 05, 2019 at 02:49:36PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Aug 01, 2019 at 05:54:19PM +0200, Jacopo Mondi wrote:\n> > Import the Android camera metadata library from the ChromiumOS build\n> > system.\n> >\n> > The camera metadata library has been copied from\n> > https://chromium.googlesource.com/chromiumos/platform2\n> > at revision ceb477360a8012ee38e80746258f4828aad6b4c7.\n> >\n> > The original path in the Cros platform2/ repository is:\n> > camera/android/libcamera_metadata/src\n> >\n> > Create a new build target for the camera metadata library to\n> > compile the libcamera Android HAL implementation against it.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/meson.build | 10 +\n> > src/android/metadata/camera_metadata.c | 1204 +++++++\n> > .../metadata/camera_metadata_tag_info.c | 2811 +++++++++++++++++\n> > 3 files changed, 4025 insertions(+)\n> > create mode 100644 src/android/meson.build\n> > create mode 100644 src/android/metadata/camera_metadata.c\n> > create mode 100644 src/android/metadata/camera_metadata_tag_info.c\n> >\n> > diff --git a/src/android/meson.build b/src/android/meson.build\n> > new file mode 100644\n> > index 000000000000..e988dfa9ee63\n> > --- /dev/null\n> > +++ b/src/android/meson.build\n> > @@ -0,0 +1,10 @@\n> > +android_camera_metadata_sources = files([\n> > + 'metadata/camera_metadata.c',\n> > +])\n> > +\n> > +# Do not install by default as the target systems (Android, ChromeOS) already\n> > +# ship a libcamera_metadata.so library.\n>\n> Shouldn't you thus compile it statically (or at least as a static\n> library) ? If we want to link at runtime against the shared library\n> provided by the system then we should compile against the corresponding\n> headers, not against our own copy of the headers. That wouldn't be\n> practical, so statically linking is likely best.\n>\n\nWouldn't this open door to mis-alignements between the metadata\nlibrary version we compile libcamera against and the one the camera\nframework on the target system provides ? In the same way we might\nhave a mis-alignemnt of headers version when linking dynamically, we\nwould have the same if we statically link our version of the metadata\nlibrary.. This is tricky to solve, as we might then need to\nship/include different version of this library depending which is the\ntarget system the HAL is compiled for... As of now, as we only target CrOS,\nkeeping in sync headers and the library from the cros sources and here\nis not hard, but what when Android support will be added?\n\nFor now, with a single system supported, if we make sure headers and\nthe metadata library implementation are in sync between libcamera and\nthe cros sources, I guess statically linking would only give us a tad\nfatter library without any additional guarantees.\n\n> Longer term I think we should replace this with a custom C++\n> implementation.\n>\n> > +android_camera_metadata = shared_library('camera_metadata',\n> > + android_camera_metadata_sources,\n> > + install : false,\n> > + include_directories : android_includes)\n> > diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> > new file mode 100644\n> > index 000000000000..6bfd02da29c7\n> > --- /dev/null\n> > +++ b/src/android/metadata/camera_metadata.c\n> > @@ -0,0 +1,1204 @@\n>\n> SPDX here too please (in a separate patch).\n>\n> What are the implications of linking LGPL-2.1+ and Apache-2.0 code\n> together ?\n>\n\nEh, how fun is to deal with legal stuff..\n\nSo, not need to say but IANAL and none of us is, so the best source I\ncould find is this stackoverflow post\nhttps://opensource.stackexchange.com/questions/5664/linking-from-lgpl-2-1-software-to-apache-2-0-library\n\nThe general question we are trying to answer is:\n\n Can copyleft-licensed code depend on non-copyleft-licensed code that\n is using a license that is deemed incompatible with a given copyleft\n license?\n\nGoing through the answers and the FAQs on the GPL license website, it\nseems the me the one that applies the most to our use case is:\n\n\n What legal issues come up if I use GPL-incompatible libraries with\n GPL software? If you want your program to link against a library\n not covered by the system library exception, you need to provide\n permission to do that.\n\n So even though the LGPL is not the GPL, I would say that the best way\n to do this would be to follow the guidelines provided at the FAQ and\n license your LGPL library with an exception stating that this does not\n extend to the Apache-licensed dependencies. Something similar to\n OpenSSL exceptions commonly found in several places such as here.\n\nThat's because I'm not sure I found a proper definition of \"system\nlibrary exception\" for LGPL-2.1. If the metadata library falls under\nthat term, we should be good the way we are, otherwise we would need\nto add a notice.\n\n> > +/*\n> > + * Copyright (C) 2012 The Android Open Source Project\n> > + *\n> > + * Licensed under the Apache License, Version 2.0 (the \"License\");\n> > + * you may not use this file except in compliance with the License.\n> > + * You may obtain a copy of the License at\n> > + *\n> > + * http://www.apache.org/licenses/LICENSE-2.0\n> > + *\n> > + * Unless required by applicable law or agreed to in writing, software\n> > + * distributed under the License is distributed on an \"AS IS\" BASIS,\n> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n> > + * See the License for the specific language governing permissions and\n> > + * limitations under the License.\n> > + */\n>\n> [snip]\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4EF876161A\n\tfor ;\n\tTue, 6 Aug 2019 12:36:41 +0200 (CEST)","from uno.localdomain\n\t(host150-24-dynamic.51-79-r.retail.telecomitalia.it [79.51.24.150])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 8F56A40005;\n\tTue, 6 Aug 2019 10:36:40 +0000 (UTC)"],"X-Originating-IP":"79.51.24.150","Date":"Tue, 6 Aug 2019 12:38:04 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190806103804.p7ntcoa6kuitl4jt@uno.localdomain>","References":"<20190801155420.24694-1-jacopo@jmondi.org>\n\t<20190801155420.24694-5-jacopo@jmondi.org>\n\t<20190805114936.GP29747@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"4np32t57iqejwzyt\"","Content-Disposition":"inline","In-Reply-To":"<20190805114936.GP29747@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 06 Aug 2019 10:36:41 -0000"}},{"id":2324,"web_url":"https://patchwork.libcamera.org/comment/2324/","msgid":"<20190806132240.GA4969@pendragon.ideasonboard.com>","date":"2019-08-06T13:22:40","subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Aug 06, 2019 at 12:38:04PM +0200, Jacopo Mondi wrote:\n> On Mon, Aug 05, 2019 at 02:49:36PM +0300, Laurent Pinchart wrote:\n> > On Thu, Aug 01, 2019 at 05:54:19PM +0200, Jacopo Mondi wrote:\n> > > Import the Android camera metadata library from the ChromiumOS build\n> > > system.\n> > >\n> > > The camera metadata library has been copied from\n> > > https://chromium.googlesource.com/chromiumos/platform2\n> > > at revision ceb477360a8012ee38e80746258f4828aad6b4c7.\n> > >\n> > > The original path in the Cros platform2/ repository is:\n> > > camera/android/libcamera_metadata/src\n> > >\n> > > Create a new build target for the camera metadata library to\n> > > compile the libcamera Android HAL implementation against it.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/android/meson.build | 10 +\n> > > src/android/metadata/camera_metadata.c | 1204 +++++++\n> > > .../metadata/camera_metadata_tag_info.c | 2811 +++++++++++++++++\n> > > 3 files changed, 4025 insertions(+)\n> > > create mode 100644 src/android/meson.build\n> > > create mode 100644 src/android/metadata/camera_metadata.c\n> > > create mode 100644 src/android/metadata/camera_metadata_tag_info.c\n> > >\n> > > diff --git a/src/android/meson.build b/src/android/meson.build\n> > > new file mode 100644\n> > > index 000000000000..e988dfa9ee63\n> > > --- /dev/null\n> > > +++ b/src/android/meson.build\n> > > @@ -0,0 +1,10 @@\n> > > +android_camera_metadata_sources = files([\n> > > + 'metadata/camera_metadata.c',\n> > > +])\n> > > +\n> > > +# Do not install by default as the target systems (Android, ChromeOS) already\n> > > +# ship a libcamera_metadata.so library.\n> >\n> > Shouldn't you thus compile it statically (or at least as a static\n> > library) ? If we want to link at runtime against the shared library\n> > provided by the system then we should compile against the corresponding\n> > headers, not against our own copy of the headers. That wouldn't be\n> > practical, so statically linking is likely best.\n> \n> Wouldn't this open door to mis-alignements between the metadata\n> library version we compile libcamera against and the one the camera\n> framework on the target system provides ? In the same way we might\n> have a mis-alignemnt of headers version when linking dynamically, we\n> would have the same if we statically link our version of the metadata\n> library.. This is tricky to solve, as we might then need to\n> ship/include different version of this library depending which is the\n> target system the HAL is compiled for... As of now, as we only target CrOS,\n> keeping in sync headers and the library from the cros sources and here\n> is not hard, but what when Android support will be added?\n\nAre there differences in the data layout between different Android\nversions ? I thought the layout was stable (but I could be wrong).\n\n> For now, with a single system supported, if we make sure headers and\n> the metadata library implementation are in sync between libcamera and\n> the cros sources, I guess statically linking would only give us a tad\n> fatter library without any additional guarantees.\n\nMaking sure they're in sync is the issue. You should either copy both\nthe header and sources to the libcamera tree, and compile statically, or\ncopy none. Especially copying the sources to link dynamically and then\nrun against a different binary is a bad idea. I have a preference for\ncopying both for now, but if you prefer copying neither, I'm OK with\nthat too. We would then need another compilation option to point to the\nheaders and library to be compiled and linked against.\n\n> > Longer term I think we should replace this with a custom C++\n> > implementation.\n> >\n> > > +android_camera_metadata = shared_library('camera_metadata',\n> > > + android_camera_metadata_sources,\n> > > + install : false,\n> > > + include_directories : android_includes)\n> > > diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> > > new file mode 100644\n> > > index 000000000000..6bfd02da29c7\n> > > --- /dev/null\n> > > +++ b/src/android/metadata/camera_metadata.c\n> > > @@ -0,0 +1,1204 @@\n> >\n> > SPDX here too please (in a separate patch).\n> >\n> > What are the implications of linking LGPL-2.1+ and Apache-2.0 code\n> > together ?\n> \n> Eh, how fun is to deal with legal stuff..\n> \n> So, not need to say but IANAL and none of us is, so the best source I\n> could find is this stackoverflow post\n> https://opensource.stackexchange.com/questions/5664/linking-from-lgpl-2-1-software-to-apache-2-0-library\n> \n> The general question we are trying to answer is:\n> \n> Can copyleft-licensed code depend on non-copyleft-licensed code that\n> is using a license that is deemed incompatible with a given copyleft\n> license?\n> \n> Going through the answers and the FAQs on the GPL license website, it\n> seems the me the one that applies the most to our use case is:\n> \n> \n> What legal issues come up if I use GPL-incompatible libraries with\n> GPL software? If you want your program to link against a library\n> not covered by the system library exception, you need to provide\n> permission to do that.\n> \n> So even though the LGPL is not the GPL, I would say that the best way\n> to do this would be to follow the guidelines provided at the FAQ and\n> license your LGPL library with an exception stating that this does not\n> extend to the Apache-licensed dependencies. Something similar to\n> OpenSSL exceptions commonly found in several places such as here.\n> \n> That's because I'm not sure I found a proper definition of \"system\n> library exception\" for LGPL-2.1. If the metadata library falls under\n> that term, we should be good the way we are, otherwise we would need\n> to add a notice.\n\nThe system library exception mostly covers glibc, and possibly a few\nother similar system libraries from the GNU project. The camera metadata\nlibrary isn't included.\n\nThe GPL and LGPL apply to binary distribution time, so linking\nstatically against the metadata library is probably an issue, while\nlinking dynamically is probably safer. I wonder how much this calls for\na custom implementation...\n\n> > > +/*\n> > > + * Copyright (C) 2012 The Android Open Source Project\n> > > + *\n> > > + * Licensed under the Apache License, Version 2.0 (the \"License\");\n> > > + * you may not use this file except in compliance with the License.\n> > > + * You may obtain a copy of the License at\n> > > + *\n> > > + * http://www.apache.org/licenses/LICENSE-2.0\n> > > + *\n> > > + * Unless required by applicable law or agreed to in writing, software\n> > > + * distributed under the License is distributed on an \"AS IS\" BASIS,\n> > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n> > > + * See the License for the specific language governing permissions and\n> > > + * limitations under the License.\n> > > + */\n> >\n> > [snip]","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8DEC76161A\n\tfor ;\n\tTue, 6 Aug 2019 15:22:43 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C17C52F9;\n\tTue, 6 Aug 2019 15:22:42 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565097762;\n\tbh=p815X58AM3A3gEDXPJkBHGH20gQX07ANIKX4A4HOuCo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jpwNfoN3deVol9ZD4qpgMLKJjsWXOvzx7K9406TPlG6+VHNNi+Xwk5roLStx4DSXt\n\tkHX6WkClxPX0gG3bOZEglvCE2eYTXoFPOso6B5D4yh5cOpgMnRutsgCOcMRIf/W3iA\n\t2g2e/4CyjPR3MU6X17QWDvelb9zSM4TfOKFFM11U=","Date":"Tue, 6 Aug 2019 16:22:40 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190806132240.GA4969@pendragon.ideasonboard.com>","References":"<20190801155420.24694-1-jacopo@jmondi.org>\n\t<20190801155420.24694-5-jacopo@jmondi.org>\n\t<20190805114936.GP29747@pendragon.ideasonboard.com>\n\t<20190806103804.p7ntcoa6kuitl4jt@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190806103804.p7ntcoa6kuitl4jt@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 06 Aug 2019 13:22:43 -0000"}},{"id":2325,"web_url":"https://patchwork.libcamera.org/comment/2325/","msgid":"<20190806142609.afskgj2xwjgovitb@uno.localdomain>","date":"2019-08-06T14:26:09","subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Aug 06, 2019 at 04:22:40PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Tue, Aug 06, 2019 at 12:38:04PM +0200, Jacopo Mondi wrote:\n> > On Mon, Aug 05, 2019 at 02:49:36PM +0300, Laurent Pinchart wrote:\n> > > On Thu, Aug 01, 2019 at 05:54:19PM +0200, Jacopo Mondi wrote:\n> > > > Import the Android camera metadata library from the ChromiumOS build\n> > > > system.\n> > > >\n> > > > The camera metadata library has been copied from\n> > > > https://chromium.googlesource.com/chromiumos/platform2\n> > > > at revision ceb477360a8012ee38e80746258f4828aad6b4c7.\n> > > >\n> > > > The original path in the Cros platform2/ repository is:\n> > > > camera/android/libcamera_metadata/src\n> > > >\n> > > > Create a new build target for the camera metadata library to\n> > > > compile the libcamera Android HAL implementation against it.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/android/meson.build | 10 +\n> > > > src/android/metadata/camera_metadata.c | 1204 +++++++\n> > > > .../metadata/camera_metadata_tag_info.c | 2811 +++++++++++++++++\n> > > > 3 files changed, 4025 insertions(+)\n> > > > create mode 100644 src/android/meson.build\n> > > > create mode 100644 src/android/metadata/camera_metadata.c\n> > > > create mode 100644 src/android/metadata/camera_metadata_tag_info.c\n> > > >\n> > > > diff --git a/src/android/meson.build b/src/android/meson.build\n> > > > new file mode 100644\n> > > > index 000000000000..e988dfa9ee63\n> > > > --- /dev/null\n> > > > +++ b/src/android/meson.build\n> > > > @@ -0,0 +1,10 @@\n> > > > +android_camera_metadata_sources = files([\n> > > > + 'metadata/camera_metadata.c',\n> > > > +])\n> > > > +\n> > > > +# Do not install by default as the target systems (Android, ChromeOS) already\n> > > > +# ship a libcamera_metadata.so library.\n> > >\n> > > Shouldn't you thus compile it statically (or at least as a static\n> > > library) ? If we want to link at runtime against the shared library\n> > > provided by the system then we should compile against the corresponding\n> > > headers, not against our own copy of the headers. That wouldn't be\n> > > practical, so statically linking is likely best.\n> >\n> > Wouldn't this open door to mis-alignements between the metadata\n> > library version we compile libcamera against and the one the camera\n> > framework on the target system provides ? In the same way we might\n> > have a mis-alignemnt of headers version when linking dynamically, we\n> > would have the same if we statically link our version of the metadata\n> > library.. This is tricky to solve, as we might then need to\n> > ship/include different version of this library depending which is the\n> > target system the HAL is compiled for... As of now, as we only target CrOS,\n> > keeping in sync headers and the library from the cros sources and here\n> > is not hard, but what when Android support will be added?\n>\n> Are there differences in the data layout between different Android\n> versions ? I thought the layout was stable (but I could be wrong).\n\nI can't tell, seems unlikely, but still... I was more concerned about\nusing tags defined in a later library versions than the one running on\nthe system more than the layout itself...\n>\n> > For now, with a single system supported, if we make sure headers and\n> > the metadata library implementation are in sync between libcamera and\n> > the cros sources, I guess statically linking would only give us a tad\n> > fatter library without any additional guarantees.\n>\n> Making sure they're in sync is the issue. You should either copy both\n> the header and sources to the libcamera tree, and compile statically, or\n> copy none. Especially copying the sources to link dynamically and then\n> run against a different binary is a bad idea. I have a preference for\n> copying both for now, but if you prefer copying neither, I'm OK with\n> that too. We would then need another compilation option to point to the\n> headers and library to be compiled and linked against.\n>\n\nOk, I indeed prefer to copy both for now instead of requiring users to\nhave their own copies on the system and point to them. We'll might\nwant to change this at a later stage or let the integration in\nandroid/cros system handle this.\n\n\n> > > Longer term I think we should replace this with a custom C++\n> > > implementation.\n> > >\n> > > > +android_camera_metadata = shared_library('camera_metadata',\n> > > > + android_camera_metadata_sources,\n> > > > + install : false,\n> > > > + include_directories : android_includes)\n> > > > diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c\n> > > > new file mode 100644\n> > > > index 000000000000..6bfd02da29c7\n> > > > --- /dev/null\n> > > > +++ b/src/android/metadata/camera_metadata.c\n> > > > @@ -0,0 +1,1204 @@\n> > >\n> > > SPDX here too please (in a separate patch).\n> > >\n> > > What are the implications of linking LGPL-2.1+ and Apache-2.0 code\n> > > together ?\n> >\n> > Eh, how fun is to deal with legal stuff..\n> >\n> > So, not need to say but IANAL and none of us is, so the best source I\n> > could find is this stackoverflow post\n> > https://opensource.stackexchange.com/questions/5664/linking-from-lgpl-2-1-software-to-apache-2-0-library\n> >\n> > The general question we are trying to answer is:\n> >\n> > Can copyleft-licensed code depend on non-copyleft-licensed code that\n> > is using a license that is deemed incompatible with a given copyleft\n> > license?\n> >\n> > Going through the answers and the FAQs on the GPL license website, it\n> > seems the me the one that applies the most to our use case is:\n> >\n> >\n> > What legal issues come up if I use GPL-incompatible libraries with\n> > GPL software? If you want your program to link against a library\n> > not covered by the system library exception, you need to provide\n> > permission to do that.\n> >\n> > So even though the LGPL is not the GPL, I would say that the best way\n> > to do this would be to follow the guidelines provided at the FAQ and\n> > license your LGPL library with an exception stating that this does not\n> > extend to the Apache-licensed dependencies. Something similar to\n> > OpenSSL exceptions commonly found in several places such as here.\n> >\n> > That's because I'm not sure I found a proper definition of \"system\n> > library exception\" for LGPL-2.1. If the metadata library falls under\n> > that term, we should be good the way we are, otherwise we would need\n> > to add a notice.\n>\n> The system library exception mostly covers glibc, and possibly a few\n> other similar system libraries from the GNU project. The camera metadata\n> library isn't included.\n\nI have guessed so.\n>\n> The GPL and LGPL apply to binary distribution time, so linking\n> statically against the metadata library is probably an issue, while\n> linking dynamically is probably safer. I wonder how much this calls for\n> a custom implementation...\n>\n\nWon't an adding an exception cover the static linking case as well?\nI agree that an internal implementation might be desirable and would\nsave us some license headaches, but it should be implemented and I'm\nnot sure it is top priority at the moment...\n\n\n> > > > +/*\n> > > > + * Copyright (C) 2012 The Android Open Source Project\n> > > > + *\n> > > > + * Licensed under the Apache License, Version 2.0 (the \"License\");\n> > > > + * you may not use this file except in compliance with the License.\n> > > > + * You may obtain a copy of the License at\n> > > > + *\n> > > > + * http://www.apache.org/licenses/LICENSE-2.0\n> > > > + *\n> > > > + * Unless required by applicable law or agreed to in writing, software\n> > > > + * distributed under the License is distributed on an \"AS IS\" BASIS,\n> > > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n> > > > + * See the License for the specific language governing permissions and\n> > > > + * limitations under the License.\n> > > > + */\n> > >\n> > > [snip]\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 960696161A\n\tfor ;\n\tTue, 6 Aug 2019 16:24:46 +0200 (CEST)","from uno.localdomain\n\t(host150-24-dynamic.51-79-r.retail.telecomitalia.it [79.51.24.150])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id B81D7C000C;\n\tTue, 6 Aug 2019 14:24:45 +0000 (UTC)"],"X-Originating-IP":"79.51.24.150","Date":"Tue, 6 Aug 2019 16:26:09 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190806142609.afskgj2xwjgovitb@uno.localdomain>","References":"<20190801155420.24694-1-jacopo@jmondi.org>\n\t<20190801155420.24694-5-jacopo@jmondi.org>\n\t<20190805114936.GP29747@pendragon.ideasonboard.com>\n\t<20190806103804.p7ntcoa6kuitl4jt@uno.localdomain>\n\t<20190806132240.GA4969@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"dgzlxkyugybwe5ui\"","Content-Disposition":"inline","In-Reply-To":"<20190806132240.GA4969@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 4/5] android: Add camera metadata\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 06 Aug 2019 14:24:46 -0000"}}]