[{"id":27151,"web_url":"https://patchwork.libcamera.org/comment/27151/","msgid":"<20230529094255.GM25984@pendragon.ideasonboard.com>","date":"2023-05-29T09:42:55","subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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, and sorry for not reviewing the last version.\n\nOn Sun, May 28, 2023 at 06:07:00PM +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 add a new dependency on android's libhardware [1]\n\ns/add/adds/\n\nI'll fix this locally.\n\nReviewed-by: Laurent Pinchart \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> 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..42eeab3bcfa9 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 += [dependency('libhardware')]\n> elif platform == 'cros'\n> android_hal_sources += files(['cros_camera_buffer.cpp',\n> 'cros_frame_buffer_allocator.cpp'])\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 1AC2FC3213\n\tfor ;\n\tMon, 29 May 2023 09:42:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 871C7626FA;\n\tMon, 29 May 2023 11:42:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29FBF60599\n\tfor ;\n\tMon, 29 May 2023 11:42:54 +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 31F60836;\n\tMon, 29 May 2023 11:42:32 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685353375;\n\tbh=mNEim3IR2vDAkF2Evx7GzxKujf8VlExUYLZpEMLtfLc=;\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=bA/2btuKdCDEBwf6T3GvqAven6q5rvGXu4MyLR9Jj8Fgp31en8+UY49hdtxfYWQ0x\n\tRu74Pd3ZnBpQnYjFCqe1knjt82S3WSdyBR8moyj/5eRS1aO8MoQCtW84mg5zvHF1a8\n\tCqNesKkg3IM/xrLyPBkTDDvheU/DE7RqfZbwUDH0aVhBUiYgJJZvH119jQSkoB4T/J\n\t7yYqi968khudx8OrxgxZmn0NZ2ivPCzYq/zkdAQ1Sj54vGh1Su/XzomAU+cpe3gsu0\n\t86DPGicQMi2xqOk0nm4UdMWfo8ZUuldoiIIUrxpo/ImPVM4usxNT1N/LUeFJsJ8tj2\n\tD7JBGJNcndPeg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685353354;\n\tbh=mNEim3IR2vDAkF2Evx7GzxKujf8VlExUYLZpEMLtfLc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=epO1CZjONznT47iGziBfJSg/3WD+pDDIzrDVD+DkT1YTCXwf0Hm4ZN9RenZw/zmLl\n\tX8X2HWjdprGNBYN0KZR1nQoi9X0d4ymrlG792ziQr5fTRaA2Rwlt6UaxPOZjmyLuWW\n\tDTrOAnx4WoEPFmsPuMpJtq1fgAyDS1CWULsdI9wY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"epO1CZjO\"; dkim-atps=neutral","Date":"Mon, 29 May 2023 12:42:55 +0300","To":"Mattijs Korpershoek ","Message-ID":"<20230529094255.GM25984@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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":27154,"web_url":"https://patchwork.libcamera.org/comment/27154/","msgid":"<20230529100935.GQ25984@pendragon.ideasonboard.com>","date":"2023-05-29T10:09:35","subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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\nI spoke a bit too fast.\n\nOn Mon, May 29, 2023 at 12:42:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Hi Mattijs,\n> \n> Thank you for the patch, and sorry for not reviewing the last version.\n> \n> On Sun, May 28, 2023 at 06:07:00PM +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 add a new dependency on android's libhardware [1]\n> \n> s/add/adds/\n> \n> I'll fix this locally.\n> \n> Reviewed-by: Laurent Pinchart \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> > 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\nThis requires linking with libdl. src/libcamera/meson.build creates a\nlibdl variable, which I think you should add to the android_deps below.\nCould you check if that compiles fine ?\n\n> > }\n> > \n> > std::unique_ptr\n> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> > index d40a3b0ba2eb..42eeab3bcfa9 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 += [dependency('libhardware')]\n> > elif platform == 'cros'\n> > android_hal_sources += files(['cros_camera_buffer.cpp',\n> > 'cros_frame_buffer_allocator.cpp'])\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 9A6DCC31E9\n\tfor ;\n\tMon, 29 May 2023 10:09:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 07302626FA;\n\tMon, 29 May 2023 12:09:35 +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 01552626F5\n\tfor ;\n\tMon, 29 May 2023 12:09:33 +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 EDFEA6D5;\n\tMon, 29 May 2023 12:09:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685354975;\n\tbh=RhogNbSGKDGHBChrPJtYkxVRt0pP1rvL+TklbKoiQ0I=;\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:\n\tFrom;\n\tb=XrXruW7P2BthorupHxkrHz9L1mSyBXLNlsK2KlpWKAMmp1GPcFmWEY8E5WklrArE7\n\trVR+hUFOVmSFIZQ0e5DiQCGc6fPuR0/UY/lQ2CqWC4fOlBEwNPVOu6nwyYw6B2N0M/\n\tPtLVFjfkluIJKlbcfQmlt6QUYtrGsTdTQ7FAUTsHCbrflSZbVDv2g3AMnfKBiqRkwt\n\tGfwjUnAKuZ+tFRQbwUpTnKK9RM5BnWLK1yj33OILg/ufk202/W1yFXGznClNoFATKu\n\t/OeHwQ7iJlvQ2Wp+qzvkNsdQF85nhH/HswyMp6f3TxulrFHw6G1tS3FaAg4PEd95IU\n\tJDy38D6sadkpQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685354954;\n\tbh=RhogNbSGKDGHBChrPJtYkxVRt0pP1rvL+TklbKoiQ0I=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=mvwgboKOJGKUwBMzqshbhhtRSZhxjCgd5QScoLXrgh21LlLSkOKuHKkVIu4RAeGgc\n\tvsSDbHc0vPLW1LJY428rQWUk/z/BWymcjvnDkx36sdagKPhmo5J8yLfJeZyIvnjlyd\n\tqnlphDk0N3fmBizUp20cq/eHStd9kSv8o06SrOvI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"mvwgboKO\"; dkim-atps=neutral","Date":"Mon, 29 May 2023 13:09:35 +0300","To":"Mattijs Korpershoek ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20230529100935.GQ25984@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>\n\t<20230529094255.GM25984@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230529094255.GM25984@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27157,"web_url":"https://patchwork.libcamera.org/comment/27157/","msgid":"<87jzwrjp50.fsf@baylibre.com>","date":"2023-05-29T12:30:51","subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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 Laurent,\n\n\nOn lun., mai 29, 2023 at 13:09, Laurent Pinchart wrote:\n\n> Hi Mattijs,\n>\n> I spoke a bit too fast.\n>\n> On Mon, May 29, 2023 at 12:42:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n>> Hi Mattijs,\n>> \n>> Thank you for the patch, and sorry for not reviewing the last version.\n\nThank you for the review. No worries, patches can split through some time.\n\n>> \n>> On Sun, May 28, 2023 at 06:07:00PM +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 add a new dependency on android's libhardware [1]\n>> \n>> s/add/adds/\n>> \n>> I'll fix this locally.\n\nWill fix in v4, since I have to respin this anyways for libdl.\n\n>> \n>> Reviewed-by: Laurent Pinchart \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>> > 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> This requires linking with libdl. src/libcamera/meson.build creates a\n> libdl variable, which I think you should add to the android_deps below.\n> Could you check if that compiles fine ?\n\nYes, I can check. I suspect it won't link in any case due to missing libhardware\nbecause we don't have access to libhardware when building on linux:\n\nsrc/android/mm/meson.build:7:4: ERROR: Dependency \"libhardware\" not found, tried pkgconfig and cmake\nlibcamera/build/../src/android/mm/generic_frame_buffer_allocator.cpp:79: undefined reference to `hw_get_module'\n\nPer my understanding from\nhttps://patchwork.libcamera.org/patch/18309/#26503, it was acceptable\nthis way.\n\nWhen commenting out hw_get_module() and the libhardware dependency in\nbuild.meson, it already compiled fine.\n\nI will add libdl to the dependency list in v4.\n\nThanks,\nMattijs\n\n>\n>> > }\n>> > \n>> > std::unique_ptr\n>> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n>> > index d40a3b0ba2eb..42eeab3bcfa9 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 += [dependency('libhardware')]\n>> > elif platform == 'cros'\n>> > android_hal_sources += files(['cros_camera_buffer.cpp',\n>> > 'cros_frame_buffer_allocator.cpp'])\n>> > \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 B8AA8C31E9\n\tfor ;\n\tMon, 29 May 2023 12:30:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 22146626F9;\n\tMon, 29 May 2023 14:30:55 +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 ACBE66038E\n\tfor ;\n\tMon, 29 May 2023 14:30:53 +0200 (CEST)","by mail-ej1-x636.google.com with SMTP id\n\ta640c23a62f3a-96ff9c0a103so468455366b.0\n\tfor ;\n\tMon, 29 May 2023 05:30:53 -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\tdv22-20020a170906b81600b0096f78953b77sm5886646ejb.147.2023.05.29.05.30.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 29 May 2023 05:30:52 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685363455;\n\tbh=YhBAMrWo7lB2gHIScwzWQfccBHyur7zj4hycUNuCD50=;\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:\n\tFrom;\n\tb=IRzDTd3aM+3P3WlaML2q6PZzNfltoNQQn257vEWlfbe0YDL4rH5GCo/K7sGwhN8NK\n\tAxORWM/Jy3JEKXj2+d2pTpYQBCO/ZTOiuqR/0Yu8FWpW/Joi2c+VWyyelY1ZZ8k4rR\n\twguExRXKX0bJuzygHDkTuGBWxW3WsWTbUoiRp5yW3cR5fbEnGBqt9dQqNASNbnFIff\n\t9a2Dam+SxquVx0M6vc+JV2JkTzuSnlnX5zxrD3sH7V1uFFzcZwEdmUShIuEJ8RLLGH\n\tgZeI6opezZtUuRZu7PgXzKqo5rbQdtIekNvI5LE2bmWShvBmkeKLVfJrqWZ3wGdVKk\n\t2OSzwWaA5AjdA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20221208.gappssmtp.com; s=20221208; t=1685363453;\n\tx=1687955453; \n\th=mime-version:message-id:date:references:in-reply-to:subject:to:from\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=YiizVmqof3tD8l+pQSqnNnjWzxuVgU8NPilMWTyDJAQ=;\n\tb=qrD39WLWEOcOpkgduWkr13k2ME70UDHDab0YJBd+6uXeJo3huMy8gB6Jk1SPQ9gISe\n\t/mwzVfIGGBhTUuOFeFOElnmqUKa+n9HynAP4BUyGUzQkJtGWjre96RTb7HEPBFKKiQKq\n\tnxSMm3i31w0MdR6tD+icimuUgXloxHh8v3fvxGELA8zVfSa+WxwvETAEElOpccYhwvOr\n\tLX5wEg42r19/X8KAFiiJVzFc2+16y4DTUcNtls6+cL9gwH2kWV2msY4qaj3f7qws/2ui\n\tDty3e3fInkO1C+qFyhTBmu4wNMRzl93GfzVODDAv9gHTrIdvDJ1guLZrSFmbG8j8c8Y0\n\t0bsg=="],"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=\"qrD39WLW\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685363453; x=1687955453;\n\th=mime-version:message-id:date:references:in-reply-to:subject:to:from\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=YiizVmqof3tD8l+pQSqnNnjWzxuVgU8NPilMWTyDJAQ=;\n\tb=h3pkk2T5xDo5eBYGj2vB6bB981IrWr1okiMD2RPehvzQcgvH/UxFituT1KTBo1UEiu\n\tbyHoh14rxE6KY6YXtqWIQRJrQDxalNvmwEqqLGvxUAmo6YoE4DcDXmY/LDlMt7nHMZvj\n\tnEkdk8ZBVMtgj4Ny80MDYkIpzMIbI9wfSiRdbfzVi5A4uxEiB326+iesDTAHPnywjTub\n\t8Sx3ZiI0NJ4YWTi3vcjNOKnsBB7sL3QAjRQBZcMOY25xgwOnghitvEcsa9RHKTYu3jYr\n\tviL6N+N2muscqTpLqfR1eCPdB+M6sFtu0leHqf0zfzouUYv1HgfrZcqWtl5JmY0xbavx\n\t4Yzw==","X-Gm-Message-State":"AC+VfDxZYZxLJe4lEjfWAzi79DehUNT3qQX08T89N2xPV7oHKPabSu3n\n\tQ2RkQc2AEuBd/m/mt6pAPe+nUQ==","X-Google-Smtp-Source":"ACHHUZ5bX36kgfWBqHEowgVmHI4ystiPxVU/ghOLKyJHNFuwX4JQnUxHir/PVG3at3aZPj8R7TIZCg==","X-Received":"by 2002:a17:907:6d17:b0:8b8:c06e:52d8 with SMTP id\n\tsa23-20020a1709076d1700b008b8c06e52d8mr10447234ejc.36.1685363453101; \n\tMon, 29 May 2023 05:30:53 -0700 (PDT)","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","In-Reply-To":"<20230529100935.GQ25984@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>\n\t<20230529094255.GM25984@pendragon.ideasonboard.com>\n\t<20230529100935.GQ25984@pendragon.ideasonboard.com>","Date":"Mon, 29 May 2023 14:30:51 +0200","Message-ID":"<87jzwrjp50.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27158,"web_url":"https://patchwork.libcamera.org/comment/27158/","msgid":"<20230529123341.GA15292@pendragon.ideasonboard.com>","date":"2023-05-29T12:33:41","subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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\nOn Mon, May 29, 2023 at 02:30:51PM +0200, Mattijs Korpershoek wrote:\n> On lun., mai 29, 2023 at 13:09, Laurent Pinchart wrote:\n> > On Mon, May 29, 2023 at 12:42:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> >> Hi Mattijs,\n> >> \n> >> Thank you for the patch, and sorry for not reviewing the last version.\n> \n> Thank you for the review. No worries, patches can split through some time.\n> \n> >> \n> >> On Sun, May 28, 2023 at 06:07:00PM +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 add a new dependency on android's libhardware [1]\n> >> \n> >> s/add/adds/\n> >> \n> >> I'll fix this locally.\n> \n> Will fix in v4, since I have to respin this anyways for libdl.\n> \n> >> Reviewed-by: Laurent Pinchart \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> >> > 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> > This requires linking with libdl. src/libcamera/meson.build creates a\n> > libdl variable, which I think you should add to the android_deps below.\n> > Could you check if that compiles fine ?\n> \n> Yes, I can check. I suspect it won't link in any case due to missing libhardware\n> because we don't have access to libhardware when building on linux:\n> \n> src/android/mm/meson.build:7:4: ERROR: Dependency \"libhardware\" not found, tried pkgconfig and cmake\n> libcamera/build/../src/android/mm/generic_frame_buffer_allocator.cpp:79: undefined reference to `hw_get_module'\n> \n> Per my understanding from\n> https://patchwork.libcamera.org/patch/18309/#26503, it was acceptable\n> this way.\n\nYes, that's fine. What I would like you to do is test linking against\nlibdl in a real Android build, either with AOSP or the Android NDK :-)\n\n> When commenting out hw_get_module() and the libhardware dependency in\n> build.meson, it already compiled fine.\n> \n> I will add libdl to the dependency list in v4.\n> \n> >> > }\n> >> > \n> >> > std::unique_ptr\n> >> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> >> > index d40a3b0ba2eb..42eeab3bcfa9 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 += [dependency('libhardware')]\n> >> > elif platform == 'cros'\n> >> > android_hal_sources += files(['cros_camera_buffer.cpp',\n> >> > 'cros_frame_buffer_allocator.cpp'])\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 4F050C3213\n\tfor ;\n\tMon, 29 May 2023 12:33:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB81C626F5;\n\tMon, 29 May 2023 14:33:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 900706038E\n\tfor ;\n\tMon, 29 May 2023 14:33:40 +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 A23036D5;\n\tMon, 29 May 2023 14:33:19 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685363621;\n\tbh=VRL3+xCnCoSKstCdBox1JRtwjyjU5aMNakpLwNO9zVc=;\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=WNJwSddB6w7qCoz/0PqpcXrHeYb2T5XZgRx1wfwrO2r9YBqG2qHz5wwyaVdNCHBIt\n\tlbz1z4jSamIac9z4lLOif5+8u7QagybmaUSAEJE2EDHbrKmvkvqhqu5/EJXswuUezP\n\t7N1d7dpxk1tL22hvyyoCveGjStJrK8YOVZZ0orbKmjyOsfld8Obvw5Tx70oh6PEsCU\n\tpahtCjS7RvoCiKg6gBEWyrC/J6+bEu1YhmwvgoGvmc7CzFk/v9670aZSppOSuSucl8\n\tkLYxGucYPMMU+EngUBcxRi8AiZ4h6Ntan5/FjCR0lXBlsPZyO25ZWSyRr7cdFOzSiF\n\tQdxZulo2dDDFQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685363600;\n\tbh=VRL3+xCnCoSKstCdBox1JRtwjyjU5aMNakpLwNO9zVc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fGya0rE5Uk7CBZ67HAAH2q4Dh0I6cUMAiEHETYNSWmhq5sw6LQa8atqSK0w0sLD7s\n\t1ffzvX0KPjp7qAti9txe7IYMu/79geECG3hmvz9I2KV8dSTs/4TlIwfqOY4V7oDztk\n\tg43O/mRpBeBs2ij1boIaRowFO+KJK+cJpb4rQH9g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"fGya0rE5\"; dkim-atps=neutral","Date":"Mon, 29 May 2023 15:33:41 +0300","To":"Mattijs Korpershoek ","Message-ID":"<20230529123341.GA15292@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>\n\t<20230529094255.GM25984@pendragon.ideasonboard.com>\n\t<20230529100935.GQ25984@pendragon.ideasonboard.com>\n\t<87jzwrjp50.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87jzwrjp50.fsf@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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":27161,"web_url":"https://patchwork.libcamera.org/comment/27161/","msgid":"<87h6rvjmln.fsf@baylibre.com>","date":"2023-05-29T13:25:40","subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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 Laurent,\n\nOn lun., mai 29, 2023 at 15:33, Laurent Pinchart wrote:\n\n> Hi Mattijs,\n>\n> On Mon, May 29, 2023 at 02:30:51PM +0200, Mattijs Korpershoek wrote:\n>> On lun., mai 29, 2023 at 13:09, Laurent Pinchart wrote:\n>> > On Mon, May 29, 2023 at 12:42:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n>> >> Hi Mattijs,\n>> >> \n>> >> Thank you for the patch, and sorry for not reviewing the last version.\n>> \n>> Thank you for the review. No worries, patches can split through some time.\n>> \n>> >> \n>> >> On Sun, May 28, 2023 at 06:07:00PM +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 add a new dependency on android's libhardware [1]\n>> >> \n>> >> s/add/adds/\n>> >> \n>> >> I'll fix this locally.\n>> \n>> Will fix in v4, since I have to respin this anyways for libdl.\n>> \n>> >> Reviewed-by: Laurent Pinchart \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>> >> > 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>> > This requires linking with libdl. src/libcamera/meson.build creates a\n>> > libdl variable, which I think you should add to the android_deps below.\n>> > Could you check if that compiles fine ?\n>> \n>> Yes, I can check. I suspect it won't link in any case due to missing libhardware\n>> because we don't have access to libhardware when building on linux:\n>> \n>> src/android/mm/meson.build:7:4: ERROR: Dependency \"libhardware\" not found, tried pkgconfig and cmake\n>> libcamera/build/../src/android/mm/generic_frame_buffer_allocator.cpp:79: undefined reference to `hw_get_module'\n>> \n>> Per my understanding from\n>> https://patchwork.libcamera.org/patch/18309/#26503, it was acceptable\n>> this way.\n>\n> Yes, that's fine. What I would like you to do is test linking against\n> libdl in a real Android build, either with AOSP or the Android NDK :-)\n\nI confirm I have build and link-tested this using AOSP (with\nhand-written Android.bp files).\n\nI have also functionally tested this: we can open the gralloc hw module.\n\nWill send a v4 shortly.\n\n>\n>> When commenting out hw_get_module() and the libhardware dependency in\n>> build.meson, it already compiled fine.\n>> \n>> I will add libdl to the dependency list in v4.\n>> \n>> >> > }\n>> >> > \n>> >> > std::unique_ptr\n>> >> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n>> >> > index d40a3b0ba2eb..42eeab3bcfa9 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 += [dependency('libhardware')]\n>> >> > elif platform == 'cros'\n>> >> > android_hal_sources += files(['cros_camera_buffer.cpp',\n>> >> > 'cros_frame_buffer_allocator.cpp'])\n>> >> > \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 0F457C3213\n\tfor ;\n\tMon, 29 May 2023 13:25:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DD16626F9;\n\tMon, 29 May 2023 15:25:43 +0200 (CEST)","from mail-ej1-x629.google.com (mail-ej1-x629.google.com\n\t[IPv6:2a00:1450:4864:20::629])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 55B766038E\n\tfor ;\n\tMon, 29 May 2023 15:25:42 +0200 (CEST)","by mail-ej1-x629.google.com with SMTP id\n\ta640c23a62f3a-96fffe11714so613284766b.0\n\tfor ;\n\tMon, 29 May 2023 06:25:42 -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\tqp28-20020a170907207c00b0096f5b48fe43sm5867366ejb.47.2023.05.29.06.25.41\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 29 May 2023 06:25:41 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685366743;\n\tbh=dinf78GHzhnB+EvTBfoKDoB8fOSkRSQhv3G+IG7YBYo=;\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=wyQgHPi3efACCfx0QLGEnEmP5Crwx4uE53nbBhOPfU0Vi+ZEBsorfyttOZkOQHuw0\n\t8O1VQP/T9OBMtM6RK10FODdgZX2TF/RBc9A+X3fGHFc9Qp6aFYqdxLXTazzzsdRHjj\n\tiTxtwSuNGJ0yi0u43HYzLTqtTtOVgfc4/XcpIO8Mb9vpIQblQLIehQ2KYR36M9vanw\n\ttUvrZQ4UpnBSp8VVboSjmlP+Xesur3B+M4G3vaD3CEaYoS654+tC9JrnPDzZi3kgYS\n\tun5k7pSXXX621Mg2NhPj1TR8YhFB6sDQFm4pVQ2BI7AUWA2J0qkibowt6cpqW6rIoF\n\tU4pdrAR/PCH3Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20221208.gappssmtp.com; s=20221208; t=1685366742;\n\tx=1687958742; \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=wXIyO2GCDckjPKhcgTtbX86SU3ilcMJ1o3fdPQh8YA4=;\n\tb=v5WARqeuqy96I9WDillr8Mo62chKtWC5TZOtp21KKBcd3j5ektpAfS0YrKFcLhxSvz\n\teomTswIuEhudA1zlGBZBPOWqG0nYVL48shel2ergXvSbIS+hfkrdLcZhPQnqj71eKvKS\n\th4G9GqQKSOFmpG5+NjaksuFY1mae09F/jEO7UWAArJUdnLZcDH6crLWTmLNCisTbIZw6\n\tm1+NCFa2f1GwoieKY3igQGG7+0YioarYqtK+3PVSR9dfntLUwt+POrMFeGpATf9zWoQ8\n\t+jsDByWgLpr+QflF2vM0RxKCLy3H9JR+N9v8mwPGZjG2Or6LBVWqN6aS4xGa0osLfqh/\n\t5biA=="],"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=\"v5WARqeu\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685366742; x=1687958742;\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=wXIyO2GCDckjPKhcgTtbX86SU3ilcMJ1o3fdPQh8YA4=;\n\tb=A1/fx1g1lUrlD+cK+PmaPcS1RhNzY9H/gjbyMN+lNl0c7BrxLcE9LU+hCFhSSPv6Lh\n\ttVYNKDw4AhYP5dwqukv4ciwQ8oyOtzl7pG4nphfFHSNKQrDjpNC7rc5kd+YWEP2CF2LA\n\ttFGw+jCaievDk7tLs5i9zZQScgcYCBMp+iYULYT6ZfoRZa9Zzsgf8NHCvNP1QTXRZRWC\n\tZzAaPwM0XiDdARWjVH6cS6KZTQGI28Rb6l+7iJsZgaEyKgtxwvHHEI/EzxDNoJNnyxU/\n\tYPnnbVMcifv7xT5e5Uo0PytoSPjkwiL10N04kDNeDAwAmKm04GjouF0Fe2c4rAM9K72z\n\tlw+w==","X-Gm-Message-State":"AC+VfDzVSh6ssT/wHnFyONRKnql8p2iB3pSC6OA4En/bvjAZyTMA0/U8\n\thMbNiBzNjalA6uuXuLLFFy+KjA==","X-Google-Smtp-Source":"ACHHUZ7950V6sphU23wS28AmZA3Xq1ejfX2cyE5wRUXE36u6NKyPbIkA7rYi1bZgTZ8akdl1pSL0Lw==","X-Received":"by 2002:a17:907:60ca:b0:96a:b12d:2fdf with SMTP id\n\thv10-20020a17090760ca00b0096ab12d2fdfmr15275138ejc.12.1685366741785; \n\tMon, 29 May 2023 06:25:41 -0700 (PDT)","To":"Laurent Pinchart ","In-Reply-To":"<20230529123341.GA15292@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>\n\t<20230529094255.GM25984@pendragon.ideasonboard.com>\n\t<20230529100935.GQ25984@pendragon.ideasonboard.com>\n\t<87jzwrjp50.fsf@baylibre.com>\n\t<20230529123341.GA15292@pendragon.ideasonboard.com>","Date":"Mon, 29 May 2023 15:25:40 +0200","Message-ID":"<87h6rvjmln.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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\" "}},{"id":27162,"web_url":"https://patchwork.libcamera.org/comment/27162/","msgid":"<20230529132911.GA22067@pendragon.ideasonboard.com>","date":"2023-05-29T13:29:11","subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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\nOn Mon, May 29, 2023 at 03:25:40PM +0200, Mattijs Korpershoek wrote:\n> On lun., mai 29, 2023 at 15:33, Laurent Pinchart wrote:\n> > On Mon, May 29, 2023 at 02:30:51PM +0200, Mattijs Korpershoek wrote:\n> >> On lun., mai 29, 2023 at 13:09, Laurent Pinchart wrote:\n> >> > On Mon, May 29, 2023 at 12:42:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> >> >> Hi Mattijs,\n> >> >> \n> >> >> Thank you for the patch, and sorry for not reviewing the last version.\n> >> \n> >> Thank you for the review. No worries, patches can split through some time.\n> >> \n> >> >> On Sun, May 28, 2023 at 06:07:00PM +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 add a new dependency on android's libhardware [1]\n> >> >> \n> >> >> s/add/adds/\n> >> >> \n> >> >> I'll fix this locally.\n> >> \n> >> Will fix in v4, since I have to respin this anyways for libdl.\n> >> \n> >> >> Reviewed-by: Laurent Pinchart \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> >> >> > 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> >> > This requires linking with libdl. src/libcamera/meson.build creates a\n> >> > libdl variable, which I think you should add to the android_deps below.\n> >> > Could you check if that compiles fine ?\n> >> \n> >> Yes, I can check. I suspect it won't link in any case due to missing libhardware\n> >> because we don't have access to libhardware when building on linux:\n> >> \n> >> src/android/mm/meson.build:7:4: ERROR: Dependency \"libhardware\" not found, tried pkgconfig and cmake\n> >> libcamera/build/../src/android/mm/generic_frame_buffer_allocator.cpp:79: undefined reference to `hw_get_module'\n> >> \n> >> Per my understanding from\n> >> https://patchwork.libcamera.org/patch/18309/#26503, it was acceptable\n> >> this way.\n> >\n> > Yes, that's fine. What I would like you to do is test linking against\n> > libdl in a real Android build, either with AOSP or the Android NDK :-)\n> \n> I confirm I have build and link-tested this using AOSP (with\n> hand-written Android.bp files).\n\nJust to make sure, did that test include linking against libdl ?\n\n> I have also functionally tested this: we can open the gralloc hw module.\n> \n> Will send a v4 shortly.\n\nThank you.\n\n> >> When commenting out hw_get_module() and the libhardware dependency in\n> >> build.meson, it already compiled fine.\n> >> \n> >> I will add libdl to the dependency list in v4.\n> >> \n> >> >> > }\n> >> >> > \n> >> >> > std::unique_ptr\n> >> >> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> >> >> > index d40a3b0ba2eb..42eeab3bcfa9 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 += [dependency('libhardware')]\n> >> >> > elif platform == 'cros'\n> >> >> > android_hal_sources += files(['cros_camera_buffer.cpp',\n> >> >> > 'cros_frame_buffer_allocator.cpp'])\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 EE769C31E9\n\tfor ;\n\tMon, 29 May 2023 13:29:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7BA796038E;\n\tMon, 29 May 2023 15:29:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BC3F06038E\n\tfor ;\n\tMon, 29 May 2023 15:29:10 +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 B2CF3836;\n\tMon, 29 May 2023 15:28:49 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685366952;\n\tbh=2JaiI03LA1KPHfAD39vCeLNr/wU2b367ilYZ/ALBcn8=;\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=pH8XcbfZ1jd8g/NQ2aRC4IMp0UtWURrDi72Avh2qnB9DW97KhNg2tw3l/xgzEprf5\n\tzwtkz4dbleSwtkJ5HbELAUgKc0RwLIa+RxDqP6Wy+sOHAW8vr6Xbu8qJo9mbiJ49sS\n\tJwZF4Hfqrr/SN1VYs0B3bYxOW3pGQv2W1Nfq5kzkgh9l0OfOQ2l46pAz640sq7PWKL\n\tAiLcnu8BGEnE4mXcAw73M4c4FcXfvhq3Nz0fxzFWwq6f61nvIZwpBoDpAMLW2Jgw6m\n\thmwu6U/KcQrtEHZlA5DLLcyBNHas2fquwQftmv7pLTdTyrFQLQnID/b7eTsO9qigPW\n\t04NpN8hGphAxA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685366930;\n\tbh=2JaiI03LA1KPHfAD39vCeLNr/wU2b367ilYZ/ALBcn8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KG0pYEi9J0TA+x1c77ulFavibXSblRuZUXGqX3rowEgLnAkH5K0vH7xLIRA67Z7jw\n\tpPhmv6ikn5vfkRwe6nBgEIyoUsguN90e2FIDwF2UkYqtdNr5i+e+hcXxngkkRgVVkS\n\ty+ZWwulDRHFuT7x7c8eAfjx5VFWQ+AhvJE9wCSKU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"KG0pYEi9\"; dkim-atps=neutral","Date":"Mon, 29 May 2023 16:29:11 +0300","To":"Mattijs Korpershoek ","Message-ID":"<20230529132911.GA22067@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>\n\t<20230529094255.GM25984@pendragon.ideasonboard.com>\n\t<20230529100935.GQ25984@pendragon.ideasonboard.com>\n\t<87jzwrjp50.fsf@baylibre.com>\n\t<20230529123341.GA15292@pendragon.ideasonboard.com>\n\t<87h6rvjmln.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87h6rvjmln.fsf@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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":27163,"web_url":"https://patchwork.libcamera.org/comment/27163/","msgid":"<87edmzjm62.fsf@baylibre.com>","date":"2023-05-29T13:35:01","subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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":"On lun., mai 29, 2023 at 16:29, Laurent Pinchart wrote:\n\n> Hi Mattijs,\n>\n> On Mon, May 29, 2023 at 03:25:40PM +0200, Mattijs Korpershoek wrote:\n>> On lun., mai 29, 2023 at 15:33, Laurent Pinchart wrote:\n>> > On Mon, May 29, 2023 at 02:30:51PM +0200, Mattijs Korpershoek wrote:\n>> >> On lun., mai 29, 2023 at 13:09, Laurent Pinchart wrote:\n>> >> > On Mon, May 29, 2023 at 12:42:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n>> >> >> Hi Mattijs,\n>> >> >> \n>> >> >> Thank you for the patch, and sorry for not reviewing the last version.\n>> >> \n>> >> Thank you for the review. No worries, patches can split through some time.\n>> >> \n>> >> >> On Sun, May 28, 2023 at 06:07:00PM +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 add a new dependency on android's libhardware [1]\n>> >> >> \n>> >> >> s/add/adds/\n>> >> >> \n>> >> >> I'll fix this locally.\n>> >> \n>> >> Will fix in v4, since I have to respin this anyways for libdl.\n>> >> \n>> >> >> Reviewed-by: Laurent Pinchart \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>> >> >> > 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>> >> > This requires linking with libdl. src/libcamera/meson.build creates a\n>> >> > libdl variable, which I think you should add to the android_deps below.\n>> >> > Could you check if that compiles fine ?\n>> >> \n>> >> Yes, I can check. I suspect it won't link in any case due to missing libhardware\n>> >> because we don't have access to libhardware when building on linux:\n>> >> \n>> >> src/android/mm/meson.build:7:4: ERROR: Dependency \"libhardware\" not found, tried pkgconfig and cmake\n>> >> libcamera/build/../src/android/mm/generic_frame_buffer_allocator.cpp:79: undefined reference to `hw_get_module'\n>> >> \n>> >> Per my understanding from\n>> >> https://patchwork.libcamera.org/patch/18309/#26503, it was acceptable\n>> >> this way.\n>> >\n>> > Yes, that's fine. What I would like you to do is test linking against\n>> > libdl in a real Android build, either with AOSP or the Android NDK :-)\n>> \n>> I confirm I have build and link-tested this using AOSP (with\n>> hand-written Android.bp files).\n>\n> Just to make sure, did that test include linking against libdl ?\n\nYes. Sorry that was not clear.\nHere is the associated Android.bp file which contains \"libdl\" in the\nshared_libs section:\n\nhttps://gitlab.baylibre.com/baylibre/ti/android/aosp/external/libcamera/-/blob/ti-android-12/src/android/Android.bp#L42\n\nAndroid has libdl implemented in bionic.\n\n>\n>> I have also functionally tested this: we can open the gralloc hw module.\n>> \n>> Will send a v4 shortly.\n>\n> Thank you.\n>\n>> >> When commenting out hw_get_module() and the libhardware dependency in\n>> >> build.meson, it already compiled fine.\n>> >> \n>> >> I will add libdl to the dependency list in v4.\n>> >> \n>> >> >> > }\n>> >> >> > \n>> >> >> > std::unique_ptr\n>> >> >> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n>> >> >> > index d40a3b0ba2eb..42eeab3bcfa9 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 += [dependency('libhardware')]\n>> >> >> > elif platform == 'cros'\n>> >> >> > android_hal_sources += files(['cros_camera_buffer.cpp',\n>> >> >> > 'cros_frame_buffer_allocator.cpp'])\n>> >> >> > \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 8BB38C3213\n\tfor ;\n\tMon, 29 May 2023 13:35:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CD2F7626FA;\n\tMon, 29 May 2023 15:35:03 +0200 (CEST)","from mail-ej1-x630.google.com (mail-ej1-x630.google.com\n\t[IPv6:2a00:1450:4864:20::630])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 03D40626F5\n\tfor ;\n\tMon, 29 May 2023 15:35:03 +0200 (CEST)","by mail-ej1-x630.google.com with SMTP id\n\ta640c23a62f3a-96fbc74fbf1so614316066b.1\n\tfor ;\n\tMon, 29 May 2023 06:35:02 -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\tsi16-20020a170906ced000b0094f1b8901e1sm5898846ejb.68.2023.05.29.06.35.01\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 29 May 2023 06:35:02 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685367303;\n\tbh=8P7Bwgr2PlvVSHyEx3rfrYJpZROLG9reJ6eISDqle2c=;\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=S6n+R1z+o0AsR2QPtU/Zs3GkSM0Mh/lbC2NVPLo+Xapb7k/W5xgm9zYTRiHyc8spR\n\t9YlgEW48QvoglCseADtkcYFY7yX6gm6R5n6qb7HzwJ269Pa8E0Fa0BSc6Vxmopst0c\n\t9S0nLNGFs0zzXCmlF7lGLmGGD93ND6LU2iFZR/XOCDo94zzQB+BXSEIdxQtGAY8n2g\n\tclSChDs6Zca41LM02YXI5C/UC7BrB1Kxi2HIZdBmBeNEG2e0W6XJyVI5ptOHJDD5RB\n\tUr+ZPIlSGXTGybt7NfkqgBG8SgaRuv6Tr2s6SMszRBt8VCr5TkI8wwHIfvUO4u/UL3\n\t4RaBfIDyvwOHw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20221208.gappssmtp.com; s=20221208; t=1685367302;\n\tx=1687959302; \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=o22zYGQKuswnCs+wUqBdc6OJ0RGpIwUVCUv0Doo102A=;\n\tb=asNPGfTlmsY5pxtBOy6V5pXr0YCqaBizNPCRRFkuepFlf0NqQvJjF+81/822St9lha\n\t4ov+sDS19Lah0TPPzoYdwTIPCmLuEW4UBm3wEvrni+00fdTKaPMPm3j8hatecnvmGyRx\n\t49JIfTmzkhKiCpFNdY3i/FNIXdGWg5t4ULMD+sP1MyHm6gUaZIH7S68O7qNloBDlOCel\n\tWRBwluSFr3UcQslyJAUm4+iDeSwZSByZqmb+x6b/zITg1HKmEpqPti8YZESJK/OQzjOR\n\tve208Gb5diIdO+w+XBmxdOi5YGcfY9ByOmxu2g7oe1BQRehvU1chCNXep5rHpl2QZxKc\n\tem7g=="],"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=\"asNPGfTl\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685367302; x=1687959302;\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=o22zYGQKuswnCs+wUqBdc6OJ0RGpIwUVCUv0Doo102A=;\n\tb=ejW6n0SaQeO5zUxQITU4dBo2RKnLLxqdywDj9dSjZ1TDCDzbLJRJHk6L2730ifPbzh\n\t5QHED6+/SHl/sfJ4l98Y7d0LBbYqHIydHhBH0Tnvj93E8L7dCDNOWJpOMKffqjPMhw2a\n\tA+pWWWD/eBc9zGcUPmkGyxoYUIq8xfUa0BZf6wnVgdZKFa5KzaboZXnC1473/UHTUvMX\n\tTuT4G9t6AJdHTYp2u7imw3mGXuAiuVjep2aX40a6KW6EMGx+2p/LKL+4iKcgzynUpOTV\n\tmK7K7lis19mk2oswrDyh2uZVSQr1W95Da5Iawdl8n/ZMguC4LbjMc01VeIbpdpwhkTj6\n\tEkMg==","X-Gm-Message-State":"AC+VfDzaGQmo0L/LnaAbv62Bjsg0shmruFGIvyiS2/Ybp8nWw4Mp1JcR\n\t+F6ez+kVGdwmkrslWjNcUqvQqQ==","X-Google-Smtp-Source":"ACHHUZ5qv7Q4XF7d3HeakQZ5DCCmeNAtKsG+on1noz/VRU+IUzTkJKvKBjCzDVYBPMKldjdtS3CL8A==","X-Received":"by 2002:a17:906:dace:b0:94f:322d:909c with SMTP id\n\txi14-20020a170906dace00b0094f322d909cmr10837565ejb.34.1685367302446; \n\tMon, 29 May 2023 06:35:02 -0700 (PDT)","To":"Laurent Pinchart ","In-Reply-To":"<20230529132911.GA22067@pendragon.ideasonboard.com>","References":"<20230227-android-gralloc-v3-1-babbdf049259@baylibre.com>\n\t<20230529094255.GM25984@pendragon.ideasonboard.com>\n\t<20230529100935.GQ25984@pendragon.ideasonboard.com>\n\t<87jzwrjp50.fsf@baylibre.com>\n\t<20230529123341.GA15292@pendragon.ideasonboard.com>\n\t<87h6rvjmln.fsf@baylibre.com>\n\t<20230529132911.GA22067@pendragon.ideasonboard.com>","Date":"Mon, 29 May 2023 15:35:01 +0200","Message-ID":"<87edmzjm62.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH RESEND v3] 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\" "}}]