[1/2] libcamera: control_serializer: Deserialize array ControlInfos

24300 diff mbox series

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 checking arrayness and size of the ControlInfo's default
value during deserialization of ControlInfoMap. Only the default value
needs to be checked as that is the only part of the ControlInfo that
will be non-scalar; min/max are defined to be scalar values.

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 <paul.elder@ideasonboard.com>
---
 include/libcamera/internal/control_serializer.h | 3 ++-
 src/libcamera/control_serializer.cpp            | 9 ++++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

Hi

2025. 09. 10. 11:35 keltezéssel, Paul Elder írta:
> 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 checking arrayness and size of the ControlInfo's default
> value during deserialization of ControlInfoMap. Only the default value
> needs to be checked as that is the only part of the ControlInfo that
> will be non-scalar; min/max are defined to be scalar values.

I wonder if maybe extending `ControlSerializer::store()` to store "isArray" and
"numElements" wouldn't provide a more generic and more robust solution?


Regards,
Barnabás Pőcze

> 
> 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 <paul.elder@ideasonboard.com>
> ---
>   include/libcamera/internal/control_serializer.h | 3 ++-
>   src/libcamera/control_serializer.cpp            | 9 ++++++---
>   2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h
> index 8a63ae44a13e..79d6eff2a940 100644
> --- a/include/libcamera/internal/control_serializer.h
> +++ b/include/libcamera/internal/control_serializer.h
> @@ -49,7 +49,8 @@ private:
>   
>   	ControlValue loadControlValue(ByteStreamBuffer &buffer,
>   				      bool isArray = false, unsigned int count = 1);
> -	ControlInfo loadControlInfo(ByteStreamBuffer &buffer);
> +	ControlInfo loadControlInfo(ByteStreamBuffer &buffer,
> +				    bool isArray, unsigned int count);
>   
>   	unsigned int serial_;
>   	unsigned int serialSeed_;
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 050f8512bd52..db3ddaaa2e78 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -397,11 +397,14 @@ ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,
>   	return value;
>   }
>   
> -ControlInfo ControlSerializer::loadControlInfo(ByteStreamBuffer &b)
> +ControlInfo ControlSerializer::loadControlInfo(ByteStreamBuffer &b,
> +					       bool isArray,
> +					       unsigned int count)
>   {
> +	/* min and max are scalars */
>   	ControlValue min = loadControlValue(b);
>   	ControlValue max = loadControlValue(b);
> -	ControlValue def = loadControlValue(b);
> +	ControlValue def = loadControlValue(b, isArray, count);
>   
>   	return ControlInfo(min, max, def);
>   }
> @@ -519,7 +522,7 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
>   		}
>   
>   		/* Create and store the ControlInfo. */
> -		ctrls.emplace(controlId, loadControlInfo(values));
> +		ctrls.emplace(controlId, loadControlInfo(values, controlId->isArray(), controlId->size()));
>   	}
>   
>   	/*