[{"id":36163,"web_url":"https://patchwork.libcamera.org/comment/36163/","msgid":"","date":"2025-10-07T10:45:44","subject":"Re: [PATCH v2 1/2] libcamera: control_serializer: Add array info to\n\tserialized ControlValue","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 07. 12:27 keltezéssel, Paul Elder írta:\n> Array controls (eg. ColourCorrectionMatrix, FrameDurationLimits,\n> ColourGains) are serialized properly by the ControlSerializer, but are\n> not deserialized properly. This is because their arrayness and size are\n> not considered during deserialization.\n> \n> Fix this by adding arrayness and size to the serialized form of all\n> ControlValues.\n> \n> This was not noticed before as the default value of the ControlInfo of\n> other array controls had been set to scalar values similar to min/max,\n> and ColourCorrectionMatrix was the first control to properly define a\n> non-scalar default value.\n> \n> Other array controls that define a scalar default value need to be\n> fixed to define a properly sized default ControlInfo value.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=285\n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v2:\n> - make it so that the *serialized form* of ControlValue includes\n> arrayness and size instead\n> - Compared to v1, this ties the arrayness and size information to\n> ControlValue instead of ControlId, which as a side effect allows us\n> to support scalar and array min/max values, not just def values.\n> This also gives us support for variable-length arrays\n> ---\n> .../libcamera/internal/control_serializer.h | 8 +++--\n> src/libcamera/control_serializer.cpp | 30 ++++++++++++++-----\n> 2 files changed, 29 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h\n> index 8a63ae44a13e..683bb13df92f 100644\n> --- a/include/libcamera/internal/control_serializer.h\n> +++ b/include/libcamera/internal/control_serializer.h\n> @@ -41,14 +41,18 @@ public:\n> \tbool isCached(const ControlInfoMap &infoMap);\n> \n> private:\n> +\tstruct ControlValueHeader {\n> +\t\tbool isArray;\n> +\t\tstd::size_t numElements;\n\nWhy not have `type` in this as well?\n\n\n> +\t};\n> +\n> \tstatic size_t binarySize(const ControlValue &value);\n> \tstatic size_t binarySize(const ControlInfo &info);\n> \n> \tstatic void store(const ControlValue &value, ByteStreamBuffer &buffer);\n> \tstatic void store(const ControlInfo &info, ByteStreamBuffer &buffer);\n> \n> -\tControlValue loadControlValue(ByteStreamBuffer &buffer,\n> -\t\t\t\t bool isArray = false, unsigned int count = 1);\n> +\tControlValue loadControlValue(ByteStreamBuffer &buffer);\n> \tControlInfo loadControlInfo(ByteStreamBuffer &buffer);\n> \n> \tunsigned int serial_;\n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index 050f8512bd52..463f6ab9118d 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -144,7 +144,12 @@ void ControlSerializer::reset()\n> \n> size_t ControlSerializer::binarySize(const ControlValue &value)\n> {\n> -\treturn sizeof(ControlType) + value.data().size_bytes();\n> +\t/*\n> +\t * Allocate extra space to save isArray and the number of elements, to\n> +\t * support array-type ControlValues.\n> +\t */\n> +\treturn sizeof(ControlType) + sizeof(ControlValueHeader) +\n> +\t value.data().size_bytes();\n> }\n> \n> size_t ControlSerializer::binarySize(const ControlInfo &info)\n> @@ -197,6 +202,13 @@ void ControlSerializer::store(const ControlValue &value,\n> {\n> \tconst ControlType type = value.type();\n> \tbuffer.write(&type);\n> +\n> +\tControlValueHeader cvh = {\n> +\t\tvalue.isArray(),\n> +\t\tvalue.numElements(),\n> +\t};\n> +\tbuffer.write(&cvh);\n> +\n> \tbuffer.write(value.data());\n> }\n> \n> @@ -368,6 +380,10 @@ int ControlSerializer::serialize(const ControlList &list,\n> \t\tstruct ipa_control_value_entry entry;\n> \t\tentry.id = id;\n> \t\tentry.type = value.type();\n> +\t\t/*\n> +\t\t * .is_array and .count are now unused as these have been moved\n> +\t\t * to ControlSerializer::store(const ControlValue &, ByteStreamBuffer &)\n> +\t\t */\n> \t\tentry.is_array = value.isArray();\n> \t\tentry.count = value.numElements();\n\nI think `type` is unused as well. So I wonder if there are any downsides to removing\nthe unused fields from `ipa_control_value_entry` and maybe even `ipa_control_info_entry`?\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \t\tentry.offset = values.offset();\n> @@ -382,16 +398,16 @@ int ControlSerializer::serialize(const ControlList &list,\n> \treturn 0;\n> }\n> \n> -ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n> -\t\t\t\t\t\t bool isArray,\n> -\t\t\t\t\t\t unsigned int count)\n> +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer)\n> {\n> \tControlType type;\n> \tbuffer.read(&type);\n> \n> -\tControlValue value;\n> +\tControlValueHeader cvh;\n> +\tbuffer.read(&cvh);\n> \n> -\tvalue.reserve(type, isArray, count);\n> +\tControlValue value;\n> +\tvalue.reserve(type, cvh.isArray, cvh.numElements);\n> \tbuffer.read(value.data());\n> \n> \treturn value;\n> @@ -633,7 +649,7 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> \t\t}\n> \n> \t\tctrls.set(entry->id,\n> -\t\t\t loadControlValue(values, entry->is_array, entry->count));\n> +\t\t\t loadControlValue(values));\n> \t}\n> \n> \treturn ctrls;","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 4A758BF415\n\tfor ;\n\tTue, 7 Oct 2025 10:45:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 486256B5F3;\n\tTue, 7 Oct 2025 12:45:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 68FA369367\n\tfor ;\n\tTue, 7 Oct 2025 12:45:48 +0200 (CEST)","from [192.168.33.24] (185.182.214.142.nat.pool.zt.hu\n\t[185.182.214.142])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D74C2929;\n\tTue, 7 Oct 2025 12:44:14 +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=\"VULA8WRm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759833855;\n\tbh=An4GZexC8htQd7xtABKSCdsAZ0PduQ0f6KC8xzbq0l4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=VULA8WRmzgKjS31F656UwQKaDFIKuSIQWw2BzleMLcwxn6VHKzCIV8Dt5LgfewIK5\n\tbClQ/ELXwnhAO+zeI+9Ix5B3imcmufvaYifylEb6kvRHYKPVGk6qCahgGrOesl5zOZ\n\tmDKUyyzAwVZRGprX0ljgOry/NbPXqFi66b2x/8CU=","Message-ID":"","Date":"Tue, 7 Oct 2025 12:45:44 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/2] libcamera: control_serializer: Add array info to\n\tserialized ControlValue","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251007102747.2746478-1-paul.elder@ideasonboard.com>\n\t<20251007102747.2746478-2-paul.elder@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251007102747.2746478-2-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":36170,"web_url":"https://patchwork.libcamera.org/comment/36170/","msgid":"<175989927136.2583768.3778221908051339128@neptunite.rasen.tech>","date":"2025-10-08T04:54:31","subject":"Re: [PATCH v2 1/2] libcamera: control_serializer: Add array info to\n\tserialized ControlValue","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-07 19:45:44)\n> 2025. 10. 07. 12:27 keltezéssel, Paul Elder írta:\n> > Array controls (eg. ColourCorrectionMatrix, FrameDurationLimits,\n> > ColourGains) are serialized properly by the ControlSerializer, but are\n> > not deserialized properly. This is because their arrayness and size are\n> > not considered during deserialization.\n> > \n> > Fix this by adding arrayness and size to the serialized form of all\n> > ControlValues.\n> > \n> > This was not noticed before as the default value of the ControlInfo of\n> > other array controls had been set to scalar values similar to min/max,\n> > and ColourCorrectionMatrix was the first control to properly define a\n> > non-scalar default value.\n> > \n> > Other array controls that define a scalar default value need to be\n> > fixed to define a properly sized default ControlInfo value.\n> > \n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=285\n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > Changes in v2:\n> > - make it so that the *serialized form* of ControlValue includes\n> > arrayness and size instead\n> > - Compared to v1, this ties the arrayness and size information to\n> > ControlValue instead of ControlId, which as a side effect allows us\n> > to support scalar and array min/max values, not just def values.\n> > This also gives us support for variable-length arrays\n> > ---\n> > .../libcamera/internal/control_serializer.h | 8 +++--\n> > src/libcamera/control_serializer.cpp | 30 ++++++++++++++-----\n> > 2 files changed, 29 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h\n> > index 8a63ae44a13e..683bb13df92f 100644\n> > --- a/include/libcamera/internal/control_serializer.h\n> > +++ b/include/libcamera/internal/control_serializer.h\n> > @@ -41,14 +41,18 @@ public:\n> > bool isCached(const ControlInfoMap &infoMap);\n> > \n> > private:\n> > + struct ControlValueHeader {\n> > + bool isArray;\n> > + std::size_t numElements;\n> \n> Why not have `type` in this as well?\n\ntype is already serialized in ipa_control_{info,value}_entry, so I thought we\ndon't need to duplicate/move it here (just kidding, it was an oversight).\n\nI suppose an ABI breaking release is our chance to change the serialized\nformat. If we're going to redesign all of this... I feel like instead of moving\nthings from ipa_control_{info,value}_entry here, it would be better to move\nthis there. Maybe it would be better to reuse ipa_control_value_entry for\nControlInfos, since a ControlInfo is just a collection of three ControlValues.\n\nWhat do you think?\n\n> \n> \n> > + };\n> > +\n> > static size_t binarySize(const ControlValue &value);\n> > static size_t binarySize(const ControlInfo &info);\n> > \n> > static void store(const ControlValue &value, ByteStreamBuffer &buffer);\n> > static void store(const ControlInfo &info, ByteStreamBuffer &buffer);\n> > \n> > - ControlValue loadControlValue(ByteStreamBuffer &buffer,\n> > - bool isArray = false, unsigned int count = 1);\n> > + ControlValue loadControlValue(ByteStreamBuffer &buffer);\n> > ControlInfo loadControlInfo(ByteStreamBuffer &buffer);\n> > \n> > unsigned int serial_;\n> > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> > index 050f8512bd52..463f6ab9118d 100644\n> > --- a/src/libcamera/control_serializer.cpp\n> > +++ b/src/libcamera/control_serializer.cpp\n> > @@ -144,7 +144,12 @@ void ControlSerializer::reset()\n> > \n> > size_t ControlSerializer::binarySize(const ControlValue &value)\n> > {\n> > - return sizeof(ControlType) + value.data().size_bytes();\n> > + /*\n> > + * Allocate extra space to save isArray and the number of elements, to\n> > + * support array-type ControlValues.\n> > + */\n> > + return sizeof(ControlType) + sizeof(ControlValueHeader) +\n> > + value.data().size_bytes();\n> > }\n> > \n> > size_t ControlSerializer::binarySize(const ControlInfo &info)\n> > @@ -197,6 +202,13 @@ void ControlSerializer::store(const ControlValue &value,\n> > {\n> > const ControlType type = value.type();\n> > buffer.write(&type);\n> > +\n> > + ControlValueHeader cvh = {\n> > + value.isArray(),\n> > + value.numElements(),\n> > + };\n> > + buffer.write(&cvh);\n> > +\n> > buffer.write(value.data());\n> > }\n> > \n> > @@ -368,6 +380,10 @@ int ControlSerializer::serialize(const ControlList &list,\n> > struct ipa_control_value_entry entry;\n> > entry.id = id;\n> > entry.type = value.type();\n> > + /*\n> > + * .is_array and .count are now unused as these have been moved\n> > + * to ControlSerializer::store(const ControlValue &, ByteStreamBuffer &)\n> > + */\n> > entry.is_array = value.isArray();\n> > entry.count = value.numElements();\n> \n> I think `type` is unused as well. So I wonder if there are any downsides to removing\n> the unused fields from `ipa_control_value_entry` and maybe even `ipa_control_info_entry`?\n\nafaict it is used in ControlSerializer::loadControlValue(ByteStreamBuffer &buffer)\nand ControlSerializer::deserialize(ByteStreamBuffer &buffer)\nto deserialize ControlValues.\n\n\nThanks,\n\nPaul\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> > entry.offset = values.offset();\n> > @@ -382,16 +398,16 @@ int ControlSerializer::serialize(const ControlList &list,\n> > return 0;\n> > }\n> > \n> > -ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n> > - bool isArray,\n> > - unsigned int count)\n> > +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer)\n> > {\n> > ControlType type;\n> > buffer.read(&type);\n> > \n> > - ControlValue value;\n> > + ControlValueHeader cvh;\n> > + buffer.read(&cvh);\n> > \n> > - value.reserve(type, isArray, count);\n> > + ControlValue value;\n> > + value.reserve(type, cvh.isArray, cvh.numElements);\n> > buffer.read(value.data());\n> > \n> > return value;\n> > @@ -633,7 +649,7 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> > }\n> > \n> > ctrls.set(entry->id,\n> > - loadControlValue(values, entry->is_array, entry->count));\n> > + loadControlValue(values));\n> > }\n> > \n> > return ctrls;\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 A04DAC324C\n\tfor ;\n\tWed, 8 Oct 2025 04:54:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 813266B5F3;\n\tWed, 8 Oct 2025 06:54:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 237336936E\n\tfor ;\n\tWed, 8 Oct 2025 06:54:38 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:a284:e18b:3861:6ee1])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id A33401E3F; \n\tWed, 8 Oct 2025 06:53:03 +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=\"QfL/g/L/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759899184;\n\tbh=MnzlI1lLHW0I5Y9XgK5TXpylHG3quyRpRdaELlk3rm4=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=QfL/g/L/eP0SA2r1hKeDqPRzGWX6YPSnI/JsJO398eRR8rpHdBA4NPLJz71s/PQoc\n\tM3V+X/F7CCxJiu5gx4/MByUeL9pcwSyBDP4WptDtK5cCJSO878d7anNFZuF0bly9d6\n\tVd01NE5K4+eNhWlutUR+T3p2SMhVUVDGabyfsaik=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20251007102747.2746478-1-paul.elder@ideasonboard.com>\n\t<20251007102747.2746478-2-paul.elder@ideasonboard.com>\n\t","Subject":"Re: [PATCH v2 1/2] libcamera: control_serializer: Add array info to\n\tserialized ControlValue","From":"Paul Elder ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 08 Oct 2025 13:54:31 +0900","Message-ID":"<175989927136.2583768.3778221908051339128@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\" "}},{"id":36171,"web_url":"https://patchwork.libcamera.org/comment/36171/","msgid":"","date":"2025-10-08T08:04:24","subject":"Re: [PATCH v2 1/2] libcamera: control_serializer: Add array info to\n\tserialized ControlValue","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 08. 6:54 keltezéssel, Paul Elder írta:\n> Quoting Barnabás Pőcze (2025-10-07 19:45:44)\n>> 2025. 10. 07. 12:27 keltezéssel, Paul Elder írta:\n>>> Array controls (eg. ColourCorrectionMatrix, FrameDurationLimits,\n>>> ColourGains) are serialized properly by the ControlSerializer, but are\n>>> not deserialized properly. This is because their arrayness and size are\n>>> not considered during deserialization.\n>>>\n>>> Fix this by adding arrayness and size to the serialized form of all\n>>> ControlValues.\n>>>\n>>> This was not noticed before as the default value of the ControlInfo of\n>>> other array controls had been set to scalar values similar to min/max,\n>>> and ColourCorrectionMatrix was the first control to properly define a\n>>> non-scalar default value.\n>>>\n>>> Other array controls that define a scalar default value need to be\n>>> fixed to define a properly sized default ControlInfo value.\n>>>\n>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=285\n>>> Signed-off-by: Paul Elder \n>>>\n>>> ---\n>>> Changes in v2:\n>>> - make it so that the *serialized form* of ControlValue includes\n>>> arrayness and size instead\n>>> - Compared to v1, this ties the arrayness and size information to\n>>> ControlValue instead of ControlId, which as a side effect allows us\n>>> to support scalar and array min/max values, not just def values.\n>>> This also gives us support for variable-length arrays\n>>> ---\n>>> .../libcamera/internal/control_serializer.h | 8 +++--\n>>> src/libcamera/control_serializer.cpp | 30 ++++++++++++++-----\n>>> 2 files changed, 29 insertions(+), 9 deletions(-)\n>>>\n>>> diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h\n>>> index 8a63ae44a13e..683bb13df92f 100644\n>>> --- a/include/libcamera/internal/control_serializer.h\n>>> +++ b/include/libcamera/internal/control_serializer.h\n>>> @@ -41,14 +41,18 @@ public:\n>>> bool isCached(const ControlInfoMap &infoMap);\n>>>\n>>> private:\n>>> + struct ControlValueHeader {\n>>> + bool isArray;\n>>> + std::size_t numElements;\n>>\n>> Why not have `type` in this as well?\n> \n> type is already serialized in ipa_control_{info,value}_entry, so I thought we\n> don't need to duplicate/move it here (just kidding, it was an oversight).\n> \n> I suppose an ABI breaking release is our chance to change the serialized\n> format. If we're going to redesign all of this... I feel like instead of moving\n> things from ipa_control_{info,value}_entry here, it would be better to move\n> this there. Maybe it would be better to reuse ipa_control_value_entry for\n> ControlInfos, since a ControlInfo is just a collection of three ControlValues.\n> \n> What do you think?\n\n`ipa_control_value_entry` has an `id`, it seems to be specifically for representing\nan entry in a `ControlList`. If that is removed and then something like\n\n struct ipa_control_list_entry {\n uint32_t id;\n ipa_control_value_entry val;\n };\n\nis added, then I suppose you could extend `ipa_control_info_entry` with\n\n ipa_control_value_entry vals[3];\n\nor similar. I believe it could work. My preference is towards having the meta\ndata (headers, etc.) right next to the data, because I feel that usually lends\nitself to easier (de)serialization, and since the main purpose here is IPC,\nlosing random access into these \"lists\" does not seem to be a great disadvantage.\nOn the other hand, that is not how this has been implemented, so maybe it's better\nto stick to the current structure.\n\n\n> \n>>\n>>\n>>> + };\n>>> +\n>>> static size_t binarySize(const ControlValue &value);\n>>> static size_t binarySize(const ControlInfo &info);\n>>>\n>>> static void store(const ControlValue &value, ByteStreamBuffer &buffer);\n>>> static void store(const ControlInfo &info, ByteStreamBuffer &buffer);\n>>>\n>>> - ControlValue loadControlValue(ByteStreamBuffer &buffer,\n>>> - bool isArray = false, unsigned int count = 1);\n>>> + ControlValue loadControlValue(ByteStreamBuffer &buffer);\n>>> ControlInfo loadControlInfo(ByteStreamBuffer &buffer);\n>>>\n>>> unsigned int serial_;\n>>> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n>>> index 050f8512bd52..463f6ab9118d 100644\n>>> --- a/src/libcamera/control_serializer.cpp\n>>> +++ b/src/libcamera/control_serializer.cpp\n>>> @@ -144,7 +144,12 @@ void ControlSerializer::reset()\n>>>\n>>> size_t ControlSerializer::binarySize(const ControlValue &value)\n>>> {\n>>> - return sizeof(ControlType) + value.data().size_bytes();\n>>> + /*\n>>> + * Allocate extra space to save isArray and the number of elements, to\n>>> + * support array-type ControlValues.\n>>> + */\n>>> + return sizeof(ControlType) + sizeof(ControlValueHeader) +\n>>> + value.data().size_bytes();\n>>> }\n>>>\n>>> size_t ControlSerializer::binarySize(const ControlInfo &info)\n>>> @@ -197,6 +202,13 @@ void ControlSerializer::store(const ControlValue &value,\n>>> {\n>>> const ControlType type = value.type();\n>>> buffer.write(&type);\n>>> +\n>>> + ControlValueHeader cvh = {\n>>> + value.isArray(),\n>>> + value.numElements(),\n>>> + };\n>>> + buffer.write(&cvh);\n>>> +\n>>> buffer.write(value.data());\n>>> }\n>>>\n>>> @@ -368,6 +380,10 @@ int ControlSerializer::serialize(const ControlList &list,\n>>> struct ipa_control_value_entry entry;\n>>> entry.id = id;\n>>> entry.type = value.type();\n>>> + /*\n>>> + * .is_array and .count are now unused as these have been moved\n>>> + * to ControlSerializer::store(const ControlValue &, ByteStreamBuffer &)\n>>> + */\n>>> entry.is_array = value.isArray();\n>>> entry.count = value.numElements();\n>>\n>> I think `type` is unused as well. So I wonder if there are any downsides to removing\n>> the unused fields from `ipa_control_value_entry` and maybe even `ipa_control_info_entry`?\n> \n> afaict it is used in ControlSerializer::loadControlValue(ByteStreamBuffer &buffer)\n> and ControlSerializer::deserialize(ByteStreamBuffer &buffer)\n> to deserialize ControlValues.\n\nAhh, you're right. `ipa_control_info_entry::type` is used at least. But\n`ipa_control_value_entry::{type,is_array}` do not seem to be, after this change.\nBut if everything is moved to the `_entry` types and the `ControlValueHeader`\nis removed, this point is moot.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> \n> Thanks,\n> \n> Paul\n> \n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>\n>>> entry.offset = values.offset();\n>>> @@ -382,16 +398,16 @@ int ControlSerializer::serialize(const ControlList &list,\n>>> return 0;\n>>> }\n>>>\n>>> -ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n>>> - bool isArray,\n>>> - unsigned int count)\n>>> +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer)\n>>> {\n>>> ControlType type;\n>>> buffer.read(&type);\n>>>\n>>> - ControlValue value;\n>>> + ControlValueHeader cvh;\n>>> + buffer.read(&cvh);\n>>>\n>>> - value.reserve(type, isArray, count);\n>>> + ControlValue value;\n>>> + value.reserve(type, cvh.isArray, cvh.numElements);\n>>> buffer.read(value.data());\n>>>\n>>> return value;\n>>> @@ -633,7 +649,7 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n>>> }\n>>>\n>>> ctrls.set(entry->id,\n>>> - loadControlValue(values, entry->is_array, entry->count));\n>>> + loadControlValue(values));\n>>> }\n>>>\n>>> return ctrls;\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 4F3C8C324C\n\tfor ;\n\tWed, 8 Oct 2025 08:04:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3B9F6B5AA;\n\tWed, 8 Oct 2025 10:04:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 38C216B5A2\n\tfor ;\n\tWed, 8 Oct 2025 10:04:28 +0200 (CEST)","from [192.168.33.20] (185.182.214.142.nat.pool.zt.hu\n\t[185.182.214.142])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0ABE5191B;\n\tWed, 8 Oct 2025 10:02:53 +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=\"Gz05Tkja\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759910574;\n\tbh=qQF5Oz6rHnVsgnU2FYwx8rFbNOF2fuCH4R52BGsQIiI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=Gz05Tkjag0CzLD7yECTgb5+2dM5mRB93H/VxpuUD/0fcDNmkRdmqGdHjC/+JXDM7b\n\tzOi7coOPVZQNU1YTnBWFjhGF0axWySp7GY0OGhHVBIrQ/JmQM7+cT2YperVRXwIA3C\n\tKTWMjSUhy8RDI900ltkUuD/Dk0u4ZpHY/PA3ST0g=","Message-ID":"","Date":"Wed, 8 Oct 2025 10:04:24 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/2] libcamera: control_serializer: Add array info to\n\tserialized ControlValue","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251007102747.2746478-1-paul.elder@ideasonboard.com>\n\t<20251007102747.2746478-2-paul.elder@ideasonboard.com>\n\t\n\t\n\t<175989927136.2583768.3778221908051339128@neptunite.rasen.tech>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<175989927136.2583768.3778221908051339128@neptunite.rasen.tech>","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\" "}}]