[{"id":17149,"web_url":"https://patchwork.libcamera.org/comment/17149/","msgid":"","date":"2021-05-24T00:54:17","subject":"Re: [libcamera-devel] [PATCH v3 2/6] ipa: mojom: Move\n\tCameraSensorInfo struct exclusively to IPA IPC","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 Fri, May 21, 2021 at 06:58:19PM +0530, Umang Jain wrote:\n> CameraSensorInfo structure is designed to pass in camera sensor related\n> information from pipeline-handler to IPA. Since the pipeline-handler\n> and IPA are connected via mojom IPC IPA interface, the interface\n> itself provides a more suitable placement of CameraSensorInfo,\n> instead of camera_sensor.h (which is a libcamera internal header\n> ultimately, at this point).\n> \n> As CameraSensorInfo is already defined in core.mojom, it is just\n> a matter of removing [skipHeader] tag to allow code-generation\n> of CameraSensorInfo.\n> \n> Finally, update header paths to include CameraSensorInfo definition\n> from IPA interfaces instead of \"libcamera/internal/camera_sensor.h\".\n> \n> Signed-off-by: Umang Jain \n> Signed-off-by: Paul Elder \n> Reviewed-by: Jean-Michel Hautbois \n> Reviewed-by: Kieran Bingham \n> ---\n> include/libcamera/internal/camera_sensor.h | 17 +--\n> include/libcamera/ipa/core.mojom | 2 +-\n> include/libcamera/ipa/ipa_interface.h | 2 -\n> src/ipa/raspberrypi/raspberrypi.cpp | 1 -\n> src/libcamera/camera_sensor.cpp | 111 -----------------\n> src/libcamera/ipa/core_ipa_interface.cpp | 132 +++++++++++++++++++++\n> 6 files changed, 134 insertions(+), 131 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 2a5c51a1..0905ebfa 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/log.h\"\n> @@ -24,22 +25,6 @@ namespace libcamera {\n> class BayerFormat;\n> class MediaEntity;\n> \n> -struct CameraSensorInfo {\n> -\tstd::string model;\n> -\n> -\tuint32_t bitsPerPixel;\n> -\n> -\tSize activeAreaSize;\n> -\tRectangle analogCrop;\n> -\tSize outputSize;\n> -\n> -\tuint64_t pixelRate;\n> -\tuint32_t lineLength;\n> -\n> -\tuint32_t minFrameLength;\n> -\tuint32_t maxFrameLength;\n> -};\n> -\n> class CameraSensor : protected Loggable\n> {\n> public:\n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index e49815d8..b95b3dc4 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -78,7 +78,7 @@ module libcamera;\n> \tuint32 height;\n> };\n> \n> -[skipHeader] struct CameraSensorInfo {\n> +struct CameraSensorInfo {\n> \tstring model;\n> \n> \tuint32 bitsPerPixel;\n> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> index dfe1b40a..4aefaa71 100644\n> --- a/include/libcamera/ipa/ipa_interface.h\n> +++ b/include/libcamera/ipa/ipa_interface.h\n> @@ -18,8 +18,6 @@\n> #include \n> #include \n> \n> -#include \"libcamera/internal/camera_sensor.h\"\n> -\n> namespace libcamera {\n> \n> /*\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 52d91db2..87774500 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -25,7 +25,6 @@\n> #include \n> \n> #include \"libcamera/internal/buffer.h\"\n> -#include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/log.h\"\n> \n> #include \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index eb84d9eb..170de827 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -33,117 +33,6 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(CameraSensor)\n> \n> -/**\n> - * \\struct CameraSensorInfo\n> - * \\brief Report the image sensor characteristics\n> - *\n> - * The structure reports image sensor characteristics used by IPA modules to\n> - * tune their algorithms based on the image sensor model currently in use and\n> - * its configuration.\n> - *\n> - * The reported information describes the sensor's intrinsics characteristics,\n> - * such as its pixel array size and the sensor model name, as well as\n> - * information relative to the currently configured mode, such as the produced\n> - * image size and the bit depth of the requested image format.\n> - *\n> - * Instances of this structure are meant to be assembled by the CameraSensor\n> - * class by inspecting the sensor static properties as well as information\n> - * relative to the current configuration.\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::model\n> - * \\brief The image sensor model name\n> - *\n> - * The sensor model name is a free-formed string that uniquely identifies the\n> - * sensor model.\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::bitsPerPixel\n> - * \\brief The number of bits per pixel of the image format produced by the\n> - * image sensor\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::activeAreaSize\n> - * \\brief The size of the pixel array active area of the sensor\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::analogCrop\n> - * \\brief The portion of the pixel array active area which is read-out and\n> - * processed\n> - *\n> - * The analog crop rectangle top-left corner is defined as the displacement\n> - * from the top-left corner of the pixel array active area. The rectangle\n> - * horizontal and vertical sizes define the portion of the pixel array which\n> - * is read-out and provided to the sensor's internal processing pipeline, before\n> - * any pixel sub-sampling method, such as pixel binning, skipping and averaging\n> - * take place.\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::outputSize\n> - * \\brief The size of the images produced by the camera sensor\n> - *\n> - * The output image size defines the horizontal and vertical sizes of the images\n> - * produced by the image sensor. The output image size is defined as the end\n> - * result of the sensor's internal image processing pipeline stages, applied on\n> - * the pixel array portion defined by the analog crop rectangle. Each image\n> - * processing stage that performs pixel sub-sampling techniques, such as pixel\n> - * binning or skipping, or perform any additional digital scaling concur in the\n> - * definition of the output image size.\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::pixelRate\n> - * \\brief The number of pixels produced in a second\n> - *\n> - * To obtain the read-out time in seconds of a full line:\n> - *\n> - * \\verbatim\n> -\tlineDuration(s) = lineLength(pixels) / pixelRate(pixels per second)\n> - \\endverbatim\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::lineLength\n> - * \\brief Total line length in pixels\n> - *\n> - * The total line length in pixel clock periods, including blanking.\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::minFrameLength\n> - * \\brief The minimum allowable frame length in units of lines\n> - *\n> - * The sensor frame length comprises of active output lines and blanking lines\n> - * in a frame. The minimum frame length value dictates the minimum allowable\n> - * frame duration of the sensor mode.\n> - *\n> - * To obtain the minimum frame duration:\n> - *\n> - * \\verbatim\n> -\tframeDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> - \\endverbatim\n> - */\n> -\n> -/**\n> - * \\var CameraSensorInfo::maxFrameLength\n> - * \\brief The maximum allowable frame length in units of lines\n> - *\n> - * The sensor frame length comprises of active output lines and blanking lines\n> - * in a frame. The maximum frame length value dictates the maximum allowable\n> - * frame duration of the sensor mode.\n> - *\n> - * To obtain the maximum frame duration:\n> - *\n> - * \\verbatim\n> -\tframeDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> - \\endverbatim\n> - */\n> -\n> /**\n> * \\class CameraSensor\n> * \\brief A camera sensor based on V4L2 subdevices\n> diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp\n> index fe1ecce6..7fef2c36 100644\n> --- a/src/libcamera/ipa/core_ipa_interface.cpp\n> +++ b/src/libcamera/ipa/core_ipa_interface.cpp\n> @@ -102,4 +102,136 @@ namespace libcamera {\n> * \\brief The stream size in pixels\n> */\n> \n> +/**\n> + * \\struct CameraSensorInfo\n> + * \\brief Report the image sensor characteristics\n> + *\n> + * The structure reports image sensor characteristics used by IPA modules to\n> + * tune their algorithms based on the image sensor model currently in use and\n> + * its configuration.\n> + *\n> + * The reported information describes the sensor's intrinsics characteristics,\n> + * such as its pixel array size and the sensor model name, as well as\n> + * information relative to the currently configured mode, such as the produced\n> + * image size and the bit depth of the requested image format.\n> + *\n> + * Instances of this structure are meant to be assembled by the CameraSensor\n> + * class by inspecting the sensor static properties as well as information\n> + * relative to the current configuration.\n> + */\n> +\n> +/**\n> + * \\fn CameraSensorInfo::CameraSensorInfo(const std::string &model,\n> + * uint32_t bitsPerPixel,\n> + * const Size &activeAreaSize,\n> + * const Rectangle &analogCrop,\n> + * const Size &outputSize,\n> + * uint64_t pixelRate,\n> + * uint32_t lineLength,\n> + * uint32_t minFrameLength,\n> + * uint32_t maxFrameLength)\n> + *\n> + * \\param[in] model\n> + * \\param[in] bitsPerPixel\n> + * \\param[in] activeAreaSize\n> + * \\param[in] analogCrop\n> + * \\param[in] outputSize\n> + * \\param[in] pixelRate\n> + * \\param[in] lineLength\n> + * \\param[in] minFrameLength\n> + * \\param[in] maxFrameLength\n\nGiven that this provides very little value in the documentation, I\nwonder if we shouldn't hide those constructors. What do you think ? If\nyou agree, the following would address the issue.\n\ndiff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\nindex cdd75f89e384..94bb49181a35 100644\n--- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n+++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n@@ -25,6 +25,7 @@ enum {{enum.mojom_name}} {\n struct {{struct.mojom_name}}\n {\n public:\n+#ifndef __DOXYGEN__\n \t{{struct.mojom_name}}() {%- if struct|has_default_fields %}\n \t\t:{% endif %}\n {%- for field in struct.fields|with_default_values -%}\n@@ -44,6 +45,8 @@ public:\n {%- endfor %}\n \t{\n \t}\n+#endif\n+\n {% for field in struct.fields %}\n \t{{field|name}} {{field.mojom_name}};\n {%- endfor %}\n\nReviewed-by: Laurent Pinchart \n\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::model\n> + * \\brief The image sensor model name\n> + *\n> + * The sensor model name is a free-formed string that uniquely identifies the\n> + * sensor model.\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::bitsPerPixel\n> + * \\brief The number of bits per pixel of the image format produced by the\n> + * image sensor\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::activeAreaSize\n> + * \\brief The size of the pixel array active area of the sensor\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::analogCrop\n> + * \\brief The portion of the pixel array active area which is read-out and\n> + * processed\n> + *\n> + * The analog crop rectangle top-left corner is defined as the displacement\n> + * from the top-left corner of the pixel array active area. The rectangle\n> + * horizontal and vertical sizes define the portion of the pixel array which\n> + * is read-out and provided to the sensor's internal processing pipeline, before\n> + * any pixel sub-sampling method, such as pixel binning, skipping and averaging\n> + * take place.\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::outputSize\n> + * \\brief The size of the images produced by the camera sensor\n> + *\n> + * The output image size defines the horizontal and vertical sizes of the images\n> + * produced by the image sensor. The output image size is defined as the end\n> + * result of the sensor's internal image processing pipeline stages, applied on\n> + * the pixel array portion defined by the analog crop rectangle. Each image\n> + * processing stage that performs pixel sub-sampling techniques, such as pixel\n> + * binning or skipping, or perform any additional digital scaling concur in the\n> + * definition of the output image size.\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::pixelRate\n> + * \\brief The number of pixels produced in a second\n> + *\n> + * To obtain the read-out time in seconds of a full line:\n> + *\n> + * \\verbatim\n> + lineDuration(s) = lineLength(pixels) / pixelRate(pixels per second)\n> + \\endverbatim\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::lineLength\n> + * \\brief Total line length in pixels\n> + *\n> + * The total line length in pixel clock periods, including blanking.\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::minFrameLength\n> + * \\brief The minimum allowable frame length in units of lines\n> + *\n> + * The sensor frame length comprises of active output lines and blanking lines\n> + * in a frame. The minimum frame length value dictates the minimum allowable\n> + * frame duration of the sensor mode.\n> + *\n> + * To obtain the minimum frame duration:\n> + *\n> + * \\verbatim\n> + frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> + \\endverbatim\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::maxFrameLength\n> + * \\brief The maximum allowable frame length in units of lines\n> + *\n> + * The sensor frame length comprises of active output lines and blanking lines\n> + * in a frame. The maximum frame length value dictates the maximum allowable\n> + * frame duration of the sensor mode.\n> + *\n> + * To obtain the maximum frame duration:\n> + *\n> + * \\verbatim\n> + frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> + \\endverbatim\n> + */\n> } /* namespace libcamera */","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 C4936C3201\n\tfor ;\n\tMon, 24 May 2021 00:54:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 297C96891E;\n\tMon, 24 May 2021 02:54:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 783F36050F\n\tfor ;\n\tMon, 24 May 2021 02:54:21 +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 E5B2E476;\n\tMon, 24 May 2021 02:54:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ghE/SO2I\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621817661;\n\tbh=CUXCaJpUDeoenVo5KU++F7nYbKgTah5wOYkEnNAHVuU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ghE/SO2IaYkFUtKSforCIchJncBt2zAOUDe8V/GNSsxqN07PDUdwCyEcLFMFpmVQ4\n\tKcRu1i8NswPUYUlDB6JarmhBom5J2MHHQVfJRZMrnI7RL5JSXo2CgpUyVfzEevjj4L\n\tVZRprbaS+0dJIDauDREgz01bs84X2h5LeQx3w5v4=","Date":"Mon, 24 May 2021 03:54:17 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210521132823.322076-1-umang.jain@ideasonboard.com>\n\t<20210521132823.322076-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210521132823.322076-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/6] ipa: mojom: Move\n\tCameraSensorInfo struct exclusively to IPA IPC","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":17175,"web_url":"https://patchwork.libcamera.org/comment/17175/","msgid":"","date":"2021-05-24T08:23:51","subject":"Re: [libcamera-devel] [PATCH v3 2/6] ipa: mojom: Move\n\tCameraSensorInfo struct exclusively to IPA IPC","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 5/24/21 6:24 AM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Fri, May 21, 2021 at 06:58:19PM +0530, Umang Jain wrote:\n>> CameraSensorInfo structure is designed to pass in camera sensor related\n>> information from pipeline-handler to IPA. Since the pipeline-handler\n>> and IPA are connected via mojom IPC IPA interface, the interface\n>> itself provides a more suitable placement of CameraSensorInfo,\n>> instead of camera_sensor.h (which is a libcamera internal header\n>> ultimately, at this point).\n>>\n>> As CameraSensorInfo is already defined in core.mojom, it is just\n>> a matter of removing [skipHeader] tag to allow code-generation\n>> of CameraSensorInfo.\n>>\n>> Finally, update header paths to include CameraSensorInfo definition\n>> from IPA interfaces instead of \"libcamera/internal/camera_sensor.h\".\n>>\n>> Signed-off-by: Umang Jain \n>> Signed-off-by: Paul Elder \n>> Reviewed-by: Jean-Michel Hautbois \n>> Reviewed-by: Kieran Bingham \n>> ---\n>> include/libcamera/internal/camera_sensor.h | 17 +--\n>> include/libcamera/ipa/core.mojom | 2 +-\n>> include/libcamera/ipa/ipa_interface.h | 2 -\n>> src/ipa/raspberrypi/raspberrypi.cpp | 1 -\n>> src/libcamera/camera_sensor.cpp | 111 -----------------\n>> src/libcamera/ipa/core_ipa_interface.cpp | 132 +++++++++++++++++++++\n>> 6 files changed, 134 insertions(+), 131 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n>> index 2a5c51a1..0905ebfa 100644\n>> --- a/include/libcamera/internal/camera_sensor.h\n>> +++ b/include/libcamera/internal/camera_sensor.h\n>> @@ -14,6 +14,7 @@\n>> #include \n>> #include \n>> #include \n>> +#include \n>> \n>> #include \"libcamera/internal/formats.h\"\n>> #include \"libcamera/internal/log.h\"\n>> @@ -24,22 +25,6 @@ namespace libcamera {\n>> class BayerFormat;\n>> class MediaEntity;\n>> \n>> -struct CameraSensorInfo {\n>> -\tstd::string model;\n>> -\n>> -\tuint32_t bitsPerPixel;\n>> -\n>> -\tSize activeAreaSize;\n>> -\tRectangle analogCrop;\n>> -\tSize outputSize;\n>> -\n>> -\tuint64_t pixelRate;\n>> -\tuint32_t lineLength;\n>> -\n>> -\tuint32_t minFrameLength;\n>> -\tuint32_t maxFrameLength;\n>> -};\n>> -\n>> class CameraSensor : protected Loggable\n>> {\n>> public:\n>> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n>> index e49815d8..b95b3dc4 100644\n>> --- a/include/libcamera/ipa/core.mojom\n>> +++ b/include/libcamera/ipa/core.mojom\n>> @@ -78,7 +78,7 @@ module libcamera;\n>> \tuint32 height;\n>> };\n>> \n>> -[skipHeader] struct CameraSensorInfo {\n>> +struct CameraSensorInfo {\n>> \tstring model;\n>> \n>> \tuint32 bitsPerPixel;\n>> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n>> index dfe1b40a..4aefaa71 100644\n>> --- a/include/libcamera/ipa/ipa_interface.h\n>> +++ b/include/libcamera/ipa/ipa_interface.h\n>> @@ -18,8 +18,6 @@\n>> #include \n>> #include \n>> \n>> -#include \"libcamera/internal/camera_sensor.h\"\n>> -\n>> namespace libcamera {\n>> \n>> /*\n>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>> index 52d91db2..87774500 100644\n>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>> @@ -25,7 +25,6 @@\n>> #include \n>> \n>> #include \"libcamera/internal/buffer.h\"\n>> -#include \"libcamera/internal/camera_sensor.h\"\n>> #include \"libcamera/internal/log.h\"\n>> \n>> #include \n>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n>> index eb84d9eb..170de827 100644\n>> --- a/src/libcamera/camera_sensor.cpp\n>> +++ b/src/libcamera/camera_sensor.cpp\n>> @@ -33,117 +33,6 @@ namespace libcamera {\n>> \n>> LOG_DEFINE_CATEGORY(CameraSensor)\n>> \n>> -/**\n>> - * \\struct CameraSensorInfo\n>> - * \\brief Report the image sensor characteristics\n>> - *\n>> - * The structure reports image sensor characteristics used by IPA modules to\n>> - * tune their algorithms based on the image sensor model currently in use and\n>> - * its configuration.\n>> - *\n>> - * The reported information describes the sensor's intrinsics characteristics,\n>> - * such as its pixel array size and the sensor model name, as well as\n>> - * information relative to the currently configured mode, such as the produced\n>> - * image size and the bit depth of the requested image format.\n>> - *\n>> - * Instances of this structure are meant to be assembled by the CameraSensor\n>> - * class by inspecting the sensor static properties as well as information\n>> - * relative to the current configuration.\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::model\n>> - * \\brief The image sensor model name\n>> - *\n>> - * The sensor model name is a free-formed string that uniquely identifies the\n>> - * sensor model.\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::bitsPerPixel\n>> - * \\brief The number of bits per pixel of the image format produced by the\n>> - * image sensor\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::activeAreaSize\n>> - * \\brief The size of the pixel array active area of the sensor\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::analogCrop\n>> - * \\brief The portion of the pixel array active area which is read-out and\n>> - * processed\n>> - *\n>> - * The analog crop rectangle top-left corner is defined as the displacement\n>> - * from the top-left corner of the pixel array active area. The rectangle\n>> - * horizontal and vertical sizes define the portion of the pixel array which\n>> - * is read-out and provided to the sensor's internal processing pipeline, before\n>> - * any pixel sub-sampling method, such as pixel binning, skipping and averaging\n>> - * take place.\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::outputSize\n>> - * \\brief The size of the images produced by the camera sensor\n>> - *\n>> - * The output image size defines the horizontal and vertical sizes of the images\n>> - * produced by the image sensor. The output image size is defined as the end\n>> - * result of the sensor's internal image processing pipeline stages, applied on\n>> - * the pixel array portion defined by the analog crop rectangle. Each image\n>> - * processing stage that performs pixel sub-sampling techniques, such as pixel\n>> - * binning or skipping, or perform any additional digital scaling concur in the\n>> - * definition of the output image size.\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::pixelRate\n>> - * \\brief The number of pixels produced in a second\n>> - *\n>> - * To obtain the read-out time in seconds of a full line:\n>> - *\n>> - * \\verbatim\n>> -\tlineDuration(s) = lineLength(pixels) / pixelRate(pixels per second)\n>> - \\endverbatim\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::lineLength\n>> - * \\brief Total line length in pixels\n>> - *\n>> - * The total line length in pixel clock periods, including blanking.\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::minFrameLength\n>> - * \\brief The minimum allowable frame length in units of lines\n>> - *\n>> - * The sensor frame length comprises of active output lines and blanking lines\n>> - * in a frame. The minimum frame length value dictates the minimum allowable\n>> - * frame duration of the sensor mode.\n>> - *\n>> - * To obtain the minimum frame duration:\n>> - *\n>> - * \\verbatim\n>> -\tframeDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n>> - \\endverbatim\n>> - */\n>> -\n>> -/**\n>> - * \\var CameraSensorInfo::maxFrameLength\n>> - * \\brief The maximum allowable frame length in units of lines\n>> - *\n>> - * The sensor frame length comprises of active output lines and blanking lines\n>> - * in a frame. The maximum frame length value dictates the maximum allowable\n>> - * frame duration of the sensor mode.\n>> - *\n>> - * To obtain the maximum frame duration:\n>> - *\n>> - * \\verbatim\n>> -\tframeDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n>> - \\endverbatim\n>> - */\n>> -\n>> /**\n>> * \\class CameraSensor\n>> * \\brief A camera sensor based on V4L2 subdevices\n>> diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp\n>> index fe1ecce6..7fef2c36 100644\n>> --- a/src/libcamera/ipa/core_ipa_interface.cpp\n>> +++ b/src/libcamera/ipa/core_ipa_interface.cpp\n>> @@ -102,4 +102,136 @@ namespace libcamera {\n>> * \\brief The stream size in pixels\n>> */\n>> \n>> +/**\n>> + * \\struct CameraSensorInfo\n>> + * \\brief Report the image sensor characteristics\n>> + *\n>> + * The structure reports image sensor characteristics used by IPA modules to\n>> + * tune their algorithms based on the image sensor model currently in use and\n>> + * its configuration.\n>> + *\n>> + * The reported information describes the sensor's intrinsics characteristics,\n>> + * such as its pixel array size and the sensor model name, as well as\n>> + * information relative to the currently configured mode, such as the produced\n>> + * image size and the bit depth of the requested image format.\n>> + *\n>> + * Instances of this structure are meant to be assembled by the CameraSensor\n>> + * class by inspecting the sensor static properties as well as information\n>> + * relative to the current configuration.\n>> + */\n>> +\n>> +/**\n>> + * \\fn CameraSensorInfo::CameraSensorInfo(const std::string &model,\n>> + * uint32_t bitsPerPixel,\n>> + * const Size &activeAreaSize,\n>> + * const Rectangle &analogCrop,\n>> + * const Size &outputSize,\n>> + * uint64_t pixelRate,\n>> + * uint32_t lineLength,\n>> + * uint32_t minFrameLength,\n>> + * uint32_t maxFrameLength)\n>> + *\n>> + * \\param[in] model\n>> + * \\param[in] bitsPerPixel\n>> + * \\param[in] activeAreaSize\n>> + * \\param[in] analogCrop\n>> + * \\param[in] outputSize\n>> + * \\param[in] pixelRate\n>> + * \\param[in] lineLength\n>> + * \\param[in] minFrameLength\n>> + * \\param[in] maxFrameLength\n> Given that this provides very little value in the documentation, I\n> wonder if we shouldn't hide those constructors. What do you think ? If\n> you agree, the following would address the issue.\n\nYes, I agree with this premise. I am actually going to squash the diff \nbelow with Patch 1/6 instead, as it will also eliminate the need of \ndocumenting IPAStream IPABuffer and IPASetting constructors there\n>\n> diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> index cdd75f89e384..94bb49181a35 100644\n> --- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> @@ -25,6 +25,7 @@ enum {{enum.mojom_name}} {\n> struct {{struct.mojom_name}}\n> {\n> public:\n> +#ifndef __DOXYGEN__\n> \t{{struct.mojom_name}}() {%- if struct|has_default_fields %}\n> \t\t:{% endif %}\n> {%- for field in struct.fields|with_default_values -%}\n> @@ -44,6 +45,8 @@ public:\n> {%- endfor %}\n> \t{\n> \t}\n> +#endif\n> +\n> {% for field in struct.fields %}\n> \t{{field|name}} {{field.mojom_name}};\n> {%- endfor %}\n>\n> Reviewed-by: Laurent Pinchart \n>\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::model\n>> + * \\brief The image sensor model name\n>> + *\n>> + * The sensor model name is a free-formed string that uniquely identifies the\n>> + * sensor model.\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::bitsPerPixel\n>> + * \\brief The number of bits per pixel of the image format produced by the\n>> + * image sensor\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::activeAreaSize\n>> + * \\brief The size of the pixel array active area of the sensor\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::analogCrop\n>> + * \\brief The portion of the pixel array active area which is read-out and\n>> + * processed\n>> + *\n>> + * The analog crop rectangle top-left corner is defined as the displacement\n>> + * from the top-left corner of the pixel array active area. The rectangle\n>> + * horizontal and vertical sizes define the portion of the pixel array which\n>> + * is read-out and provided to the sensor's internal processing pipeline, before\n>> + * any pixel sub-sampling method, such as pixel binning, skipping and averaging\n>> + * take place.\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::outputSize\n>> + * \\brief The size of the images produced by the camera sensor\n>> + *\n>> + * The output image size defines the horizontal and vertical sizes of the images\n>> + * produced by the image sensor. The output image size is defined as the end\n>> + * result of the sensor's internal image processing pipeline stages, applied on\n>> + * the pixel array portion defined by the analog crop rectangle. Each image\n>> + * processing stage that performs pixel sub-sampling techniques, such as pixel\n>> + * binning or skipping, or perform any additional digital scaling concur in the\n>> + * definition of the output image size.\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::pixelRate\n>> + * \\brief The number of pixels produced in a second\n>> + *\n>> + * To obtain the read-out time in seconds of a full line:\n>> + *\n>> + * \\verbatim\n>> + lineDuration(s) = lineLength(pixels) / pixelRate(pixels per second)\n>> + \\endverbatim\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::lineLength\n>> + * \\brief Total line length in pixels\n>> + *\n>> + * The total line length in pixel clock periods, including blanking.\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::minFrameLength\n>> + * \\brief The minimum allowable frame length in units of lines\n>> + *\n>> + * The sensor frame length comprises of active output lines and blanking lines\n>> + * in a frame. The minimum frame length value dictates the minimum allowable\n>> + * frame duration of the sensor mode.\n>> + *\n>> + * To obtain the minimum frame duration:\n>> + *\n>> + * \\verbatim\n>> + frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n>> + \\endverbatim\n>> + */\n>> +\n>> +/**\n>> + * \\var CameraSensorInfo::maxFrameLength\n>> + * \\brief The maximum allowable frame length in units of lines\n>> + *\n>> + * The sensor frame length comprises of active output lines and blanking lines\n>> + * in a frame. The maximum frame length value dictates the maximum allowable\n>> + * frame duration of the sensor mode.\n>> + *\n>> + * To obtain the maximum frame duration:\n>> + *\n>> + * \\verbatim\n>> + frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n>> + \\endverbatim\n>> + */\n>> } /* namespace libcamera */","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 1F1FAC3200\n\tfor ;\n\tMon, 24 May 2021 08:24:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 778736891E;\n\tMon, 24 May 2021 10:24:00 +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 98C3F602B1\n\tfor ;\n\tMon, 24 May 2021 10:23:58 +0200 (CEST)","from localhost.localdomain (unknown [103.251.226.203])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 336EF1315;\n\tMon, 24 May 2021 10:23:56 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"b3nf+tKH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621844638;\n\tbh=T7W6Ql2am3g35T7cZwElVDZMOz275wSOtIWeGFFrHsM=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=b3nf+tKHTM4Qf18CfkCAWbODSnWxCe2S5fQIt2JS8cyAV6M5xpi0qsARuWw29f5mu\n\t6p1acQqMKMcYRXJGOZE63ikdrCkQlUoCMUwbq88LHdIo1X4edjck/wqf+4u9l8k2Jd\n\tF17Z4avVCdLk1MGwTrP+Ft8LgtaGyoSvKqiOVCtc=","To":"Laurent Pinchart ","References":"<20210521132823.322076-1-umang.jain@ideasonboard.com>\n\t<20210521132823.322076-3-umang.jain@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"","Date":"Mon, 24 May 2021 13:53:51 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.11.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 2/6] ipa: mojom: Move\n\tCameraSensorInfo struct exclusively to IPA IPC","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\" "}}]