[{"id":22498,"web_url":"https://patchwork.libcamera.org/comment/22498/","msgid":"","date":"2022-03-28T22:28:48","subject":"Re: [libcamera-devel] [PATCH v3 IPAIPU3 2/2] ipu3: Use the new\n\tIPAIPU3 interface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Mon, Mar 28, 2022 at 11:06:12PM +0530, Umang Jain via libcamera-devel wrote:\n> Use the new IPAIPU3 interface that migrates the event-based ops\n> to dedication functions.\n> \n> Signed-off-by: Umang Jain \n> ---\n> ipu3.cpp | 108 +++++++++++++++++++++++--------------------------------\n> 1 file changed, 45 insertions(+), 63 deletions(-)\n> \n> diff --git a/ipu3.cpp b/ipu3.cpp\n> index 6ab86e0..d02581d 100644\n> --- a/ipu3.cpp\n> +++ b/ipu3.cpp\n> @@ -49,13 +49,16 @@ public:\n> \n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> -\tvoid processEvent(const IPU3Event &event) override;\n> +\n> +\tvoid processParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;\n> +\tvoid processControls(const uint32_t frame, const ControlList &controls) override;\n\nSame comment as in 1/2 regarding the name of those two functions.\n\n> +\tvoid processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,\n> +\t\t\t\tconst uint32_t bufferId, const ControlList &sensorControls) override;\n> \n> private:\n> \tvoid updateControls(const IPACameraSensorInfo &sensorInfo,\n> \t\t\t const ControlInfoMap &sensorControls,\n> \t\t\t ControlInfoMap *ipaControls);\n> -\tvoid processControls(unsigned int frame, const ControlList &metadata);\n> \tvoid runAiq(unsigned int frame);\n> \tvoid fillParams(unsigned int frame, ipu3_uapi_params *params);\n> \tvoid parseStatistics(unsigned int frame,\n> @@ -321,54 +324,41 @@ void IPAIPU3::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPAIPU3::processEvent(const IPU3Event &event)\n> +void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n> +\t\t\t [[maybe_unused]] const ControlList &controls)\n> {\n> -\tswitch (event.op) {\n> -\tcase EventProcessControls: {\n> -\t\tprocessControls(event.frame, event.controls);\n> -\t\tbreak;\n> -\t}\n> -\tcase EventStatReady: {\n> -\t\tauto it = buffers_.find(event.bufferId);\n> -\t\tif (it == buffers_.end()) {\n> -\t\t\tLOG(IPAIPU3, Error) << \"Could not find stats buffer!\";\n> -\t\t\treturn;\n> -\t\t}\n> -\n> -\t\tSpan mem = it->second.maps()[0];\n> -\t\tconst ipu3_uapi_stats_3a *stats =\n> -\t\t\treinterpret_cast(mem.data());\n> -\n> -\t\tparseStatistics(event.frame,\n> -\t\t\t\tevent.frameTimestamp,\n> -\t\t\t\tstats,\n> -\t\t\t\tevent.sensorControls);\n> -\t\tbreak;\n> -\t}\n> -\tcase EventFillParams: {\n> -\t\tauto it = buffers_.find(event.bufferId);\n> -\t\tif (it == buffers_.end()) {\n> -\t\t\tLOG(IPAIPU3, Error) << \"Could not find param buffer!\";\n> -\t\t\treturn;\n> -\t\t}\n> -\n> -\t\tSpan mem = it->second.maps()[0];\n> -\t\tipu3_uapi_params *params =\n> -\t\t\treinterpret_cast(mem.data());\n> -\n> -\t\tfillParams(event.frame, params);\n> -\t\tbreak;\n> -\t}\n> -\tdefault:\n> -\t\tLOG(IPAIPU3, Error) << \"Unknown event \" << event.op;\n> -\t\tbreak;\n> +\t/* \\todo Start processing for 'frame' based on 'controls'. */\n> +}\n> +\n> +void IPAIPU3::processParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> +{\n> +\tauto it = buffers_.find(bufferId);\n> +\tif (it == buffers_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Could not find params buffer\";\n> +\t\treturn;\n> \t}\n> +\n> +\tSpan mem = it->second.maps()[0];\n> +\tipu3_uapi_params *params =\n> +\t\treinterpret_cast(mem.data());\n> +\n> +\tfillParams(frame, params);\n> }\n> \n> -void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n> -\t\t\t [[maybe_unused]] const ControlList &controls)\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> {\n> -\t/* \\todo Start processing for 'frame' based on 'controls'. */\n> +\tauto it = buffers_.find(bufferId);\n> +\tif (it == buffers_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Could not find stats buffer\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tSpan mem = it->second.maps()[0];\n> +\tconst ipu3_uapi_stats_3a *stats =\n> +\t\treinterpret_cast(mem.data());\n> +\n> +\tparseStatistics(frame, frameTimestamp, stats, sensorControls);\n> }\n> \n> void IPAIPU3::runAiq([[maybe_unused]] unsigned int frame)\n> @@ -424,10 +414,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> \n> \tsetControls(frame);\n> \n> -\tIPU3Action op;\n> -\top.op = ActionParamFilled;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tparamsBufferReady.emit(frame);\n> }\n> \n> void IPAIPU3::parseStatistics(unsigned int frame,\n> @@ -488,27 +475,22 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> \t\t\t\t(sensorInfo_.pixelRate / 1e6);\n> \tctrls.set(controls::FrameDuration, frameDuration);\n> \n> -\tIPU3Action op;\n> -\top.op = ActionMetadataReady;\n> -\top.controls = ctrls;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tmetadataReady.emit(frame, ctrls);\n> }\n> \n> void IPAIPU3::setControls(unsigned int frame)\n> {\n> -\tIPU3Action op;\n> -\top.op = ActionSetSensorControls;\n> -\n> -\tControlList sensorCtrls(ctrls_);\n> -\tsensorCtrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> -\tsensorCtrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain_));\n> +\tControlList ctrls(ctrls_);\n> +\tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain_));\n\nHow about keeping the name sensorCtrls to differentiate it from\nlensCtrls ?\n\n> \n> -\top.sensorControls = sensorCtrls;\n> +\tControlList lensCtrls(ctrls_);\n> +\t/*\n> +\t * \\todo: lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, ));\n> +\t */\n\nThis doesn't look right.\n\n> \n> -\top.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n> \n> -\tqueueFrameAction.emit(frame, op);\n> +\tsetSensorControls.emit(frame, ctrls, lensCtrls);\n> }\n> \n> } /* namespace ipa::ipu3 */","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 C61AAC0F1B\n\tfor ;\n\tMon, 28 Mar 2022 22:28:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3951165633;\n\tTue, 29 Mar 2022 00:28:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A890600AB\n\tfor ;\n\tTue, 29 Mar 2022 00:28:52 +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 6DED72F7;\n\tTue, 29 Mar 2022 00:28:51 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648506533;\n\tbh=ihshK4Z6FTnnxVSMk0XkSucxYAJn3sL/5QtAQUgXqxw=;\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=SGBdwx/BadXgSSqdhhF7Cp5DgbGtoUu5MYWJktRg4gll9ihYS5CJOJWh9fZfuxreN\n\tNwOqdNeK1v303ndfF3eyM6WXXU99UjMpsMe69VzvoBdtWyKfkIEk8mWsAUYfmeknzq\n\teCsYesF0N3KNrzRJn+Q17caMxssvVdWJf4r7DRLwlvOOZWIuIop0AdBpA2/tcZUi+E\n\t8MPgRojZ6eJOSs171Jz1MWKbTj0quao839I3ii0TiqCkDCGfR+08YtsOjP8bHnj8pJ\n\tZbqjPgnTSZAJ+Y6nLQLcXFUUQEIZpsQ7FU3+pJOiNGfL02CH/G5Uh4Vg0SVDk56iAi\n\tMz1CsuL8UhxmA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648506531;\n\tbh=ihshK4Z6FTnnxVSMk0XkSucxYAJn3sL/5QtAQUgXqxw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Fy8FAoL/WMymr48cGXxZYwM59FytRNdwB8Db7wWgQDXJRzt6VF4Y1XYZRvGKtxA3G\n\t4S1vk3FveZBaijm3nanwdpj9vpdG03yGCzBkAgYbm0ZfcypuGShmhFsI2q6WfptIqL\n\tsm2heRR5J/972WUOgLP+sBIJ5ijHU/eM9jh4G+O8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Fy8FAoL/\"; dkim-atps=neutral","Date":"Tue, 29 Mar 2022 01:28:48 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220328173612.138332-1-umang.jain@ideasonboard.com>\n\t<20220328173612.138332-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220328173612.138332-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 IPAIPU3 2/2] ipu3: Use the new\n\tIPAIPU3 interface","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\" "}}]