[{"id":39495,"web_url":"https://patchwork.libcamera.org/comment/39495/","msgid":"<178272709247.36676.6673868360871511120@ping.linuxembedded.co.uk>","date":"2026-06-29T09:58:12","subject":"Re: [PATCH v2 10/12] ipa: ipu3: Don't pass sensorInfo to\n\tupdateControls()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Daniel Scally (2026-06-26 14:05:57)\n> The updateControls() function uses the configured output size of the\n> sensor to set the ranges for vertical blanking and frame duration\n> controls. We currently pass sensorInfo as a parameter, but it's also\n> stored as a member of class IPAIPU3 and so doesn't need to be passed\n> to updateControls at all().\n> \n> Drop the parameter and just use the class member instead.\n> \n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v2:\n> \n> - New patch\n\nIs this the same pattern as the other libipa modules ?\n\nI'd be keen to work towards consistency here throughout.\n\nSo if it's the right way:\n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 21 ++++++++++-----------\n> 1 file changed, 10 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 4bdc4b7677fe9703fb968ca91967c68742454514..b1c176e68098b24d8fd37e7a0e278cabf0e1c3b2 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -165,8 +165,7 @@ protected:\n> std::string logPrefix() const override;\n> \n> private:\n> - void updateControls(const IPACameraSensorInfo &sensorInfo,\n> - const ControlInfoMap &sensorControls,\n> + void updateControls(const ControlInfoMap &sensorControls,\n> ControlInfoMap *ipaControls);\n> void updateSessionConfiguration(const ControlInfoMap &sensorControls);\n> \n> @@ -239,8 +238,7 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)\n> * - controls::ExposureTime\n> * - controls::FrameDurationLimits\n> */\n> -void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> - const ControlInfoMap &sensorControls,\n> +void IPAIPU3::updateControls(const ControlInfoMap &sensorControls,\n> ControlInfoMap *ipaControls)\n> {\n> ControlInfoMap::Map controls{};\n> @@ -267,19 +265,19 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> */\n> const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;\n> uint32_t hblank = v4l2HBlank.def().get();\n> - uint32_t lineLength = sensorInfo.outputSize.width + hblank;\n> + uint32_t lineLength = sensorInfo_.outputSize.width + hblank;\n> \n> const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n> std::array frameHeights{\n> - v4l2VBlank.min().get() + sensorInfo.outputSize.height,\n> - v4l2VBlank.max().get() + sensorInfo.outputSize.height,\n> - v4l2VBlank.def().get() + sensorInfo.outputSize.height,\n> + v4l2VBlank.min().get() + sensorInfo_.outputSize.height,\n> + v4l2VBlank.max().get() + sensorInfo_.outputSize.height,\n> + v4l2VBlank.def().get() + sensorInfo_.outputSize.height,\n> };\n> \n> std::array frameDurations;\n> for (unsigned int i = 0; i < frameHeights.size(); ++i) {\n> uint64_t frameSize = lineLength * frameHeights[i];\n> - frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n> + frameDurations[i] = frameSize / (sensorInfo_.pixelRate / 1000000U);\n> }\n> controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n> frameDurations[1],\n> @@ -313,6 +311,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> context_.configuration = {};\n> context_.configuration.sensor.lineDuration =\n> sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;\n> + sensorInfo_ = sensorInfo;\n> \n> /* Load the tuning data file. */\n> File file(settings.configurationFile);\n> @@ -346,7 +345,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> return ret;\n> \n> /* Initialize controls. */\n> - updateControls(sensorInfo, sensorControls, ipaControls);\n> + updateControls(sensorControls, ipaControls);\n> \n> return 0;\n> }\n> @@ -488,7 +487,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> calculateBdsGrid(configInfo.bdsOutputSize);\n> \n> /* Update the camera controls using the new sensor settings. */\n> - updateControls(sensorInfo_, sensorCtrls_, ipaControls);\n> + updateControls(sensorCtrls_, ipaControls);\n> \n> /* Update the IPASessionConfiguration using the sensor settings. */\n> updateSessionConfiguration(sensorCtrls_);\n> \n> -- \n> 2.43.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 DCB76C3261\n\tfor ;\n\tMon, 29 Jun 2026 09:58:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 013FB65F19;\n\tMon, 29 Jun 2026 11:58: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 220D965F11\n\tfor ;\n\tMon, 29 Jun 2026 11:58:15 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0BB2134;\n\tMon, 29 Jun 2026 11:57:32 +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=\"vYsicN29\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782727052;\n\tbh=MuUgaHhQDC5M+sEsRFrNwwiOX6ocMBXNcSRkBUo8Hh4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=vYsicN29E+rFcL2W1k3qW93rlhuJ8fTlYXJhTSX0BY9V2R9WPMy0JGW4w59BBLA11\n\tPySu1UT0DNNZyi3N9dEUoYlAi47fe+3FxNbjtLftzc4PhaQOo1SU44gTlDDtXgU/mj\n\t+LKNeAO3wvxYGiYLsIhks7XcssJIHskGmt7lZZqw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260626-ipu3-libipa-rework-v2-10-41546e23de3e@ideasonboard.com>","References":"<20260626-ipu3-libipa-rework-v2-0-41546e23de3e@ideasonboard.com>\n\t<20260626-ipu3-libipa-rework-v2-10-41546e23de3e@ideasonboard.com>","Subject":"Re: [PATCH v2 10/12] ipa: ipu3: Don't pass sensorInfo to\n\tupdateControls()","From":"Kieran Bingham ","Cc":"Daniel Scally ","To":"Daniel Scally ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 29 Jun 2026 10:58:12 +0100","Message-ID":"<178272709247.36676.6673868360871511120@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":39526,"web_url":"https://patchwork.libcamera.org/comment/39526/","msgid":"<6f49a3f6-ca02-4a7d-a7eb-1824301fe254@ideasonboard.com>","date":"2026-06-30T11:36:22","subject":"Re: [PATCH v2 10/12] ipa: ipu3: Don't pass sensorInfo to\n\tupdateControls()","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 06. 26. 15:05 keltezéssel, Daniel Scally írta:\n> The updateControls() function uses the configured output size of the\n> sensor to set the ranges for vertical blanking and frame duration\n> controls. We currently pass sensorInfo as a parameter, but it's also\n> stored as a member of class IPAIPU3 and so doesn't need to be passed\n> to updateControls at all().\n> \n> Drop the parameter and just use the class member instead.\n> \n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v2:\n> \n> - New patch\n> ---\n\nLooks ok to me.\n\nReviewed-by: Barnabás Pőcze \n\n\n> src/ipa/ipu3/ipu3.cpp | 21 ++++++++++-----------\n> 1 file changed, 10 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 4bdc4b7677fe9703fb968ca91967c68742454514..b1c176e68098b24d8fd37e7a0e278cabf0e1c3b2 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -165,8 +165,7 @@ protected:\n> \tstd::string logPrefix() const override;\n> \n> private:\n> -\tvoid updateControls(const IPACameraSensorInfo &sensorInfo,\n> -\t\t\t const ControlInfoMap &sensorControls,\n> +\tvoid updateControls(const ControlInfoMap &sensorControls,\n> \t\t\t ControlInfoMap *ipaControls);\n> \tvoid updateSessionConfiguration(const ControlInfoMap &sensorControls);\n> \n> @@ -239,8 +238,7 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)\n> * - controls::ExposureTime\n> * - controls::FrameDurationLimits\n> */\n> -void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> -\t\t\t const ControlInfoMap &sensorControls,\n> +void IPAIPU3::updateControls(const ControlInfoMap &sensorControls,\n> \t\t\t ControlInfoMap *ipaControls)\n> {\n> \tControlInfoMap::Map controls{};\n> @@ -267,19 +265,19 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> \t */\n> \tconst ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;\n> \tuint32_t hblank = v4l2HBlank.def().get();\n> -\tuint32_t lineLength = sensorInfo.outputSize.width + hblank;\n> +\tuint32_t lineLength = sensorInfo_.outputSize.width + hblank;\n> \n> \tconst ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n> \tstd::array frameHeights{\n> -\t\tv4l2VBlank.min().get() + sensorInfo.outputSize.height,\n> -\t\tv4l2VBlank.max().get() + sensorInfo.outputSize.height,\n> -\t\tv4l2VBlank.def().get() + sensorInfo.outputSize.height,\n> +\t\tv4l2VBlank.min().get() + sensorInfo_.outputSize.height,\n> +\t\tv4l2VBlank.max().get() + sensorInfo_.outputSize.height,\n> +\t\tv4l2VBlank.def().get() + sensorInfo_.outputSize.height,\n> \t};\n> \n> \tstd::array frameDurations;\n> \tfor (unsigned int i = 0; i < frameHeights.size(); ++i) {\n> \t\tuint64_t frameSize = lineLength * frameHeights[i];\n> -\t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n> +\t\tframeDurations[i] = frameSize / (sensorInfo_.pixelRate / 1000000U);\n> \t}\n> \tcontrols[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n> \t\t\t\t\t\t\t frameDurations[1],\n> @@ -313,6 +311,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> \tcontext_.configuration = {};\n> \tcontext_.configuration.sensor.lineDuration =\n> \t\tsensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;\n> +\tsensorInfo_ = sensorInfo;\n> \n> \t/* Load the tuning data file. */\n> \tFile file(settings.configurationFile);\n> @@ -346,7 +345,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> \t\treturn ret;\n> \n> \t/* Initialize controls. */\n> -\tupdateControls(sensorInfo, sensorControls, ipaControls);\n> +\tupdateControls(sensorControls, ipaControls);\n> \n> \treturn 0;\n> }\n> @@ -488,7 +487,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \tcalculateBdsGrid(configInfo.bdsOutputSize);\n> \n> \t/* Update the camera controls using the new sensor settings. */\n> -\tupdateControls(sensorInfo_, sensorCtrls_, ipaControls);\n> +\tupdateControls(sensorCtrls_, ipaControls);\n> \n> \t/* Update the IPASessionConfiguration using the sensor settings. */\n> \tupdateSessionConfiguration(sensorCtrls_);\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 70F11C3303\n\tfor ;\n\tTue, 30 Jun 2026 11:36:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74AA465F6F;\n\tTue, 30 Jun 2026 13:36:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0649D65F55\n\tfor ;\n\tTue, 30 Jun 2026 13:36:26 +0200 (CEST)","from [192.168.33.31] (185.221.140.128.nat.pool.zt.hu\n\t[185.221.140.128])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E3E5622B;\n\tTue, 30 Jun 2026 13:35:41 +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=\"v3uGCWcH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782819342;\n\tbh=r8ey91Jqz8Ub4ZN8mkk62KhdVYQLMwPdMbj6VqZPRf4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=v3uGCWcHVjpELSYc9w5+7VR9sOWnZ9+gtw47kpPSYfoMHZl7QXRM0fu26XHFSAmkG\n\tpZjrYeAizRsc7wVATVuRWq4Kei2va2uP56edd9ekUstNgDwH05A/HQqN1NKXmAYS9t\n\tXwe2JgPFo8DWqp0c/FXzL874zPMAfcbc748lQUnI=","Message-ID":"<6f49a3f6-ca02-4a7d-a7eb-1824301fe254@ideasonboard.com>","Date":"Tue, 30 Jun 2026 13:36:22 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 10/12] ipa: ipu3: Don't pass sensorInfo to\n\tupdateControls()","To":"Daniel Scally ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260626-ipu3-libipa-rework-v2-0-41546e23de3e@ideasonboard.com>\n\t<20260626-ipu3-libipa-rework-v2-10-41546e23de3e@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260626-ipu3-libipa-rework-v2-10-41546e23de3e@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]