[{"id":17315,"web_url":"https://patchwork.libcamera.org/comment/17315/","msgid":"","date":"2021-05-27T07:52:12","subject":"Re: [libcamera-devel] [PATCH v3 3/5] ipa: core: Move documentation\n\tfrom cpp file back into the mojom file","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nOn 5/27/21 12:58 PM, Paul Elder wrote:\n> Move the documentation back to the mojom file from the cpp file. While\n> at it, move the documentation for IPAInterface::init() and\n> IPAInterface::stop() to the IPA guide.\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n>\n> ---\n> Changes in v3:\n> - remove core_ipa_interface.cpp\nThere was also the change to update .mojom \\todo. Apologies that I \ndidn't notice and forgot to mention that in previous review. I have sent \none, in reply. Feel free to squash or retain as is, whatever you like.\n\nReviewed-by: Umang Jain \n> ---\n> Documentation/guides/ipa.rst | 6 +\n> include/libcamera/ipa/core.mojom | 200 +++++++++++++++++++++--\n> src/libcamera/ipa/core_ipa_interface.cpp | 199 ----------------------\n> 3 files changed, 190 insertions(+), 215 deletions(-)\n> delete mode 100644 src/libcamera/ipa/core_ipa_interface.cpp\n>\n> diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst\n> index 6195b2b7..c612470f 100644\n> --- a/Documentation/guides/ipa.rst\n> +++ b/Documentation/guides/ipa.rst\n> @@ -166,6 +166,12 @@ At a minimum, the following three functions must be present (and implemented):\n> All three of these functions are synchronous. The parameters for start() and\n> init() may be customized.\n> \n> +init() initializes the IPA interface. It shall be called before any other\n> +function of the IPAInterface.\n> +\n> +stop() informs the IPA module that the camera is stopped. The IPA module shall\n> +release resources prepared in start().\n> +\n> A configure() method is recommended. Any ControlInfoMap instances that will be\n> used by the IPA must be sent to the IPA from the pipeline handler, at configure\n> time, for example.\n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index 34a799c2..b32f3093 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -2,6 +2,11 @@\n> \n> module libcamera;\n> \n> +/**\n> + * \\file core_ipa_interface.h\n> + * \\brief libcamera structs for IPAs\n> + */\n> +\n> /*\n> * Things that can be defined here (and in other mojom files):\n> * - consts\n> @@ -78,6 +83,116 @@ module libcamera;\n> \tuint32 height;\n> };\n> \n> +/**\n> + * \\struct IPACameraSensorInfo\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 IPACameraSensorInfo::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 IPACameraSensorInfo::bitsPerPixel\n> + * \\brief The number of bits per pixel of the image format produced by the\n> + * image sensor\n> + */\n> +\n> +/**\n> + * \\var IPACameraSensorInfo::activeAreaSize\n> + * \\brief The size of the pixel array active area of the sensor\n> + */\n> +\n> +/**\n> + * \\var IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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> struct IPACameraSensorInfo {\n> \tstring model;\n> \n> @@ -94,35 +209,88 @@ struct IPACameraSensorInfo {\n> \tuint32 maxFrameLength;\n> };\n> \n> +/**\n> + * \\struct IPABuffer\n> + * \\brief Buffer information for the IPA interface\n> + *\n> + * The IPABuffer structure associates buffer memory with a unique ID. It is\n> + * used to map buffers to the IPA with IPAInterface::mapBuffers(), after which\n> + * buffers will be identified by their ID in the IPA interface.\n> + */\n> +\n> +/**\n> + * \\var IPABuffer::id\n> + * \\brief The buffer unique ID\n> + *\n> + * Buffers mapped to the IPA are identified by numerical unique IDs. The IDs\n> + * are chosen by the pipeline handler to fulfil the following constraints:\n> + *\n> + * - IDs shall be positive integers different than zero\n> + * - IDs shall be unique among all mapped buffers\n> + *\n> + * When buffers are unmapped with IPAInterface::unmapBuffers() their IDs are\n> + * freed and may be reused for new buffer mappings.\n> + */\n> +\n> +/**\n> + * \\var IPABuffer::planes\n> + * \\brief The buffer planes description\n> + *\n> + * Stores the dmabuf handle and length for each plane of the buffer.\n> + */\n> struct IPABuffer {\n> \tuint32 id;\n> \t[hasFd] array planes;\n> };\n> \n> +/**\n> + * \\struct IPASettings\n> + * \\brief IPA interface initialization settings\n> + *\n> + * The IPASettings structure stores data passed to the IPAInterface::init()\n> + * function. The data contains settings that don't depend on a particular camera\n> + * or pipeline configuration and are valid for the whole life time of the IPA\n> + * interface.\n> + */\n> +\n> +/**\n> + * \\var IPASettings::configurationFile\n> + * \\brief The name of the IPA configuration file\n> + *\n> + * This field may be an empty string if the IPA doesn't require a configuration\n> + * file.\n> + */\n> +\n> +/**\n> + * \\var IPASettings::sensorModel\n> + * \\brief The sensor model name\n> + *\n> + * Provides the sensor model name to the IPA.\n> + */\n> struct IPASettings {\n> \tstring configurationFile;\n> \tstring sensorModel;\n> };\n> \n> -struct IPAStream {\n> -\tuint32 pixelFormat;\n> -\tSize size;\n> -};\n> -\n> /**\n> - * \\fn IPAInterface::init()\n> - * \\brief Initialise the IPAInterface\n> - * \\param[in] settings The IPA initialization settings\n> + * \\struct IPAStream\n> + * \\brief Stream configuration for the IPA interface\n> *\n> - * This function initializes the IPA interface. It shall be called before any\n> - * other function of the IPAInterface. The \\a settings carry initialization\n> - * parameters that are valid for the whole life time of the IPA interface.\n> + * The IPAStream structure stores stream configuration parameters needed by the\n> + * IPAInterface::configure() method. It mirrors the StreamConfiguration class\n> + * that is not suitable for this purpose due to not being serializable.\n> */\n> \n> /**\n> - * \\fn IPAInterface::stop()\n> - * \\brief Stop the IPA\n> - *\n> - * This method informs the IPA module that the camera is stopped. The IPA module\n> - * shall release resources prepared in start().\n> + * \\var IPAStream::pixelFormat\n> + * \\brief The stream pixel format\n> + */\n> +\n> +/**\n> + * \\var IPAStream::size\n> + * \\brief The stream size in pixels\n> */\n> +struct IPAStream {\n> +\tuint32 pixelFormat;\n> +\tSize size;\n> +};\n> diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp\n> deleted file mode 100644\n> index 6ab689cd..00000000\n> --- a/src/libcamera/ipa/core_ipa_interface.cpp\n> +++ /dev/null\n> @@ -1,199 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2021, Google Inc.\n> - *\n> - * core_ipa_interface.cpp - Docs file for core.mojom generated header\n> - */\n> -\n> -namespace libcamera {\n> -\n> -/**\n> - * \\file core_ipa_interface.h\n> - * \\brief Core IPA inteface\n> - */\n> -\n> -/**\n> - * \\struct IPABuffer\n> - * \\brief Buffer information for the IPA interface\n> - *\n> - * The IPABuffer structure associates buffer memory with a unique ID. It is\n> - * used to map buffers to the IPA with IPAInterface::mapBuffers(), after which\n> - * buffers will be identified by their ID in the IPA interface.\n> - */\n> -\n> -/**\n> - * \\var IPABuffer::id\n> - * \\brief The buffer unique ID\n> - *\n> - * Buffers mapped to the IPA are identified by numerical unique IDs. The IDs\n> - * are chosen by the pipeline handler to fulfil the following constraints:\n> - *\n> - * - IDs shall be positive integers different than zero\n> - * - IDs shall be unique among all mapped buffers\n> - *\n> - * When buffers are unmapped with IPAInterface::unmapBuffers() their IDs are\n> - * freed and may be reused for new buffer mappings.\n> - */\n> -\n> -/**\n> - * \\var IPABuffer::planes\n> - * \\brief The buffer planes description\n> - *\n> - * Stores the dmabuf handle and length for each plane of the buffer.\n> - */\n> -\n> -/**\n> - * \\struct IPASettings\n> - * \\brief IPA interface initialization settings\n> - *\n> - * The IPASettings structure stores data passed to the IPAInterface::init()\n> - * function. The data contains settings that don't depend on a particular camera\n> - * or pipeline configuration and are valid for the whole life time of the IPA\n> - * interface.\n> - */\n> -\n> -/**\n> - * \\var IPASettings::configurationFile\n> - * \\brief The name of the IPA configuration file\n> - *\n> - * This field may be an empty string if the IPA doesn't require a configuration\n> - * file.\n> - */\n> -\n> -/**\n> - * \\var IPASettings::sensorModel\n> - * \\brief The sensor model name\n> - *\n> - * Provides the sensor model name to the IPA.\n> - */\n> -\n> -/**\n> - * \\struct IPAStream\n> - * \\brief Stream configuration for the IPA interface\n> - *\n> - * The IPAStream structure stores stream configuration parameters needed by the\n> - * IPAInterface::configure() method. It mirrors the StreamConfiguration class\n> - * that is not suitable for this purpose due to not being serializable.\n> - */\n> -\n> -/**\n> - * \\var IPAStream::pixelFormat\n> - * \\brief The stream pixel format\n> - */\n> -\n> -/**\n> - * \\var IPAStream::size\n> - * \\brief The stream size in pixels\n> - */\n> -\n> -/**\n> - * \\struct IPACameraSensorInfo\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 IPACameraSensorInfo::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 IPACameraSensorInfo::bitsPerPixel\n> - * \\brief The number of bits per pixel of the image format produced by the\n> - * image sensor\n> - */\n> -\n> -/**\n> - * \\var IPACameraSensorInfo::activeAreaSize\n> - * \\brief The size of the pixel array active area of the sensor\n> - */\n> -\n> -/**\n> - * \\var IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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 IPACameraSensorInfo::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 CFC74C3203\n\tfor ;\n\tThu, 27 May 2021 07:52:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3440D68925;\n\tThu, 27 May 2021 09:52:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E04F602AA\n\tfor ;\n\tThu, 27 May 2021 09:52:17 +0200 (CEST)","from localhost.localdomain (unknown [103.251.226.180])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 61C2E8DE;\n\tThu, 27 May 2021 09:52:16 +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=\"iK3LRCsB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622101937;\n\tbh=VmbB9yt0M1VGQvY78+hAJm2e92ga1/LDooyjIF/Iu1g=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=iK3LRCsB0HVy9LaL/gJjb2tkRLDFfBlP+jjr6uhFi5j5r2qDAX1lpqvZN0l1B3+Y6\n\tuafxb08c6WiJg2NlXEDY/OIEE6TKZL9sIsGrXQaTQXsVbPQxeYOIr0gyOR0rs2ETVI\n\tYN0Ivc20PRc5wDjsSqkvpNOotX6LXIdvvpBf9HrI=","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210527072805.1333870-1-paul.elder@ideasonboard.com>\n\t<20210527072805.1333870-4-paul.elder@ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Thu, 27 May 2021 13:22:12 +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":"<20210527072805.1333870-4-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 3/5] ipa: core: Move documentation\n\tfrom cpp file back into the mojom file","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]