[{"id":36188,"web_url":"https://patchwork.libcamera.org/comment/36188/","msgid":"<15e01f48-7e6b-46c7-8251-3724825fc0d1@ideasonboard.com>","date":"2025-10-10T12:44:52","subject":"Re: [PATCH v3 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\n2025. 10. 10. 13:33 keltezéssel, Paul Elder írta:\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.\n> \n> Currently this is relevant to the following 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\nSorry, maybe I am missing something, but doesn't the previous change fix that?\nIf so, then I think the justification should mention that the goal is (at least\nas far as I am aware) that applications are able to take the default value of a\ncontrol (if there is any), and set it \"as is\". Which implies that array-like controls\nneed array-like default values with the same type (and same size if fixed size).\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> Fix the scalarness of these controls where relevant.\n> \n> Signed-off-by: Paul Elder \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 | 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 \n> #include \n> +#include \n> #include \n> \n> #include \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 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{ 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 \n> #include \n> #include \n> #include \n> @@ -14,6 +15,7 @@\n> \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \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 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{ 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 defColourGains = { 1.0f, 1.0f };\n> +\tcmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f,\n> +\t\t\t\t\t\t Span{ 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 fa22bfc34904..3f098610a06a 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -436,10 +436,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 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{ 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 8dfe35cc3267..67dab6cd970c 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> @@ -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 defFrameDurations = {\n> +\t\tstatic_cast(defaultMinFrameDuration.get()),\n> +\t\tstatic_cast(defaultMinFrameDuration.get())\n> +\t};\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{ defFrameDurations });\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 0D21ABE080\n\tfor ;\n\tFri, 10 Oct 2025 12:44:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 45B8B6B5C4;\n\tFri, 10 Oct 2025 14:44:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8162E6B599\n\tfor ;\n\tFri, 10 Oct 2025 14:44:56 +0200 (CEST)","from [192.168.33.28] (185.182.214.121.nat.pool.zt.hu\n\t[185.182.214.121])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D369B5B3;\n\tFri, 10 Oct 2025 14:43:20 +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=\"TFJjj4Vr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760100201;\n\tbh=y3bMrJsDVZUCxXjYY3YF+mgWVl2Y3jAwnXT28Y3kOHw=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=TFJjj4Vr6su+SaRrzauFcCnt19wXuPdcUfsOFpufittwNdie7jGarSDNW+VeqhuYf\n\tJN85gho3AmMIUehjnKlyflYnsJSIeY1rXsdIHNeWISygzGzgOc9SamyPss+dzVS2nA\n\t19VqnlW7CzORsq7ltWw6cXHfYNrB0IlGebKpd2tA=","Message-ID":"<15e01f48-7e6b-46c7-8251-3724825fc0d1@ideasonboard.com>","Date":"Fri, 10 Oct 2025 14:44:52 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 2/2] ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting\n\tnon-scalar controls","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251010113332.3030598-1-paul.elder@ideasonboard.com>\n\t<20251010113332.3030598-3-paul.elder@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251010113332.3030598-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":"","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":36204,"web_url":"https://patchwork.libcamera.org/comment/36204/","msgid":"<176015891548.2583768.14506587274047533870@neptunite.rasen.tech>","date":"2025-10-11T05:01:55","subject":"Re: [PATCH v3 2/2] ipa: ipu3, mali-c55, rkisp1,\n\trpi: Fix reporting non-scalar controls","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-10-10 21:44:52)\n> Hi\n> \n> 2025. 10. 10. 13:33 keltezéssel, Paul Elder írta:\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.\n> > \n> > Currently this is relevant to the following 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> Sorry, maybe I am missing something, but doesn't the previous change fix that?\n\nRight, it does.\n\n> If so, then I think the justification should mention that the goal is (at least\n> as far as I am aware) that applications are able to take the default value of a\n> control (if there is any), and set it \"as is\". Which implies that array-like controls\n> need array-like default values with the same type (and same size if fixed size).\n\nYes that is right.\n\n\nThanks,\n\nPaul\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> > \n> > Fix the scalarness of these controls where relevant.\n> > \n> > Signed-off-by: Paul Elder \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 | 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 \n> > #include \n> > +#include \n> > #include \n> > \n> > #include \n> > @@ -280,10 +281,11 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> > uint64_t frameSize = lineLength * frameHeights[i];\n> > frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n> > }\n> > + std::array defFrameDurations = { frameDurations[2], frameDurations[2] };\n> > \n> > controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n> > frameDurations[1],\n> > - frameDurations[2]);\n> > + Span{ defFrameDurations });\n> > \n> > controls.merge(context_.ctrlMap);\n> > *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 \n> > #include \n> > #include \n> > #include \n> > @@ -14,6 +15,7 @@\n> > \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -234,9 +236,10 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,\n> > frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n> > }\n> > \n> > + std::array defFrameDurations = { frameDurations[2], frameDurations[2] };\n> > ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n> > frameDurations[1],\n> > - frameDurations[2]);\n> > + Span{ defFrameDurations });\n> > \n> > /*\n> > * 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> > kMaxColourTemperature,\n> > kDefaultColourTemperature);\n> > cmap[&controls::AwbEnable] = ControlInfo(false, true);\n> > - cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, 1.0f);\n> > +\n> > + std::array defColourGains = { 1.0f, 1.0f };\n> > + cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f,\n> > + Span{ defColourGains });\n> > \n> > if (!tuningData.contains(\"algorithm\"))\n> > LOG(RkISP1Awb, Info) << \"No AWB algorithm specified.\"\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index fa22bfc34904..3f098610a06a 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -436,10 +436,12 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,\n> > uint64_t frameSize = lineLength * frameHeights[i];\n> > frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n> > }\n> > + std::array defFrameDurations = { frameDurations[2], frameDurations[2] };\n> > \n> > /* \\todo Move this (and other agc-related controls) to agc */\n> > context_.ctrlMap[&controls::FrameDurationLimits] =\n> > - ControlInfo(frameDurations[0], frameDurations[1], frameDurations[2]);\n> > + ControlInfo(frameDurations[0], frameDurations[1],\n> > + ControlValue(Span{ defFrameDurations }));\n> > \n> > ctrlMap.insert(context_.ctrlMap.begin(), context_.ctrlMap.end());\n> > *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..67dab6cd970c 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> > @@ -243,10 +244,14 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa\n> > * based on the current sensor mode.\n> > */\n> > ControlInfoMap::Map ctrlMap = ipaControls;\n> > + std::array defFrameDurations = {\n> > + static_cast(defaultMinFrameDuration.get()),\n> > + static_cast(defaultMinFrameDuration.get())\n> > + };\n> > ctrlMap[&controls::FrameDurationLimits] =\n> > ControlInfo(static_cast(mode_.minFrameDuration.get()),\n> > static_cast(mode_.maxFrameDuration.get()),\n> > - static_cast(defaultMinFrameDuration.get()));\n> > + Span{ defFrameDurations });\n> > \n> > ctrlMap[&controls::AnalogueGain] =\n> > ControlInfo(static_cast(mode_.minAnalogueGain),\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 BD58FBE080\n\tfor ;\n\tSat, 11 Oct 2025 05:02:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8289D60316;\n\tSat, 11 Oct 2025 07:02:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEF636030E\n\tfor ;\n\tSat, 11 Oct 2025 07:02:02 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:42b2:5255:36e:e46a])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 71ED256D;\n\tSat, 11 Oct 2025 07:00: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=\"S7jjNq8A\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760158827;\n\tbh=KjDVm8K6b3MAhOWBhbOLX16/tZYuAl6izipVckSeKeY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=S7jjNq8AC7tcOzfOh5Vnlh97dbqWTgCwQD9CFuhl35pdVNouNHHZxlQ9zL8/QBUzd\n\tnsgiiSY3WLSXQ0wOoEA+9Dog7IQeKdJvi3gzPsx6eG1KGA7m1tltUrWjQONYnQAtkv\n\tog/TBPOwmr2fM28VEgDYHH/gFHh7UM1p/VBr+PBo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<15e01f48-7e6b-46c7-8251-3724825fc0d1@ideasonboard.com>","References":"<20251010113332.3030598-1-paul.elder@ideasonboard.com>\n\t<20251010113332.3030598-3-paul.elder@ideasonboard.com>\n\t<15e01f48-7e6b-46c7-8251-3724825fc0d1@ideasonboard.com>","Subject":"Re: [PATCH v3 2/2] ipa: ipu3, mali-c55, rkisp1,\n\trpi: Fix reporting non-scalar controls","From":"Paul Elder ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Sat, 11 Oct 2025 14:01:55 +0900","Message-ID":"<176015891548.2583768.14506587274047533870@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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\" "}}]