[{"id":27164,"web_url":"https://patchwork.libcamera.org/comment/27164/","msgid":"<20230529155818.GD15264@pendragon.ideasonboard.com>","date":"2023-05-29T15:58:18","subject":"Re: [libcamera-devel] [PATCH v4] android: mm: generic: use\n\tGRALLOC_HARDWARE_MODULE_ID","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Mattijs,\n\nThank you for the patch.\n\nOn Mon, May 29, 2023 at 03:30:17PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n> PlatformFrameBufferAllocator is an abstraction over gralloc.\n> Right now hardwareModule_ points towards a CAMERA_HARDWARE_MODULE_ID.\n> \n> When gralloc_open() is called we observe:\n> libcamera: DEBUG HAL camera3_hal.cpp:75 Open camera gpu0\n> libcamera: ERROR Camera camera.cpp:524 Camera in Configured state trying acquire() requiring state Available\n> 01-23 14:14:04.742 370 416 E libcamera: FATAL HAL generic_frame_buffer_allocator.cpp:105 gralloc_open() failed: -87\n> \n> Which is wrong, gralloc_open() is attempting to re-open the camera HAL,\n> instead of the gralloc HAL.\n> \n> Point to a GRALLOC_HARDWARE_MODULE_ID instead so that we can request\n> buffers from gralloc in android.\n> \n> Note: this adds new dependencies on android's libhardware [1] and on libdl.\n> \n> [1] https://android.googlesource.com/platform/hardware/libhardware\n> \n> Fixes: c58662c5770e (\"android: Introduce PlatformFrameBufferAllocator\")\n> Signed-off-by: Mattijs Korpershoek \n> ---\n> * Tested on an integration branch in Android 12 (Using android.bp)\n> * Build tested with meson setup build -Dandroid=enabled -Dandroid_platform=generic\n> \n> (Laurent, I dropped your reviewed-by since there are additional changes)\n> ---\n> Changes in v4:\n> - Add missing meson.build change to link against libdl\n> - Link to v3: https://lists.libcamera.org/pipermail/libcamera-devel/2023-May/037926.html\n> \n> Changes in v3:\n> - Add missing meson.build change to define libhardware dependency\n> - Link to v2: https://lists.libcamera.org/pipermail/libcamera-devel/2023-February/036884.html\n> \n> Changes in v2:\n> - Add dlclose() in PlatformFrameBufferAllocator destructor (Laurent)\n> - Removed RFC, as linking against an AOSP lib for android is OK.\n> - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2023-February/036857.html\n> ---\n> src/android/mm/generic_frame_buffer_allocator.cpp | 7 +++++--\n> src/android/mm/meson.build | 1 +\n> 2 files changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp\n> index 3750e1bf52a9..7ecef2c669df 100644\n> --- a/src/android/mm/generic_frame_buffer_allocator.cpp\n> +++ b/src/android/mm/generic_frame_buffer_allocator.cpp\n> @@ -5,6 +5,7 @@\n> * generic_camera_buffer.cpp - Allocate FrameBuffer using gralloc API\n> */\n> \n> +#include \n> #include \n> #include \n> \n> @@ -72,9 +73,10 @@ class PlatformFrameBufferAllocator::Private : public Extensible::Private\n> public:\n> \tPrivate(CameraDevice *const cameraDevice)\n> \t\t: cameraDevice_(cameraDevice),\n> -\t\t hardwareModule_(cameraDevice->camera3Device()->common.module),\n> +\t\t hardwareModule_(nullptr),\n> \t\t allocDevice_(nullptr)\n> \t{\n> +\t\thw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hardwareModule_);\n> \t\tASSERT(hardwareModule_);\n> \t}\n> \n> @@ -85,7 +87,7 @@ public:\n> \n> private:\n> \tconst CameraDevice *const cameraDevice_;\n> -\tstruct hw_module_t *const hardwareModule_;\n> +\tconst struct hw_module_t *hardwareModule_;\n> \tstruct alloc_device_t *allocDevice_;\n> };\n> \n> @@ -93,6 +95,7 @@ PlatformFrameBufferAllocator::Private::~Private()\n> {\n> \tif (allocDevice_)\n> \t\tgralloc_close(allocDevice_);\n> +\tdlclose(hardwareModule_->dso);\n> }\n> \n> std::unique_ptr\n> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> index d40a3b0ba2eb..c92416bd3a6f 100644\n> --- a/src/android/mm/meson.build\n> +++ b/src/android/mm/meson.build\n> @@ -4,6 +4,7 @@ platform = get_option('android_platform')\n> if platform == 'generic'\n> android_hal_sources += files(['generic_camera_buffer.cpp',\n> 'generic_frame_buffer_allocator.cpp'])\n> + android_deps += [libdl, dependency('libhardware')]\n\nWe usually split arrays with multiple entries on multiple lines:\n\n android_deps += [\n libdl,\n\tdependency('libhardware'),\n ]\n\nI'll do this locally.\n\nReviewed-by: Laurent Pinchart \n\n> elif platform == 'cros'\n> android_hal_sources += files(['cros_camera_buffer.cpp',\n> 'cros_frame_buffer_allocator.cpp'])\n> \n> ---\n> base-commit: 76e1cb9f7176b4e7a935295d4e633ee24a0fef67\n> change-id: 20230227-android-gralloc-04e31d5edc68","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 8B9B3C31E9\n\tfor ;\n\tMon, 29 May 2023 15:58:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 93868626F9;\n\tMon, 29 May 2023 17:58:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CCA616038E\n\tfor ;\n\tMon, 29 May 2023 17:58:17 +0200 (CEST)","from pendragon.ideasonboard.com (om126255106133.24.openmobile.ne.jp\n\t[126.255.106.133])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB421836;\n\tMon, 29 May 2023 17:57:56 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685375899;\n\tbh=21AIJkVh20J+p8/V9cfT2pSm0CZoyX8YQXb/H+eSOIU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=G7hPCdlnOxPgg0Gjj8NXrJQDTPFvjhcyEG1oJOu2Jy8K8kMkfVoWsalQR/v447Saz\n\taKGTFjfKlMUwwiT1M7V4AG9lHKvUhRY1TdpLwuqXbXkTqIFuNHiNDEMFyj6T8aHGR+\n\tcoyHZEjP2o9obaqHH7mR+6BgGBeXnNx1PJVwzF0OKTlBm7hqsoC/lY0BNovDEJvSgR\n\tpmGQLF0JUczCjTQ0NKBPlFiyfmKAugCwfL03IwHu1MHLkdw77ootySBM8Q14cRNM6I\n\tDIfyYU+TW17kGBpGZsUy25jEyUpl6BPOgs+MJ2jnI2IPMGn+Mkj3a0A1bEq6goHgP9\n\t8To6hNjPa5WOg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685375877;\n\tbh=21AIJkVh20J+p8/V9cfT2pSm0CZoyX8YQXb/H+eSOIU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZB3GqiYUkxcpDkU4jsoeUTgVx2f1UqRcY4qiDbAZW0nY75GUMfE01RlSkSPG3Xnt5\n\tPiFdAcRcTUxNP+kd3XtnmCpGxn/W/bCg0thZDV2/jURvpJ+FFg9JfbD1ja5fML0YS7\n\tWt37hBDqr2rmSwfjJLy7Esf7bw5LSvk6e8BfShYw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ZB3GqiYU\"; dkim-atps=neutral","Date":"Mon, 29 May 2023 18:58:18 +0300","To":"Mattijs Korpershoek ","Message-ID":"<20230529155818.GD15264@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v4-1-8cd3a44c5b70@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230227-android-gralloc-v4-1-8cd3a44c5b70@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH v4] android: mm: generic: use\n\tGRALLOC_HARDWARE_MODULE_ID","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27178,"web_url":"https://patchwork.libcamera.org/comment/27178/","msgid":"<168544909877.130277.18288798045979326882@Monstersaurus>","date":"2023-05-30T12:18:18","subject":"Re: [libcamera-devel] [PATCH v4] android: mm: generic: use\n\tGRALLOC_HARDWARE_MODULE_ID","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2023-05-29 16:58:18)\n> Hi Mattijs,\n> \n> Thank you for the patch.\n> \n> On Mon, May 29, 2023 at 03:30:17PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n> > PlatformFrameBufferAllocator is an abstraction over gralloc.\n> > Right now hardwareModule_ points towards a CAMERA_HARDWARE_MODULE_ID.\n> > \n> > When gralloc_open() is called we observe:\n> > libcamera: DEBUG HAL camera3_hal.cpp:75 Open camera gpu0\n> > libcamera: ERROR Camera camera.cpp:524 Camera in Configured state trying acquire() requiring state Available\n> > 01-23 14:14:04.742 370 416 E libcamera: FATAL HAL generic_frame_buffer_allocator.cpp:105 gralloc_open() failed: -87\n> > \n> > Which is wrong, gralloc_open() is attempting to re-open the camera HAL,\n> > instead of the gralloc HAL.\n> > \n> > Point to a GRALLOC_HARDWARE_MODULE_ID instead so that we can request\n> > buffers from gralloc in android.\n> > \n> > Note: this adds new dependencies on android's libhardware [1] and on libdl.\n> > \n> > [1] https://android.googlesource.com/platform/hardware/libhardware\n> > \n> > Fixes: c58662c5770e (\"android: Introduce PlatformFrameBufferAllocator\")\n> > Signed-off-by: Mattijs Korpershoek \n> > ---\n> > * Tested on an integration branch in Android 12 (Using android.bp)\n> > * Build tested with meson setup build -Dandroid=enabled -Dandroid_platform=generic\n> > \n> > (Laurent, I dropped your reviewed-by since there are additional changes)\n> > ---\n> > Changes in v4:\n> > - Add missing meson.build change to link against libdl\n> > - Link to v3: https://lists.libcamera.org/pipermail/libcamera-devel/2023-May/037926.html\n> > \n> > Changes in v3:\n> > - Add missing meson.build change to define libhardware dependency\n> > - Link to v2: https://lists.libcamera.org/pipermail/libcamera-devel/2023-February/036884.html\n> > \n> > Changes in v2:\n> > - Add dlclose() in PlatformFrameBufferAllocator destructor (Laurent)\n> > - Removed RFC, as linking against an AOSP lib for android is OK.\n> > - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2023-February/036857.html\n> > ---\n> > src/android/mm/generic_frame_buffer_allocator.cpp | 7 +++++--\n> > src/android/mm/meson.build | 1 +\n> > 2 files changed, 6 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp\n> > index 3750e1bf52a9..7ecef2c669df 100644\n> > --- a/src/android/mm/generic_frame_buffer_allocator.cpp\n> > +++ b/src/android/mm/generic_frame_buffer_allocator.cpp\n> > @@ -5,6 +5,7 @@\n> > * generic_camera_buffer.cpp - Allocate FrameBuffer using gralloc API\n> > */\n> > \n> > +#include \n> > #include \n> > #include \n> > \n> > @@ -72,9 +73,10 @@ class PlatformFrameBufferAllocator::Private : public Extensible::Private\n> > public:\n> > Private(CameraDevice *const cameraDevice)\n> > : cameraDevice_(cameraDevice),\n> > - hardwareModule_(cameraDevice->camera3Device()->common.module),\n> > + hardwareModule_(nullptr),\n> > allocDevice_(nullptr)\n> > {\n> > + hw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hardwareModule_);\n\nAs discussed with Laurent directly, I think keeping compilation\nsupported for non-aosp builds is helpful to continue maintaining the\nAndroid build with compile tests.\n\nBut we can do this easily by having a compilation unit\n(libhardware-stubs.cpp perhaps) that gets compiled when libhardware is\nnot available, to provide linkage to a stubbed hw_get_module().\n\nI don't mind if that's on top as long as all the builds don't stay\nbroken for long! (There will still be regular builds for ChromeOS\nthough so it wouldn't get forgotten, so I'm not too worried).\n\nReviewed-by: Kieran Bingham \n\n> > ASSERT(hardwareModule_);\n> > }\n> > \n> > @@ -85,7 +87,7 @@ public:\n> > \n> > private:\n> > const CameraDevice *const cameraDevice_;\n> > - struct hw_module_t *const hardwareModule_;\n> > + const struct hw_module_t *hardwareModule_;\n> > struct alloc_device_t *allocDevice_;\n> > };\n> > \n> > @@ -93,6 +95,7 @@ PlatformFrameBufferAllocator::Private::~Private()\n> > {\n> > if (allocDevice_)\n> > gralloc_close(allocDevice_);\n> > + dlclose(hardwareModule_->dso);\n> > }\n> > \n> > std::unique_ptr\n> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> > index d40a3b0ba2eb..c92416bd3a6f 100644\n> > --- a/src/android/mm/meson.build\n> > +++ b/src/android/mm/meson.build\n> > @@ -4,6 +4,7 @@ platform = get_option('android_platform')\n> > if platform == 'generic'\n> > android_hal_sources += files(['generic_camera_buffer.cpp',\n> > 'generic_frame_buffer_allocator.cpp'])\n> > + android_deps += [libdl, dependency('libhardware')]\n> \n> We usually split arrays with multiple entries on multiple lines:\n> \n> android_deps += [\n> libdl,\n> dependency('libhardware'),\n> ]\n> \n> I'll do this locally.\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > elif platform == 'cros'\n> > android_hal_sources += files(['cros_camera_buffer.cpp',\n> > 'cros_frame_buffer_allocator.cpp'])\n> > \n> > ---\n> > base-commit: 76e1cb9f7176b4e7a935295d4e633ee24a0fef67\n> > change-id: 20230227-android-gralloc-04e31d5edc68\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 505F4C31E9\n\tfor ;\n\tTue, 30 May 2023 12:18:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 96A03626F8;\n\tTue, 30 May 2023 14:18:23 +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 BA5AE60595\n\tfor ;\n\tTue, 30 May 2023 14:18:21 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E1BA8E4;\n\tTue, 30 May 2023 14:18:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685449103;\n\tbh=rOMx+nW5/0OUxk93cdcSJif7lt2cBaSRRsbUTHmmI98=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=uMsU8xx95/CXVtw40uzwDvOvwtCRC6K0ZAOdVnZs+rDE3qwOvn9bqjaJc3nm7lwGN\n\tJ0gZiEvvkGtzuGEuuTwOdgEjbgKu+DIc16pwRFOsPUesgRGMoSmhcBp3c7mF0tspoL\n\to0r4BywWeoK1mVZmvbs/wT5+qXOF9ZU33F0shF+Ek+8z6+Jc2PEwLbLJqR1K7eNQwg\n\tkaIqSt1qHWguBzODpsxQzYC5J3gMe3nj6CLq46i1cmP1PPrXjx3YV2EzzGvabfxQSP\n\te8VfpYGx6zgMUY3GFCi7FSibzcHgqLLG933jD1xhPMAwB7Ol/lkDb/bR1O38Oo6gsR\n\tfp8zadajA5krQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685449081;\n\tbh=rOMx+nW5/0OUxk93cdcSJif7lt2cBaSRRsbUTHmmI98=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=VllNhizIs6gGydCramRzENBF9qeWwJkZ1YjIxSpOn13ZpFkWmkapE+mgwN9TQUuMh\n\tEo/f4pP4CZHYNUhPKyL1/4O1SDYEkE+H/GSafDh4yGEBczUoDykxLx+GV1wuP9yvhx\n\twGdNmVXcKjFGuyuaVBgg05GcpAsfgLw18E4bqosM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VllNhizI\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230529155818.GD15264@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v4-1-8cd3a44c5b70@baylibre.com>\n\t<20230529155818.GD15264@pendragon.ideasonboard.com>","To":"Laurent Pinchart ,\n\tLaurent Pinchart via libcamera-devel\n\t, \n\tMattijs Korpershoek ","Date":"Tue, 30 May 2023 13:18:18 +0100","Message-ID":"<168544909877.130277.18288798045979326882@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4] android: mm: generic: use\n\tGRALLOC_HARDWARE_MODULE_ID","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27183,"web_url":"https://patchwork.libcamera.org/comment/27183/","msgid":"<871qixkfhd.fsf@baylibre.com>","date":"2023-05-30T15:26:22","subject":"Re: [libcamera-devel] [PATCH v4] android: mm: generic: use\n\tGRALLOC_HARDWARE_MODULE_ID","submitter":{"id":153,"url":"https://patchwork.libcamera.org/api/people/153/","name":"Mattijs Korpershoek","email":"mkorpershoek@baylibre.com"},"content":"Hi Kieran,\n\nOn mar., mai 30, 2023 at 13:18, Kieran Bingham wrote:\n\n> Quoting Laurent Pinchart via libcamera-devel (2023-05-29 16:58:18)\n>> Hi Mattijs,\n>> \n>> Thank you for the patch.\n>> \n>> On Mon, May 29, 2023 at 03:30:17PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n>> > PlatformFrameBufferAllocator is an abstraction over gralloc.\n>> > Right now hardwareModule_ points towards a CAMERA_HARDWARE_MODULE_ID.\n>> > \n>> > When gralloc_open() is called we observe:\n>> > libcamera: DEBUG HAL camera3_hal.cpp:75 Open camera gpu0\n>> > libcamera: ERROR Camera camera.cpp:524 Camera in Configured state trying acquire() requiring state Available\n>> > 01-23 14:14:04.742 370 416 E libcamera: FATAL HAL generic_frame_buffer_allocator.cpp:105 gralloc_open() failed: -87\n>> > \n>> > Which is wrong, gralloc_open() is attempting to re-open the camera HAL,\n>> > instead of the gralloc HAL.\n>> > \n>> > Point to a GRALLOC_HARDWARE_MODULE_ID instead so that we can request\n>> > buffers from gralloc in android.\n>> > \n>> > Note: this adds new dependencies on android's libhardware [1] and on libdl.\n>> > \n>> > [1] https://android.googlesource.com/platform/hardware/libhardware\n>> > \n>> > Fixes: c58662c5770e (\"android: Introduce PlatformFrameBufferAllocator\")\n>> > Signed-off-by: Mattijs Korpershoek \n>> > ---\n>> > * Tested on an integration branch in Android 12 (Using android.bp)\n>> > * Build tested with meson setup build -Dandroid=enabled -Dandroid_platform=generic\n>> > \n>> > (Laurent, I dropped your reviewed-by since there are additional changes)\n>> > ---\n>> > Changes in v4:\n>> > - Add missing meson.build change to link against libdl\n>> > - Link to v3: https://lists.libcamera.org/pipermail/libcamera-devel/2023-May/037926.html\n>> > \n>> > Changes in v3:\n>> > - Add missing meson.build change to define libhardware dependency\n>> > - Link to v2: https://lists.libcamera.org/pipermail/libcamera-devel/2023-February/036884.html\n>> > \n>> > Changes in v2:\n>> > - Add dlclose() in PlatformFrameBufferAllocator destructor (Laurent)\n>> > - Removed RFC, as linking against an AOSP lib for android is OK.\n>> > - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2023-February/036857.html\n>> > ---\n>> > src/android/mm/generic_frame_buffer_allocator.cpp | 7 +++++--\n>> > src/android/mm/meson.build | 1 +\n>> > 2 files changed, 6 insertions(+), 2 deletions(-)\n>> > \n>> > diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp\n>> > index 3750e1bf52a9..7ecef2c669df 100644\n>> > --- a/src/android/mm/generic_frame_buffer_allocator.cpp\n>> > +++ b/src/android/mm/generic_frame_buffer_allocator.cpp\n>> > @@ -5,6 +5,7 @@\n>> > * generic_camera_buffer.cpp - Allocate FrameBuffer using gralloc API\n>> > */\n>> > \n>> > +#include \n>> > #include \n>> > #include \n>> > \n>> > @@ -72,9 +73,10 @@ class PlatformFrameBufferAllocator::Private : public Extensible::Private\n>> > public:\n>> > Private(CameraDevice *const cameraDevice)\n>> > : cameraDevice_(cameraDevice),\n>> > - hardwareModule_(cameraDevice->camera3Device()->common.module),\n>> > + hardwareModule_(nullptr),\n>> > allocDevice_(nullptr)\n>> > {\n>> > + hw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hardwareModule_);\n>\n> As discussed with Laurent directly, I think keeping compilation\n> supported for non-aosp builds is helpful to continue maintaining the\n> Android build with compile tests.\n\nThank you for the quick review. I agree that breaking the meson build\nfor -Dandroid=enabled -Dandroid_platform=generic is preferable.\n\n>\n> But we can do this easily by having a compilation unit\n> (libhardware-stubs.cpp perhaps) that gets compiled when libhardware is\n> not available, to provide linkage to a stubbed hw_get_module().\n\nI'm not super fluent with meson, but I see that Laurent already\nimplemented your suggestion here:\n\nhttps://lists.libcamera.org/pipermail/libcamera-devel/2023-May/037975.html\n\n>\n> I don't mind if that's on top as long as all the builds don't stay\n> broken for long! (There will still be regular builds for ChromeOS\n> though so it wouldn't get forgotten, so I'm not too worried).\n\nThat's understandable. I hope that both patches can be merged together.\n\n>\n> Reviewed-by: Kieran Bingham \n\nThanks again for the help to both of you.\n\nRegards\nMattijs\n\n>\n>> > ASSERT(hardwareModule_);\n>> > }\n>> > \n>> > @@ -85,7 +87,7 @@ public:\n>> > \n>> > private:\n>> > const CameraDevice *const cameraDevice_;\n>> > - struct hw_module_t *const hardwareModule_;\n>> > + const struct hw_module_t *hardwareModule_;\n>> > struct alloc_device_t *allocDevice_;\n>> > };\n>> > \n>> > @@ -93,6 +95,7 @@ PlatformFrameBufferAllocator::Private::~Private()\n>> > {\n>> > if (allocDevice_)\n>> > gralloc_close(allocDevice_);\n>> > + dlclose(hardwareModule_->dso);\n>> > }\n>> > \n>> > std::unique_ptr\n>> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n>> > index d40a3b0ba2eb..c92416bd3a6f 100644\n>> > --- a/src/android/mm/meson.build\n>> > +++ b/src/android/mm/meson.build\n>> > @@ -4,6 +4,7 @@ platform = get_option('android_platform')\n>> > if platform == 'generic'\n>> > android_hal_sources += files(['generic_camera_buffer.cpp',\n>> > 'generic_frame_buffer_allocator.cpp'])\n>> > + android_deps += [libdl, dependency('libhardware')]\n>> \n>> We usually split arrays with multiple entries on multiple lines:\n>> \n>> android_deps += [\n>> libdl,\n>> dependency('libhardware'),\n>> ]\n>> \n>> I'll do this locally.\n>> \n>> Reviewed-by: Laurent Pinchart \n>> \n>> > elif platform == 'cros'\n>> > android_hal_sources += files(['cros_camera_buffer.cpp',\n>> > 'cros_frame_buffer_allocator.cpp'])\n>> > \n>> > ---\n>> > base-commit: 76e1cb9f7176b4e7a935295d4e633ee24a0fef67\n>> > change-id: 20230227-android-gralloc-04e31d5edc68\n>> \n>> -- \n>> Regards,\n>> \n>> Laurent Pinchart","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 5AECDC31E9\n\tfor ;\n\tTue, 30 May 2023 15:26:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F805626FA;\n\tTue, 30 May 2023 17:26:26 +0200 (CEST)","from mail-ej1-x636.google.com (mail-ej1-x636.google.com\n\t[IPv6:2a00:1450:4864:20::636])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 449E560599\n\tfor ;\n\tTue, 30 May 2023 17:26:24 +0200 (CEST)","by mail-ej1-x636.google.com with SMTP id\n\ta640c23a62f3a-974265a1a40so138972966b.0\n\tfor ;\n\tTue, 30 May 2023 08:26:24 -0700 (PDT)","from localhost (abordeaux-655-1-129-86.w90-5.abo.wanadoo.fr.\n\t[90.5.10.86]) by smtp.gmail.com with ESMTPSA id\n\tk12-20020a170906128c00b00965a52d2bf6sm7374662ejb.88.2023.05.30.08.26.23\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 30 May 2023 08:26:23 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685460386;\n\tbh=og2GXAdkcA2p198Ay8bvV2fLiCHflxdOorNzXvS1X1A=;\n\th=To:In-Reply-To:References:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Qe12+Pd636DP4swIJU4+27SgSwN1EPjl0mOk/3MRdSlUEGGBGbGHr4QH46SrXigzw\n\to6bAxn0Dkv8NNa1wr9na0zTqJR4H2Pj6fSw9CQI/e1B4JM976JqcLjgIDvZSVc6xsx\n\tDEDHW3TqbasBo+Oyvwl6bZSuPOocRoOQqAnm6fIcoB3aCd5Z8Nn1UNskIpnmEwFqGP\n\tyV1nb6xCNYeSrHllEijxQfLmnCimTfnkHMgA5N5cIju8l7jig/rab5FFJBpvKI/sim\n\tnMBKrf2fEpF1w8c+QAluSIuarwvCJEfLw9964kOmSHhzIwXBmbXHVUOn/Sm4XoVmPT\n\tF6ynBRCR8krZg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20221208.gappssmtp.com; s=20221208; t=1685460384;\n\tx=1688052384; \n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:from:to:cc:subject:date:message-id:reply-to;\n\tbh=4eGHLv4yTgP6dvqZ/HD62JT1PXV1OtMpjl0X7gPzXWQ=;\n\tb=PZasGEGpzHo7+0I/iMoW+CI5ZoWLuAwlmRRX0X9FaJ3kAkPnKyy+hLo912vuT9TBdX\n\tzQvC8Z5ou0MmlzEDqqqsSJfhqj9LecNhLW+Fq2cnqtIH1IjoU1lLmcal6RV+3GAW57Ib\n\toCxcQmJFhp9+rPJA/QcLBlVv2rMPy3jH5IhdBqxGUSF74Z68bEE7HNKkj/IfHE1DR7sX\n\tAHaOA16+xkD1XAl+8rxPlDykiCK9fC2w7ADhBMk/8hNX955hwDbopEmq2mlP00GB0Hds\n\tHq0cpgkaeaJ3TqEjmxtWjUj8jsyacfJO8X3o49oUF6tzqA9e9HBpTMFLhwvzUFIKjPSu\n\t504g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=baylibre-com.20221208.gappssmtp.com\n\theader.i=@baylibre-com.20221208.gappssmtp.com header.b=\"PZasGEGp\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685460384; x=1688052384;\n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; \n\tbh=4eGHLv4yTgP6dvqZ/HD62JT1PXV1OtMpjl0X7gPzXWQ=;\n\tb=bOCFRVucrfYCnHY6KK2WA6PShpXSTY7AxYbIho+esXC3K35dyUxif4vKemhFpHZzWL\n\tEQnWLoTmT7GtUKhqDeEXqJgbHPGd1V9YhwXUL6SxNjRpo1S79RZuNclB1PGo2Q2uHlGG\n\t0N40LM4VMT8gcdwDtMniQ44s2wpHnNbrmMhu5ViGFAE9jVmKkuFF4LaelDTWdoqhvyuW\n\tHWs2oIAYIt59mwJZ6X2CujhrY0O/H2FstEdn0ygVuAVp+o3PBQX/7hnapUJrqwGXEDbD\n\t4gLj0DJP8BG9NhYDPp7kggwh71SysU9gu7MKSQc5Uh1Ed2n0shAYY2KWOpOLOzZrn3BU\n\tGcXg==","X-Gm-Message-State":"AC+VfDxTZpda3W9lwbgMP9EGhxE7vlhfkNQC3ANxYuphwfplpGwZ2ZuZ\n\tRLpwYeFeiKukDuGnEZe9orklblsxVIyApE+UGq4=","X-Google-Smtp-Source":"ACHHUZ4kaoSHHiz8r+DCKyo6CcpXsS7gSvQqjVcUZ6Utu2fywmm4sfe1O7fTVyGC87Z4tyOwLGdWWw==","X-Received":"by 2002:a17:907:6ea8:b0:973:9443:c5fc with SMTP id\n\tsh40-20020a1709076ea800b009739443c5fcmr11451356ejc.14.1685460383702; \n\tTue, 30 May 2023 08:26:23 -0700 (PDT)","To":"Kieran Bingham , Laurent Pinchart\n\t","In-Reply-To":"<168544909877.130277.18288798045979326882@Monstersaurus>","References":"<20230227-android-gralloc-v4-1-8cd3a44c5b70@baylibre.com>\n\t<20230529155818.GD15264@pendragon.ideasonboard.com>\n\t<168544909877.130277.18288798045979326882@Monstersaurus>","Date":"Tue, 30 May 2023 17:26:22 +0200","Message-ID":"<871qixkfhd.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH v4] android: mm: generic: use\n\tGRALLOC_HARDWARE_MODULE_ID","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","From":"Mattijs Korpershoek via libcamera-devel\n\t","Reply-To":"Mattijs Korpershoek ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]