[libcamera-devel] [PATCH v1.1 22/23] libcamera: control_serializer: Add support for compound controls

Jacopo Mondi jacopo at jmondi.org
Tue Jan 14 17:31:53 CET 2020


Add support for serializing and deserializing control values which
transport compound values.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---

sa rightfully noticed by Laurent in offline discussion, there was quite a big
leak in the code as the Span<> class does not enforce memory ownership, so the
memory location that is provided to it during data loading should be manually
released.

Fix this by wrapping it in an unique_ptr<>

 void ControlSerializer::loadData(ByteStreamBuffer &buffer, unsigned int count,
                                 ControlValue *value)
 {
-       Span<T> data(new T[count], count);
+       std::unique_ptr<T> mem(new T[count]);
+       Span<T> data(mem.get(), count);
        buffer.read(&data);

        /*


Valgrind does not report leaks anymore by complains about unique_ptr<>
destructor getting confused.
==38940== Mismatched free() / delete / delete []
==38940==    at 0x4839EAB: operator delete(void*) (vg_replace_malloc.c:586)
==38940==    by 0x4A02E6D: std::default_delete<int>::operator()(int*) const (unique_ptr.h:81)
==38940==    by 0x4A02BBC: std::unique_ptr<int, std::default_delete<int> >::~unique_ptr() (unique_ptr.h:284)
==38940==    by 0x49FEF84: void libcamera::ControlSerializer::loadData<int>(libcamera::ByteStreamBuffer&, unsigned int, libcamera::ControlValue*) (control_serializer.cpp:377)

Might be a false positive as the heap is clean

==38940== HEAP SUMMARY:
==38940==     in use at exit: 0 bytes in 0 blocks
==38940==   total heap usage: 4,497 allocs, 4,497 frees, 1,350,869 bytes allocated

---
 src/libcamera/control_serializer.cpp       | 124 ++++++++++++++++++---
 src/libcamera/include/control_serializer.h |  10 +-
 2 files changed, 118 insertions(+), 16 deletions(-)

diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
index fd91baf15156..c24cb7a57195 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"
@@ -30,11 +31,15 @@ LOG_DEFINE_CATEGORY(Serializer)
 namespace {

 static constexpr size_t ControlValueSize[] = {
-	[ControlTypeNone]	= 1,
-	[ControlTypeBool]	= sizeof(bool),
-	[ControlTypeInteger32]	= sizeof(int32_t),
-	[ControlTypeInteger64]	= sizeof(int64_t),
-	[ControlTypeFloat]	= sizeof(float),
+	[ControlTypeNone]		= 1,
+	[ControlTypeBool]		= sizeof(bool),
+	[ControlTypeInteger32]		= sizeof(int32_t),
+	[ControlTypeInteger64]		= sizeof(int64_t),
+	[ControlTypeFloat]		= sizeof(float),
+	[ControlTypeCompoundBool]	= sizeof(bool),
+	[ControlTypeCompoundInt32]	= sizeof(int32_t),
+	[ControlTypeCompoundInt64]	= sizeof(int64_t),
+	[ControlTypeCompoundFloat]	= sizeof(float),
 };

 } /* namespace */
@@ -107,7 +112,7 @@ void ControlSerializer::reset()

 size_t ControlSerializer::binarySize(const ControlValue &value)
 {
-	return ControlValueSize[value.type()];
+	return ControlValueSize[value.type()] * value.numElements();
 }

 size_t ControlSerializer::binarySize(const ControlRange &range)
@@ -183,6 +188,30 @@ void ControlSerializer::store(const ControlValue &value,
 		break;
 	}

+	case ControlTypeCompoundBool: {
+		Span<bool> data = value.get<Span<bool>>();
+		buffer.write(&data);
+		break;
+	}
+
+	case ControlTypeCompoundInt32: {
+		Span<int32_t> data = value.get<Span<int32_t>>();
+		buffer.write(data);
+		break;
+	}
+
+	case ControlTypeCompoundInt64: {
+		Span<int64_t> data = value.get<Span<int64_t>>();
+		buffer.write(data);
+		break;
+	}
+
+	case ControlTypeCompoundFloat: {
+		Span<float> data = value.get<Span<float>>();
+		buffer.write(data);
+		break;
+	}
+
 	default:
 		break;
 	}
@@ -318,7 +347,7 @@ int ControlSerializer::serialize(const ControlList &list,

 		struct ipa_control_value_entry entry;
 		entry.id = id;
-		entry.count = 1;
+		entry.count = value.numElements();
 		entry.type = value.type();
 		entry.offset = values.offset();
 		entries.write(&entry);
@@ -332,35 +361,75 @@ int ControlSerializer::serialize(const ControlList &list,
 	return 0;
 }

+template<typename T>
+void ControlSerializer::loadData(ByteStreamBuffer &buffer, unsigned int count,
+				 ControlValue *value)
+{
+	std::unique_ptr<T> mem(new T[count]);
+	Span<T> data(mem.get(), count);
+	buffer.read(&data);
+
+	/*
+	 * Use of ControlValue::set() guarantees the memory content
+	 * is copied into the ControlValue.
+	 */
+	value->set(data);
+}
+
 template<>
 ControlValue ControlSerializer::load<ControlValue>(ControlType type,
-						   ByteStreamBuffer &b)
+						   ByteStreamBuffer &buffer,
+						   unsigned int count)
 {
 	switch (type) {
 	case ControlTypeBool: {
 		bool value;
-		b.read(&value);
+		buffer.read(&value);
 		return ControlValue(value);
 	}

 	case ControlTypeInteger32: {
 		int32_t value;
-		b.read(&value);
+		buffer.read(&value);
 		return ControlValue(value);
 	}

 	case ControlTypeInteger64: {
 		int64_t value;
-		b.read(&value);
+		buffer.read(&value);
 		return ControlValue(value);
 	}

 	case ControlTypeFloat: {
 		float value;
-		b.read(&value);
+		buffer.read(&value);
 		return ControlValue(value);
 	}

+	case ControlTypeCompoundBool: {
+		ControlValue value;
+		loadData<bool>(buffer, count, &value);
+		return value;
+	}
+
+	case ControlTypeCompoundInt32: {
+		ControlValue value;
+		loadData<int32_t>(buffer, count, &value);
+		return value;
+	}
+
+	case ControlTypeCompoundInt64: {
+		ControlValue value;
+		loadData<int64_t>(buffer, count, &value);
+		return value;
+	}
+
+	case ControlTypeCompoundFloat: {
+		ControlValue value;
+		loadData<float>(buffer, count, &value);
+		return value;
+	}
+
 	default:
 		return ControlValue();
 	}
@@ -370,8 +439,32 @@ template<>
 ControlRange ControlSerializer::load<ControlRange>(ControlType type,
 						   ByteStreamBuffer &b)
 {
-	ControlValue min = load<ControlValue>(type, b);
-	ControlValue max = load<ControlValue>(type, b);
+	/*
+	 * The 'type' parameter represents the type of the Control
+	 * the ControlRange refers to. Even if the Control is a compound,
+	 * its range elements are not: adjust the type opportunely.
+	 */
+	ControlType rangeType;
+	switch (type) {
+	case ControlTypeCompoundBool:
+		rangeType = ControlTypeBool;
+		break;
+	case ControlTypeCompoundInt32:
+		rangeType = ControlTypeInteger32;
+		break;
+	case ControlTypeCompoundInt64:
+		rangeType = ControlTypeInteger64;
+		break;
+	case ControlTypeCompoundFloat:
+		rangeType = ControlTypeFloat;
+		break;
+	default:
+		rangeType = type;
+		break;
+	}
+
+	ControlValue min = load<ControlValue>(rangeType, b);
+	ControlValue max = load<ControlValue>(rangeType, b);

 	return ControlRange(min, max);
 }
@@ -519,7 +612,8 @@ 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,
+			  load<ControlValue>(type, values, entry.count));
 	}

 	return ctrls;
diff --git a/src/libcamera/include/control_serializer.h b/src/libcamera/include/control_serializer.h
index 55259913a2ca..2d76ffe7ed84 100644
--- a/src/libcamera/include/control_serializer.h
+++ b/src/libcamera/include/control_serializer.h
@@ -10,6 +10,7 @@
 #include <map>
 #include <memory>
 #include <vector>
+#include <type_traits>

 #include <libcamera/controls.h>

@@ -40,8 +41,15 @@ private:
 	static void store(const ControlValue &value, ByteStreamBuffer &buffer);
 	static void store(const ControlRange &range, ByteStreamBuffer &buffer);

+	template<typename T,
+		 typename std::enable_if<std::is_same<ControlValue, T>::value>::type * = nullptr>
+	T load(ControlType type, ByteStreamBuffer &buffer, unsigned int count = 1);
+	template<typename T,
+		 typename std::enable_if<std::is_same<ControlRange, T>::value>::type * = nullptr>
+	T load(ControlType type, ByteStreamBuffer &buffer);
 	template<typename T>
-	T load(ControlType type, ByteStreamBuffer &b);
+	void loadData(ByteStreamBuffer &buffer, unsigned int count,
+		      ControlValue *value);

 	unsigned int serial_;
 	std::vector<std::unique_ptr<ControlId>> controlIds_;
--
2.24.1



More information about the libcamera-devel mailing list