[libcamera-devel,27/31] libcamera: control_serializer: Add support for array controls

Message ID 20200229164254.23604-28-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Add support for array controls
Related show

Commit Message

Laurent Pinchart Feb. 29, 2020, 4:42 p.m. UTC
From: Jacopo Mondi <jacopo@jmondi.org>

Add support for serializing and deserializing control values that store
arrays of values. The serialized format is extended to explicitly handle
arrays.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/control_serializer.cpp       | 80 ++++++++++++----------
 src/libcamera/include/control_serializer.h |  6 +-
 2 files changed, 49 insertions(+), 37 deletions(-)

Comments

Kieran Bingham March 5, 2020, 5:29 p.m. UTC | #1
On 29/02/2020 16:42, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>
> 
> Add support for serializing and deserializing control values that store
> arrays of values. The serialized format is extended to explicitly handle
> arrays.

Does this need any update in any documentation of the serialization format?


> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/control_serializer.cpp       | 80 ++++++++++++----------
>  src/libcamera/include/control_serializer.h |  6 +-
>  2 files changed, 49 insertions(+), 37 deletions(-)
> 
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 6cac70739468..31b03e6e6d7c 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -14,6 +14,7 @@
>  #include <ipa/ipa_controls.h>
>  #include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
> +#include <libcamera/span.h>
>  
>  #include "byte_stream_buffer.h"
>  #include "log.h"
> @@ -279,8 +280,9 @@ int ControlSerializer::serialize(const ControlList &list,
>  
>  		struct ipa_control_value_entry entry;
>  		entry.id = id;
> -		entry.count = 1;
>  		entry.type = value.type();
> +		entry.is_array = value.isArray();
> +		entry.count = value.numElements();
>  		entry.offset = values.offset();
>  		entries.write(&entry);
>  
> @@ -293,52 +295,56 @@ int ControlSerializer::serialize(const ControlList &list,
>  	return 0;
>  }
>  
> -template<>
> -ControlValue ControlSerializer::load<ControlValue>(ControlType type,
> -						   ByteStreamBuffer &b)
> +template<typename T>
> +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,
> +						 bool isArray,
> +						 unsigned int count)
> +{
> +	ControlValue value;
> +
> +	const T *data = buffer.read<T>(count);
> +	if (!data)
> +		return value;
> +
> +	if (isArray)
> +		value.set(Span<const T>{ data, count });
> +	else
> +		value.set(*data);
> +
> +	return value;
> +}
> +
> +ControlValue ControlSerializer::loadControlValue(ControlType type,
> +						 ByteStreamBuffer &buffer,
> +						 bool isArray,
> +						 unsigned int count)
>  {
>  	switch (type) {
> -	case ControlTypeBool: {
> -		bool value;
> -		b.read(&value);
> -		return ControlValue(value);
> -	}
> +	case ControlTypeBool:
> +		return loadControlValue<bool>(buffer, isArray, count);
>  
> -	case ControlTypeInteger8: {
> -		int8_t value;
> -		b.read(&value);
> -		return ControlValue(value);
> -	}
> +	case ControlTypeInteger8:
> +		return loadControlValue<int8_t>(buffer, isArray, count);
>  
> -	case ControlTypeInteger32: {
> -		int32_t value;
> -		b.read(&value);
> -		return ControlValue(value);
> -	}
> +	case ControlTypeInteger32:
> +		return loadControlValue<int32_t>(buffer, isArray, count);
>  
> -	case ControlTypeInteger64: {
> -		int64_t value;
> -		b.read(&value);
> -		return ControlValue(value);
> -	}
> +	case ControlTypeInteger64:
> +		return loadControlValue<int64_t>(buffer, isArray, count);
>  
> -	case ControlTypeFloat: {
> -		float value;
> -		b.read(&value);
> -		return ControlValue(value);
> -	}
> +	case ControlTypeFloat:
> +		return loadControlValue<float>(buffer, isArray, count);
>  
>  	default:
>  		return ControlValue();
>  	}
>  }
>  
> -template<>
> -ControlRange ControlSerializer::load<ControlRange>(ControlType type,
> -						   ByteStreamBuffer &b)
> +ControlRange ControlSerializer::loadControlRange(ControlType type,
> +						 ByteStreamBuffer &b)
>  {
> -	ControlValue min = load<ControlValue>(type, b);
> -	ControlValue max = load<ControlValue>(type, b);
> +	ControlValue min = loadControlValue(type, b);
> +	ControlValue max = loadControlValue(type, b);
>  
>  	return ControlRange(min, max);
>  }
> @@ -412,7 +418,7 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
>  
>  		/* Create and store the ControlRange. */
>  		ctrls.emplace(controlIds_.back().get(),
> -			      load<ControlRange>(type, values));
> +			      loadControlRange(type, values));
>  	}
>  
>  	/*
> @@ -500,7 +506,9 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer
>  		}
>  
>  		ControlType type = static_cast<ControlType>(entry->type);
> -		ctrls.set(entry->id, load<ControlValue>(type, values));
> +		ctrls.set(entry->id,
> +			  loadControlValue(type, values, entry->is_array,
> +					   entry->count));
>  	}
>  
>  	return ctrls;
> diff --git a/src/libcamera/include/control_serializer.h b/src/libcamera/include/control_serializer.h
> index 55259913a2ca..b91d13155f5e 100644
> --- a/src/libcamera/include/control_serializer.h
> +++ b/src/libcamera/include/control_serializer.h
> @@ -41,7 +41,11 @@ private:
>  	static void store(const ControlRange &range, ByteStreamBuffer &buffer);
>  
>  	template<typename T>
> -	T load(ControlType type, ByteStreamBuffer &b);
> +	ControlValue loadControlValue(ByteStreamBuffer &buffer, bool isArray,
> +				      unsigned int count);
> +	ControlValue loadControlValue(ControlType type, ByteStreamBuffer &buffer,
> +				      bool isArray = false, unsigned int count = 1);
> +	ControlRange loadControlRange(ControlType type, ByteStreamBuffer &buffer);
>  
>  	unsigned int serial_;
>  	std::vector<std::unique_ptr<ControlId>> controlIds_;
>
Laurent Pinchart March 6, 2020, 3 p.m. UTC | #2
Hi Kieran,

On Thu, Mar 05, 2020 at 05:29:36PM +0000, Kieran Bingham wrote:
> On 29/02/2020 16:42, Laurent Pinchart wrote:
> > From: Jacopo Mondi <jacopo@jmondi.org>
> > 
> > Add support for serializing and deserializing control values that store
> > arrays of values. The serialized format is extended to explicitly handle
> > arrays.
> 
> Does this need any update in any documentation of the serialization format?

For once the documentation was a step ahead and already described
support for controls with an array of values :-)

> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  src/libcamera/control_serializer.cpp       | 80 ++++++++++++----------
> >  src/libcamera/include/control_serializer.h |  6 +-
> >  2 files changed, 49 insertions(+), 37 deletions(-)
> > 
> > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> > index 6cac70739468..31b03e6e6d7c 100644
> > --- a/src/libcamera/control_serializer.cpp
> > +++ b/src/libcamera/control_serializer.cpp
> > @@ -14,6 +14,7 @@
> >  #include <ipa/ipa_controls.h>
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/controls.h>
> > +#include <libcamera/span.h>
> >  
> >  #include "byte_stream_buffer.h"
> >  #include "log.h"
> > @@ -279,8 +280,9 @@ int ControlSerializer::serialize(const ControlList &list,
> >  
> >  		struct ipa_control_value_entry entry;
> >  		entry.id = id;
> > -		entry.count = 1;
> >  		entry.type = value.type();
> > +		entry.is_array = value.isArray();
> > +		entry.count = value.numElements();
> >  		entry.offset = values.offset();
> >  		entries.write(&entry);
> >  
> > @@ -293,52 +295,56 @@ int ControlSerializer::serialize(const ControlList &list,
> >  	return 0;
> >  }
> >  
> > -template<>
> > -ControlValue ControlSerializer::load<ControlValue>(ControlType type,
> > -						   ByteStreamBuffer &b)
> > +template<typename T>
> > +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,
> > +						 bool isArray,
> > +						 unsigned int count)
> > +{
> > +	ControlValue value;
> > +
> > +	const T *data = buffer.read<T>(count);
> > +	if (!data)
> > +		return value;
> > +
> > +	if (isArray)
> > +		value.set(Span<const T>{ data, count });
> > +	else
> > +		value.set(*data);
> > +
> > +	return value;
> > +}
> > +
> > +ControlValue ControlSerializer::loadControlValue(ControlType type,
> > +						 ByteStreamBuffer &buffer,
> > +						 bool isArray,
> > +						 unsigned int count)
> >  {
> >  	switch (type) {
> > -	case ControlTypeBool: {
> > -		bool value;
> > -		b.read(&value);
> > -		return ControlValue(value);
> > -	}
> > +	case ControlTypeBool:
> > +		return loadControlValue<bool>(buffer, isArray, count);
> >  
> > -	case ControlTypeInteger8: {
> > -		int8_t value;
> > -		b.read(&value);
> > -		return ControlValue(value);
> > -	}
> > +	case ControlTypeInteger8:
> > +		return loadControlValue<int8_t>(buffer, isArray, count);
> >  
> > -	case ControlTypeInteger32: {
> > -		int32_t value;
> > -		b.read(&value);
> > -		return ControlValue(value);
> > -	}
> > +	case ControlTypeInteger32:
> > +		return loadControlValue<int32_t>(buffer, isArray, count);
> >  
> > -	case ControlTypeInteger64: {
> > -		int64_t value;
> > -		b.read(&value);
> > -		return ControlValue(value);
> > -	}
> > +	case ControlTypeInteger64:
> > +		return loadControlValue<int64_t>(buffer, isArray, count);
> >  
> > -	case ControlTypeFloat: {
> > -		float value;
> > -		b.read(&value);
> > -		return ControlValue(value);
> > -	}
> > +	case ControlTypeFloat:
> > +		return loadControlValue<float>(buffer, isArray, count);
> >  
> >  	default:
> >  		return ControlValue();
> >  	}
> >  }
> >  
> > -template<>
> > -ControlRange ControlSerializer::load<ControlRange>(ControlType type,
> > -						   ByteStreamBuffer &b)
> > +ControlRange ControlSerializer::loadControlRange(ControlType type,
> > +						 ByteStreamBuffer &b)
> >  {
> > -	ControlValue min = load<ControlValue>(type, b);
> > -	ControlValue max = load<ControlValue>(type, b);
> > +	ControlValue min = loadControlValue(type, b);
> > +	ControlValue max = loadControlValue(type, b);
> >  
> >  	return ControlRange(min, max);
> >  }
> > @@ -412,7 +418,7 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
> >  
> >  		/* Create and store the ControlRange. */
> >  		ctrls.emplace(controlIds_.back().get(),
> > -			      load<ControlRange>(type, values));
> > +			      loadControlRange(type, values));
> >  	}
> >  
> >  	/*
> > @@ -500,7 +506,9 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer
> >  		}
> >  
> >  		ControlType type = static_cast<ControlType>(entry->type);
> > -		ctrls.set(entry->id, load<ControlValue>(type, values));
> > +		ctrls.set(entry->id,
> > +			  loadControlValue(type, values, entry->is_array,
> > +					   entry->count));
> >  	}
> >  
> >  	return ctrls;
> > diff --git a/src/libcamera/include/control_serializer.h b/src/libcamera/include/control_serializer.h
> > index 55259913a2ca..b91d13155f5e 100644
> > --- a/src/libcamera/include/control_serializer.h
> > +++ b/src/libcamera/include/control_serializer.h
> > @@ -41,7 +41,11 @@ private:
> >  	static void store(const ControlRange &range, ByteStreamBuffer &buffer);
> >  
> >  	template<typename T>
> > -	T load(ControlType type, ByteStreamBuffer &b);
> > +	ControlValue loadControlValue(ByteStreamBuffer &buffer, bool isArray,
> > +				      unsigned int count);
> > +	ControlValue loadControlValue(ControlType type, ByteStreamBuffer &buffer,
> > +				      bool isArray = false, unsigned int count = 1);
> > +	ControlRange loadControlRange(ControlType type, ByteStreamBuffer &buffer);
> >  
> >  	unsigned int serial_;
> >  	std::vector<std::unique_ptr<ControlId>> controlIds_;

Patch

diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
index 6cac70739468..31b03e6e6d7c 100644
--- a/src/libcamera/control_serializer.cpp
+++ b/src/libcamera/control_serializer.cpp
@@ -14,6 +14,7 @@ 
 #include <ipa/ipa_controls.h>
 #include <libcamera/control_ids.h>
 #include <libcamera/controls.h>
+#include <libcamera/span.h>
 
 #include "byte_stream_buffer.h"
 #include "log.h"
@@ -279,8 +280,9 @@  int ControlSerializer::serialize(const ControlList &list,
 
 		struct ipa_control_value_entry entry;
 		entry.id = id;
-		entry.count = 1;
 		entry.type = value.type();
+		entry.is_array = value.isArray();
+		entry.count = value.numElements();
 		entry.offset = values.offset();
 		entries.write(&entry);
 
@@ -293,52 +295,56 @@  int ControlSerializer::serialize(const ControlList &list,
 	return 0;
 }
 
-template<>
-ControlValue ControlSerializer::load<ControlValue>(ControlType type,
-						   ByteStreamBuffer &b)
+template<typename T>
+ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,
+						 bool isArray,
+						 unsigned int count)
+{
+	ControlValue value;
+
+	const T *data = buffer.read<T>(count);
+	if (!data)
+		return value;
+
+	if (isArray)
+		value.set(Span<const T>{ data, count });
+	else
+		value.set(*data);
+
+	return value;
+}
+
+ControlValue ControlSerializer::loadControlValue(ControlType type,
+						 ByteStreamBuffer &buffer,
+						 bool isArray,
+						 unsigned int count)
 {
 	switch (type) {
-	case ControlTypeBool: {
-		bool value;
-		b.read(&value);
-		return ControlValue(value);
-	}
+	case ControlTypeBool:
+		return loadControlValue<bool>(buffer, isArray, count);
 
-	case ControlTypeInteger8: {
-		int8_t value;
-		b.read(&value);
-		return ControlValue(value);
-	}
+	case ControlTypeInteger8:
+		return loadControlValue<int8_t>(buffer, isArray, count);
 
-	case ControlTypeInteger32: {
-		int32_t value;
-		b.read(&value);
-		return ControlValue(value);
-	}
+	case ControlTypeInteger32:
+		return loadControlValue<int32_t>(buffer, isArray, count);
 
-	case ControlTypeInteger64: {
-		int64_t value;
-		b.read(&value);
-		return ControlValue(value);
-	}
+	case ControlTypeInteger64:
+		return loadControlValue<int64_t>(buffer, isArray, count);
 
-	case ControlTypeFloat: {
-		float value;
-		b.read(&value);
-		return ControlValue(value);
-	}
+	case ControlTypeFloat:
+		return loadControlValue<float>(buffer, isArray, count);
 
 	default:
 		return ControlValue();
 	}
 }
 
-template<>
-ControlRange ControlSerializer::load<ControlRange>(ControlType type,
-						   ByteStreamBuffer &b)
+ControlRange ControlSerializer::loadControlRange(ControlType type,
+						 ByteStreamBuffer &b)
 {
-	ControlValue min = load<ControlValue>(type, b);
-	ControlValue max = load<ControlValue>(type, b);
+	ControlValue min = loadControlValue(type, b);
+	ControlValue max = loadControlValue(type, b);
 
 	return ControlRange(min, max);
 }
@@ -412,7 +418,7 @@  ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
 
 		/* Create and store the ControlRange. */
 		ctrls.emplace(controlIds_.back().get(),
-			      load<ControlRange>(type, values));
+			      loadControlRange(type, values));
 	}
 
 	/*
@@ -500,7 +506,9 @@  ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer
 		}
 
 		ControlType type = static_cast<ControlType>(entry->type);
-		ctrls.set(entry->id, load<ControlValue>(type, values));
+		ctrls.set(entry->id,
+			  loadControlValue(type, values, entry->is_array,
+					   entry->count));
 	}
 
 	return ctrls;
diff --git a/src/libcamera/include/control_serializer.h b/src/libcamera/include/control_serializer.h
index 55259913a2ca..b91d13155f5e 100644
--- a/src/libcamera/include/control_serializer.h
+++ b/src/libcamera/include/control_serializer.h
@@ -41,7 +41,11 @@  private:
 	static void store(const ControlRange &range, ByteStreamBuffer &buffer);
 
 	template<typename T>
-	T load(ControlType type, ByteStreamBuffer &b);
+	ControlValue loadControlValue(ByteStreamBuffer &buffer, bool isArray,
+				      unsigned int count);
+	ControlValue loadControlValue(ControlType type, ByteStreamBuffer &buffer,
+				      bool isArray = false, unsigned int count = 1);
+	ControlRange loadControlRange(ControlType type, ByteStreamBuffer &buffer);
 
 	unsigned int serial_;
 	std::vector<std::unique_ptr<ControlId>> controlIds_;