[{"id":20535,"web_url":"https://patchwork.libcamera.org/comment/20535/","msgid":"","date":"2021-10-26T10:19:50","subject":"Re: [libcamera-devel] [PATCH v4 03/19] ipa: ipu3: Document the\n\tIPAIPU3 class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Tue, Oct 26, 2021 at 11:55:18AM +0200, Jean-Michel Hautbois wrote:\n> Clarify the roles and interactions between the pipeline handler events\n> and the algorithm calls by documenting all the remaining functions of\n> the class.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 87 +++++++++++++++++++++++++++++++++++++------\n> 1 file changed, 76 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 1f501821..177b67b0 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -305,9 +305,9 @@ private:\n> \tstruct IPAContext context_;\n> };\n> \n> -/*\n> - * Compute IPASessionConfiguration using the sensor information and the sensor\n> - * v4l2 controls.\n> +/**\n> + * \\brief Compute IPASessionConfiguration using the sensor information and the\n> + * sensor V4L2 controls\n> */\n> void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,\n> \t\t\t\t\t const ControlInfoMap &sensorControls)\n> @@ -336,9 +336,9 @@ void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,\n> \tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n> }\n> \n> -/*\n> - * Compute camera controls using the sensor information and the sensor\n> - * v4l2 controls.\n> +/**\n> + * \\brief Compute camera controls using the sensor information and the sensor\n> + * V4L2 controls\n> *\n> * Some of the camera controls are computed by the pipeline handler, some others\n> * by the IPA module which is in charge of handling, for example, the exposure\n> @@ -399,7 +399,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> }\n> \n> /**\n> - * Initialize the IPA module and its controls.\n> + * \\brief Initialize the IPA module and its controls\n> *\n> * This function receives the camera sensor information from the pipeline\n> * handler, computes the limits of the controls it handles and returns\n> @@ -430,8 +430,15 @@ int IPAIPU3::init(const IPASettings &settings,\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Perform any processing required before the first frame\n> + */\n> int IPAIPU3::start()\n> {\n> +\t/*\n> +\t * Set the sensors V4L2 controls before the first frame to ensure that\n> +\t * we have an expected and known configuration from the start.\n> +\t */\n> \tsetControls(0);\n> \n> \treturn 0;\n> @@ -512,7 +519,7 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> * handler\n> * \\param[in] ipaControls The IPA controls to update\n> *\n> - * Calculate the best grid for the statistics based on the Pipeline Handler BDS\n> + * Calculate the best grid for the statistics based on the pipeline handler BDS\n> * output, and parse the minimum and maximum exposure and analogue gain control\n> * values.\n> *\n> @@ -585,6 +592,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Map the parameters and stats buffers allocated in the pipeline handler\n> + * \\param[in] buffers The buffers to map\n> + */\n> void IPAIPU3::mapBuffers(const std::vector &buffers)\n> {\n> \tfor (const IPABuffer &buffer : buffers) {\n> @@ -594,6 +605,10 @@ void IPAIPU3::mapBuffers(const std::vector &buffers)\n> \t}\n> }\n> \n> +/**\n> + * \\brief Unmap the parameters and stats buffers\n> + * \\param[in] ids The IDs of the buffers to unmap\n> + */\n> void IPAIPU3::unmapBuffers(const std::vector &ids)\n> {\n> \tfor (unsigned int id : ids) {\n> @@ -605,6 +620,10 @@ void IPAIPU3::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> +/**\n> + * \\brief Process an event generated by the pipeline handler\n> + * \\param[in] event The event sent from pipeline handler\n> + */\n> void IPAIPU3::processEvent(const IPU3Event &event)\n> {\n> \tswitch (event.op) {\n> @@ -646,12 +665,28 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n> \t}\n> }\n> \n> +/**\n> + * \\brief Process a control list for a request from the application\n> + * \\param[in] frame The number of the frame which will be processed next\n> + * \\param[in] controls The controls for the \\a frame\n> + *\n> + * Parse the request to handle any IPA-managed controls that were set from the\n> + * application such as manual sensor settings.\n> + */\n> void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n> \t\t\t [[maybe_unused]] const ControlList &controls)\n> {\n> \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> }\n> \n> +/**\n> + * \\brief Fill the ImgU parameter buffer for the next frame\n> + * \\param[in] frame The number of the latest frame processed\n> + * \\param[out] params The parameter buffer to fill\n> + *\n> + * Algorithms are expected to fill the IPU3 parameter buffer for the next\n> + * frame given their most recent processing of the ImgU statistics.\n> + */\n> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> {\n> \t/*\n> @@ -674,6 +709,16 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> \tqueueFrameAction.emit(frame, op);\n> }\n> \n> +/**\n> + * \\brief Process the statistics generated by the ImgU\n> + * \\param[in] frame The number of the latest frame processed\n> + * \\param[in] frameTimestamp The current frame timestamp\n> + * \\param[in] stats The IPU3 statistics and ISP results\n> + *\n> + * Parse the most recently processed image statistics from the ImgU. The\n> + * statistics are passed to each algorithm module to run their calculations and\n> + * update their state accordingly.\n> + */\n> void IPAIPU3::parseStatistics(unsigned int frame,\n> \t\t\t [[maybe_unused]] int64_t frameTimestamp,\n> \t\t\t [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> @@ -697,6 +742,13 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> \tqueueFrameAction.emit(frame, op);\n> }\n> \n> +/**\n> + * \\brief Handle sensor controls for a given \\a frame number\n> + * \\param[in] frame The frame on which the sensor controls should be set\n> + *\n> + * Send the desired sensor control values to the pipeline handler to request\n> + * that they are applied on the camera sensor.\n> + */\n> void IPAIPU3::setControls(unsigned int frame)\n> {\n> \tIPU3Action op;\n> @@ -715,10 +767,15 @@ void IPAIPU3::setControls(unsigned int frame)\n> \n> } /* namespace ipa::ipu3 */\n> \n> -/*\n> - * External IPA module interface\n> +/**\n> + * \\brief External IPA module interface\n> + *\n> + * The IPAModuleInfo is required to match an IPA module construction against the\n> + * intented pipeline handler with the module. The API and pipeline handler\n> + * versions must match the corresponding IPA interface and pipeline handler.\n> + *\n> + * \\sa struct IPAModuleInfo\n> */\n> -\n> extern \"C\" {\n> const struct IPAModuleInfo ipaModuleInfo = {\n> \tIPA_MODULE_API_VERSION,\n> @@ -727,6 +784,14 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \t\"ipu3\",\n> };\n> \n> +/**\n> + * \\brief Create an instance of the IPA interface\n> + *\n> + * This function is the entry point of the IPA module. It is called by the IPA\n> + * manager to create an instance of the IPA interface for each camera. When\n> + * matched against with a pipeline handler, the IPAManager will construct an IPA\n> + * instance for each associated Camera.\n> + */\n> IPAInterface *ipaCreate()\n> {\n> \treturn new ipa::ipu3::IPAIPU3();","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 D619ABDB1C\n\tfor ;\n\tTue, 26 Oct 2021 10:20:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 491E960124;\n\tTue, 26 Oct 2021 12:20:15 +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 39B2A60123\n\tfor ;\n\tTue, 26 Oct 2021 12:20:13 +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 A76233F0;\n\tTue, 26 Oct 2021 12:20:12 +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=\"An4yTuvG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635243612;\n\tbh=6G/W+UYUQERh2i3oTpfbVZ4RHo3XmFk+7fFNqEt6ocg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=An4yTuvGlniFgBvKGI2vDpCgorb2pl3wSQB4v3ptpzvzCWqQ1sr9920sHVndmExw9\n\t7/G7zRbkNcNprCwPyo//49As6G/34INKRPDL+K0hTWgNoyBEqmCtSiNS7ZU1t3tAc/\n\tB/EvQAo3gTknyxKphizkJErrH0c5KId2o026F960=","Date":"Tue, 26 Oct 2021 13:19:50 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211026095534.90348-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211026095534.90348-4-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211026095534.90348-4-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 03/19] ipa: ipu3: Document the\n\tIPAIPU3 class","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":20539,"web_url":"https://patchwork.libcamera.org/comment/20539/","msgid":"<163524433287.1182057.12494502703647615561@Monstersaurus>","date":"2021-10-26T10:32:12","subject":"Re: [libcamera-devel] [PATCH v4 03/19] ipa: ipu3: Document the\n\tIPAIPU3 class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-10-26 10:55:18)\n> Clarify the roles and interactions between the pipeline handler events\n> and the algorithm calls by documenting all the remaining functions of\n> the class.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 87 +++++++++++++++++++++++++++++++++++++------\n> 1 file changed, 76 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 1f501821..177b67b0 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -305,9 +305,9 @@ private:\n> struct IPAContext context_;\n> };\n> \n> -/*\n> - * Compute IPASessionConfiguration using the sensor information and the sensor\n> - * v4l2 controls.\n> +/**\n> + * \\brief Compute IPASessionConfiguration using the sensor information and the\n> + * sensor V4L2 controls\n> */\n> void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,\n> const ControlInfoMap &sensorControls)\n> @@ -336,9 +336,9 @@ void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,\n> context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n> }\n> \n> -/*\n> - * Compute camera controls using the sensor information and the sensor\n> - * v4l2 controls.\n> +/**\n> + * \\brief Compute camera controls using the sensor information and the sensor\n> + * V4L2 controls\n> *\n> * Some of the camera controls are computed by the pipeline handler, some others\n> * by the IPA module which is in charge of handling, for example, the exposure\n> @@ -399,7 +399,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> }\n> \n> /**\n> - * Initialize the IPA module and its controls.\n> + * \\brief Initialize the IPA module and its controls\n> *\n> * This function receives the camera sensor information from the pipeline\n> * handler, computes the limits of the controls it handles and returns\n> @@ -430,8 +430,15 @@ int IPAIPU3::init(const IPASettings &settings,\n> return 0;\n> }\n> \n> +/**\n> + * \\brief Perform any processing required before the first frame\n> + */\n> int IPAIPU3::start()\n> {\n> + /*\n> + * Set the sensors V4L2 controls before the first frame to ensure that\n> + * we have an expected and known configuration from the start.\n> + */\n> setControls(0);\n> \n> return 0;\n> @@ -512,7 +519,7 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> * handler\n> * \\param[in] ipaControls The IPA controls to update\n> *\n> - * Calculate the best grid for the statistics based on the Pipeline Handler BDS\n> + * Calculate the best grid for the statistics based on the pipeline handler BDS\n> * output, and parse the minimum and maximum exposure and analogue gain control\n> * values.\n> *\n> @@ -585,6 +592,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> return 0;\n> }\n> \n> +/**\n> + * \\brief Map the parameters and stats buffers allocated in the pipeline handler\n> + * \\param[in] buffers The buffers to map\n> + */\n> void IPAIPU3::mapBuffers(const std::vector &buffers)\n> {\n> for (const IPABuffer &buffer : buffers) {\n> @@ -594,6 +605,10 @@ void IPAIPU3::mapBuffers(const std::vector &buffers)\n> }\n> }\n> \n> +/**\n> + * \\brief Unmap the parameters and stats buffers\n> + * \\param[in] ids The IDs of the buffers to unmap\n> + */\n> void IPAIPU3::unmapBuffers(const std::vector &ids)\n> {\n> for (unsigned int id : ids) {\n> @@ -605,6 +620,10 @@ void IPAIPU3::unmapBuffers(const std::vector &ids)\n> }\n> }\n> \n> +/**\n> + * \\brief Process an event generated by the pipeline handler\n> + * \\param[in] event The event sent from pipeline handler\n> + */\n> void IPAIPU3::processEvent(const IPU3Event &event)\n> {\n> switch (event.op) {\n> @@ -646,12 +665,28 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n> }\n> }\n> \n> +/**\n> + * \\brief Process a control list for a request from the application\n> + * \\param[in] frame The number of the frame which will be processed next\n> + * \\param[in] controls The controls for the \\a frame\n> + *\n> + * Parse the request to handle any IPA-managed controls that were set from the\n> + * application such as manual sensor settings.\n> + */\n> void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n> [[maybe_unused]] const ControlList &controls)\n> {\n> /* \\todo Start processing for 'frame' based on 'controls'. */\n> }\n> \n> +/**\n> + * \\brief Fill the ImgU parameter buffer for the next frame\n> + * \\param[in] frame The number of the latest frame processed\n> + * \\param[out] params The parameter buffer to fill\n> + *\n> + * Algorithms are expected to fill the IPU3 parameter buffer for the next\n> + * frame given their most recent processing of the ImgU statistics.\n> + */\n> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> {\n> /*\n> @@ -674,6 +709,16 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> queueFrameAction.emit(frame, op);\n> }\n> \n> +/**\n> + * \\brief Process the statistics generated by the ImgU\n> + * \\param[in] frame The number of the latest frame processed\n> + * \\param[in] frameTimestamp The current frame timestamp\n> + * \\param[in] stats The IPU3 statistics and ISP results\n> + *\n> + * Parse the most recently processed image statistics from the ImgU. The\n> + * statistics are passed to each algorithm module to run their calculations and\n> + * update their state accordingly.\n> + */\n> void IPAIPU3::parseStatistics(unsigned int frame,\n> [[maybe_unused]] int64_t frameTimestamp,\n> [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> @@ -697,6 +742,13 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> queueFrameAction.emit(frame, op);\n> }\n> \n> +/**\n> + * \\brief Handle sensor controls for a given \\a frame number\n> + * \\param[in] frame The frame on which the sensor controls should be set\n> + *\n> + * Send the desired sensor control values to the pipeline handler to request\n> + * that they are applied on the camera sensor.\n> + */\n> void IPAIPU3::setControls(unsigned int frame)\n> {\n> IPU3Action op;\n> @@ -715,10 +767,15 @@ void IPAIPU3::setControls(unsigned int frame)\n> \n> } /* namespace ipa::ipu3 */\n> \n> -/*\n> - * External IPA module interface\n> +/**\n> + * \\brief External IPA module interface\n> + *\n> + * The IPAModuleInfo is required to match an IPA module construction against the\n> + * intented pipeline handler with the module. The API and pipeline handler\n> + * versions must match the corresponding IPA interface and pipeline handler.\n> + *\n> + * \\sa struct IPAModuleInfo\n> */\n> -\n> extern \"C\" {\n> const struct IPAModuleInfo ipaModuleInfo = {\n> IPA_MODULE_API_VERSION,\n> @@ -727,6 +784,14 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \"ipu3\",\n> };\n> \n> +/**\n> + * \\brief Create an instance of the IPA interface\n> + *\n> + * This function is the entry point of the IPA module. It is called by the IPA\n> + * manager to create an instance of the IPA interface for each camera. When\n> + * matched against with a pipeline handler, the IPAManager will construct an IPA\n> + * instance for each associated Camera.\n> + */\n> IPAInterface *ipaCreate()\n> {\n> return new ipa::ipu3::IPAIPU3();\n> -- \n> 2.32.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 0083DBDB1C\n\tfor ;\n\tTue, 26 Oct 2021 10:32:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3F5B60237;\n\tTue, 26 Oct 2021 12:32:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E47FB60123\n\tfor ;\n\tTue, 26 Oct 2021 12:32:15 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 99B973F0;\n\tTue, 26 Oct 2021 12:32:15 +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=\"sG5mGCnp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635244335;\n\tbh=7KzGUkfHaimJXAHRsicet3nVESnQ6QfpIemWircNqhM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=sG5mGCnp0b4PF6ileO9SC/XGO70PiZ8PFOaarmTRnPEn3IT/20cW1rBK210JVmt83\n\t+/r1wKLMdIpsLeoHHAINkLnu6Dw26eI//s4hBoi93BdL7H2o8LCSY7WPS4L45rZu87\n\tJqzKDnCtmYOoGGZnmlwhhx/QjIqHvVJp70veVknE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211026095534.90348-4-jeanmichel.hautbois@ideasonboard.com>","References":"<20211026095534.90348-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211026095534.90348-4-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 26 Oct 2021 11:32:12 +0100","Message-ID":"<163524433287.1182057.12494502703647615561@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v4 03/19] ipa: ipu3: Document the\n\tIPAIPU3 class","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\" "}},{"id":20546,"web_url":"https://patchwork.libcamera.org/comment/20546/","msgid":"","date":"2021-10-26T11:00:01","subject":"Re: [libcamera-devel] [PATCH v4 03/19] ipa: ipu3: Document the\n\tIPAIPU3 class","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi JM,\n\nOn 10/26/21 3:25 PM, Jean-Michel Hautbois wrote:\n> Clarify the roles and interactions between the pipeline handler events\n> and the algorithm calls by documenting all the remaining functions of\n> the class.\n>\n> Signed-off-by: Jean-Michel Hautbois \n> Signed-off-by: Kieran Bingham \n\n\nReviewed-by: Umang Jain \n> ---\n> src/ipa/ipu3/ipu3.cpp | 87 +++++++++++++++++++++++++++++++++++++------\n> 1 file changed, 76 insertions(+), 11 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 1f501821..177b67b0 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -305,9 +305,9 @@ private:\n> \tstruct IPAContext context_;\n> };\n> \n> -/*\n> - * Compute IPASessionConfiguration using the sensor information and the sensor\n> - * v4l2 controls.\n> +/**\n> + * \\brief Compute IPASessionConfiguration using the sensor information and the\n> + * sensor V4L2 controls\n> */\n> void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,\n> \t\t\t\t\t const ControlInfoMap &sensorControls)\n> @@ -336,9 +336,9 @@ void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,\n> \tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n> }\n> \n> -/*\n> - * Compute camera controls using the sensor information and the sensor\n> - * v4l2 controls.\n> +/**\n> + * \\brief Compute camera controls using the sensor information and the sensor\n> + * V4L2 controls\n> *\n> * Some of the camera controls are computed by the pipeline handler, some others\n> * by the IPA module which is in charge of handling, for example, the exposure\n> @@ -399,7 +399,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> }\n> \n> /**\n> - * Initialize the IPA module and its controls.\n> + * \\brief Initialize the IPA module and its controls\n> *\n> * This function receives the camera sensor information from the pipeline\n> * handler, computes the limits of the controls it handles and returns\n> @@ -430,8 +430,15 @@ int IPAIPU3::init(const IPASettings &settings,\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Perform any processing required before the first frame\n> + */\n> int IPAIPU3::start()\n> {\n> +\t/*\n> +\t * Set the sensors V4L2 controls before the first frame to ensure that\n> +\t * we have an expected and known configuration from the start.\n> +\t */\n> \tsetControls(0);\n> \n> \treturn 0;\n> @@ -512,7 +519,7 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> * handler\n> * \\param[in] ipaControls The IPA controls to update\n> *\n> - * Calculate the best grid for the statistics based on the Pipeline Handler BDS\n> + * Calculate the best grid for the statistics based on the pipeline handler BDS\n> * output, and parse the minimum and maximum exposure and analogue gain control\n> * values.\n> *\n> @@ -585,6 +592,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Map the parameters and stats buffers allocated in the pipeline handler\n> + * \\param[in] buffers The buffers to map\n> + */\n> void IPAIPU3::mapBuffers(const std::vector &buffers)\n> {\n> \tfor (const IPABuffer &buffer : buffers) {\n> @@ -594,6 +605,10 @@ void IPAIPU3::mapBuffers(const std::vector &buffers)\n> \t}\n> }\n> \n> +/**\n> + * \\brief Unmap the parameters and stats buffers\n> + * \\param[in] ids The IDs of the buffers to unmap\n> + */\n> void IPAIPU3::unmapBuffers(const std::vector &ids)\n> {\n> \tfor (unsigned int id : ids) {\n> @@ -605,6 +620,10 @@ void IPAIPU3::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> +/**\n> + * \\brief Process an event generated by the pipeline handler\n> + * \\param[in] event The event sent from pipeline handler\n> + */\n> void IPAIPU3::processEvent(const IPU3Event &event)\n> {\n> \tswitch (event.op) {\n> @@ -646,12 +665,28 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n> \t}\n> }\n> \n> +/**\n> + * \\brief Process a control list for a request from the application\n> + * \\param[in] frame The number of the frame which will be processed next\n> + * \\param[in] controls The controls for the \\a frame\n> + *\n> + * Parse the request to handle any IPA-managed controls that were set from the\n> + * application such as manual sensor settings.\n> + */\n> void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n> \t\t\t [[maybe_unused]] const ControlList &controls)\n> {\n> \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> }\n> \n> +/**\n> + * \\brief Fill the ImgU parameter buffer for the next frame\n> + * \\param[in] frame The number of the latest frame processed\n> + * \\param[out] params The parameter buffer to fill\n> + *\n> + * Algorithms are expected to fill the IPU3 parameter buffer for the next\n> + * frame given their most recent processing of the ImgU statistics.\n> + */\n> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> {\n> \t/*\n> @@ -674,6 +709,16 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> \tqueueFrameAction.emit(frame, op);\n> }\n> \n> +/**\n> + * \\brief Process the statistics generated by the ImgU\n> + * \\param[in] frame The number of the latest frame processed\n> + * \\param[in] frameTimestamp The current frame timestamp\n> + * \\param[in] stats The IPU3 statistics and ISP results\n> + *\n> + * Parse the most recently processed image statistics from the ImgU. The\n> + * statistics are passed to each algorithm module to run their calculations and\n> + * update their state accordingly.\n> + */\n> void IPAIPU3::parseStatistics(unsigned int frame,\n> \t\t\t [[maybe_unused]] int64_t frameTimestamp,\n> \t\t\t [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> @@ -697,6 +742,13 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> \tqueueFrameAction.emit(frame, op);\n> }\n> \n> +/**\n> + * \\brief Handle sensor controls for a given \\a frame number\n> + * \\param[in] frame The frame on which the sensor controls should be set\n> + *\n> + * Send the desired sensor control values to the pipeline handler to request\n> + * that they are applied on the camera sensor.\n> + */\n> void IPAIPU3::setControls(unsigned int frame)\n> {\n> \tIPU3Action op;\n> @@ -715,10 +767,15 @@ void IPAIPU3::setControls(unsigned int frame)\n> \n> } /* namespace ipa::ipu3 */\n> \n> -/*\n> - * External IPA module interface\n> +/**\n> + * \\brief External IPA module interface\n> + *\n> + * The IPAModuleInfo is required to match an IPA module construction against the\n> + * intented pipeline handler with the module. The API and pipeline handler\n> + * versions must match the corresponding IPA interface and pipeline handler.\n> + *\n> + * \\sa struct IPAModuleInfo\n> */\n> -\n> extern \"C\" {\n> const struct IPAModuleInfo ipaModuleInfo = {\n> \tIPA_MODULE_API_VERSION,\n> @@ -727,6 +784,14 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \t\"ipu3\",\n> };\n> \n> +/**\n> + * \\brief Create an instance of the IPA interface\n> + *\n> + * This function is the entry point of the IPA module. It is called by the IPA\n> + * manager to create an instance of the IPA interface for each camera. When\n> + * matched against with a pipeline handler, the IPAManager will construct an IPA\n> + * instance for each associated Camera.\n> + */\n> IPAInterface *ipaCreate()\n> {\n> \treturn new ipa::ipu3::IPAIPU3();","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 6C7B7BDB1C\n\tfor ;\n\tTue, 26 Oct 2021 11:00:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3B9960237;\n\tTue, 26 Oct 2021 13:00:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A90160123\n\tfor ;\n\tTue, 26 Oct 2021 13:00:06 +0200 (CEST)","from [192.168.1.106] (unknown [103.251.226.211])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C44203F0;\n\tTue, 26 Oct 2021 13:00:05 +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=\"E8C4Zfxd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635246006;\n\tbh=/6d3QTRFMAZlPOv1BPVOooFgnHtX4vDqKcKdiTwfDlA=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=E8C4ZfxdOtk1/ejPAEqiuSiuO/+tNLsIFye36keMOzRgylMWIrfLIQLJ75fSetylh\n\tf6nGVj9vwQgEb3eQjHRuEElI8VmJmq/J/vGLJh79HHYmSUVualhMRAohedy9bmHVWs\n\tvwrj5s82fnQSAI/PnEu7kxEfsIkXL/oVS8anUNrI=","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211026095534.90348-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211026095534.90348-4-jeanmichel.hautbois@ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Tue, 26 Oct 2021 16:30:01 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20211026095534.90348-4-jeanmichel.hautbois@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v4 03/19] ipa: ipu3: Document the\n\tIPAIPU3 class","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\" "}}]