[{"id":15331,"web_url":"https://patchwork.libcamera.org/comment/15331/","msgid":"","date":"2021-02-28T18:20:34","subject":"Re: [libcamera-devel] [PATCH 02/12] android: Introduce CameraBuffer\n\tinterface","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 Fri, Feb 26, 2021 at 02:29:22PM +0100, Jacopo Mondi wrote:\n> In order to provide support for different memory backends,\n> move the MappedCamera3Buffer class definition outside of the\n> CameraDevice class to its own file.\n> \n> The interface defined in camera_buffer.h will be implemented by\n> different backends that will be placed in the src/android/mm\n> subdirectory.\n> \n> Provide a first implementation for the 'generic android' backend which\n> matches the existing one.\n> \n> The MappedCamera3Buffer interface will be renamed in CameraBuffer\n> in the next patch to match the name of the file and not in this patch\n> to ease review.\n\nThe next patch is fairly small, I'd squash it with this one. Splitting\nthe two makes 02/12 a bit weird, as you mention introducing CameraBuffer\nin the subject while this patch doesn't create such a class.\n\nIf you really want to keep the two separate, I'd rename\nMappedCamera3Buffer to CameraBuffer first.\n\nWith this handled,\n\nReviewed-by: Laurent Pinchart \n\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_buffer.h | 21 ++++++++++\n> src/android/camera_device.cpp | 30 --------------\n> src/android/camera_device.h | 7 +---\n> src/android/meson.build | 2 +\n> src/android/mm/android_generic_buffer.cpp | 48 +++++++++++++++++++++++\n> src/android/mm/meson.build | 6 +++\n> 6 files changed, 78 insertions(+), 36 deletions(-)\n> create mode 100644 src/android/camera_buffer.h\n> create mode 100644 src/android/mm/android_generic_buffer.cpp\n> create mode 100644 src/android/mm/meson.build\n> \n> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h\n> new file mode 100644\n> index 000000000000..a1fb97a3c6db\n> --- /dev/null\n> +++ b/src/android/camera_buffer.h\n> @@ -0,0 +1,21 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * camera_buffer.h - Frame buffer handling interface definition\n> + */\n> +#ifndef __ANDROID_CAMERA_BUFFER_H__\n> +#define __ANDROID_CAMERA_BUFFER_H__\n> +\n> +#include \n> +\n> +#include \n> +\n> +class MappedCamera3Buffer : public libcamera::MappedBuffer\n> +{\n> +public:\n> +\tMappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);\n> +\t~MappedCamera3Buffer();\n> +};\n> +\n> +#endif /* __ANDROID_CAMERA_BUFFER_H__ */\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 16cb8c6d2b84..a7a5b7986aa4 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -257,36 +257,6 @@ void sortCamera3StreamConfigs(std::vector &unsortedConfigs,\n> \n> } /* namespace */\n> \n> -MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,\n> -\t\t\t\t\t int flags)\n> -{\n> -\tmaps_.reserve(camera3buffer->numFds);\n> -\terror_ = 0;\n> -\n> -\tfor (int i = 0; i < camera3buffer->numFds; i++) {\n> -\t\tif (camera3buffer->data[i] == -1)\n> -\t\t\tcontinue;\n> -\n> -\t\toff_t length = lseek(camera3buffer->data[i], 0, SEEK_END);\n> -\t\tif (length < 0) {\n> -\t\t\terror_ = -errno;\n> -\t\t\tLOG(HAL, Error) << \"Failed to query plane length\";\n> -\t\t\tbreak;\n> -\t\t}\n> -\n> -\t\tvoid *address = mmap(nullptr, length, flags, MAP_SHARED,\n> -\t\t\t\t camera3buffer->data[i], 0);\n> -\t\tif (address == MAP_FAILED) {\n> -\t\t\terror_ = -errno;\n> -\t\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n> -\t\t\tbreak;\n> -\t\t}\n> -\n> -\t\tmaps_.emplace_back(static_cast(address),\n> -\t\t\t\t static_cast(length));\n> -\t}\n> -}\n> -\n> /*\n> * \\struct Camera3RequestDescriptor\n> *\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 9cbfcad38433..e6c192c2100b 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -24,17 +24,12 @@\n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/message.h\"\n> \n> +#include \"camera_buffer.h\"\n> #include \"camera_metadata.h\"\n> #include \"camera_stream.h\"\n> #include \"camera_worker.h\"\n> #include \"jpeg/encoder.h\"\n> \n> -class MappedCamera3Buffer : public libcamera::MappedBuffer\n> -{\n> -public:\n> -\tMappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);\n> -};\n> -\n> class CameraDevice : protected libcamera::Loggable\n> {\n> public:\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index a13ce63b1d58..fd41c74f78ef 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -57,6 +57,8 @@ android_hal_sources = files([\n> 'yuv/post_processor_yuv.cpp'\n> ])\n> \n> +subdir('mm')\n> +\n> android_camera_metadata_sources = files([\n> 'metadata/camera_metadata.c',\n> ])\n> diff --git a/src/android/mm/android_generic_buffer.cpp b/src/android/mm/android_generic_buffer.cpp\n> new file mode 100644\n> index 000000000000..2504d9276e9e\n> --- /dev/null\n> +++ b/src/android/mm/android_generic_buffer.cpp\n> @@ -0,0 +1,48 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * android_generic_buffer.cpp - Generic Android frame buffer backend\n> + */\n> +\n> +#include \"../camera_buffer.h\"\n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +using namespace libcamera;\n> +\n> +LOG_DECLARE_CATEGORY(HAL)\n> +\n> +MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,\n> +\t\t\t\t\t int flags)\n> +{\n> +\tmaps_.reserve(camera3buffer->numFds);\n> +\terror_ = 0;\n> +\n> +\tfor (int i = 0; i < camera3buffer->numFds; i++) {\n> +\t\tif (camera3buffer->data[i] == -1)\n> +\t\t\tcontinue;\n> +\n> +\t\toff_t length = lseek(camera3buffer->data[i], 0, SEEK_END);\n> +\t\tif (length < 0) {\n> +\t\t\terror_ = -errno;\n> +\t\t\tLOG(HAL, Error) << \"Failed to query plane length\";\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tvoid *address = mmap(nullptr, length, flags, MAP_SHARED,\n> +\t\t\t\t camera3buffer->data[i], 0);\n> +\t\tif (address == MAP_FAILED) {\n> +\t\t\terror_ = -errno;\n> +\t\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tmaps_.emplace_back(static_cast(address),\n> +\t\t\t\t static_cast(length));\n> +\t}\n> +}\n> +\n> +MappedCamera3Buffer::~MappedCamera3Buffer()\n> +{\n> +}\n> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> new file mode 100644\n> index 000000000000..39be8fec8567\n> --- /dev/null\n> +++ b/src/android/mm/meson.build\n> @@ -0,0 +1,6 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +memory_backend = get_option('android_memory_backend')\n> +if memory_backend == 'android_generic'\n> + android_hal_sources += files(['android_generic_buffer.cpp'])\n> +endif","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 5DD2DBD808\n\tfor ;\n\tSun, 28 Feb 2021 18:21:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0B3A68A45;\n\tSun, 28 Feb 2021 19:21:04 +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 1983768A45\n\tfor ;\n\tSun, 28 Feb 2021 19:21:03 +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 8CC1B80F;\n\tSun, 28 Feb 2021 19:21:02 +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=\"gCM/fCFO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614536462;\n\tbh=WfISDv6qHa+QlXNtm2o3F8q/EzLv3cwUREK56VipcQU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gCM/fCFOEXzfFyuwwTR5GRs0JdCeur9P1ED9zDkjUvbXxyY5Z3BcX2RCox2hINnla\n\tFoq2UYSPm4VeRCTFb1yr6xIVZHZKivwH5Dm7B0MadGbT2P4ENW+i7HJSTT6yMOJ9pV\n\tWnnMzyyrthK75ddQQCpxgHU0UCFKK8OscvpStyKA=","Date":"Sun, 28 Feb 2021 20:20:34 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210226132932.165484-1-jacopo@jmondi.org>\n\t<20210226132932.165484-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210226132932.165484-3-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 02/12] android: Introduce CameraBuffer\n\tinterface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]