[{"id":16727,"web_url":"https://patchwork.libcamera.org/comment/16727/","msgid":"","date":"2021-05-01T20:54:12","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n> The libcamera Android HAL implementation should not be an integral part\n> of libcamera, but a support library that utilises the libcamera public\n> API.\n> \n> Move the implementation to it's own distinct library.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> \n> Please note that this no longer requires ChromeOS to manually symlink\n> into the camera_hal directory.\n> \n> A modification similar to the following should be applied to the Chrome\n> ebuilds if this is applied:\n> \n> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n> index 433357f1ef74..cf90e438cfb3 100644\n> --- a/media-libs/libcamera/libcamera-9999.ebuild\n> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n> @@ -57,6 +57,4 @@ src_compile() {\n> \n> src_install() {\n> meson_src_install\n> -\n> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n> }\n\nCould you submit a patch to CrOS ?\n\n> src/android/meson.build | 10 ++++++++++\n> src/libcamera/meson.build | 11 -----------\n> src/meson.build | 7 +++----\n> 3 files changed, 13 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index 2be20c97e118..6170448a73ce 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -3,6 +3,7 @@\n> android_deps = [\n> dependency('libexif', required : get_option('android')),\n> dependency('libjpeg', required : get_option('android')),\n> + libcamera_dep,\n> ]\n> \n> android_enabled = true\n> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n> android_camera_metadata_sources,\n> c_args : '-Wno-shadow',\n> include_directories : android_includes)\n> +\n> +android_hal = shared_library('android-hal',\n\nandroid-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n?\n\n> + android_hal_sources,\n> + name_prefix : '',\n> + link_with : android_camera_metadata,\n> + install : true,\n> + cpp_args : libcamera_cpp_args,\n> + include_directories : android_includes,\n> + dependencies : android_deps)\n\nWithout install_dir, this will get installed in /usr/lib/, which isn't\nwhere the CrOS camera service will look for it. We probably need a\nconfiguration option to set the path, as it will differ between CrOS and\nAndroid.\n\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index e0a48aa23ea5..f77b80b878d8 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -129,16 +129,6 @@ libcamera_deps = [\n> dependency('threads'),\n> ]\n> \n> -libcamera_link_with = []\n> -\n> -if android_enabled\n> - libcamera_sources += android_hal_sources\n> - includes += android_includes\n> - libcamera_link_with += android_camera_metadata\n> -\n> - libcamera_deps += android_deps\n> -endif\n> -\n> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n> # The build_rpath is stripped at install time by meson, so we determine at\n> # runtime if the library is running from an installed location by checking\n> @@ -147,7 +137,6 @@ endif\n> libcamera = shared_library('camera',\n> libcamera_sources,\n> install : true,\n> - link_with : libcamera_link_with,\n> cpp_args : libcamera_cpp_args,\n> include_directories : includes,\n> objects : libcamera_objects,\n> diff --git a/src/meson.build b/src/meson.build\n> index 64d22b9ce5b6..9dc32751bbb1 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -31,11 +31,10 @@ endif\n> libcamera_cpp_args = []\n> libcamera_objects = []\n> \n> -# The 'android' subdir must be processed first, and the build targets\n> -# are included directly into the libcamera library when this is enabled.\n> -subdir('android')\n> -\n> +# libcamera must be built first as a dependency to the other components.\n> subdir('libcamera')\n\nThis is really nice, having android first has been bugging me for a\nwhile.\n\n> +\n> +subdir('android')\n> subdir('ipa')\n> \n> subdir('lc-compliance')","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4A485BDE73\n\tfor ;\n\tSat, 1 May 2021 20:54:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94EA968909;\n\tSat, 1 May 2021 22:54:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E2EC60516\n\tfor ;\n\tSat, 1 May 2021 22:54:15 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A8EED547;\n\tSat, 1 May 2021 22:54:14 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"b1kL4dpz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619902454;\n\tbh=RMPQ2t1GTJhMAfvWRWtq8rkgzI/w0SjYfic7OUxKS2c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b1kL4dpz0OQxrzgSIR5muX0dlqmkuWk81L7thQpGGvPhaGrHpf0xrv8dXOltqRKoD\n\trOqi7cmAJ2mMMCDODwq1DGh3mY1haQdIaOhqMemAvftv7EGUQTxf+w7TA/Vf63Z0wv\n\tZXsD3Nrw7TCTz+6IYDBsCfYguKybHuDASaqxc3no=","Date":"Sat, 1 May 2021 23:54:12 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16728,"web_url":"https://patchwork.libcamera.org/comment/16728/","msgid":"<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>","date":"2021-05-02T20:02:45","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 01/05/2021 21:54, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n>> The libcamera Android HAL implementation should not be an integral part\n>> of libcamera, but a support library that utilises the libcamera public\n>> API.\n>>\n>> Move the implementation to it's own distinct library.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>>\n>> Please note that this no longer requires ChromeOS to manually symlink\n>> into the camera_hal directory.\n>>\n>> A modification similar to the following should be applied to the Chrome\n>> ebuilds if this is applied:\n>>\n>> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n>> index 433357f1ef74..cf90e438cfb3 100644\n>> --- a/media-libs/libcamera/libcamera-9999.ebuild\n>> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n>> @@ -57,6 +57,4 @@ src_compile() {\n>> \n>> src_install() {\n>> meson_src_install\n>> -\n>> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n>> }\n> \n> Could you submit a patch to CrOS ?\n> \n>> src/android/meson.build | 10 ++++++++++\n>> src/libcamera/meson.build | 11 -----------\n>> src/meson.build | 7 +++----\n>> 3 files changed, 13 insertions(+), 15 deletions(-)\n>>\n>> diff --git a/src/android/meson.build b/src/android/meson.build\n>> index 2be20c97e118..6170448a73ce 100644\n>> --- a/src/android/meson.build\n>> +++ b/src/android/meson.build\n>> @@ -3,6 +3,7 @@\n>> android_deps = [\n>> dependency('libexif', required : get_option('android')),\n>> dependency('libjpeg', required : get_option('android')),\n>> + libcamera_dep,\n>> ]\n>> \n>> android_enabled = true\n>> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n>> android_camera_metadata_sources,\n>> c_args : '-Wno-shadow',\n>> include_directories : android_includes)\n>> +\n>> +android_hal = shared_library('android-hal',\n> \n> android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n\nI started out with libcamera-hal, but then I decided I didn't like it so\nI updated to android-hal ...\n\nGiven that this is an android-hal implementation I thought that was\n/more/ descriptive ...\n\notherwise libcamera-hal sounds like it's an abstraction layer of libcamera.\n\nIf you prefer libcamera-hal I'll change it back.\n\n> ?\n> \n>> + android_hal_sources,\n>> + name_prefix : '',\n>> + link_with : android_camera_metadata,\n>> + install : true,\n>> + cpp_args : libcamera_cpp_args,\n>> + include_directories : android_includes,\n>> + dependencies : android_deps)\n> \n> Without install_dir, this will get installed in /usr/lib/, which isn't\n> where the CrOS camera service will look for it. We probably need a\n> configuration option to set the path, as it will differ between CrOS and\n> Android.\n\nBut I have install_dir ... uhm ... Ok so I forgot to save out the\nupdated patch before sending ...\n\nIt's there in the next version already ;-) ...\n\n> +android_installdir = get_option('libdir')\n> +\n> if get_option('android_platform') == 'cros'\n> libcamera_cpp_args += [ '-DOS_CHROMEOS']\n> + android_installdir = get_option('libdir') / 'camera_hal'\n> endif\n\nWill install it to camera_hal directly when the platform is set.\n\n\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index e0a48aa23ea5..f77b80b878d8 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -129,16 +129,6 @@ libcamera_deps = [\n>> dependency('threads'),\n>> ]\n>> \n>> -libcamera_link_with = []\n>> -\n>> -if android_enabled\n>> - libcamera_sources += android_hal_sources\n>> - includes += android_includes\n>> - libcamera_link_with += android_camera_metadata\n>> -\n>> - libcamera_deps += android_deps\n>> -endif\n>> -\n>> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n>> # The build_rpath is stripped at install time by meson, so we determine at\n>> # runtime if the library is running from an installed location by checking\n>> @@ -147,7 +137,6 @@ endif\n>> libcamera = shared_library('camera',\n>> libcamera_sources,\n>> install : true,\n>> - link_with : libcamera_link_with,\n>> cpp_args : libcamera_cpp_args,\n>> include_directories : includes,\n>> objects : libcamera_objects,\n>> diff --git a/src/meson.build b/src/meson.build\n>> index 64d22b9ce5b6..9dc32751bbb1 100644\n>> --- a/src/meson.build\n>> +++ b/src/meson.build\n>> @@ -31,11 +31,10 @@ endif\n>> libcamera_cpp_args = []\n>> libcamera_objects = []\n>> \n>> -# The 'android' subdir must be processed first, and the build targets\n>> -# are included directly into the libcamera library when this is enabled.\n>> -subdir('android')\n>> -\n>> +# libcamera must be built first as a dependency to the other components.\n>> subdir('libcamera')\n> \n> This is really nice, having android first has been bugging me for a\n> while.\n\nMe to, I somehow assumed last time I looked at this that there were\ndeeper reasons for integrating into the libcamera.so - but there wasn't\nanything preventing the split really.\n\nAnyway, v2 to come anyway with the correct install paths, and if desired\n- a rename.\n\n--\nKieran\n\n\n> \n>> +\n>> +subdir('android')\n>> subdir('ipa')\n>> \n>> subdir('lc-compliance')\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8C71CBDE76\n\tfor ;\n\tSun, 2 May 2021 20:02:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB89168910;\n\tSun, 2 May 2021 22:02:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 28F18688AF\n\tfor ;\n\tSun, 2 May 2021 22:02:51 +0200 (CEST)","from [IPv6:2a02:c7f:18f1:100:b12f:c62f:79f3:7d84] (unknown\n\t[IPv6:2a02:c7f:18f1:100:b12f:c62f:79f3:7d84])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7235E883;\n\tSun, 2 May 2021 22:02:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZYg0MEZA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619985770;\n\tbh=jo9Z4Xv+ArUq64bhdmyoscDjIl6sIlEo3tPAxH1Gb3Y=;\n\th=Reply-To:To:Cc:References:From:Subject:Date:In-Reply-To:From;\n\tb=ZYg0MEZAudvTsiLEb6gTVvOeme8mhy0QwECZnVIReYe7n17Elz9koJivtBFJBsSiR\n\tlRhOT2drbBzDvPRBM5oMSUOd7XAyJZnvhKtKEr32zmsCaFYHQOm6whNIbeG9MXSEIC\n\tx6AEBdajnwY77aNiwhfhGq2ZaTczbWhRC9MO/hYk=","To":"Laurent Pinchart ","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>","Date":"Sun, 2 May 2021 21:02:45 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16730,"web_url":"https://patchwork.libcamera.org/comment/16730/","msgid":"","date":"2021-05-03T07:58:01","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Sun, May 02, 2021 at 09:02:45PM +0100, Kieran Bingham wrote:\n> On 01/05/2021 21:54, Laurent Pinchart wrote:\n> > On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n> >> The libcamera Android HAL implementation should not be an integral part\n> >> of libcamera, but a support library that utilises the libcamera public\n> >> API.\n> >>\n> >> Move the implementation to it's own distinct library.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >>\n> >> Please note that this no longer requires ChromeOS to manually symlink\n> >> into the camera_hal directory.\n> >>\n> >> A modification similar to the following should be applied to the Chrome\n> >> ebuilds if this is applied:\n> >>\n> >> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n> >> index 433357f1ef74..cf90e438cfb3 100644\n> >> --- a/media-libs/libcamera/libcamera-9999.ebuild\n> >> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n> >> @@ -57,6 +57,4 @@ src_compile() {\n> >> \n> >> src_install() {\n> >> meson_src_install\n> >> -\n> >> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n> >> }\n> > \n> > Could you submit a patch to CrOS ?\n> > \n> >> src/android/meson.build | 10 ++++++++++\n> >> src/libcamera/meson.build | 11 -----------\n> >> src/meson.build | 7 +++----\n> >> 3 files changed, 13 insertions(+), 15 deletions(-)\n> >>\n> >> diff --git a/src/android/meson.build b/src/android/meson.build\n> >> index 2be20c97e118..6170448a73ce 100644\n> >> --- a/src/android/meson.build\n> >> +++ b/src/android/meson.build\n> >> @@ -3,6 +3,7 @@\n> >> android_deps = [\n> >> dependency('libexif', required : get_option('android')),\n> >> dependency('libjpeg', required : get_option('android')),\n> >> + libcamera_dep,\n> >> ]\n> >> \n> >> android_enabled = true\n> >> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n> >> android_camera_metadata_sources,\n> >> c_args : '-Wno-shadow',\n> >> include_directories : android_includes)\n> >> +\n> >> +android_hal = shared_library('android-hal',\n> > \n> > android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n> \n> I started out with libcamera-hal, but then I decided I didn't like it so\n> I updated to android-hal ...\n> \n> Given that this is an android-hal implementation I thought that was\n> /more/ descriptive ...\n> \n> otherwise libcamera-hal sounds like it's an abstraction layer of libcamera.\n> \n> If you prefer libcamera-hal I'll change it back.\n\nFollowing that logic, every shared object on a Linux system should be\ncalled linux-* :-) Given that we may have different HAL implementations\nin the same directory (both multiple camera HAL implementations in\nChrome OS, and multiple HALs for unrelated devices in Android), it's\nimportant for the file name to specify the project and possibly the fact\nthat it's related to cameras. With 'libcamera' as the project name, both\ngoals are reached :-)\n\n> > ?\n> > \n> >> + android_hal_sources,\n> >> + name_prefix : '',\n> >> + link_with : android_camera_metadata,\n> >> + install : true,\n> >> + cpp_args : libcamera_cpp_args,\n> >> + include_directories : android_includes,\n> >> + dependencies : android_deps)\n> > \n> > Without install_dir, this will get installed in /usr/lib/, which isn't\n> > where the CrOS camera service will look for it. We probably need a\n> > configuration option to set the path, as it will differ between CrOS and\n> > Android.\n> \n> But I have install_dir ... uhm ... Ok so I forgot to save out the\n> updated patch before sending ...\n> \n> It's there in the next version already ;-) ...\n> \n> > +android_installdir = get_option('libdir')\n> > +\n> > if get_option('android_platform') == 'cros'\n> > libcamera_cpp_args += [ '-DOS_CHROMEOS']\n\nExtra space.\n\n> > + android_installdir = get_option('libdir') / 'camera_hal'\n> > endif\n> \n> Will install it to camera_hal directly when the platform is set.\n\nShould install be set to true on CrOS only ? On Android meson isn't used\nanyway, and on other systems, the Android HAL isn't needed.\n\n> >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >> index e0a48aa23ea5..f77b80b878d8 100644\n> >> --- a/src/libcamera/meson.build\n> >> +++ b/src/libcamera/meson.build\n> >> @@ -129,16 +129,6 @@ libcamera_deps = [\n> >> dependency('threads'),\n> >> ]\n> >> \n> >> -libcamera_link_with = []\n> >> -\n> >> -if android_enabled\n> >> - libcamera_sources += android_hal_sources\n> >> - includes += android_includes\n> >> - libcamera_link_with += android_camera_metadata\n> >> -\n> >> - libcamera_deps += android_deps\n> >> -endif\n> >> -\n> >> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n> >> # The build_rpath is stripped at install time by meson, so we determine at\n> >> # runtime if the library is running from an installed location by checking\n> >> @@ -147,7 +137,6 @@ endif\n> >> libcamera = shared_library('camera',\n> >> libcamera_sources,\n> >> install : true,\n> >> - link_with : libcamera_link_with,\n> >> cpp_args : libcamera_cpp_args,\n> >> include_directories : includes,\n> >> objects : libcamera_objects,\n> >> diff --git a/src/meson.build b/src/meson.build\n> >> index 64d22b9ce5b6..9dc32751bbb1 100644\n> >> --- a/src/meson.build\n> >> +++ b/src/meson.build\n> >> @@ -31,11 +31,10 @@ endif\n> >> libcamera_cpp_args = []\n> >> libcamera_objects = []\n> >> \n> >> -# The 'android' subdir must be processed first, and the build targets\n> >> -# are included directly into the libcamera library when this is enabled.\n> >> -subdir('android')\n> >> -\n> >> +# libcamera must be built first as a dependency to the other components.\n> >> subdir('libcamera')\n> > \n> > This is really nice, having android first has been bugging me for a\n> > while.\n> \n> Me to, I somehow assumed last time I looked at this that there were\n> deeper reasons for integrating into the libcamera.so - but there wasn't\n> anything preventing the split really.\n> \n> Anyway, v2 to come anyway with the correct install paths, and if desired\n> - a rename.\n> \n> >> +\n> >> +subdir('android')\n> >> subdir('ipa')\n> >> \n> >> subdir('lc-compliance')","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 866AEBDE77\n\tfor ;\n\tMon, 3 May 2021 07:58:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C144968910;\n\tMon, 3 May 2021 09:58:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 635B160511\n\tfor ;\n\tMon, 3 May 2021 09:58:03 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BE14087C;\n\tMon, 3 May 2021 09:58:02 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VXd7qm0x\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620028682;\n\tbh=7/DVI/2R32Vh3Gx1FhjsysDLPN00jyiHQ85f5Oa5PMc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VXd7qm0xiWbSUIGj5rWcn2TUeeZuUUwOWyNWqm+qSmY17R5Sk8T6vXxBBBPedNE28\n\tsQ55/U3b+M8tSmcfAbJ8QZ+a8UTdXueDPNqblUdx8igb4e57jv4vN+pQH9akzSZCal\n\trSHwoVnVS20UWBYGThHl8KWC/rJYPcngUvppVLUE=","Date":"Mon, 3 May 2021 10:58:01 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t\n\t<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16731,"web_url":"https://patchwork.libcamera.org/comment/16731/","msgid":"<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>","date":"2021-05-03T08:13:31","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 03/05/2021 08:58, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Sun, May 02, 2021 at 09:02:45PM +0100, Kieran Bingham wrote:\n>> On 01/05/2021 21:54, Laurent Pinchart wrote:\n>>> On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n>>>> The libcamera Android HAL implementation should not be an integral part\n>>>> of libcamera, but a support library that utilises the libcamera public\n>>>> API.\n>>>>\n>>>> Move the implementation to it's own distinct library.\n>>>>\n>>>> Signed-off-by: Kieran Bingham \n>>>> ---\n>>>>\n>>>> Please note that this no longer requires ChromeOS to manually symlink\n>>>> into the camera_hal directory.\n>>>>\n>>>> A modification similar to the following should be applied to the Chrome\n>>>> ebuilds if this is applied:\n>>>>\n>>>> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n>>>> index 433357f1ef74..cf90e438cfb3 100644\n>>>> --- a/media-libs/libcamera/libcamera-9999.ebuild\n>>>> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n>>>> @@ -57,6 +57,4 @@ src_compile() {\n>>>> \n>>>> src_install() {\n>>>> meson_src_install\n>>>> -\n>>>> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n>>>> }\n>>>\n>>> Could you submit a patch to CrOS ?\n>>>\n>>>> src/android/meson.build | 10 ++++++++++\n>>>> src/libcamera/meson.build | 11 -----------\n>>>> src/meson.build | 7 +++----\n>>>> 3 files changed, 13 insertions(+), 15 deletions(-)\n>>>>\n>>>> diff --git a/src/android/meson.build b/src/android/meson.build\n>>>> index 2be20c97e118..6170448a73ce 100644\n>>>> --- a/src/android/meson.build\n>>>> +++ b/src/android/meson.build\n>>>> @@ -3,6 +3,7 @@\n>>>> android_deps = [\n>>>> dependency('libexif', required : get_option('android')),\n>>>> dependency('libjpeg', required : get_option('android')),\n>>>> + libcamera_dep,\n>>>> ]\n>>>> \n>>>> android_enabled = true\n>>>> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n>>>> android_camera_metadata_sources,\n>>>> c_args : '-Wno-shadow',\n>>>> include_directories : android_includes)\n>>>> +\n>>>> +android_hal = shared_library('android-hal',\n>>>\n>>> android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n>>\n>> I started out with libcamera-hal, but then I decided I didn't like it so\n>> I updated to android-hal ...\n>>\n>> Given that this is an android-hal implementation I thought that was\n>> /more/ descriptive ...\n>>\n>> otherwise libcamera-hal sounds like it's an abstraction layer of libcamera.\n>>\n>> If you prefer libcamera-hal I'll change it back.\n> \n> Following that logic, every shared object on a Linux system should be\n> called linux-* :-) Given that we may have different HAL implementations> in the same directory (both multiple camera HAL implementations in\n> Chrome OS, and multiple HALs for unrelated devices in Android), it's\n> important for the file name to specify the project and possibly the fact\n> that it's related to cameras. With 'libcamera' as the project name, both\n> goals are reached :-)\n\nWell ... maybe the Linux Abstraction Layer for Android could be called\nlinux-hal (ok, too meta, because Android is on top of Linux) ... but the\ndifference is the perspective. If this was installed on android then\nyes, I see your point - but we don't (currently) install on android,\nonly chrome ;-) - where it is the 'android-hal' much like the 'libc' is\nthe c library...\n\nI guess it could be the libcamera-android-hal.so too\n\nPerhaps if Chrome decided not to use the Android HAL but to use a\nwrapper on top of libcamera directly, I could imagine that could be\ncalled the libcamera-hal for instance.\n\n--\nKieran\n\n\n>>> ?\n>>>\n>>>> + android_hal_sources,\n>>>> + name_prefix : '',\n>>>> + link_with : android_camera_metadata,\n>>>> + install : true,\n>>>> + cpp_args : libcamera_cpp_args,\n>>>> + include_directories : android_includes,\n>>>> + dependencies : android_deps)\n>>>\n>>> Without install_dir, this will get installed in /usr/lib/, which isn't\n>>> where the CrOS camera service will look for it. We probably need a\n>>> configuration option to set the path, as it will differ between CrOS and\n>>> Android.\n>>\n>> But I have install_dir ... uhm ... Ok so I forgot to save out the\n>> updated patch before sending ...\n>>\n>> It's there in the next version already ;-) ...\n>>\n>>> +android_installdir = get_option('libdir')\n>>> +\n>>> if get_option('android_platform') == 'cros'\n>>> libcamera_cpp_args += [ '-DOS_CHROMEOS']\n> \n> Extra space.\n> \n>>> + android_installdir = get_option('libdir') / 'camera_hal'\n>>> endif\n>>\n>> Will install it to camera_hal directly when the platform is set.\n> \n> Should install be set to true on CrOS only ? On Android meson isn't used\n> anyway, and on other systems, the Android HAL isn't needed.\n\nIf it isn't needed, then don't build it (i.e. don't enable android)?\n\nAnd if it doesn't need to be installed (i.e. just compile testing) well\nthen there's no need to call the install ...\n\n\n\n>>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>>> index e0a48aa23ea5..f77b80b878d8 100644\n>>>> --- a/src/libcamera/meson.build\n>>>> +++ b/src/libcamera/meson.build\n>>>> @@ -129,16 +129,6 @@ libcamera_deps = [\n>>>> dependency('threads'),\n>>>> ]\n>>>> \n>>>> -libcamera_link_with = []\n>>>> -\n>>>> -if android_enabled\n>>>> - libcamera_sources += android_hal_sources\n>>>> - includes += android_includes\n>>>> - libcamera_link_with += android_camera_metadata\n>>>> -\n>>>> - libcamera_deps += android_deps\n>>>> -endif\n>>>> -\n>>>> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n>>>> # The build_rpath is stripped at install time by meson, so we determine at\n>>>> # runtime if the library is running from an installed location by checking\n>>>> @@ -147,7 +137,6 @@ endif\n>>>> libcamera = shared_library('camera',\n>>>> libcamera_sources,\n>>>> install : true,\n>>>> - link_with : libcamera_link_with,\n>>>> cpp_args : libcamera_cpp_args,\n>>>> include_directories : includes,\n>>>> objects : libcamera_objects,\n>>>> diff --git a/src/meson.build b/src/meson.build\n>>>> index 64d22b9ce5b6..9dc32751bbb1 100644\n>>>> --- a/src/meson.build\n>>>> +++ b/src/meson.build\n>>>> @@ -31,11 +31,10 @@ endif\n>>>> libcamera_cpp_args = []\n>>>> libcamera_objects = []\n>>>> \n>>>> -# The 'android' subdir must be processed first, and the build targets\n>>>> -# are included directly into the libcamera library when this is enabled.\n>>>> -subdir('android')\n>>>> -\n>>>> +# libcamera must be built first as a dependency to the other components.\n>>>> subdir('libcamera')\n>>>\n>>> This is really nice, having android first has been bugging me for a\n>>> while.\n>>\n>> Me to, I somehow assumed last time I looked at this that there were\n>> deeper reasons for integrating into the libcamera.so - but there wasn't\n>> anything preventing the split really.\n>>\n>> Anyway, v2 to come anyway with the correct install paths, and if desired\n>> - a rename.\n>>\n>>>> +\n>>>> +subdir('android')\n>>>> subdir('ipa')\n>>>> \n>>>> subdir('lc-compliance')\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 77E6DBDE74\n\tfor ;\n\tMon, 3 May 2021 08:13:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D3A3068919;\n\tMon, 3 May 2021 10:13:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AE870602C0\n\tfor ;\n\tMon, 3 May 2021 10:13:35 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2FB3787C;\n\tMon, 3 May 2021 10:13:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mDDz9PXq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620029615;\n\tbh=5VTQFndn3l6jorYqu6Np40Y62N0aMnqPrci4MLybLjs=;\n\th=Reply-To:To:Cc:References:From:Subject:Date:In-Reply-To:From;\n\tb=mDDz9PXqiyxgmi+GvstL0zA5cHJIwA5ljTWtPFOv37gp/XSxUfLuxsRrA33gUNbvG\n\tyHZT39THqtuJFRFmSVjQgAVMJ67BjQbLFvEbr6QRfqSNjkfgkUazYoi4VUUSngKlHt\n\t9r9UVIG1JHV2s0UrC4matJt5c5n6i5J3/3hnMPBY=","To":"Laurent Pinchart ","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t\n\t<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>\n\t","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>","Date":"Mon, 3 May 2021 09:13:31 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16732,"web_url":"https://patchwork.libcamera.org/comment/16732/","msgid":"<8a2d6592-b7c7-2f9d-6724-f96ce27c25eb@ideasonboard.com>","date":"2021-05-03T08:18:26","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 03/05/2021 08:58, Laurent Pinchart wrote:\n> Hi Kieran,\n\n>>> +android_installdir = get_option('libdir')\n>>> +\n>>> if get_option('android_platform') == 'cros'\n>>> libcamera_cpp_args += [ '-DOS_CHROMEOS']\n> \n> Extra space.\n\nNot my addition. But I'll fix it up in the same patch. It shouldn't be\ntoo obfuscating.\n\nOh - this indent is also only three spaces, so I'll also push it up to\nfour as well.\n\n>>> + android_installdir = get_option('libdir') / 'camera_hal'\n>>> endif\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 42954BDE77\n\tfor ;\n\tMon, 3 May 2021 08:18:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB11A6890C;\n\tMon, 3 May 2021 10:18:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB3D7602C0\n\tfor ;\n\tMon, 3 May 2021 10:18:29 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5924187C;\n\tMon, 3 May 2021 10:18:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tmAHO51M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620029909;\n\tbh=59TzGmx9vtEX7nzBXHKNfQFLHPRgOAXRqN/UaTPUxc0=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=tmAHO51M68bMzVJFFiXX8IeBDJvIw8AdU7OABYiMeh5ESMfjL/0Hax1ir2jhYNenL\n\tBVglrDTPwuvAc+4bXS4UXj5SH3KqojGUE3SUP1FGdAnr1xzWWBMpNmergbNQCH2Hxl\n\tjepvjPfJiuViPYgdvP6c/z0ziwEPbSjxCKxVZEPk=","To":"Laurent Pinchart ","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t\n\t<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>\n\t","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<8a2d6592-b7c7-2f9d-6724-f96ce27c25eb@ideasonboard.com>","Date":"Mon, 3 May 2021 09:18:26 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16733,"web_url":"https://patchwork.libcamera.org/comment/16733/","msgid":"","date":"2021-05-03T08:21:53","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, May 03, 2021 at 09:13:31AM +0100, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> On 03/05/2021 08:58, Laurent Pinchart wrote:\n> > Hi Kieran,\n> > \n> > On Sun, May 02, 2021 at 09:02:45PM +0100, Kieran Bingham wrote:\n> >> On 01/05/2021 21:54, Laurent Pinchart wrote:\n> >>> On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n> >>>> The libcamera Android HAL implementation should not be an integral part\n> >>>> of libcamera, but a support library that utilises the libcamera public\n> >>>> API.\n> >>>>\n> >>>> Move the implementation to it's own distinct library.\n> >>>>\n> >>>> Signed-off-by: Kieran Bingham \n> >>>> ---\n> >>>>\n> >>>> Please note that this no longer requires ChromeOS to manually symlink\n> >>>> into the camera_hal directory.\n> >>>>\n> >>>> A modification similar to the following should be applied to the Chrome\n> >>>> ebuilds if this is applied:\n> >>>>\n> >>>> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n> >>>> index 433357f1ef74..cf90e438cfb3 100644\n> >>>> --- a/media-libs/libcamera/libcamera-9999.ebuild\n> >>>> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n> >>>> @@ -57,6 +57,4 @@ src_compile() {\n> >>>> \n> >>>> src_install() {\n> >>>> meson_src_install\n> >>>> -\n> >>>> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n> >>>> }\n> >>>\n> >>> Could you submit a patch to CrOS ?\n> >>>\n> >>>> src/android/meson.build | 10 ++++++++++\n> >>>> src/libcamera/meson.build | 11 -----------\n> >>>> src/meson.build | 7 +++----\n> >>>> 3 files changed, 13 insertions(+), 15 deletions(-)\n> >>>>\n> >>>> diff --git a/src/android/meson.build b/src/android/meson.build\n> >>>> index 2be20c97e118..6170448a73ce 100644\n> >>>> --- a/src/android/meson.build\n> >>>> +++ b/src/android/meson.build\n> >>>> @@ -3,6 +3,7 @@\n> >>>> android_deps = [\n> >>>> dependency('libexif', required : get_option('android')),\n> >>>> dependency('libjpeg', required : get_option('android')),\n> >>>> + libcamera_dep,\n> >>>> ]\n> >>>> \n> >>>> android_enabled = true\n> >>>> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n> >>>> android_camera_metadata_sources,\n> >>>> c_args : '-Wno-shadow',\n> >>>> include_directories : android_includes)\n> >>>> +\n> >>>> +android_hal = shared_library('android-hal',\n> >>>\n> >>> android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n> >>\n> >> I started out with libcamera-hal, but then I decided I didn't like it so\n> >> I updated to android-hal ...\n> >>\n> >> Given that this is an android-hal implementation I thought that was\n> >> /more/ descriptive ...\n> >>\n> >> otherwise libcamera-hal sounds like it's an abstraction layer of libcamera.\n> >>\n> >> If you prefer libcamera-hal I'll change it back.\n> > \n> > Following that logic, every shared object on a Linux system should be\n> > called linux-* :-) Given that we may have different HAL implementations> in the same directory (both multiple camera HAL implementations in\n> > Chrome OS, and multiple HALs for unrelated devices in Android), it's\n> > important for the file name to specify the project and possibly the fact\n> > that it's related to cameras. With 'libcamera' as the project name, both\n> > goals are reached :-)\n> \n> Well ... maybe the Linux Abstraction Layer for Android could be called\n> linux-hal (ok, too meta, because Android is on top of Linux) ... but the\n> difference is the perspective. If this was installed on android then\n> yes, I see your point - but we don't (currently) install on android,\n> only chrome ;-) - where it is the 'android-hal' much like the 'libc' is\n> the c library...\n> \n> I guess it could be the libcamera-android-hal.so too\n> \n> Perhaps if Chrome decided not to use the Android HAL but to use a\n> wrapper on top of libcamera directly, I could imagine that could be\n> called the libcamera-hal for instance.\n\nThere are more HALs than the camera HAL on Android, there's one for\naudio, for graphicss, for sensors (as in motion sensors), ... Those\nother HALs are possibly not used on Chrome OS (not entirely sure\nthough), but it's not just about cameras.\n\n> >>> ?\n> >>>\n> >>>> + android_hal_sources,\n> >>>> + name_prefix : '',\n> >>>> + link_with : android_camera_metadata,\n> >>>> + install : true,\n> >>>> + cpp_args : libcamera_cpp_args,\n> >>>> + include_directories : android_includes,\n> >>>> + dependencies : android_deps)\n> >>>\n> >>> Without install_dir, this will get installed in /usr/lib/, which isn't\n> >>> where the CrOS camera service will look for it. We probably need a\n> >>> configuration option to set the path, as it will differ between CrOS and\n> >>> Android.\n> >>\n> >> But I have install_dir ... uhm ... Ok so I forgot to save out the\n> >> updated patch before sending ...\n> >>\n> >> It's there in the next version already ;-) ...\n> >>\n> >>> +android_installdir = get_option('libdir')\n> >>> +\n> >>> if get_option('android_platform') == 'cros'\n> >>> libcamera_cpp_args += [ '-DOS_CHROMEOS']\n> > \n> > Extra space.\n> > \n> >>> + android_installdir = get_option('libdir') / 'camera_hal'\n> >>> endif\n> >>\n> >> Will install it to camera_hal directly when the platform is set.\n> > \n> > Should install be set to true on CrOS only ? On Android meson isn't used\n> > anyway, and on other systems, the Android HAL isn't needed.\n> \n> If it isn't needed, then don't build it (i.e. don't enable android)?\n> \n> And if it doesn't need to be installed (i.e. just compile testing) well\n> then there's no need to call the install ...\n\nFair enough.\n\n> >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >>>> index e0a48aa23ea5..f77b80b878d8 100644\n> >>>> --- a/src/libcamera/meson.build\n> >>>> +++ b/src/libcamera/meson.build\n> >>>> @@ -129,16 +129,6 @@ libcamera_deps = [\n> >>>> dependency('threads'),\n> >>>> ]\n> >>>> \n> >>>> -libcamera_link_with = []\n> >>>> -\n> >>>> -if android_enabled\n> >>>> - libcamera_sources += android_hal_sources\n> >>>> - includes += android_includes\n> >>>> - libcamera_link_with += android_camera_metadata\n> >>>> -\n> >>>> - libcamera_deps += android_deps\n> >>>> -endif\n> >>>> -\n> >>>> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n> >>>> # The build_rpath is stripped at install time by meson, so we determine at\n> >>>> # runtime if the library is running from an installed location by checking\n> >>>> @@ -147,7 +137,6 @@ endif\n> >>>> libcamera = shared_library('camera',\n> >>>> libcamera_sources,\n> >>>> install : true,\n> >>>> - link_with : libcamera_link_with,\n> >>>> cpp_args : libcamera_cpp_args,\n> >>>> include_directories : includes,\n> >>>> objects : libcamera_objects,\n> >>>> diff --git a/src/meson.build b/src/meson.build\n> >>>> index 64d22b9ce5b6..9dc32751bbb1 100644\n> >>>> --- a/src/meson.build\n> >>>> +++ b/src/meson.build\n> >>>> @@ -31,11 +31,10 @@ endif\n> >>>> libcamera_cpp_args = []\n> >>>> libcamera_objects = []\n> >>>> \n> >>>> -# The 'android' subdir must be processed first, and the build targets\n> >>>> -# are included directly into the libcamera library when this is enabled.\n> >>>> -subdir('android')\n> >>>> -\n> >>>> +# libcamera must be built first as a dependency to the other components.\n> >>>> subdir('libcamera')\n> >>>\n> >>> This is really nice, having android first has been bugging me for a\n> >>> while.\n> >>\n> >> Me to, I somehow assumed last time I looked at this that there were\n> >> deeper reasons for integrating into the libcamera.so - but there wasn't\n> >> anything preventing the split really.\n> >>\n> >> Anyway, v2 to come anyway with the correct install paths, and if desired\n> >> - a rename.\n> >>\n> >>>> +\n> >>>> +subdir('android')\n> >>>> subdir('ipa')\n> >>>> \n> >>>> subdir('lc-compliance')\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 12683BDE74\n\tfor ;\n\tMon, 3 May 2021 08:21:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74D7E6890C;\n\tMon, 3 May 2021 10:21:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02FAE602C0\n\tfor ;\n\tMon, 3 May 2021 10:21:55 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 71AD387C;\n\tMon, 3 May 2021 10:21:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FsgMfOf/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620030115;\n\tbh=WH/Pnr44l5mP84yApTzVUNA3P4ThlgeQabDYQSHESkU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FsgMfOf/TUD169gbk4s3oDnBXvXZ7a/WaF3NVX6Pwqg/UZJMbARjd7MzHNaDiui9y\n\tA3HHnFhzTa1jLnYVXkESIDs0RZBoPoVbN+bvJ2OCqPfEfiKa2Kgyyjz/S3pRP9uZ/b\n\t/ekFr5PfCbWhzdSXsJeWEVGPL69BcRgoZ/Ibt4nQ=","Date":"Mon, 3 May 2021 11:21:53 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t\n\t<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>\n\t\n\t<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16734,"web_url":"https://patchwork.libcamera.org/comment/16734/","msgid":"<20210503082958.iblpovrzdgfbg7ft@uno.localdomain>","date":"2021-05-03T08:29:58","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello,\n\nOn Mon, May 03, 2021 at 09:13:31AM +0100, Kieran Bingham wrote:\n> Hi Laurent,\n>\n> On 03/05/2021 08:58, Laurent Pinchart wrote:\n> > Hi Kieran,\n> >\n> > On Sun, May 02, 2021 at 09:02:45PM +0100, Kieran Bingham wrote:\n> >> On 01/05/2021 21:54, Laurent Pinchart wrote:\n> >>> On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n> >>>> The libcamera Android HAL implementation should not be an integral part\n> >>>> of libcamera, but a support library that utilises the libcamera public\n> >>>> API.\n> >>>>\n> >>>> Move the implementation to it's own distinct library.\n> >>>>\n> >>>> Signed-off-by: Kieran Bingham \n> >>>> ---\n> >>>>\n> >>>> Please note that this no longer requires ChromeOS to manually symlink\n> >>>> into the camera_hal directory.\n> >>>>\n> >>>> A modification similar to the following should be applied to the Chrome\n> >>>> ebuilds if this is applied:\n> >>>>\n> >>>> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n> >>>> index 433357f1ef74..cf90e438cfb3 100644\n> >>>> --- a/media-libs/libcamera/libcamera-9999.ebuild\n> >>>> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n> >>>> @@ -57,6 +57,4 @@ src_compile() {\n> >>>>\n> >>>> src_install() {\n> >>>> meson_src_install\n> >>>> -\n> >>>> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n> >>>> }\n> >>>\n> >>> Could you submit a patch to CrOS ?\n> >>>\n> >>>> src/android/meson.build | 10 ++++++++++\n> >>>> src/libcamera/meson.build | 11 -----------\n> >>>> src/meson.build | 7 +++----\n> >>>> 3 files changed, 13 insertions(+), 15 deletions(-)\n> >>>>\n> >>>> diff --git a/src/android/meson.build b/src/android/meson.build\n> >>>> index 2be20c97e118..6170448a73ce 100644\n> >>>> --- a/src/android/meson.build\n> >>>> +++ b/src/android/meson.build\n> >>>> @@ -3,6 +3,7 @@\n> >>>> android_deps = [\n> >>>> dependency('libexif', required : get_option('android')),\n> >>>> dependency('libjpeg', required : get_option('android')),\n> >>>> + libcamera_dep,\n> >>>> ]\n> >>>>\n> >>>> android_enabled = true\n> >>>> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n> >>>> android_camera_metadata_sources,\n> >>>> c_args : '-Wno-shadow',\n> >>>> include_directories : android_includes)\n> >>>> +\n> >>>> +android_hal = shared_library('android-hal',\n> >>>\n> >>> android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n> >>\n> >> I started out with libcamera-hal, but then I decided I didn't like it so\n> >> I updated to android-hal ...\n> >>\n> >> Given that this is an android-hal implementation I thought that was\n> >> /more/ descriptive ...\n> >>\n> >> otherwise libcamera-hal sounds like it's an abstraction layer of libcamera.\n> >>\n> >> If you prefer libcamera-hal I'll change it back.\n> >\n> > Following that logic, every shared object on a Linux system should be\n> > called linux-* :-) Given that we may have different HAL implementations> in the same directory (both multiple camera HAL implementations in\n> > Chrome OS, and multiple HALs for unrelated devices in Android), it's\n> > important for the file name to specify the project and possibly the fact\n> > that it's related to cameras. With 'libcamera' as the project name, both\n> > goals are reached :-)\n>\n> Well ... maybe the Linux Abstraction Layer for Android could be called\n> linux-hal (ok, too meta, because Android is on top of Linux) ... but the\n> difference is the perspective. If this was installed on android then\n> yes, I see your point - but we don't (currently) install on android,\n> only chrome ;-) - where it is the 'android-hal' much like the 'libc' is\n> the c library...\n\nFWIW I second naming the newly introduced .so as libcamera-hal.so.\n\nAs an example Cros currently ships two HALs on Soraka, intel and usb.\nBoth implement the Android Camera3 API, why aren't they both called\nandroid-hal.so ?\n\nThe HAL naming scheme usually reports the platform/camera manufacturer\n(unless you know there will always be a single HAL on your systems, so\nyou can call it camera.so and that's it). Having 'libcamera' in the\nname is important to make sure we won't conflict with other components\nat install time, as they can claim to be the 'android-hal' of the\nsystem as well.\n\nNot to mention Android has several HAL, one for each subsystem, so\nthis should at least be 'camera-hal.so' if we want to stay generic.\n\nFurthermore HAL is a pretty Android-specific thing, I don't think it\nneeds to have 'android' in the name to make clear what API we\nimplement.\n\n>\n> I guess it could be the libcamera-android-hal.so too\n\nRepeating 'android' in the 'hal' name is of no use imho.\n\nlibcamera-hal.so is a perfectly fine name for the libcamera-based camera\nHAL. What are you afraid this can be confused with ?\n\n>\n> Perhaps if Chrome decided not to use the Android HAL but to use a\n> wrapper on top of libcamera directly, I could imagine that could be\n> called the libcamera-hal for instance.\n\nThey would call into libcamera.so in that case, don't they ? What's\nthe purpose of an HAL if not to behave like an HAL? HAL is not just a\nnaming suffix, but a combination of the right exported symbols and the\nright C-API that makes it possible for the library to be loaded at\nrun-time by the Android framework, and all of this is -very- Android\nspecific.\n\nThanks\n j\n\n>\n> --\n> Kieran\n>\n>\n> >>> ?\n> >>>\n> >>>> + android_hal_sources,\n> >>>> + name_prefix : '',\n> >>>> + link_with : android_camera_metadata,\n> >>>> + install : true,\n> >>>> + cpp_args : libcamera_cpp_args,\n> >>>> + include_directories : android_includes,\n> >>>> + dependencies : android_deps)\n> >>>\n> >>> Without install_dir, this will get installed in /usr/lib/, which isn't\n> >>> where the CrOS camera service will look for it. We probably need a\n> >>> configuration option to set the path, as it will differ between CrOS and\n> >>> Android.\n> >>\n> >> But I have install_dir ... uhm ... Ok so I forgot to save out the\n> >> updated patch before sending ...\n> >>\n> >> It's there in the next version already ;-) ...\n> >>\n> >>> +android_installdir = get_option('libdir')\n> >>> +\n> >>> if get_option('android_platform') == 'cros'\n> >>> libcamera_cpp_args += [ '-DOS_CHROMEOS']\n> >\n> > Extra space.\n> >\n> >>> + android_installdir = get_option('libdir') / 'camera_hal'\n> >>> endif\n> >>\n> >> Will install it to camera_hal directly when the platform is set.\n> >\n> > Should install be set to true on CrOS only ? On Android meson isn't used\n> > anyway, and on other systems, the Android HAL isn't needed.\n>\n> If it isn't needed, then don't build it (i.e. don't enable android)?\n>\n> And if it doesn't need to be installed (i.e. just compile testing) well\n> then there's no need to call the install ...\n>\n>\n>\n> >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >>>> index e0a48aa23ea5..f77b80b878d8 100644\n> >>>> --- a/src/libcamera/meson.build\n> >>>> +++ b/src/libcamera/meson.build\n> >>>> @@ -129,16 +129,6 @@ libcamera_deps = [\n> >>>> dependency('threads'),\n> >>>> ]\n> >>>>\n> >>>> -libcamera_link_with = []\n> >>>> -\n> >>>> -if android_enabled\n> >>>> - libcamera_sources += android_hal_sources\n> >>>> - includes += android_includes\n> >>>> - libcamera_link_with += android_camera_metadata\n> >>>> -\n> >>>> - libcamera_deps += android_deps\n> >>>> -endif\n> >>>> -\n> >>>> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n> >>>> # The build_rpath is stripped at install time by meson, so we determine at\n> >>>> # runtime if the library is running from an installed location by checking\n> >>>> @@ -147,7 +137,6 @@ endif\n> >>>> libcamera = shared_library('camera',\n> >>>> libcamera_sources,\n> >>>> install : true,\n> >>>> - link_with : libcamera_link_with,\n> >>>> cpp_args : libcamera_cpp_args,\n> >>>> include_directories : includes,\n> >>>> objects : libcamera_objects,\n> >>>> diff --git a/src/meson.build b/src/meson.build\n> >>>> index 64d22b9ce5b6..9dc32751bbb1 100644\n> >>>> --- a/src/meson.build\n> >>>> +++ b/src/meson.build\n> >>>> @@ -31,11 +31,10 @@ endif\n> >>>> libcamera_cpp_args = []\n> >>>> libcamera_objects = []\n> >>>>\n> >>>> -# The 'android' subdir must be processed first, and the build targets\n> >>>> -# are included directly into the libcamera library when this is enabled.\n> >>>> -subdir('android')\n> >>>> -\n> >>>> +# libcamera must be built first as a dependency to the other components.\n> >>>> subdir('libcamera')\n> >>>\n> >>> This is really nice, having android first has been bugging me for a\n> >>> while.\n> >>\n> >> Me to, I somehow assumed last time I looked at this that there were\n> >> deeper reasons for integrating into the libcamera.so - but there wasn't\n> >> anything preventing the split really.\n> >>\n> >> Anyway, v2 to come anyway with the correct install paths, and if desired\n> >> - a rename.\n> >>\n> >>>> +\n> >>>> +subdir('android')\n> >>>> subdir('ipa')\n> >>>>\n> >>>> subdir('lc-compliance')\n> >\n>\n> --\n> Regards\n> --\n> Kieran\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0EEF9BDE77\n\tfor ;\n\tMon, 3 May 2021 08:29:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8FE6B6890C;\n\tMon, 3 May 2021 10:29:15 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7760E602C0\n\tfor ;\n\tMon, 3 May 2021 10:29:14 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id AFF7D240006;\n\tMon, 3 May 2021 08:29:13 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 3 May 2021 10:29:58 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20210503082958.iblpovrzdgfbg7ft@uno.localdomain>","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t\n\t<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>\n\t\n\t<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16809,"web_url":"https://patchwork.libcamera.org/comment/16809/","msgid":"<923646d2-d4e4-9d7a-055f-27d87f8f12d6@ideasonboard.com>","date":"2021-05-06T12:25:23","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 03/05/2021 09:29, Jacopo Mondi wrote:\n> Hello,\n> \n> On Mon, May 03, 2021 at 09:13:31AM +0100, Kieran Bingham wrote:\n>> Hi Laurent,\n>>\n>> On 03/05/2021 08:58, Laurent Pinchart wrote:\n>>> Hi Kieran,\n>>>\n>>> On Sun, May 02, 2021 at 09:02:45PM +0100, Kieran Bingham wrote:\n>>>> On 01/05/2021 21:54, Laurent Pinchart wrote:\n>>>>> On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n>>>>>> The libcamera Android HAL implementation should not be an integral part\n>>>>>> of libcamera, but a support library that utilises the libcamera public\n>>>>>> API.\n>>>>>>\n>>>>>> Move the implementation to it's own distinct library.\n>>>>>>\n>>>>>> Signed-off-by: Kieran Bingham \n>>>>>> ---\n>>>>>>\n>>>>>> Please note that this no longer requires ChromeOS to manually symlink\n>>>>>> into the camera_hal directory.\n>>>>>>\n>>>>>> A modification similar to the following should be applied to the Chrome\n>>>>>> ebuilds if this is applied:\n>>>>>>\n>>>>>> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n>>>>>> index 433357f1ef74..cf90e438cfb3 100644\n>>>>>> --- a/media-libs/libcamera/libcamera-9999.ebuild\n>>>>>> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n>>>>>> @@ -57,6 +57,4 @@ src_compile() {\n>>>>>>\n>>>>>> src_install() {\n>>>>>> meson_src_install\n>>>>>> -\n>>>>>> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n>>>>>> }\n>>>>>\n>>>>> Could you submit a patch to CrOS ?\n>>>>>\n>>>>>> src/android/meson.build | 10 ++++++++++\n>>>>>> src/libcamera/meson.build | 11 -----------\n>>>>>> src/meson.build | 7 +++----\n>>>>>> 3 files changed, 13 insertions(+), 15 deletions(-)\n>>>>>>\n>>>>>> diff --git a/src/android/meson.build b/src/android/meson.build\n>>>>>> index 2be20c97e118..6170448a73ce 100644\n>>>>>> --- a/src/android/meson.build\n>>>>>> +++ b/src/android/meson.build\n>>>>>> @@ -3,6 +3,7 @@\n>>>>>> android_deps = [\n>>>>>> dependency('libexif', required : get_option('android')),\n>>>>>> dependency('libjpeg', required : get_option('android')),\n>>>>>> + libcamera_dep,\n>>>>>> ]\n>>>>>>\n>>>>>> android_enabled = true\n>>>>>> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n>>>>>> android_camera_metadata_sources,\n>>>>>> c_args : '-Wno-shadow',\n>>>>>> include_directories : android_includes)\n>>>>>> +\n>>>>>> +android_hal = shared_library('android-hal',\n>>>>>\n>>>>> android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n>>>>\n>>>> I started out with libcamera-hal, but then I decided I didn't like it so\n>>>> I updated to android-hal ...\n>>>>\n>>>> Given that this is an android-hal implementation I thought that was\n>>>> /more/ descriptive ...\n>>>>\n>>>> otherwise libcamera-hal sounds like it's an abstraction layer of libcamera.\n>>>>\n>>>> If you prefer libcamera-hal I'll change it back.\n>>>\n>>> Following that logic, every shared object on a Linux system should be\n>>> called linux-* :-) Given that we may have different HAL implementations> in the same directory (both multiple camera HAL implementations in\n>>> Chrome OS, and multiple HALs for unrelated devices in Android), it's\n>>> important for the file name to specify the project and possibly the fact\n>>> that it's related to cameras. With 'libcamera' as the project name, both\n>>> goals are reached :-)\n>>\n>> Well ... maybe the Linux Abstraction Layer for Android could be called\n>> linux-hal (ok, too meta, because Android is on top of Linux) ... but the\n>> difference is the perspective. If this was installed on android then\n>> yes, I see your point - but we don't (currently) install on android,\n>> only chrome ;-) - where it is the 'android-hal' much like the 'libc' is\n>> the c library...\n> \n> FWIW I second naming the newly introduced .so as libcamera-hal.so.\n> \n> As an example Cros currently ships two HALs on Soraka, intel and usb.\n> Both implement the Android Camera3 API, why aren't they both called\n> android-hal.so ?\n\n\nInterestingly, On CrOS for IPU3 the intel hal builds a libcamera_hal.so\nwhich is renamed to intel-ipu3.so by the installation:\n\n\nchipset-kbl/media-libs/cros-camera-hal-intel-ipu3/cros-camera-hal-intel-ipu3-0.0.1-r617.ebuild\n cros-camera_dohal \"${OUT}/lib/libcamera_hal.so\" intel-ipu3.so\n\n\nand the chipset-rk3399 variant does the same thing:\n\n cros-camera_dohal \"${OUT}/lib/libcamera_hal.so\" rockchip-isp1.so\n\n\nOf course the catch is here, that our .so supports both, or all\nplatforms ;-)\n\n\n> The HAL naming scheme usually reports the platform/camera manufacturer\n> (unless you know there will always be a single HAL on your systems, so\n> you can call it camera.so and that's it). Having 'libcamera' in the\n> name is important to make sure we won't conflict with other components\n> at install time, as they can claim to be the 'android-hal' of the\n> system as well.\n> \n> Not to mention Android has several HAL, one for each subsystem, so\n> this should at least be 'camera-hal.so' if we want to stay generic.\n> \n> Furthermore HAL is a pretty Android-specific thing, I don't think it\n\nNo - I believe HAL means \"Hardware Abstraction Library\" It's a very\ncommon terminology, and I've encountered in in many places.\n\n\n> needs to have 'android' in the name to make clear what API we\n> implement.\n\nAs it stands here, this library is a libcamera based implementation of\nthe Android Camera HAL.\n\nIf we had to implement other Camera API's on top of libcamera, perhaps\nimagine if we implemented a GenICam API ... - That is a Hardware\nAbstraction Layer too ...\n\nWhat would that be called?\n\n\tlibcamera-genicam.so?\n\nOr would that also be handled in this same .so object?\n\nAnd (ok, pushing the boat out) but what if we ended up supporting\nwindows, mac, or freebsd, fuchsia?\nWould the support for those be\n\tlibcamera-hal.so\nor\n\tlibcamera-mac-hal.so\n\tlibcamera-windows-hal.so\n\tlibcamera-freebsd-hal.so\n\tlibcamera-fuchsia-hal.so\n\n(Wow, are OS's supposed to have 7 letters or something)\n\n>> I guess it could be the libcamera-android-hal.so too\n> \n> Repeating 'android' in the 'hal' name is of no use imho.\n> \n> libcamera-hal.so is a perfectly fine name for the libcamera-based camera\n> HAL. What are you afraid this can be confused with ?\n\n\nMy concern is that 'HAL' does not say much. It means 'android' to you -\nbut that's not a given.\n\nAnd maybe that's the point we're arguing. Perhaps\n\n \tlibcamera-android.so\n\nwould be acceptable?\n\n(If you're suggesting that it's the '-hal' suffix you don't think is needed)\n\n\n>> Perhaps if Chrome decided not to use the Android HAL but to use a\n>> wrapper on top of libcamera directly, I could imagine that could be\n>> called the libcamera-hal for instance.\n> \n> They would call into libcamera.so in that case, don't they ?\n\nI guess it depends on how they construct it. They might want a\ncentralised wrapper component to call into to handle all the things like\nwhat chrome-specific hardware acceleration to apply on top...\n\nAnd perhaps that would be the libcamera-hal from their perspective.\n\n\n> What's\n> the purpose of an HAL if not to behave like an HAL? HAL is not just a\n> naming suffix, but a combination of the right exported symbols and the\n> right C-API that makes it possible for the library to be loaded at\n> run-time by the Android framework, and all of this is -very- Android\n> specific.\n\nI think we're talking across terms here. Yes, a HAL is an implementation\nof a set of exported symbols to provide an interface to talk to hardware.\n\nAn *android camera HAL* is indeed that. But it's to implement the\nandroid camera hal specifically.\n\nA 'HAL' is ... just a generic term though, and it doesn't say \"Which\"\nAPI it is implementing.\n\n\n> Thanks\n> j\n> \n>>\n>> --\n>> Kieran\n>>\n>>\n>>>>> ?\n>>>>>\n>>>>>> + android_hal_sources,\n>>>>>> + name_prefix : '',\n>>>>>> + link_with : android_camera_metadata,\n>>>>>> + install : true,\n>>>>>> + cpp_args : libcamera_cpp_args,\n>>>>>> + include_directories : android_includes,\n>>>>>> + dependencies : android_deps)\n>>>>>\n>>>>> Without install_dir, this will get installed in /usr/lib/, which isn't\n>>>>> where the CrOS camera service will look for it. We probably need a\n>>>>> configuration option to set the path, as it will differ between CrOS and\n>>>>> Android.\n>>>>\n>>>> But I have install_dir ... uhm ... Ok so I forgot to save out the\n>>>> updated patch before sending ...\n>>>>\n>>>> It's there in the next version already ;-) ...\n>>>>\n>>>>> +android_installdir = get_option('libdir')\n>>>>> +\n>>>>> if get_option('android_platform') == 'cros'\n>>>>> libcamera_cpp_args += [ '-DOS_CHROMEOS']\n>>>\n>>> Extra space.\n>>>\n>>>>> + android_installdir = get_option('libdir') / 'camera_hal'\n>>>>> endif\n>>>>\n>>>> Will install it to camera_hal directly when the platform is set.\n>>>\n>>> Should install be set to true on CrOS only ? On Android meson isn't used\n>>> anyway, and on other systems, the Android HAL isn't needed.\n>>\n>> If it isn't needed, then don't build it (i.e. don't enable android)?\n>>\n>> And if it doesn't need to be installed (i.e. just compile testing) well\n>> then there's no need to call the install ...\n>>\n>>\n>>\n>>>>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>>>>> index e0a48aa23ea5..f77b80b878d8 100644\n>>>>>> --- a/src/libcamera/meson.build\n>>>>>> +++ b/src/libcamera/meson.build\n>>>>>> @@ -129,16 +129,6 @@ libcamera_deps = [\n>>>>>> dependency('threads'),\n>>>>>> ]\n>>>>>>\n>>>>>> -libcamera_link_with = []\n>>>>>> -\n>>>>>> -if android_enabled\n>>>>>> - libcamera_sources += android_hal_sources\n>>>>>> - includes += android_includes\n>>>>>> - libcamera_link_with += android_camera_metadata\n>>>>>> -\n>>>>>> - libcamera_deps += android_deps\n>>>>>> -endif\n>>>>>> -\n>>>>>> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n>>>>>> # The build_rpath is stripped at install time by meson, so we determine at\n>>>>>> # runtime if the library is running from an installed location by checking\n>>>>>> @@ -147,7 +137,6 @@ endif\n>>>>>> libcamera = shared_library('camera',\n>>>>>> libcamera_sources,\n>>>>>> install : true,\n>>>>>> - link_with : libcamera_link_with,\n>>>>>> cpp_args : libcamera_cpp_args,\n>>>>>> include_directories : includes,\n>>>>>> objects : libcamera_objects,\n>>>>>> diff --git a/src/meson.build b/src/meson.build\n>>>>>> index 64d22b9ce5b6..9dc32751bbb1 100644\n>>>>>> --- a/src/meson.build\n>>>>>> +++ b/src/meson.build\n>>>>>> @@ -31,11 +31,10 @@ endif\n>>>>>> libcamera_cpp_args = []\n>>>>>> libcamera_objects = []\n>>>>>>\n>>>>>> -# The 'android' subdir must be processed first, and the build targets\n>>>>>> -# are included directly into the libcamera library when this is enabled.\n>>>>>> -subdir('android')\n>>>>>> -\n>>>>>> +# libcamera must be built first as a dependency to the other components.\n>>>>>> subdir('libcamera')\n>>>>>\n>>>>> This is really nice, having android first has been bugging me for a\n>>>>> while.\n>>>>\n>>>> Me to, I somehow assumed last time I looked at this that there were\n>>>> deeper reasons for integrating into the libcamera.so - but there wasn't\n>>>> anything preventing the split really.\n>>>>\n>>>> Anyway, v2 to come anyway with the correct install paths, and if desired\n>>>> - a rename.\n>>>>\n>>>>>> +\n>>>>>> +subdir('android')\n>>>>>> subdir('ipa')\n>>>>>>\n>>>>>> subdir('lc-compliance')\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 30C28BF825\n\tfor ;\n\tThu, 6 May 2021 12:25:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A975268915;\n\tThu, 6 May 2021 14:25:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7133A68901\n\tfor ;\n\tThu, 6 May 2021 14:25:27 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 99C2C4A5;\n\tThu, 6 May 2021 14:25:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"U7rvOV+Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620303927;\n\tbh=i6S30dBGFR3atzYmAx2nh+j6lkxCi6Mx5gchTOr4XE8=;\n\th=Reply-To:To:Cc:References:From:Subject:Date:In-Reply-To:From;\n\tb=U7rvOV+ZZlrMKhi4VblP+uexmnsOugLcYM43e4xkjfK5EZU/diilGNH/qsvk/PMTv\n\t4pUDKGixC2yoy8dytHarNJeio5Y03p5XdxViPUFrU+rQlfRtiDRRY2dLjmxUqoNmr4\n\tsZjdHEdmfo901Fu0Og5h58niPBuAeHv51hVRevLw=","To":"Jacopo Mondi ","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t\n\t<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>\n\t\n\t<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>\n\t<20210503082958.iblpovrzdgfbg7ft@uno.localdomain>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<923646d2-d4e4-9d7a-055f-27d87f8f12d6@ideasonboard.com>","Date":"Thu, 6 May 2021 13:25:23 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210503082958.iblpovrzdgfbg7ft@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16810,"web_url":"https://patchwork.libcamera.org/comment/16810/","msgid":"","date":"2021-05-06T12:41:42","subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Thu, May 06, 2021 at 01:25:23PM +0100, Kieran Bingham wrote:\n> On 03/05/2021 09:29, Jacopo Mondi wrote:\n> > On Mon, May 03, 2021 at 09:13:31AM +0100, Kieran Bingham wrote:\n> >> On 03/05/2021 08:58, Laurent Pinchart wrote:\n> >>> On Sun, May 02, 2021 at 09:02:45PM +0100, Kieran Bingham wrote:\n> >>>> On 01/05/2021 21:54, Laurent Pinchart wrote:\n> >>>>> On Fri, Apr 30, 2021 at 04:02:19PM +0100, Kieran Bingham wrote:\n> >>>>>> The libcamera Android HAL implementation should not be an integral part\n> >>>>>> of libcamera, but a support library that utilises the libcamera public\n> >>>>>> API.\n> >>>>>>\n> >>>>>> Move the implementation to it's own distinct library.\n> >>>>>>\n> >>>>>> Signed-off-by: Kieran Bingham \n> >>>>>> ---\n> >>>>>>\n> >>>>>> Please note that this no longer requires ChromeOS to manually symlink\n> >>>>>> into the camera_hal directory.\n> >>>>>>\n> >>>>>> A modification similar to the following should be applied to the Chrome\n> >>>>>> ebuilds if this is applied:\n> >>>>>>\n> >>>>>> diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild\n> >>>>>> index 433357f1ef74..cf90e438cfb3 100644\n> >>>>>> --- a/media-libs/libcamera/libcamera-9999.ebuild\n> >>>>>> +++ b/media-libs/libcamera/libcamera-9999.ebuild\n> >>>>>> @@ -57,6 +57,4 @@ src_compile() {\n> >>>>>>\n> >>>>>> src_install() {\n> >>>>>> meson_src_install\n> >>>>>> -\n> >>>>>> - dosym ../libcamera.so \"/usr/$(get_libdir)/camera_hal/libcamera.so\"\n> >>>>>> }\n> >>>>>\n> >>>>> Could you submit a patch to CrOS ?\n> >>>>>\n> >>>>>> src/android/meson.build | 10 ++++++++++\n> >>>>>> src/libcamera/meson.build | 11 -----------\n> >>>>>> src/meson.build | 7 +++----\n> >>>>>> 3 files changed, 13 insertions(+), 15 deletions(-)\n> >>>>>>\n> >>>>>> diff --git a/src/android/meson.build b/src/android/meson.build\n> >>>>>> index 2be20c97e118..6170448a73ce 100644\n> >>>>>> --- a/src/android/meson.build\n> >>>>>> +++ b/src/android/meson.build\n> >>>>>> @@ -3,6 +3,7 @@\n> >>>>>> android_deps = [\n> >>>>>> dependency('libexif', required : get_option('android')),\n> >>>>>> dependency('libjpeg', required : get_option('android')),\n> >>>>>> + libcamera_dep,\n> >>>>>> ]\n> >>>>>>\n> >>>>>> android_enabled = true\n> >>>>>> @@ -66,3 +67,12 @@ android_camera_metadata = static_library('camera_metadata',\n> >>>>>> android_camera_metadata_sources,\n> >>>>>> c_args : '-Wno-shadow',\n> >>>>>> include_directories : android_includes)\n> >>>>>> +\n> >>>>>> +android_hal = shared_library('android-hal',\n> >>>>>\n> >>>>> android-hal.so won't be a very descriptive name. Maybe libcamera-hal.so\n> >>>>\n> >>>> I started out with libcamera-hal, but then I decided I didn't like it so\n> >>>> I updated to android-hal ...\n> >>>>\n> >>>> Given that this is an android-hal implementation I thought that was\n> >>>> /more/ descriptive ...\n> >>>>\n> >>>> otherwise libcamera-hal sounds like it's an abstraction layer of libcamera.\n> >>>>\n> >>>> If you prefer libcamera-hal I'll change it back.\n> >>>\n> >>> Following that logic, every shared object on a Linux system should be\n> >>> called linux-* :-) Given that we may have different HAL implementations> in the same directory (both multiple camera HAL implementations in\n> >>> Chrome OS, and multiple HALs for unrelated devices in Android), it's\n> >>> important for the file name to specify the project and possibly the fact\n> >>> that it's related to cameras. With 'libcamera' as the project name, both\n> >>> goals are reached :-)\n> >>\n> >> Well ... maybe the Linux Abstraction Layer for Android could be called\n> >> linux-hal (ok, too meta, because Android is on top of Linux) ... but the\n> >> difference is the perspective. If this was installed on android then\n> >> yes, I see your point - but we don't (currently) install on android,\n> >> only chrome ;-) - where it is the 'android-hal' much like the 'libc' is\n> >> the c library...\n> > \n> > FWIW I second naming the newly introduced .so as libcamera-hal.so.\n> > \n> > As an example Cros currently ships two HALs on Soraka, intel and usb.\n> > Both implement the Android Camera3 API, why aren't they both called\n> > android-hal.so ?\n> \n> Interestingly, On CrOS for IPU3 the intel hal builds a libcamera_hal.so\n> which is renamed to intel-ipu3.so by the installation:\n> \n> chipset-kbl/media-libs/cros-camera-hal-intel-ipu3/cros-camera-hal-intel-ipu3-0.0.1-r617.ebuild\n> cros-camera_dohal \"${OUT}/lib/libcamera_hal.so\" intel-ipu3.so\n> \n> and the chipset-rk3399 variant does the same thing:\n> \n> cros-camera_dohal \"${OUT}/lib/libcamera_hal.so\" rockchip-isp1.so\n> \n> Of course the catch is here, that our .so supports both, or all\n> platforms ;-)\n\nThis gives us the luxury of naming the binary in any way we want, and\nrenaming it to match the Chrome OS naming scheme at install time in the\nebuild. We can thus decouple the two discussions, please let's note that\nthe HAL implementation doesn't really exist in isolation, it's only used\nfor Chrome OS and Android devices, so we don't have to name it in a way\nthat would take into account how they would be installed in other\nsystems. The name we're picking will not be seen by anyone.\n\n> > The HAL naming scheme usually reports the platform/camera manufacturer\n> > (unless you know there will always be a single HAL on your systems, so\n> > you can call it camera.so and that's it). Having 'libcamera' in the\n> > name is important to make sure we won't conflict with other components\n> > at install time, as they can claim to be the 'android-hal' of the\n> > system as well.\n> > \n> > Not to mention Android has several HAL, one for each subsystem, so\n> > this should at least be 'camera-hal.so' if we want to stay generic.\n> > \n> > Furthermore HAL is a pretty Android-specific thing, I don't think it\n> \n> No - I believe HAL means \"Hardware Abstraction Library\" It's a very\n> common terminology, and I've encountered in in many places.\n\nIn theory, I agree, but in the context of cameras, it's very linked to\nAndroid in practice.\n\n> > needs to have 'android' in the name to make clear what API we\n> > implement.\n> \n> As it stands here, this library is a libcamera based implementation of\n> the Android Camera HAL.\n> \n> If we had to implement other Camera API's on top of libcamera, perhaps\n> imagine if we implemented a GenICam API ... - That is a Hardware\n> Abstraction Layer too ...\n> \n> What would that be called?\n> \n> \tlibcamera-genicam.so?\n\nSounds good, or any another name that would be mandated by GenICam.\n\n> Or would that also be handled in this same .so object?\n> \n> And (ok, pushing the boat out) but what if we ended up supporting\n> windows, mac, or freebsd, fuchsia?\n> Would the support for those be\n> \tlibcamera-hal.so\n> or\n> \tlibcamera-mac-hal.so\n> \tlibcamera-windows-hal.so\n> \tlibcamera-freebsd-hal.so\n> \tlibcamera-fuchsia-hal.so\n\nWe would name those based on the requirements of these platforms. I\ndoubt you'd have a .so on windows ;-)\n\n> (Wow, are OS's supposed to have 7 letters or something)\n> \n> >> I guess it could be the libcamera-android-hal.so too\n> > \n> > Repeating 'android' in the 'hal' name is of no use imho.\n> > \n> > libcamera-hal.so is a perfectly fine name for the libcamera-based camera\n> > HAL. What are you afraid this can be confused with ?\n> \n> My concern is that 'HAL' does not say much. It means 'android' to you -\n> but that's not a given.\n> \n> And maybe that's the point we're arguing. Perhaps\n> \n> \tlibcamera-android.so\n> \n> would be acceptable?\n> \n> (If you're suggesting that it's the '-hal' suffix you don't think is needed)\n> \n> >> Perhaps if Chrome decided not to use the Android HAL but to use a\n> >> wrapper on top of libcamera directly, I could imagine that could be\n> >> called the libcamera-hal for instance.\n> > \n> > They would call into libcamera.so in that case, don't they ?\n> \n> I guess it depends on how they construct it. They might want a\n> centralised wrapper component to call into to handle all the things like\n> what chrome-specific hardware acceleration to apply on top...\n> \n> And perhaps that would be the libcamera-hal from their perspective.\n> \n> > What's\n> > the purpose of an HAL if not to behave like an HAL? HAL is not just a\n> > naming suffix, but a combination of the right exported symbols and the\n> > right C-API that makes it possible for the library to be loaded at\n> > run-time by the Android framework, and all of this is -very- Android\n> > specific.\n> \n> I think we're talking across terms here. Yes, a HAL is an implementation\n> of a set of exported symbols to provide an interface to talk to hardware.\n> \n> An *android camera HAL* is indeed that. But it's to implement the\n> android camera hal specifically.\n> \n> A 'HAL' is ... just a generic term though, and it doesn't say \"Which\"\n> API it is implementing.\n> \n> >>>>> ?\n> >>>>>\n> >>>>>> + android_hal_sources,\n> >>>>>> + name_prefix : '',\n> >>>>>> + link_with : android_camera_metadata,\n> >>>>>> + install : true,\n> >>>>>> + cpp_args : libcamera_cpp_args,\n> >>>>>> + include_directories : android_includes,\n> >>>>>> + dependencies : android_deps)\n> >>>>>\n> >>>>> Without install_dir, this will get installed in /usr/lib/, which isn't\n> >>>>> where the CrOS camera service will look for it. We probably need a\n> >>>>> configuration option to set the path, as it will differ between CrOS and\n> >>>>> Android.\n> >>>>\n> >>>> But I have install_dir ... uhm ... Ok so I forgot to save out the\n> >>>> updated patch before sending ...\n> >>>>\n> >>>> It's there in the next version already ;-) ...\n> >>>>\n> >>>>> +android_installdir = get_option('libdir')\n> >>>>> +\n> >>>>> if get_option('android_platform') == 'cros'\n> >>>>> libcamera_cpp_args += [ '-DOS_CHROMEOS']\n> >>>\n> >>> Extra space.\n> >>>\n> >>>>> + android_installdir = get_option('libdir') / 'camera_hal'\n> >>>>> endif\n> >>>>\n> >>>> Will install it to camera_hal directly when the platform is set.\n> >>>\n> >>> Should install be set to true on CrOS only ? On Android meson isn't used\n> >>> anyway, and on other systems, the Android HAL isn't needed.\n> >>\n> >> If it isn't needed, then don't build it (i.e. don't enable android)?\n> >>\n> >> And if it doesn't need to be installed (i.e. just compile testing) well\n> >> then there's no need to call the install ...\n> >>\n> >>>>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >>>>>> index e0a48aa23ea5..f77b80b878d8 100644\n> >>>>>> --- a/src/libcamera/meson.build\n> >>>>>> +++ b/src/libcamera/meson.build\n> >>>>>> @@ -129,16 +129,6 @@ libcamera_deps = [\n> >>>>>> dependency('threads'),\n> >>>>>> ]\n> >>>>>>\n> >>>>>> -libcamera_link_with = []\n> >>>>>> -\n> >>>>>> -if android_enabled\n> >>>>>> - libcamera_sources += android_hal_sources\n> >>>>>> - includes += android_includes\n> >>>>>> - libcamera_link_with += android_camera_metadata\n> >>>>>> -\n> >>>>>> - libcamera_deps += android_deps\n> >>>>>> -endif\n> >>>>>> -\n> >>>>>> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n> >>>>>> # The build_rpath is stripped at install time by meson, so we determine at\n> >>>>>> # runtime if the library is running from an installed location by checking\n> >>>>>> @@ -147,7 +137,6 @@ endif\n> >>>>>> libcamera = shared_library('camera',\n> >>>>>> libcamera_sources,\n> >>>>>> install : true,\n> >>>>>> - link_with : libcamera_link_with,\n> >>>>>> cpp_args : libcamera_cpp_args,\n> >>>>>> include_directories : includes,\n> >>>>>> objects : libcamera_objects,\n> >>>>>> diff --git a/src/meson.build b/src/meson.build\n> >>>>>> index 64d22b9ce5b6..9dc32751bbb1 100644\n> >>>>>> --- a/src/meson.build\n> >>>>>> +++ b/src/meson.build\n> >>>>>> @@ -31,11 +31,10 @@ endif\n> >>>>>> libcamera_cpp_args = []\n> >>>>>> libcamera_objects = []\n> >>>>>>\n> >>>>>> -# The 'android' subdir must be processed first, and the build targets\n> >>>>>> -# are included directly into the libcamera library when this is enabled.\n> >>>>>> -subdir('android')\n> >>>>>> -\n> >>>>>> +# libcamera must be built first as a dependency to the other components.\n> >>>>>> subdir('libcamera')\n> >>>>>\n> >>>>> This is really nice, having android first has been bugging me for a\n> >>>>> while.\n> >>>>\n> >>>> Me to, I somehow assumed last time I looked at this that there were\n> >>>> deeper reasons for integrating into the libcamera.so - but there wasn't\n> >>>> anything preventing the split really.\n> >>>>\n> >>>> Anyway, v2 to come anyway with the correct install paths, and if desired\n> >>>> - a rename.\n> >>>>\n> >>>>>> +\n> >>>>>> +subdir('android')\n> >>>>>> subdir('ipa')\n> >>>>>>\n> >>>>>> subdir('lc-compliance')","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2A76EBDE7F\n\tfor ;\n\tThu, 6 May 2021 12:41:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9468268909;\n\tThu, 6 May 2021 14:41:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AE87C68901\n\tfor ;\n\tThu, 6 May 2021 14:41:47 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 37C394A5;\n\tThu, 6 May 2021 14:41:47 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dgRXNqNf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620304907;\n\tbh=o9ISwBzNbj5HyEn1JEHOUE5NYlEYVDe8/bjuypkDmuc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dgRXNqNfsgdZvJV7SPGQ69O91lQkTyWp+A42YqO04lqhuoXjEiMjjPzY+txDi3wAR\n\ttQ3lPAeqzRu4ZXvZpzvE2a5l32w0n3WInu/QHxBmVshhM1sMxxYtiDCies910gjzjG\n\tGZZZFD3kaCIfb90mvt7XYVtw4ANGVwv8Bx0jEANk=","Date":"Thu, 6 May 2021 15:41:42 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210430150219.299389-1-kieran.bingham@ideasonboard.com>\n\t\n\t<023d6d08-e4c0-82d1-c370-58b54a499a94@ideasonboard.com>\n\t\n\t<80a12663-ee01-f4b7-d702-33462f28a5cf@ideasonboard.com>\n\t<20210503082958.iblpovrzdgfbg7ft@uno.localdomain>\n\t<923646d2-d4e4-9d7a-055f-27d87f8f12d6@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<923646d2-d4e4-9d7a-055f-27d87f8f12d6@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] android: Split HAL to it's own shared\n\tlibrary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]