[libcamera-devel] [PATCH 6/6] libcamera: control_serializer: Serialize info::def()

[paul.elder at ideasonboard.com]

Hi Jacopo,

On Wed, Sep 01, 2021 at 04:38:00PM +0200, Jacopo Mondi wrote:
> The ControlInfo class was originally designed to only transport
> the control's minimum and maximum values which represent the control's
> valid limits.
> 
> Later the default value of the control has been added to the ControlInfo
> class, but the control serializer implementation has not been updated
> accordingly.
> 
> This causes issues to IPA modules making use of ControlInfo::def() as,

s/to/in/

> when running in isolation, they would receive a 0 value as default.

s/ as default//

There's no way to turn off receiving the "default" :D

> 
> Fix that by serializing and deserializing the additional ControlValue
> and update the protocol description accordingly.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/control_serializer.cpp |  6 ++++--
>  src/libcamera/ipa_controls.cpp       | 14 ++++++++------
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 7eef041a16ee..4e8045251938 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -105,7 +105,7 @@ size_t ControlSerializer::binarySize(const ControlValue &value)
>  
>  size_t ControlSerializer::binarySize(const ControlInfo &info)
>  {
> -	return binarySize(info.min()) + binarySize(info.max());
> +	return binarySize(info.min()) + binarySize(info.max()) + binarySize(info.def());
>  }
>  
>  /**
> @@ -158,6 +158,7 @@ void ControlSerializer::store(const ControlInfo &info, ByteStreamBuffer &buffer)
>  {
>  	store(info.min(), buffer);
>  	store(info.max(), buffer);
> +	store(info.def(), buffer);
>  }
>  
>  /**
> @@ -346,8 +347,9 @@ ControlInfo ControlSerializer::loadControlInfo(ControlType type,
>  
>  	ControlValue min = loadControlValue(type, b);
>  	ControlValue max = loadControlValue(type, b);
> +	ControlValue def = loadControlValue(type, b);
>  
> -	return ControlInfo(min, max);
> +	return ControlInfo(min, max, def);
>  }
>  
>  /**
> diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp
> index fb98cda30ede..3d172d66b30f 100644
> --- a/src/libcamera/ipa_controls.cpp
> +++ b/src/libcamera/ipa_controls.cpp
> @@ -108,17 +108,19 @@
>   *           +-------------------------+       .
>   *         / | ...                     |       | entry[n].offset
>   *         | +-------------------------+ <-----´
> - *    Data | | minimum value (#n)      | \
> - * section | +-------------------------+ | Entry #n
> - *         | | maximum value (#n)      | /
> + *         | | minimum value (#n)      | \
> + *    Data | +-------------------------+ |
> + * section | | maximum value (#n)      | | Entry #n
> + *         | +-------------------------+ |
> + *         | | default value (#n)      | /
>   *         | +-------------------------+
>   *         \ | ...                     |
>   *           +-------------------------+
>   * ~~~~
>   *
> - * The minimum and maximum value are stored in the platform's native data
> - * format. The ipa_control_info_entry::offset field stores the offset from the
> - * beginning of the data section to the info data.
> + * The minimum, maximum and default value are stored in the platform's native

s/maximum/maximum,/

s/value/values/


Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> + * data format. The ipa_control_info_entry::offset field stores the offset from
> + * the beginning of the data section to the info data.
>   *
>   * Info data in the data section shall be stored in the same order as the
>   * entries array, shall be aligned to a multiple of 8 bytes, and shall be
> -- 
> 2.32.0
>