[{"id":20266,"web_url":"https://patchwork.libcamera.org/comment/20266/","msgid":"","date":"2021-10-18T13:41:51","subject":"Re: [libcamera-devel] [PATCH 01/11] camera_device: Remove private\n\tscope of Camera3RequestDescriptor","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Mon, Oct 18, 2021 at 06:59:13PM +0530, Umang Jain wrote:\n> Camera3RequestDescriptor is a utility structure that groups information\n> about a capture request. It can be and will be extended to preserve the\n> context of a capture overall. Since the context of a capture needs to\n> be shared among other classes (for e.g. CameraStream) having a private\n> definition of the struct in CameraDevice class doesn't help.\n> \n> Hence, de-scope the structure so that it can be shared with other\n> components (through references or pointers). Splitting the structure to\n> a separate file will help avoiding circular dependencies when using it\n> through the HAL implementation.\n> \n> Signed-off-by: Umang Jain \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/android/camera_device.cpp | 43 ++++---------------------------\n> src/android/camera_device.h | 27 ++------------------\n> src/android/camera_request.cpp | 45 +++++++++++++++++++++++++++++++++\n> src/android/camera_request.h | 46 ++++++++++++++++++++++++++++++++++\n> src/android/meson.build | 1 +\n> 5 files changed, 99 insertions(+), 63 deletions(-)\n> create mode 100644 src/android/camera_request.cpp\n> create mode 100644 src/android/camera_request.h\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 90186710..b4ab5da1 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -6,9 +6,6 @@\n> */\n> \n> #include \"camera_device.h\"\n> -#include \"camera_hal_config.h\"\n> -#include \"camera_ops.h\"\n> -#include \"post_processor.h\"\n> \n> #include \n> #include \n> @@ -27,6 +24,11 @@\n> \n> #include \"system/graphics.h\"\n> \n> +#include \"camera_hal_config.h\"\n> +#include \"camera_ops.h\"\n> +#include \"camera_request.h\"\n> +#include \"post_processor.h\"\n> +\n> using namespace libcamera;\n> \n> LOG_DECLARE_CATEGORY(HAL)\n> @@ -213,41 +215,6 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList)\n> \n> } /* namespace */\n> \n> -/*\n> - * \\struct Camera3RequestDescriptor\n> - *\n> - * A utility structure that groups information about a capture request to be\n> - * later re-used at request complete time to notify the framework.\n> - */\n> -\n> -CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(\n> -\tCamera *camera, const camera3_capture_request_t *camera3Request)\n> -{\n> -\tframeNumber_ = camera3Request->frame_number;\n> -\n> -\t/* Copy the camera3 request stream information for later access. */\n> -\tconst uint32_t numBuffers = camera3Request->num_output_buffers;\n> -\tbuffers_.resize(numBuffers);\n> -\tfor (uint32_t i = 0; i < numBuffers; i++)\n> -\t\tbuffers_[i] = camera3Request->output_buffers[i];\n> -\n> -\t/*\n> -\t * FrameBuffer instances created by wrapping a camera3 provided dmabuf\n> -\t * are emplaced in this vector of unique_ptr<> for lifetime management.\n> -\t */\n> -\tframeBuffers_.reserve(numBuffers);\n> -\n> -\t/* Clone the controls associated with the camera3 request. */\n> -\tsettings_ = CameraMetadata(camera3Request->settings);\n> -\n> -\t/*\n> -\t * Create the CaptureRequest, stored as a unique_ptr<> to tie its\n> -\t * lifetime to the descriptor.\n> -\t */\n> -\trequest_ = std::make_unique(camera,\n> -\t\t\t\t\t\t reinterpret_cast(this));\n> -}\n> -\n> /*\n> * \\class CameraDevice\n> *\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index b7d774fe..86224aa1 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -33,7 +33,9 @@\n> #include \"camera_worker.h\"\n> #include \"jpeg/encoder.h\"\n> \n> +struct Camera3RequestDescriptor;\n> struct CameraConfigData;\n> +\n> class CameraDevice : protected libcamera::Loggable\n> {\n> public:\n> @@ -73,31 +75,6 @@ private:\n> \n> \tCameraDevice(unsigned int id, std::shared_ptr camera);\n> \n> -\tstruct Camera3RequestDescriptor {\n> -\t\tenum class Status {\n> -\t\t\tPending,\n> -\t\t\tSuccess,\n> -\t\t\tError,\n> -\t\t};\n> -\n> -\t\tCamera3RequestDescriptor() = default;\n> -\t\t~Camera3RequestDescriptor() = default;\n> -\t\tCamera3RequestDescriptor(libcamera::Camera *camera,\n> -\t\t\t\t\t const camera3_capture_request_t *camera3Request);\n> -\t\tCamera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;\n> -\n> -\t\tbool isPending() const { return status_ == Status::Pending; }\n> -\n> -\t\tuint32_t frameNumber_ = 0;\n> -\t\tstd::vector buffers_;\n> -\t\tstd::vector> frameBuffers_;\n> -\t\tCameraMetadata settings_;\n> -\t\tstd::unique_ptr request_;\n> -\n> -\t\tcamera3_capture_result_t captureResult_ = {};\n> -\t\tStatus status_ = Status::Pending;\n> -\t};\n> -\n> \tenum class State {\n> \t\tStopped,\n> \t\tFlushing,\n> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp\n> new file mode 100644\n> index 00000000..93e546bf\n> --- /dev/null\n> +++ b/src/android/camera_request.cpp\n> @@ -0,0 +1,45 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019-2021, Google Inc.\n> + *\n> + * camera_request.cpp - libcamera Android Camera Request Descriptor\n> + */\n> +\n> +#include \"camera_request.h\"\n> +\n> +using namespace libcamera;\n> +\n> +/*\n> + * \\struct Camera3RequestDescriptor\n> + *\n> + * A utility structure that groups information about a capture request to be\n> + * later re-used at request complete time to notify the framework.\n> + */\n> +\n> +Camera3RequestDescriptor::Camera3RequestDescriptor(\n> +\tCamera *camera, const camera3_capture_request_t *camera3Request)\n> +{\n> +\tframeNumber_ = camera3Request->frame_number;\n> +\n> +\t/* Copy the camera3 request stream information for later access. */\n> +\tconst uint32_t numBuffers = camera3Request->num_output_buffers;\n> +\tbuffers_.resize(numBuffers);\n> +\tfor (uint32_t i = 0; i < numBuffers; i++)\n> +\t\tbuffers_[i] = camera3Request->output_buffers[i];\n> +\n> +\t/*\n> +\t * FrameBuffer instances created by wrapping a camera3 provided dmabuf\n> +\t * are emplaced in this vector of unique_ptr<> for lifetime management.\n> +\t */\n> +\tframeBuffers_.reserve(numBuffers);\n> +\n> +\t/* Clone the controls associated with the camera3 request. */\n> +\tsettings_ = CameraMetadata(camera3Request->settings);\n> +\n> +\t/*\n> +\t * Create the CaptureRequest, stored as a unique_ptr<> to tie its\n> +\t * lifetime to the descriptor.\n> +\t */\n> +\trequest_ = std::make_unique(camera,\n> +\t\t\t\t\t\t reinterpret_cast(this));\n> +}\n> diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> new file mode 100644\n> index 00000000..1346f6fa\n> --- /dev/null\n> +++ b/src/android/camera_request.h\n> @@ -0,0 +1,46 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019-2021, Google Inc.\n> + *\n> + * camera_request.h - libcamera Android Camera Request Descriptor\n> + */\n> +#ifndef __ANDROID_CAMERA_REQUEST_H__\n> +#define __ANDROID_CAMERA_REQUEST_H__\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"camera_metadata.h\"\n> +#include \"camera_worker.h\"\n> +\n> +struct Camera3RequestDescriptor {\n> +\tenum class Status {\n> +\t\tPending,\n> +\t\tSuccess,\n> +\t\tError,\n> +\t};\n> +\n> +\tCamera3RequestDescriptor() = default;\n> +\t~Camera3RequestDescriptor() = default;\n> +\tCamera3RequestDescriptor(libcamera::Camera *camera,\n> +\t\t\t\t const camera3_capture_request_t *camera3Request);\n> +\tCamera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;\n> +\n> +\tbool isPending() const { return status_ == Status::Pending; }\n> +\n> +\tuint32_t frameNumber_ = 0;\n> +\tstd::vector buffers_;\n> +\tstd::vector> frameBuffers_;\n> +\tCameraMetadata settings_;\n> +\tstd::unique_ptr request_;\n> +\n> +\tcamera3_capture_result_t captureResult_ = {};\n> +\tStatus status_ = Status::Pending;\n> +};\n> +\n> +#endif /* __ANDROID_CAMERA_REQUEST_H__ */\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index 7d1e7e85..332b177c 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -45,6 +45,7 @@ android_hal_sources = files([\n> 'camera_hal_manager.cpp',\n> 'camera_metadata.cpp',\n> 'camera_ops.cpp',\n> + 'camera_request.cpp',\n> 'camera_stream.cpp',\n> 'camera_worker.cpp',\n> 'jpeg/encoder_libjpeg.cpp',","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 15698C323E\n\tfor ;\n\tMon, 18 Oct 2021 13:42:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5842668F59;\n\tMon, 18 Oct 2021 15:42:11 +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 0AFB968F56\n\tfor ;\n\tMon, 18 Oct 2021 15:42:09 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 93ABF18F1;\n\tMon, 18 Oct 2021 15:42:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dL/bDSem\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634564528;\n\tbh=+45OekZF29wVN4Le2i2twoXdlrdv4xwYQg6azm851tg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dL/bDSemRA55gyh4od/r9VG1A7FIYoh7VEJ8xsbByzym96rEEJLiMB2d20JyvwJYw\n\tHWY6f4qaknEX3eH7jxg1KW5ZWknPdrL71taBxnf9O39896fdWem9xPHw6cJOWNCyBM\n\tgqsSG3plFCriJXmDWuIXTrkHeqxHyHkk/fF4qwSk=","Date":"Mon, 18 Oct 2021 16:41:51 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20211018132923.476242-1-umang.jain@ideasonboard.com>\n\t<20211018132923.476242-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211018132923.476242-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 01/11] camera_device: Remove private\n\tscope of Camera3RequestDescriptor","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20268,"web_url":"https://patchwork.libcamera.org/comment/20268/","msgid":"<20211018152610.5gw77vcjwvm6zbay@uno.localdomain>","date":"2021-10-18T15:26:10","subject":"Re: [libcamera-devel] [PATCH 01/11] camera_device: Remove private\n\tscope of Camera3RequestDescriptor","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Umang,\n\nOn Mon, Oct 18, 2021 at 06:59:13PM +0530, Umang Jain wrote:\n> Camera3RequestDescriptor is a utility structure that groups information\n> about a capture request. It can be and will be extended to preserve the\n> context of a capture overall. Since the context of a capture needs to\n> be shared among other classes (for e.g. CameraStream) having a private\n> definition of the struct in CameraDevice class doesn't help.\n>\n> Hence, de-scope the structure so that it can be shared with other\n> components (through references or pointers). Splitting the structure to\n> a separate file will help avoiding circular dependencies when using it\n> through the HAL implementation.\n>\n> Signed-off-by: Umang Jain \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Jacopo Mondi \n\n> ---\n> src/android/camera_device.cpp | 43 ++++---------------------------\n> src/android/camera_device.h | 27 ++------------------\n> src/android/camera_request.cpp | 45 +++++++++++++++++++++++++++++++++\n> src/android/camera_request.h | 46 ++++++++++++++++++++++++++++++++++\n> src/android/meson.build | 1 +\n> 5 files changed, 99 insertions(+), 63 deletions(-)\n> create mode 100644 src/android/camera_request.cpp\n> create mode 100644 src/android/camera_request.h\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 90186710..b4ab5da1 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -6,9 +6,6 @@\n> */\n>\n> #include \"camera_device.h\"\n> -#include \"camera_hal_config.h\"\n> -#include \"camera_ops.h\"\n> -#include \"post_processor.h\"\n>\n> #include \n> #include \n> @@ -27,6 +24,11 @@\n>\n> #include \"system/graphics.h\"\n>\n> +#include \"camera_hal_config.h\"\n> +#include \"camera_ops.h\"\n> +#include \"camera_request.h\"\n> +#include \"post_processor.h\"\n> +\n> using namespace libcamera;\n>\n> LOG_DECLARE_CATEGORY(HAL)\n> @@ -213,41 +215,6 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList)\n>\n> } /* namespace */\n>\n> -/*\n> - * \\struct Camera3RequestDescriptor\n> - *\n> - * A utility structure that groups information about a capture request to be\n> - * later re-used at request complete time to notify the framework.\n> - */\n> -\n> -CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(\n> -\tCamera *camera, const camera3_capture_request_t *camera3Request)\n> -{\n> -\tframeNumber_ = camera3Request->frame_number;\n> -\n> -\t/* Copy the camera3 request stream information for later access. */\n> -\tconst uint32_t numBuffers = camera3Request->num_output_buffers;\n> -\tbuffers_.resize(numBuffers);\n> -\tfor (uint32_t i = 0; i < numBuffers; i++)\n> -\t\tbuffers_[i] = camera3Request->output_buffers[i];\n> -\n> -\t/*\n> -\t * FrameBuffer instances created by wrapping a camera3 provided dmabuf\n> -\t * are emplaced in this vector of unique_ptr<> for lifetime management.\n> -\t */\n> -\tframeBuffers_.reserve(numBuffers);\n> -\n> -\t/* Clone the controls associated with the camera3 request. */\n> -\tsettings_ = CameraMetadata(camera3Request->settings);\n> -\n> -\t/*\n> -\t * Create the CaptureRequest, stored as a unique_ptr<> to tie its\n> -\t * lifetime to the descriptor.\n> -\t */\n> -\trequest_ = std::make_unique(camera,\n> -\t\t\t\t\t\t reinterpret_cast(this));\n> -}\n> -\n> /*\n> * \\class CameraDevice\n> *\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index b7d774fe..86224aa1 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -33,7 +33,9 @@\n> #include \"camera_worker.h\"\n> #include \"jpeg/encoder.h\"\n>\n> +struct Camera3RequestDescriptor;\n> struct CameraConfigData;\n> +\n> class CameraDevice : protected libcamera::Loggable\n> {\n> public:\n> @@ -73,31 +75,6 @@ private:\n>\n> \tCameraDevice(unsigned int id, std::shared_ptr camera);\n>\n> -\tstruct Camera3RequestDescriptor {\n> -\t\tenum class Status {\n> -\t\t\tPending,\n> -\t\t\tSuccess,\n> -\t\t\tError,\n> -\t\t};\n> -\n> -\t\tCamera3RequestDescriptor() = default;\n> -\t\t~Camera3RequestDescriptor() = default;\n> -\t\tCamera3RequestDescriptor(libcamera::Camera *camera,\n> -\t\t\t\t\t const camera3_capture_request_t *camera3Request);\n> -\t\tCamera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;\n> -\n> -\t\tbool isPending() const { return status_ == Status::Pending; }\n> -\n> -\t\tuint32_t frameNumber_ = 0;\n> -\t\tstd::vector buffers_;\n> -\t\tstd::vector> frameBuffers_;\n> -\t\tCameraMetadata settings_;\n> -\t\tstd::unique_ptr request_;\n> -\n> -\t\tcamera3_capture_result_t captureResult_ = {};\n> -\t\tStatus status_ = Status::Pending;\n> -\t};\n> -\n> \tenum class State {\n> \t\tStopped,\n> \t\tFlushing,\n> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp\n> new file mode 100644\n> index 00000000..93e546bf\n> --- /dev/null\n> +++ b/src/android/camera_request.cpp\n> @@ -0,0 +1,45 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019-2021, Google Inc.\n> + *\n> + * camera_request.cpp - libcamera Android Camera Request Descriptor\n> + */\n> +\n> +#include \"camera_request.h\"\n> +\n> +using namespace libcamera;\n> +\n> +/*\n> + * \\struct Camera3RequestDescriptor\n> + *\n> + * A utility structure that groups information about a capture request to be\n> + * later re-used at request complete time to notify the framework.\n> + */\n> +\n> +Camera3RequestDescriptor::Camera3RequestDescriptor(\n> +\tCamera *camera, const camera3_capture_request_t *camera3Request)\n> +{\n> +\tframeNumber_ = camera3Request->frame_number;\n> +\n> +\t/* Copy the camera3 request stream information for later access. */\n> +\tconst uint32_t numBuffers = camera3Request->num_output_buffers;\n> +\tbuffers_.resize(numBuffers);\n> +\tfor (uint32_t i = 0; i < numBuffers; i++)\n> +\t\tbuffers_[i] = camera3Request->output_buffers[i];\n> +\n> +\t/*\n> +\t * FrameBuffer instances created by wrapping a camera3 provided dmabuf\n> +\t * are emplaced in this vector of unique_ptr<> for lifetime management.\n> +\t */\n> +\tframeBuffers_.reserve(numBuffers);\n> +\n> +\t/* Clone the controls associated with the camera3 request. */\n> +\tsettings_ = CameraMetadata(camera3Request->settings);\n> +\n> +\t/*\n> +\t * Create the CaptureRequest, stored as a unique_ptr<> to tie its\n> +\t * lifetime to the descriptor.\n> +\t */\n> +\trequest_ = std::make_unique(camera,\n> +\t\t\t\t\t\t reinterpret_cast(this));\n> +}\n> diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> new file mode 100644\n> index 00000000..1346f6fa\n> --- /dev/null\n> +++ b/src/android/camera_request.h\n> @@ -0,0 +1,46 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019-2021, Google Inc.\n> + *\n> + * camera_request.h - libcamera Android Camera Request Descriptor\n> + */\n> +#ifndef __ANDROID_CAMERA_REQUEST_H__\n> +#define __ANDROID_CAMERA_REQUEST_H__\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"camera_metadata.h\"\n> +#include \"camera_worker.h\"\n> +\n> +struct Camera3RequestDescriptor {\n> +\tenum class Status {\n> +\t\tPending,\n> +\t\tSuccess,\n> +\t\tError,\n> +\t};\n> +\n> +\tCamera3RequestDescriptor() = default;\n> +\t~Camera3RequestDescriptor() = default;\n> +\tCamera3RequestDescriptor(libcamera::Camera *camera,\n> +\t\t\t\t const camera3_capture_request_t *camera3Request);\n> +\tCamera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;\n> +\n> +\tbool isPending() const { return status_ == Status::Pending; }\n> +\n> +\tuint32_t frameNumber_ = 0;\n> +\tstd::vector buffers_;\n> +\tstd::vector> frameBuffers_;\n> +\tCameraMetadata settings_;\n> +\tstd::unique_ptr request_;\n> +\n> +\tcamera3_capture_result_t captureResult_ = {};\n> +\tStatus status_ = Status::Pending;\n> +};\n> +\n> +#endif /* __ANDROID_CAMERA_REQUEST_H__ */\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index 7d1e7e85..332b177c 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -45,6 +45,7 @@ android_hal_sources = files([\n> 'camera_hal_manager.cpp',\n> 'camera_metadata.cpp',\n> 'camera_ops.cpp',\n> + 'camera_request.cpp',\n> 'camera_stream.cpp',\n> 'camera_worker.cpp',\n> 'jpeg/encoder_libjpeg.cpp',\n> --\n> 2.31.0\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 60D6EC324C\n\tfor ;\n\tMon, 18 Oct 2021 15:25:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C71768F5B;\n\tMon, 18 Oct 2021 17:25:23 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B29168F56\n\tfor ;\n\tMon, 18 Oct 2021 17:25:22 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 9EB04240007;\n\tMon, 18 Oct 2021 15:25:21 +0000 (UTC)"],"Date":"Mon, 18 Oct 2021 17:26:10 +0200","From":"Jacopo Mondi ","To":"Umang Jain ","Message-ID":"<20211018152610.5gw77vcjwvm6zbay@uno.localdomain>","References":"<20211018132923.476242-1-umang.jain@ideasonboard.com>\n\t<20211018132923.476242-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211018132923.476242-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 01/11] camera_device: Remove private\n\tscope of Camera3RequestDescriptor","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20286,"web_url":"https://patchwork.libcamera.org/comment/20286/","msgid":"","date":"2021-10-19T03:56:51","subject":"Re: [libcamera-devel] [PATCH 01/11] camera_device: Remove private\n\tscope of Camera3RequestDescriptor","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Umang,\n\nOn Tue, Oct 19, 2021 at 12:25 AM Jacopo Mondi wrote:\n>\n> Hi Umang,\n>\n> On Mon, Oct 18, 2021 at 06:59:13PM +0530, Umang Jain wrote:\n> > Camera3RequestDescriptor is a utility structure that groups information\n> > about a capture request. It can be and will be extended to preserve the\n> > context of a capture overall. Since the context of a capture needs to\n> > be shared among other classes (for e.g. CameraStream) having a private\n> > definition of the struct in CameraDevice class doesn't help.\n> >\n> > Hence, de-scope the structure so that it can be shared with other\n> > components (through references or pointers). Splitting the structure to\n> > a separate file will help avoiding circular dependencies when using it\n> > through the HAL implementation.\n> >\n> > Signed-off-by: Umang Jain \n> > Signed-off-by: Laurent Pinchart \n>\n> Reviewed-by: Jacopo Mondi \n>\n\nReviewed-by: Hirokazu Honda \n> > ---\n> > src/android/camera_device.cpp | 43 ++++---------------------------\n> > src/android/camera_device.h | 27 ++------------------\n> > src/android/camera_request.cpp | 45 +++++++++++++++++++++++++++++++++\n> > src/android/camera_request.h | 46 ++++++++++++++++++++++++++++++++++\n> > src/android/meson.build | 1 +\n> > 5 files changed, 99 insertions(+), 63 deletions(-)\n> > create mode 100644 src/android/camera_request.cpp\n> > create mode 100644 src/android/camera_request.h\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 90186710..b4ab5da1 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -6,9 +6,6 @@\n> > */\n> >\n> > #include \"camera_device.h\"\n> > -#include \"camera_hal_config.h\"\n> > -#include \"camera_ops.h\"\n> > -#include \"post_processor.h\"\n> >\n> > #include \n> > #include \n> > @@ -27,6 +24,11 @@\n> >\n> > #include \"system/graphics.h\"\n> >\n> > +#include \"camera_hal_config.h\"\n> > +#include \"camera_ops.h\"\n> > +#include \"camera_request.h\"\n> > +#include \"post_processor.h\"\n> > +\n> > using namespace libcamera;\n> >\n> > LOG_DECLARE_CATEGORY(HAL)\n> > @@ -213,41 +215,6 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList)\n> >\n> > } /* namespace */\n> >\n> > -/*\n> > - * \\struct Camera3RequestDescriptor\n> > - *\n> > - * A utility structure that groups information about a capture request to be\n> > - * later re-used at request complete time to notify the framework.\n> > - */\n> > -\n> > -CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(\n> > - Camera *camera, const camera3_capture_request_t *camera3Request)\n> > -{\n> > - frameNumber_ = camera3Request->frame_number;\n> > -\n> > - /* Copy the camera3 request stream information for later access. */\n> > - const uint32_t numBuffers = camera3Request->num_output_buffers;\n> > - buffers_.resize(numBuffers);\n> > - for (uint32_t i = 0; i < numBuffers; i++)\n> > - buffers_[i] = camera3Request->output_buffers[i];\n> > -\n> > - /*\n> > - * FrameBuffer instances created by wrapping a camera3 provided dmabuf\n> > - * are emplaced in this vector of unique_ptr<> for lifetime management.\n> > - */\n> > - frameBuffers_.reserve(numBuffers);\n> > -\n> > - /* Clone the controls associated with the camera3 request. */\n> > - settings_ = CameraMetadata(camera3Request->settings);\n> > -\n> > - /*\n> > - * Create the CaptureRequest, stored as a unique_ptr<> to tie its\n> > - * lifetime to the descriptor.\n> > - */\n> > - request_ = std::make_unique(camera,\n> > - reinterpret_cast(this));\n> > -}\n> > -\n> > /*\n> > * \\class CameraDevice\n> > *\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index b7d774fe..86224aa1 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -33,7 +33,9 @@\n> > #include \"camera_worker.h\"\n> > #include \"jpeg/encoder.h\"\n> >\n> > +struct Camera3RequestDescriptor;\n> > struct CameraConfigData;\n> > +\n> > class CameraDevice : protected libcamera::Loggable\n> > {\n> > public:\n> > @@ -73,31 +75,6 @@ private:\n> >\n> > CameraDevice(unsigned int id, std::shared_ptr camera);\n> >\n> > - struct Camera3RequestDescriptor {\n> > - enum class Status {\n> > - Pending,\n> > - Success,\n> > - Error,\n> > - };\n> > -\n> > - Camera3RequestDescriptor() = default;\n> > - ~Camera3RequestDescriptor() = default;\n> > - Camera3RequestDescriptor(libcamera::Camera *camera,\n> > - const camera3_capture_request_t *camera3Request);\n> > - Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;\n> > -\n> > - bool isPending() const { return status_ == Status::Pending; }\n> > -\n> > - uint32_t frameNumber_ = 0;\n> > - std::vector buffers_;\n> > - std::vector> frameBuffers_;\n> > - CameraMetadata settings_;\n> > - std::unique_ptr request_;\n> > -\n> > - camera3_capture_result_t captureResult_ = {};\n> > - Status status_ = Status::Pending;\n> > - };\n> > -\n> > enum class State {\n> > Stopped,\n> > Flushing,\n> > diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp\n> > new file mode 100644\n> > index 00000000..93e546bf\n> > --- /dev/null\n> > +++ b/src/android/camera_request.cpp\n> > @@ -0,0 +1,45 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019-2021, Google Inc.\n> > + *\n> > + * camera_request.cpp - libcamera Android Camera Request Descriptor\n> > + */\n> > +\n> > +#include \"camera_request.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +/*\n> > + * \\struct Camera3RequestDescriptor\n> > + *\n> > + * A utility structure that groups information about a capture request to be\n> > + * later re-used at request complete time to notify the framework.\n> > + */\n> > +\n> > +Camera3RequestDescriptor::Camera3RequestDescriptor(\n> > + Camera *camera, const camera3_capture_request_t *camera3Request)\n> > +{\n> > + frameNumber_ = camera3Request->frame_number;\n> > +\n> > + /* Copy the camera3 request stream information for later access. */\n> > + const uint32_t numBuffers = camera3Request->num_output_buffers;\n> > + buffers_.resize(numBuffers);\n> > + for (uint32_t i = 0; i < numBuffers; i++)\n> > + buffers_[i] = camera3Request->output_buffers[i];\n> > +\n> > + /*\n> > + * FrameBuffer instances created by wrapping a camera3 provided dmabuf\n> > + * are emplaced in this vector of unique_ptr<> for lifetime management.\n> > + */\n> > + frameBuffers_.reserve(numBuffers);\n> > +\n> > + /* Clone the controls associated with the camera3 request. */\n> > + settings_ = CameraMetadata(camera3Request->settings);\n> > +\n> > + /*\n> > + * Create the CaptureRequest, stored as a unique_ptr<> to tie its\n> > + * lifetime to the descriptor.\n> > + */\n> > + request_ = std::make_unique(camera,\n> > + reinterpret_cast(this));\n> > +}\n> > diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> > new file mode 100644\n> > index 00000000..1346f6fa\n> > --- /dev/null\n> > +++ b/src/android/camera_request.h\n> > @@ -0,0 +1,46 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019-2021, Google Inc.\n> > + *\n> > + * camera_request.h - libcamera Android Camera Request Descriptor\n> > + */\n> > +#ifndef __ANDROID_CAMERA_REQUEST_H__\n> > +#define __ANDROID_CAMERA_REQUEST_H__\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"camera_metadata.h\"\n> > +#include \"camera_worker.h\"\n> > +\n> > +struct Camera3RequestDescriptor {\n> > + enum class Status {\n> > + Pending,\n> > + Success,\n> > + Error,\n> > + };\n> > +\n> > + Camera3RequestDescriptor() = default;\n> > + ~Camera3RequestDescriptor() = default;\n> > + Camera3RequestDescriptor(libcamera::Camera *camera,\n> > + const camera3_capture_request_t *camera3Request);\n> > + Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;\n> > +\n> > + bool isPending() const { return status_ == Status::Pending; }\n> > +\n> > + uint32_t frameNumber_ = 0;\n> > + std::vector buffers_;\n> > + std::vector> frameBuffers_;\n> > + CameraMetadata settings_;\n> > + std::unique_ptr request_;\n> > +\n> > + camera3_capture_result_t captureResult_ = {};\n> > + Status status_ = Status::Pending;\n> > +};\n> > +\n> > +#endif /* __ANDROID_CAMERA_REQUEST_H__ */\n> > diff --git a/src/android/meson.build b/src/android/meson.build\n> > index 7d1e7e85..332b177c 100644\n> > --- a/src/android/meson.build\n> > +++ b/src/android/meson.build\n> > @@ -45,6 +45,7 @@ android_hal_sources = files([\n> > 'camera_hal_manager.cpp',\n> > 'camera_metadata.cpp',\n> > 'camera_ops.cpp',\n> > + 'camera_request.cpp',\n> > 'camera_stream.cpp',\n> > 'camera_worker.cpp',\n> > 'jpeg/encoder_libjpeg.cpp',\n> > --\n> > 2.31.0\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 9D400C323E\n\tfor ;\n\tTue, 19 Oct 2021 03:57:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E64F68F5A;\n\tTue, 19 Oct 2021 05:57:04 +0200 (CEST)","from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com\n\t[IPv6:2a00:1450:4864:20::52c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5AF8560126\n\tfor ;\n\tTue, 19 Oct 2021 05:57:03 +0200 (CEST)","by mail-ed1-x52c.google.com with SMTP id y30so6624672edi.0\n\tfor ;\n\tMon, 18 Oct 2021 20:57:03 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"T9PiSgns\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=SwXS3j/aLCRN4wLLnY2xU8hGA/UTbg+AMrCAY46Y618=;\n\tb=T9PiSgns/a4lbS0NGR4xsOTlLTG/qc+ImBgTZt13+wjm/FteZqg61CEj4YRMWqKuSh\n\t/DlhoHdicsuz9PFhv2L6TEJF6AHZJUqoJx3nNNxfKdb+xWk85iQ2/88b7RtHpsLtav9W\n\tRnGUsESNrQhsIM8eeikaJwTK/Q6TnjOdYN++o=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=SwXS3j/aLCRN4wLLnY2xU8hGA/UTbg+AMrCAY46Y618=;\n\tb=pNsZ7yCoxmSYL38cjerBFUkCWq3enVlDjxoWU9nAyJ5UJHop5PB2MHfiOZ7d8jULC0\n\tZv3Glqlf8GCR6Yrq+1e6URL+CuWVf1hOQOVPE2qjFSGdDnzMpCsfBA41Y8UUbtYuEKqt\n\trPFW8+Wt9tK0NFDZnDf3rP1URPXzj90r0srPUmk2BsJ+lqpu3kHxM0jf9U9H0KtpO9V9\n\tBnGuPxLtIHV9LHG6wFyTV3svBfYd2dko1pQLjJHdtUVRscmJ3rKw6fmPh+fmtzK2ppOo\n\tTRJBH0IYwHNgeP3i8DawoWHSJN7HWgGlsuBbNnv+kXwZjH1b6+8CGd+1hd3HQ2Cihn4A\n\t9Wvw==","X-Gm-Message-State":"AOAM5305fsFWPtUqIWM5f3ZofbDkcA8qI5qqMysWer3mqCybCLvKvnH7\n\t7YfnoDo9vMijIZFE231KQYhYJBr8XxQMZOPmceQoEg==","X-Google-Smtp-Source":"ABdhPJzj4/RuHCPsf72lvtu0zIBK5NxHgUTsxhZQRSwcZF4nWohCCH92mDEKGU48wOiXd+u8Lal4zh/d8ag/H2tsnQ8=","X-Received":"by 2002:a17:906:f208:: with SMTP id\n\tgt8mr33508273ejb.522.1634615822839; \n\tMon, 18 Oct 2021 20:57:02 -0700 (PDT)","MIME-Version":"1.0","References":"<20211018132923.476242-1-umang.jain@ideasonboard.com>\n\t<20211018132923.476242-2-umang.jain@ideasonboard.com>\n\t<20211018152610.5gw77vcjwvm6zbay@uno.localdomain>","In-Reply-To":"<20211018152610.5gw77vcjwvm6zbay@uno.localdomain>","From":"Hirokazu Honda ","Date":"Tue, 19 Oct 2021 12:56:51 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 01/11] camera_device: Remove private\n\tscope of Camera3RequestDescriptor","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]