From patchwork Tue Oct 7 10:27:43 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 24566 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 02E82BF415 for ; Tue, 7 Oct 2025 10:28:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DCF826B5F9; Tue, 7 Oct 2025 12:28:02 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="NbnZUeL8"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 351D16B5AA for ; Tue, 7 Oct 2025 12:28:00 +0200 (CEST) Received: from neptunite.hamster-moth.ts.net (unknown [IPv6:2404:7a81:160:2100:859f:ca72:88bf:8f12]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0F1ADEB5; Tue, 7 Oct 2025 12:26:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1759832787; bh=ZK499pj8Jsi7nEuwyLKsBr+PNimy3AgfGwuqd00xnew=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NbnZUeL8CKPa0tz78BIfLdwywOjfcoNt0OavJcjL8AizaU/BXrZBQ1Ibyp1e5XiTb 44sepgu6bOrksNOJOS6vCtZDGtQJavujUYZKdeUTLn+t4UbbgMjecKwsyNb87TBo4B mDm/bQeYzgrtSBg4hn7YJX6Hly2sMMlk82vM5DQU= From: Paul Elder To: libcamera-devel@lists.libcamera.org Cc: Paul Elder Subject: [PATCH v2 1/2] libcamera: control_serializer: Add array info to serialized ControlValue Date: Tue, 7 Oct 2025 19:27:43 +0900 Message-ID: <20251007102747.2746478-2-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20251007102747.2746478-1-paul.elder@ideasonboard.com> References: <20251007102747.2746478-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Array controls (eg. ColourCorrectionMatrix, FrameDurationLimits, ColourGains) are serialized properly by the ControlSerializer, but are not deserialized properly. This is because their arrayness and size are not considered during deserialization. Fix this by adding arrayness and size to the serialized form of all ControlValues. This was not noticed before as the default value of the ControlInfo of other array controls had been set to scalar values similar to min/max, and ColourCorrectionMatrix was the first control to properly define a non-scalar default value. Other array controls that define a scalar default value need to be fixed to define a properly sized default ControlInfo value. Bug: https://bugs.libcamera.org/show_bug.cgi?id=285 Signed-off-by: Paul Elder --- Changes in v2: - make it so that the *serialized form* of ControlValue includes arrayness and size instead - Compared to v1, this ties the arrayness and size information to ControlValue instead of ControlId, which as a side effect allows us to support scalar and array min/max values, not just def values. This also gives us support for variable-length arrays --- .../libcamera/internal/control_serializer.h | 8 +++-- src/libcamera/control_serializer.cpp | 30 ++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h index 8a63ae44a13e..683bb13df92f 100644 --- a/include/libcamera/internal/control_serializer.h +++ b/include/libcamera/internal/control_serializer.h @@ -41,14 +41,18 @@ public: bool isCached(const ControlInfoMap &infoMap); private: + struct ControlValueHeader { + bool isArray; + std::size_t numElements; + }; + static size_t binarySize(const ControlValue &value); static size_t binarySize(const ControlInfo &info); static void store(const ControlValue &value, ByteStreamBuffer &buffer); static void store(const ControlInfo &info, ByteStreamBuffer &buffer); - ControlValue loadControlValue(ByteStreamBuffer &buffer, - bool isArray = false, unsigned int count = 1); + ControlValue loadControlValue(ByteStreamBuffer &buffer); ControlInfo loadControlInfo(ByteStreamBuffer &buffer); unsigned int serial_; diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index 050f8512bd52..463f6ab9118d 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -144,7 +144,12 @@ void ControlSerializer::reset() size_t ControlSerializer::binarySize(const ControlValue &value) { - return sizeof(ControlType) + value.data().size_bytes(); + /* + * Allocate extra space to save isArray and the number of elements, to + * support array-type ControlValues. + */ + return sizeof(ControlType) + sizeof(ControlValueHeader) + + value.data().size_bytes(); } size_t ControlSerializer::binarySize(const ControlInfo &info) @@ -197,6 +202,13 @@ void ControlSerializer::store(const ControlValue &value, { const ControlType type = value.type(); buffer.write(&type); + + ControlValueHeader cvh = { + value.isArray(), + value.numElements(), + }; + buffer.write(&cvh); + buffer.write(value.data()); } @@ -368,6 +380,10 @@ int ControlSerializer::serialize(const ControlList &list, struct ipa_control_value_entry entry; entry.id = id; entry.type = value.type(); + /* + * .is_array and .count are now unused as these have been moved + * to ControlSerializer::store(const ControlValue &, ByteStreamBuffer &) + */ entry.is_array = value.isArray(); entry.count = value.numElements(); entry.offset = values.offset(); @@ -382,16 +398,16 @@ int ControlSerializer::serialize(const ControlList &list, return 0; } -ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer, - bool isArray, - unsigned int count) +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer) { ControlType type; buffer.read(&type); - ControlValue value; + ControlValueHeader cvh; + buffer.read(&cvh); - value.reserve(type, isArray, count); + ControlValue value; + value.reserve(type, cvh.isArray, cvh.numElements); buffer.read(value.data()); return value; @@ -633,7 +649,7 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer } ctrls.set(entry->id, - loadControlValue(values, entry->is_array, entry->count)); + loadControlValue(values)); } return ctrls; From patchwork Tue Oct 7 10:27:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 24567 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id D3594BF415 for ; Tue, 7 Oct 2025 10:28:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EEFC66B601; Tue, 7 Oct 2025 12:28:06 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="KqBXglSr"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BF526B5AA for ; Tue, 7 Oct 2025 12:28:02 +0200 (CEST) Received: from neptunite.hamster-moth.ts.net (unknown [IPv6:2404:7a81:160:2100:859f:ca72:88bf:8f12]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AAD2B6F3; Tue, 7 Oct 2025 12:26:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1759832788; bh=0ouQwnbj8kZY4gSteuzFCsZORxLSKTufq+f/ztLzDZI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KqBXglSrzTDKoU9mMaL5LhPeBRHjHim2zSLig/FDpxWYr75sTjFeYlxyf+qYKLHGb 1C8zxsyh9ZYH13m/1qqJisWzgHpjv0HmdSkDG3v2XG57/N2bHcaNSVW3qJz6BDIziN qEPVerMBWdNZSx7vYfRdFXOFBEuNUl//OFRyCDv8= From: Paul Elder To: libcamera-devel@lists.libcamera.org Cc: Paul Elder Subject: [PATCH v2 2/2] ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting non-scalar controls Date: Tue, 7 Oct 2025 19:27:44 +0900 Message-ID: <20251007102747.2746478-3-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20251007102747.2746478-1-paul.elder@ideasonboard.com> References: <20251007102747.2746478-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The ControlInfos of non-scalar controls that are reported in controls() must have scalar min/max and non-scalar default values for controls that have a defined size. Currently this is relevant to the following controls: - ColourGains - ColourCorrectionMatrix - FrameDurationLimits A mismatch of scalarness in these fields causes deserialization errors in ControlSerializer which manifest when IPAs are run in isolation. Fix the scalarness of these controls where relevant. Signed-off-by: Paul Elder --- src/ipa/ipu3/ipu3.cpp | 4 +++- src/ipa/mali-c55/mali-c55.cpp | 5 ++++- src/ipa/rkisp1/algorithms/awb.cpp | 5 ++++- src/ipa/rkisp1/rkisp1.cpp | 4 +++- src/ipa/rpi/common/ipa_base.cpp | 7 ++++++- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 1cae08bf255f..e71639a16522 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -280,10 +281,11 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo, uint64_t frameSize = lineLength * frameHeights[i]; frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); } + std::array defFrameDurations = { frameDurations[2], frameDurations[2] }; controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0], frameDurations[1], - frameDurations[2]); + Span{ defFrameDurations }); controls.merge(context_.ctrlMap); *ipaControls = ControlInfoMap(std::move(controls), controls::controls); diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp index 7d45e7310aec..c63d3b2bb7be 100644 --- a/src/ipa/mali-c55/mali-c55.cpp +++ b/src/ipa/mali-c55/mali-c55.cpp @@ -5,6 +5,7 @@ * Mali-C55 ISP image processing algorithms */ +#include #include #include #include @@ -14,6 +15,7 @@ #include #include +#include #include #include @@ -234,9 +236,10 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo, frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); } + std::array defFrameDurations = { frameDurations[2], frameDurations[2] }; ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0], frameDurations[1], - frameDurations[2]); + Span{ defFrameDurations }); /* * Compute exposure time limits from the V4L2_CID_EXPOSURE control diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 399fb51be414..a664011a9f0d 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -91,7 +91,10 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData) kMaxColourTemperature, kDefaultColourTemperature); cmap[&controls::AwbEnable] = ControlInfo(false, true); - cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, 1.0f); + + std::array defColourGains = { 1.0f, 1.0f }; + cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, + Span{ defColourGains }); if (!tuningData.contains("algorithm")) LOG(RkISP1Awb, Info) << "No AWB algorithm specified." diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index fa22bfc34904..3f098610a06a 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -436,10 +436,12 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, uint64_t frameSize = lineLength * frameHeights[i]; frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); } + std::array defFrameDurations = { frameDurations[2], frameDurations[2] }; /* \todo Move this (and other agc-related controls) to agc */ context_.ctrlMap[&controls::FrameDurationLimits] = - ControlInfo(frameDurations[0], frameDurations[1], frameDurations[2]); + ControlInfo(frameDurations[0], frameDurations[1], + ControlValue(Span{ defFrameDurations })); ctrlMap.insert(context_.ctrlMap.begin(), context_.ctrlMap.end()); *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 8dfe35cc3267..67dab6cd970c 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -7,6 +7,7 @@ #include "ipa_base.h" +#include #include #include @@ -243,10 +244,14 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa * based on the current sensor mode. */ ControlInfoMap::Map ctrlMap = ipaControls; + std::array defFrameDurations = { + static_cast(defaultMinFrameDuration.get()), + static_cast(defaultMinFrameDuration.get()) + }; ctrlMap[&controls::FrameDurationLimits] = ControlInfo(static_cast(mode_.minFrameDuration.get()), static_cast(mode_.maxFrameDuration.get()), - static_cast(defaultMinFrameDuration.get())); + Span{ defFrameDurations }); ctrlMap[&controls::AnalogueGain] = ControlInfo(static_cast(mode_.minAnalogueGain),