[{"id":22587,"web_url":"https://patchwork.libcamera.org/comment/22587/","msgid":"","date":"2022-04-05T22:55:10","subject":"Re: [libcamera-devel] [PATCH v4 3/6] ipa: ipu3: Inlink\n\tparseStatistics() into processStatsBuffer()","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\ns/Inlink/Inline/ in the subject line.\n\nOn Thu, Mar 31, 2022 at 10:00:55PM +0530, Umang Jain via libcamera-devel wrote:\n> Since we have moved away from switch/case on the operation ID,\n> there's little reason to split the operation in two functions.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Laurent Pinchart \n\nI'm not sure I would have split 2/6 and 3/6 in two patches (but there's\nno need to squash them).\n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 54 +++++++++++++++----------------------------\n> 1 file changed, 19 insertions(+), 35 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 23a9033e..605ca9a5 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -157,9 +157,6 @@ private:\n> \t\t\t ControlInfoMap *ipaControls);\n> \tvoid updateSessionConfiguration(const ControlInfoMap &sensorControls);\n> \n> -\tvoid parseStatistics(unsigned int frame,\n> -\t\t\t int64_t frameTimestamp,\n> -\t\t\t const ipu3_uapi_stats_3a *stats);\n> \tbool validateSensorControls();\n> \n> \tvoid setControls(unsigned int frame);\n> @@ -547,11 +544,15 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> }\n> \n> /**\n> - * \\brief Process statistics after ISP completion\n> + * \\brief Process the statistics generated by the ImgU\n> * \\param[in] frame The frame number\n> * \\param[in] frameTimestamp Timestamp of the frame\n> * \\param[in] bufferId ID of the statistics buffer\n> * \\param[in] sensorControls Sensor controls\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::processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,\n> \t\t\t\t const uint32_t bufferId, const ControlList &sensorControls)\n> @@ -569,37 +570,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimest\n> \tcontext_.frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> \tcontext_.frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> \n> -\tparseStatistics(frame, frameTimestamp, stats);\n> -}\n> -\n> -/**\n> - * \\brief Queue a request and process the control list 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::queueRequest(const uint32_t 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 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 const ipu3_uapi_stats_3a *stats)\n> -{\n> \tdouble lineDuration = context_.configuration.sensor.lineDuration.get();\n> \tint32_t vBlank = context_.configuration.sensor.defVBlank;\n> \tControlList ctrls(controls::controls);\n> @@ -630,6 +600,20 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> \tmetadataReady.emit(frame, ctrls);\n> }\n> \n> +/**\n> + * \\brief Queue a request and process the control list 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::queueRequest(const uint32_t 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 Handle sensor controls for a given \\a frame number\n> * \\param[in] frame The frame on which the sensor controls should be set","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 554E1C3256\n\tfor ;\n\tTue, 5 Apr 2022 22:55:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BFDEB6563F;\n\tWed, 6 Apr 2022 00:55: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 E2C80633A5\n\tfor ;\n\tWed, 6 Apr 2022 00:55:13 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 75F22482;\n\tWed, 6 Apr 2022 00:55:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649199315;\n\tbh=outsS8a19s38j8SmhARA2j2AEKuVA2JxBJrZVCPeCNI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=PDgZXEQpGzP/n8gX7udyfJEf799eCjZBz0NtdlQTHk8+phfW8EAYSXKGDIdbWMleA\n\t8ksaqwwzB7nxza2jxGk2a2bnHsGphadbNFrpE1KK+AWeB+Hox1FKVyWUctFMMz96TI\n\tHMvZnMvGWZMuodnrnZYYS0+OJjjQW8RAKYLLo6/ujd72V9z3Ha/qsmn5H46AMMpllb\n\tHAIfIn1E1H6TJB82ylE4Magz8LeZnAHbQUiyZjvFHwYn6PjwhsaHm2liec0nhkVqUr\n\tU4zd7sVK1ONfaQ3Dt1tMPzmGyQSzccIY3O63kLQsUuxEANT2E4Of6LPen4uV4S327t\n\tTOAv9RsHeWiSw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649199313;\n\tbh=outsS8a19s38j8SmhARA2j2AEKuVA2JxBJrZVCPeCNI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gaZgJG7amXJ5ZyCczTf+Dakk742A0DFtqQW9KuO7F9oKR8OK6JkSisfgCo7KK/ckF\n\tEFDq+TGLKDAUkbSViHdklqyv2JvfaIzrUJ4b55vmpZO03VDsJP8o9bHAuOrSjfN/sp\n\tT+SiVdwRXqyVsAEGWN5HmR4OQKPTzcULdPwBoFUA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gaZgJG7a\"; dkim-atps=neutral","Date":"Wed, 6 Apr 2022 01:55:10 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220331163058.171418-1-umang.jain@ideasonboard.com>\n\t<20220331163058.171418-4-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220331163058.171418-4-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 3/6] ipa: ipu3: Inlink\n\tparseStatistics() into processStatsBuffer()","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]