[{"id":35767,"web_url":"https://patchwork.libcamera.org/comment/35767/","msgid":"<4e9a9f8e-cfff-4d51-bf03-269a088d7389@ideasonboard.com>","date":"2025-09-10T14:11:36","subject":"Re: [PATCH 2/2] ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting\n\tnon-scalar controls","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 09. 10. 11:35 keltezéssel, Paul Elder írta:\n> The ControlInfos of non-scalar controls that are reported in controls()\n> must have scalar min/max and non-scalar default values for controls that\n> have a defined size. Currently this is relevant to the following\n> controls:\n> - ColourGains\n> - ColourCorrectionMatrix\n> - FrameDurationLimits\n> \n> A mismatch of scalarness in these fields causes deserialization errors\n> in ControlSerializer which manifest when IPAs are run in isolation.\n> \n> Fix the scalarness of these controls where relevant.\n\nRelated to https://bugs.libcamera.org/show_bug.cgi?id=101. Maybe the time has\nfinally come to define a concrete policy about the default values of array-like\ncontrols.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>   src/ipa/ipu3/ipu3.cpp             | 4 +++-\n>   src/ipa/mali-c55/mali-c55.cpp     | 5 ++++-\n>   src/ipa/rkisp1/algorithms/awb.cpp | 5 ++++-\n>   src/ipa/rkisp1/rkisp1.cpp         | 4 +++-\n>   src/ipa/rpi/common/ipa_base.cpp   | 7 ++++++-\n>   5 files changed, 20 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 1cae08bf255f..e71639a16522 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -20,6 +20,7 @@\n>   \n>   #include <libcamera/base/file.h>\n>   #include <libcamera/base/log.h>\n> +#include <libcamera/base/span.h>\n>   #include <libcamera/base/utils.h>\n>   \n>   #include <libcamera/control_ids.h>\n> @@ -280,10 +281,11 @@ 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> +\tstd::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] };\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<const int64_t, 2>{ defFrameDurations });\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..c63d3b2bb7be 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 <array>\n>   #include <map>\n>   #include <string.h>\n>   #include <vector>\n> @@ -14,6 +15,7 @@\n>   \n>   #include <libcamera/base/file.h>\n>   #include <libcamera/base/log.h>\n> +#include <libcamera/base/span.h>\n>   \n>   #include <libcamera/control_ids.h>\n>   #include <libcamera/ipa/ipa_interface.h>\n> @@ -234,9 +236,10 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,\n>   \t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n>   \t}\n>   \n> +\tstd::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] };\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<const int64_t, 2>{ defFrameDurations });\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..a664011a9f0d 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -91,7 +91,10 @@ 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> +\tstd::array<float, 2> defColourGains = { 1.0f, 1.0f };\n> +\tcmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f,\n> +\t\t\t\t\t\t   Span<const float, 2>{ defColourGains });\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 cf66d5553dcd..6eb3ae9c6310 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -430,10 +430,12 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,\n>   \t\tuint64_t frameSize = lineLength * frameHeights[i];\n>   \t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n>   \t}\n> +\tstd::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] };\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<const int64_t, 2>{ defFrameDurations }));\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 6448e6abd735..e34e890f0bdc 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 <array>\n>   #include <cmath>\n>   \n>   #include <libcamera/base/log.h>\n> @@ -243,10 +244,14 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa\n>   \t * based on the current sensor mode.\n>   \t */\n>   \tControlInfoMap::Map ctrlMap = ipaControls;\n> +\tstd::array<int64_t, 2> defFrameDurations = {\n> +\t\tstatic_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()),\n> +\t\tstatic_cast<int64_t>(defaultMinFrameDuration.get<std::micro>())\n> +\t};\n>   \tctrlMap[&controls::FrameDurationLimits] =\n>   \t\tControlInfo(static_cast<int64_t>(mode_.minFrameDuration.get<std::micro>()),\n>   \t\t\t    static_cast<int64_t>(mode_.maxFrameDuration.get<std::micro>()),\n> -\t\t\t    static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()));\n> +\t\t\t    Span<const int64_t, 2>{ defFrameDurations });\n>   \n>   \tctrlMap[&controls::AnalogueGain] =\n>   \t\tControlInfo(static_cast<float>(mode_.minAnalogueGain),","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E9044C324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 10 Sep 2025 14:11:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F9B66936E;\n\tWed, 10 Sep 2025 16:11:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6476869357\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Sep 2025 16:11:44 +0200 (CEST)","from [192.168.33.8] (185.221.142.115.nat.pool.zt.hu\n\t[185.221.142.115])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 53541C6F;\n\tWed, 10 Sep 2025 16:10:26 +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=\"jYyCVqi/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757513430;\n\tbh=nFD6SimA3YLRp/81Vc+xOFPTVvZaszgTLd2tCfEa4BU=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=jYyCVqi/qtZDKL5sHb4gbCkZB6147YVYP/JZg/YR0SeEMF6tyPHQ49lGG6DsOh753\n\tQFNpTkjbXx4TicJnCZtkvVrYHJ0a/xQCPNMx6/6p909B2Gr1//E6oI9QPiFRRE/w8q\n\tYSqMK/G6s0UNAMblvuc3yDmvUolGy9k8wgb1Y8bM=","Message-ID":"<4e9a9f8e-cfff-4d51-bf03-269a088d7389@ideasonboard.com>","Date":"Wed, 10 Sep 2025 16:11:36 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/2] ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting\n\tnon-scalar controls","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20250910093539.3216782-1-paul.elder@ideasonboard.com>\n\t<20250910093539.3216782-3-paul.elder@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250910093539.3216782-3-paul.elder@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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]