[{"id":36162,"web_url":"https://patchwork.libcamera.org/comment/36162/","msgid":"<175983338901.2583768.9328083240280131667@neptunite.rasen.tech>","date":"2025-10-07T10:36:29","subject":"Re: [PATCH v2 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":"Hi me,\n\nQuoting Paul Elder (2025-10-07 19:27:44)\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\nmin/max don't have this restriction; only the default value has this\nrestriction. The code is fine though, it's just the commit message.\n\nWe should upgrade the documentation (where?) though to mention that min/max can\nbe scalar or non-scalar depending on what makes sense for the control, and that\nmin/max are not designed to be set as a control value directly whereas default is\ndesigned to be able to be set directly.\n\n\nPaul\n\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> \n> Signed-off-by: Paul Elder \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> -- \n> 2.47.2\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 B1F61C324C\n\tfor ;\n\tTue, 7 Oct 2025 10:36:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AA406B5F3;\n\tTue, 7 Oct 2025 12:36:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 88C0769367\n\tfor ;\n\tTue, 7 Oct 2025 12:36:35 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:859f:ca72:88bf:8f12])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id BEDC76F3\n\tfor ;\n\tTue, 7 Oct 2025 12:35:01 +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=\"S3xMJXY8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759833302;\n\tbh=l+3oKT1Tln09mLFj1CUyo8+SyZgu8diO4+uDZBQgIgg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=S3xMJXY8FGpN1v7h8t5k70mvWJhAX4XAendcqXAplhaIAmXKenEuibcnpvwURQ6p7\n\t2ulnJykB8Cj8mVEYb5/ZkgcDyBvidK1EhMMvx55TLxnpuSct1G1+ex5HjJgFftAkXT\n\tueuK2ctgO9yoyTEReRoCJ/wX1Y28B0fQF1kx8jP4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251007102747.2746478-3-paul.elder@ideasonboard.com>","References":"<20251007102747.2746478-1-paul.elder@ideasonboard.com>\n\t<20251007102747.2746478-3-paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v2 2/2] ipa: ipu3, mali-c55, rkisp1,\n\trpi: Fix reporting non-scalar controls","From":"Paul Elder ","Cc":"","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 07 Oct 2025 19:36:29 +0900","Message-ID":"<175983338901.2583768.9328083240280131667@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\" "}}]