[libcamera-devel] [PATCH v4 1/2] libcamera: control_serializer: store/load all ControlValue types
Umang Jain
umang.jain at ideasonboard.com
Mon Sep 12 11:25:27 CEST 2022
Hi Christian
Thank you for the patch,
On 9/4/22 3:03 AM, Christian Rauch via libcamera-devel wrote:
> The min/max/def ControlValue of a ControlInfo can take arbitrary types that
> are different from each other and different from the ControlId type.
> The serialiser serialises these ControlValue separately by their type but
> does not store the type. The deserialiser assumes that ControlValue types
> match the ControlId type. If this is not the case, deserialisation will try
> to deserialise values of the wrong type.
>
> Fix this by serialising each of the min/max/def ControlValue's ControlType
> and storing it just before the serialised ControlValue.
>
> Fixes: https://bugs.libcamera.org/show_bug.cgi?id=137
>
> Signed-off-by: Christian Rauch <Rauch.Christian at gmx.de>
> Tested-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
Adding a unit test for this in
test/serialization/control_serialization.cpp might be good idea,
but for this patch,
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> .../libcamera/internal/control_serializer.h | 4 +--
> src/libcamera/control_serializer.cpp | 28 +++++++++----------
> 2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h
> index 99e57fee..a38ca6b0 100644
> --- a/include/libcamera/internal/control_serializer.h
> +++ b/include/libcamera/internal/control_serializer.h
> @@ -47,9 +47,9 @@ private:
> static void store(const ControlValue &value, ByteStreamBuffer &buffer);
> static void store(const ControlInfo &info, ByteStreamBuffer &buffer);
>
> - ControlValue loadControlValue(ControlType type, ByteStreamBuffer &buffer,
> + ControlValue loadControlValue(ByteStreamBuffer &buffer,
> bool isArray = false, unsigned int count = 1);
> - ControlInfo loadControlInfo(ControlType type, ByteStreamBuffer &buffer);
> + ControlInfo loadControlInfo(ByteStreamBuffer &buffer);
>
> unsigned int serial_;
> unsigned int serialSeed_;
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index e87d2362..0cf719bd 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -144,7 +144,7 @@ void ControlSerializer::reset()
>
> size_t ControlSerializer::binarySize(const ControlValue &value)
> {
> - return value.data().size_bytes();
> + return sizeof(ControlType) + value.data().size_bytes();
> }
>
> size_t ControlSerializer::binarySize(const ControlInfo &info)
> @@ -195,6 +195,8 @@ size_t ControlSerializer::binarySize(const ControlList &list)
> void ControlSerializer::store(const ControlValue &value,
> ByteStreamBuffer &buffer)
> {
> + const ControlType type = value.type();
> + buffer.write(&type);
> buffer.write(value.data());
> }
>
> @@ -379,11 +381,13 @@ int ControlSerializer::serialize(const ControlList &list,
> return 0;
> }
>
> -ControlValue ControlSerializer::loadControlValue(ControlType type,
> - ByteStreamBuffer &buffer,
> +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,
> bool isArray,
> unsigned int count)
> {
> + ControlType type;
> + buffer.read(&type);
> +
> ControlValue value;
>
> value.reserve(type, isArray, count);
> @@ -392,15 +396,11 @@ ControlValue ControlSerializer::loadControlValue(ControlType type,
> return value;
> }
>
> -ControlInfo ControlSerializer::loadControlInfo(ControlType type,
> - ByteStreamBuffer &b)
> +ControlInfo ControlSerializer::loadControlInfo(ByteStreamBuffer &b)
> {
> - if (type == ControlTypeString)
> - type = ControlTypeInteger32;
> -
> - ControlValue min = loadControlValue(type, b);
> - ControlValue max = loadControlValue(type, b);
> - ControlValue def = loadControlValue(type, b);
> + ControlValue min = loadControlValue(b);
> + ControlValue max = loadControlValue(b);
> + ControlValue def = loadControlValue(b);
>
> return ControlInfo(min, max, def);
> }
> @@ -513,7 +513,7 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
> }
>
> /* Create and store the ControlInfo. */
> - ctrls.emplace(controlId, loadControlInfo(type, values));
> + ctrls.emplace(controlId, loadControlInfo(values));
> }
>
> /*
> @@ -624,10 +624,8 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer
> return {};
> }
>
> - ControlType type = static_cast<ControlType>(entry->type);
> ctrls.set(entry->id,
> - loadControlValue(type, values, entry->is_array,
> - entry->count));
> + loadControlValue(values, entry->is_array, entry->count));
> }
>
> return ctrls;
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list