[{"id":15370,"web_url":"https://patchwork.libcamera.org/comment/15370/","msgid":"","date":"2021-03-01T23:44:34","subject":"Re: [libcamera-devel] [PATCH 06/10] android: camera_buffer:\n\tImplement libcamera::Extensible","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Mon, Mar 01, 2021 at 04:01:07PM +0100, Jacopo Mondi wrote:\n> In order to prepare to support more memory backends, make the\n> CameraBuffer class implement the PIMPL (pointer-to-implementation)\n> pattern by inheriting from the libcamera::Extensible class.\n> \n> Temporary maintain libcamera::MappedBuffer as the CameraBuffer base\n> class to maintain compatibility of the CameraStream::process() interface\n> that requires a MappedBuffer * as second argument and will be converted\n> to use a CameraBuffer in the next patch.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_buffer.h | 14 ++++-\n> src/android/mm/generic_camera_buffer.cpp | 74 +++++++++++++++++++++++-\n> 2 files changed, 86 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h\n> index 0590cd84652b..ca4f4527c690 100644\n> --- a/src/android/camera_buffer.h\n> +++ b/src/android/camera_buffer.h\n> @@ -9,13 +9,25 @@\n> \n> #include \n> \n> +#include \n> #include \n> +#include \n> \n> -class CameraBuffer : public libcamera::MappedBuffer\n> +class CameraBuffer final : public libcamera::Extensible,\n> +\t\t\t public libcamera::MappedBuffer\n> {\n> +\tLIBCAMERA_DECLARE_PRIVATE(CameraBuffer)\n> +\n> public:\n> \tCameraBuffer(buffer_handle_t camera3Buffer, int flags);\n> \t~CameraBuffer();\n> +\n> +\tbool isValid() const;\n> +\n> +\tunsigned int numPlanes() const;\n> +\n> +\tlibcamera::Span plane(unsigned int plane) const;\n> +\tlibcamera::Span plane(unsigned int plane);\n> };\n> \n> #endif /* __ANDROID_CAMERA_BUFFER_H__ */\n> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp\n> index d1407f9cddf4..eedf16b76542 100644\n> --- a/src/android/mm/generic_camera_buffer.cpp\n> +++ b/src/android/mm/generic_camera_buffer.cpp\n> @@ -13,7 +13,25 @@ using namespace libcamera;\n> \n> LOG_DECLARE_CATEGORY(HAL)\n> \n> -CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)\n> +class CameraBuffer::Private : public Extensible::Private,\n> +\t\t\t public libcamera::MappedBuffer\n> +{\n> +\tLIBCAMERA_DECLARE_PUBLIC(CameraBuffer)\n> +\n> +public:\n> +\tPrivate(CameraBuffer *cameraBuffer,\n> +\t\tbuffer_handle_t camera3Buffer, int flags);\n> +\t~Private();\n> +\n> +\tunsigned int numPlanes() const;\n> +\n> +\tSpan plane(unsigned int plane) const;\n> +\tSpan plane(unsigned int plane);\n> +};\n> +\n> +CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,\n> +\t\t\t buffer_handle_t camera3Buffer, int flags)\n> +\t: Extensible::Private(cameraBuffer)\n> {\n> \tmaps_.reserve(camera3Buffer->numFds);\n> \terror_ = 0;\n> @@ -42,6 +60,60 @@ CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)\n> \t}\n> }\n> \n> +CameraBuffer::Private::~Private()\n> +{\n> +}\n> +\n> +unsigned int CameraBuffer::Private::numPlanes() const\n> +{\n> +\treturn maps_.size();\n> +}\n> +\n> +Span CameraBuffer::Private::plane(unsigned int plane) const\n> +{\n> +\tif (plane >= maps_.size())\n> +\t\treturn {};\n> +\n> +\treturn maps_[plane];\n> +}\n> +\n> +Span CameraBuffer::Private::plane(unsigned int plane)\n> +{\n> +\tif (plane >= maps_.size())\n> +\t\treturn {};\n> +\n> +\treturn maps_[plane];\n> +}\n> +\n> +CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)\n> +\t: Extensible(new Private(this, camera3Buffer, flags))\n> +{\n> +}\n> +\n> CameraBuffer::~CameraBuffer()\n> {\n> }\n> +\n> +bool CameraBuffer::isValid() const\n> +{\n> +\tconst Private *const d = LIBCAMERA_D_PTR();\n> +\treturn d->isValid();\n> +}\n> +\n> +unsigned int CameraBuffer::numPlanes() const\n> +{\n> +\tconst Private *const d = LIBCAMERA_D_PTR();\n> +\treturn d->numPlanes();\n> +}\n> +\n> +Span CameraBuffer::plane(unsigned int plane) const\n> +{\n> +\tconst Private *const d = LIBCAMERA_D_PTR();\n> +\treturn d->plane(plane);\n\nHow about implementing this as\n\n\treturn const_cast(d)->plane(plane);\n\nto avoid two plane() function in the private class ? You could then drop\nthe const version of Private::plane() (it would be possible to do it the\nother way around, but conceptually, keeping the non-const version is\nbetter I think as we return a pointer to non-const memory).\n\nReviewed-by: Laurent Pinchart \n\n> +}\n> +\n> +Span CameraBuffer::plane(unsigned int plane)\n> +{\n> +\tPrivate *const d = LIBCAMERA_D_PTR();\n> +\treturn d->plane(plane);\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 838E6BD808\n\tfor ;\n\tMon, 1 Mar 2021 23:45:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA97B68A8F;\n\tTue, 2 Mar 2021 00:45:05 +0100 (CET)","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 236A068A45\n\tfor ;\n\tTue, 2 Mar 2021 00:45:04 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2628545D;\n\tTue, 2 Mar 2021 00:45:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oQEWDpBg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614642303;\n\tbh=2wPXsauhePySPAm563pWF0qqfq2bBqSnGyBPojR476Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oQEWDpBgoh5VkygCQFBIYil/cO6aFiewzBEZM8sR05T+o5m3o38NWjb1GUTzoGy6Z\n\tDr31b8PGH1Q5EWen9Qq74LV9r5jTngJ7f1VZ38QljpkJneompRieiSNH/pomS8N9BT\n\tKpnH9T6CaFeXeCN3Y1puizp4cYOj3GwL2S/RMeJs=","Date":"Tue, 2 Mar 2021 01:44:34 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210301150111.61791-1-jacopo@jmondi.org>\n\t<20210301150111.61791-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210301150111.61791-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 06/10] android: camera_buffer:\n\tImplement libcamera::Extensible","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Han-lin Chen ,\n\tlibcamera-devel@lists.libcamera.org, Daniel Hung-yu Wu ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]