[{"id":36895,"web_url":"https://patchwork.libcamera.org/comment/36895/","msgid":"<20251119042545.GP10711@pendragon.ideasonboard.com>","date":"2025-11-19T04:25:45","subject":"Re: [PATCH v6 2/2] ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting\n\tnon-scalar controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Mon, Nov 17, 2025 at 05:08:15PM +0900, Paul Elder wrote:\n> The ControlInfos of non-scalar controls that are reported in controls()\n> must have non-scalar default values for controls that have a defined\n> size. This is because applications should be able to directly set the\n> default value from a ControlInfo to the control.\n> \n> Currently this is relevant to the following controls:\n> - ColourGains\n> - ColourCorrectionMatrix\n> - FrameDurationLimits\n> - AfWindows\n> \n> Fix the scalarness of these controls where relevant.\n> \n> Signed-off-by: Paul Elder \n> Tested-by: Barnabás Pőcze # rkisp1\n> Reviewed-by: Naushir Patuck \n> \n> ---\n> No change in v6\n> \n> Changes in v5:\n> - add AfWindows\n> - create Span arrays on-the-fly\n> \n> Changes in v4:\n> - improve commit message\n> \n> No change in v3\n> \n> No change in v2\n> ---\n> src/ipa/ipu3/ipu3.cpp | 4 ++--\n> src/ipa/mali-c55/mali-c55.cpp | 4 +++-\n> src/ipa/rkisp1/algorithms/awb.cpp | 4 +++-\n> src/ipa/rkisp1/rkisp1.cpp | 3 ++-\n> src/ipa/rpi/common/ipa_base.cpp | 7 +++++--\n> 5 files changed, 15 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 1cae08bf255f..b926f579a9a3 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -20,6 +20,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -280,10 +281,9 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> \t\tuint64_t frameSize = lineLength * frameHeights[i];\n> \t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n> \t}\n> -\n> \tcontrols[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n> \t\t\t\t\t\t\t frameDurations[1],\n> -\t\t\t\t\t\t\t frameDurations[2]);\n> +\t\t\t\t\t\t\t Span{ { frameDurations[2], frameDurations[2] } });\n\nThat's a veryyyyyy long line.\n\n\tcontrols[&controls::FrameDurationLimits] =\n\t\tControlInfo(frameDurations[0], frameDurations[1],\n\t\t\t Span{ { frameDurations[2], frameDurations[2] } });\n\nor maybe\n\n\tstd::array frameDurationsDefault = {\n\t\tframeDurations[2], frameDurations[2]\n\t};\n\n\tcontrols[&controls::FrameDurationLimits] =\n\t\tControlInfo(frameDurations[0], frameDurations[1],\n\t\t\t frameDurationsDefault);\n\nas the Span constructor from an std::array is not explicit.\n\nSame below.\n\nReviewed-by: Laurent Pinchart \n\n> \n> \tcontrols.merge(context_.ctrlMap);\n> \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\n> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp\n> index 7d45e7310aec..4eaedabb47b8 100644\n> --- a/src/ipa/mali-c55/mali-c55.cpp\n> +++ b/src/ipa/mali-c55/mali-c55.cpp\n> @@ -5,6 +5,7 @@\n> * Mali-C55 ISP image processing algorithms\n> */\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -14,6 +15,7 @@\n> \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -236,7 +238,7 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,\n> \n> \tctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n> \t\t\t\t\t\t\t frameDurations[1],\n> -\t\t\t\t\t\t\t frameDurations[2]);\n> +\t\t\t\t\t\t\t Span{ { frameDurations[2], frameDurations[2] } });\n> \n> \t/*\n> \t * Compute exposure time limits from the V4L2_CID_EXPOSURE control\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 399fb51be414..e8da7974a1d6 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -91,7 +91,9 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> \t\t\t\t\t\t\t kMaxColourTemperature,\n> \t\t\t\t\t\t\t kDefaultColourTemperature);\n> \tcmap[&controls::AwbEnable] = ControlInfo(false, true);\n> -\tcmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, 1.0f);\n> +\n> +\tcmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f,\n> +\t\t\t\t\t\t Span{ { 1.0f, 1.0f } });\n> \n> \tif (!tuningData.contains(\"algorithm\"))\n> \t\tLOG(RkISP1Awb, Info) << \"No AWB algorithm specified.\"\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index fa22bfc34904..61d3d1f6f96b 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -439,7 +439,8 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,\n> \n> \t/* \\todo Move this (and other agc-related controls) to agc */\n> \tcontext_.ctrlMap[&controls::FrameDurationLimits] =\n> -\t\tControlInfo(frameDurations[0], frameDurations[1], frameDurations[2]);\n> +\t\tControlInfo(frameDurations[0], frameDurations[1],\n> +\t\t\t ControlValue(Span{ { frameDurations[2], frameDurations[2] } }));\n> \n> \tctrlMap.insert(context_.ctrlMap.begin(), context_.ctrlMap.end());\n> \t*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index 8dfe35cc3267..14aba4500ae4 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \"ipa_base.h\"\n> \n> +#include \n> #include \n> \n> #include \n> @@ -105,7 +106,8 @@ const ControlInfoMap::Map ipaAfControls{\n> \t{ &controls::AfRange, ControlInfo(controls::AfRangeValues) },\n> \t{ &controls::AfSpeed, ControlInfo(controls::AfSpeedValues) },\n> \t{ &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> -\t{ &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> +\t{ &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535),\n> +\t\t\t\t\t Span{ { Rectangle{} } }) },\n> \t{ &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> \t{ &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> \t{ &controls::LensPosition, ControlInfo(0.0f, 32.0f, 1.0f) }\n> @@ -246,7 +248,8 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa\n> \tctrlMap[&controls::FrameDurationLimits] =\n> \t\tControlInfo(static_cast(mode_.minFrameDuration.get()),\n> \t\t\t static_cast(mode_.maxFrameDuration.get()),\n> -\t\t\t static_cast(defaultMinFrameDuration.get()));\n> +\t\t\t Span{ { static_cast(defaultMinFrameDuration.get()),\n> +\t\t\t\t\t\t static_cast(defaultMinFrameDuration.get()) } });\n> \n> \tctrlMap[&controls::AnalogueGain] =\n> \t\tControlInfo(static_cast(mode_.minAnalogueGain),","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 46A96BD80A\n\tfor ;\n\tWed, 19 Nov 2025 04:26:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 913F960A80;\n\tWed, 19 Nov 2025 05:26:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60A1660856\n\tfor ;\n\tWed, 19 Nov 2025 05:26:15 +0100 (CET)","from pendragon.ideasonboard.com (unknown [205.220.129.225])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 88AE814B0; \n\tWed, 19 Nov 2025 05:24:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZCxsCtjk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763526251;\n\tbh=ncBY9FZO/8Li3RGz3N1EvSZMyi7CC+lnPzeYn+gcLvM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZCxsCtjkzJ/pcHAwSOMfPwPjPdErhWbcqgR3Fpqx0p33sR3JHmAcbrx2XTCYbGlrr\n\tVs6zw+wLp9W3mXhu2oB6Inuxq8pHPt6Ds+6PjWdNraJi12P+WgXu+JzcsX06xsyCYk\n\tHKeUOCnE0dXxNYnNdRh4F4Z1o08DChV+v2KX51Tw=","Date":"Wed, 19 Nov 2025 13:25:45 +0900","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, =?utf-8?b?QmFybmFiw6FzIFDFkWN6?=\n\t=?utf-8?q?e?= ,\n\tNaushir Patuck ","Subject":"Re: [PATCH v6 2/2] ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting\n\tnon-scalar controls","Message-ID":"<20251119042545.GP10711@pendragon.ideasonboard.com>","References":"<20251117080818.3009835-1-paul.elder@ideasonboard.com>\n\t<20251117080818.3009835-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20251117080818.3009835-3-paul.elder@ideasonboard.com>","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\" "}}]