[libcamera-devel,v2,1/7] libcamera: controls: Add zero-copy set API for ControlValue

Message ID 20200321003640.2156-2-laurent.pinchart@ideasonboard.com
State Accepted
Commit 8daf20485b90af2065e3db0e3fd0cd5b72fd7ac4
Headers show
Series
  • Add support for V4L2 array controls
Related show

Commit Message

Laurent Pinchart March 21, 2020, 12:36 a.m. UTC
Extend the ControlValue class with a reserve() function to set the value
without actually copying data, and a non-const data() function that
allows writing data directly to the ControlValue storage. This allows
allocating memory directly in ControlValue, potentially removing a data
copy.

Note that this change was implemented before ByteStreamBuffer gained the
zero-copy read() variant, and doesn't actually save a copy in the
control serializer. It however still simplifies
ControlSerializer::loadControlValue().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/controls.h               |  4 ++
 src/libcamera/control_serializer.cpp       | 61 ++--------------------
 src/libcamera/controls.cpp                 | 54 ++++++++++++++-----
 src/libcamera/include/control_serializer.h |  3 --
 4 files changed, 50 insertions(+), 72 deletions(-)

Patch

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index 2a6657128f17..4b2e7e9cdd6c 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -115,6 +115,7 @@  public:
 	bool isArray() const { return isArray_; }
 	std::size_t numElements() const { return numElements_; }
 	Span<const uint8_t> data() const;
+	Span<uint8_t> data();
 
 	std::string toString() const;
 
@@ -174,6 +175,9 @@  public:
 		    value.data(), value.size(), sizeof(typename T::value_type));
 	}
 
+	void reserve(ControlType type, bool isArray = false,
+		     std::size_t numElements = 1);
+
 private:
 	ControlType type_ : 8;
 	bool isArray_;
diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
index 808419f246c0..fcff5e56fbf7 100644
--- a/src/libcamera/control_serializer.cpp
+++ b/src/libcamera/control_serializer.cpp
@@ -295,70 +295,17 @@  int ControlSerializer::serialize(const ControlList &list,
 	return 0;
 }
 
-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;
-}
-
-template<>
-ControlValue ControlSerializer::loadControlValue<std::string>(ByteStreamBuffer &buffer,
-						 bool isArray,
-						 unsigned int count)
-{
-	ControlValue value;
-
-	const char *data = buffer.read<char>(count);
-	if (!data)
-		return value;
-
-	value.set(std::string{ data, count });
-
-	return value;
-}
-
 ControlValue ControlSerializer::loadControlValue(ControlType type,
 						 ByteStreamBuffer &buffer,
 						 bool isArray,
 						 unsigned int count)
 {
-	switch (type) {
-	case ControlTypeBool:
-		return loadControlValue<bool>(buffer, isArray, count);
-
-	case ControlTypeByte:
-		return loadControlValue<uint8_t>(buffer, isArray, count);
-
-	case ControlTypeInteger32:
-		return loadControlValue<int32_t>(buffer, isArray, count);
-
-	case ControlTypeInteger64:
-		return loadControlValue<int64_t>(buffer, isArray, count);
-
-	case ControlTypeFloat:
-		return loadControlValue<float>(buffer, isArray, count);
-
-	case ControlTypeString:
-		return loadControlValue<std::string>(buffer, isArray, count);
+	ControlValue value;
 
-	case ControlTypeNone:
-		return ControlValue();
-	}
+	value.reserve(type, isArray, count);
+	buffer.read(value.data());
 
-	return ControlValue();
+	return value;
 }
 
 ControlInfo ControlSerializer::loadControlInfo(ControlType type,
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 2568447478d5..540cc026417a 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -189,6 +189,15 @@  Span<const uint8_t> ControlValue::data() const
 	return { data, size };
 }
 
+/**
+ * \copydoc ControlValue::data() const
+ */
+Span<uint8_t> ControlValue::data()
+{
+	Span<const uint8_t> data = const_cast<const ControlValue *>(this)->data();
+	return { const_cast<uint8_t *>(data.data()), data.size() };
+}
+
 /**
  * \brief Assemble and return a string describing the value
  * \return A string describing the ControlValue
@@ -312,23 +321,44 @@  void ControlValue::set(ControlType type, bool isArray, const void *data,
 {
 	ASSERT(elementSize == ControlValueSize[type]);
 
-	release();
+	reserve(type, isArray, numElements);
+
+	Span<uint8_t> storage = ControlValue::data();
+	memcpy(storage.data(), data, storage.size());
+}
+
+/**
+ * \brief Set the control type and reserve memory
+ * \param[in] type The control type
+ * \param[in] isArray True to make the value an array
+ * \param[in] numElements The number of elements
+ *
+ * This function sets the type of the control value to \a type, and reserves
+ * memory to store the control value. If \a isArray is true, the instance
+ * becomes an array control and storage for \a numElements is reserved.
+ * Otherwise the instance becomes a simple control, numElements is ignored, and
+ * storage for the single element is reserved.
+ */
+void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElements)
+{
+	if (!isArray)
+		numElements = 1;
+
+	std::size_t oldSize = numElements_ * ControlValueSize[type_];
+	std::size_t newSize = numElements * ControlValueSize[type];
+
+	if (oldSize != newSize)
+		release();
 
 	type_ = type;
-	numElements_ = numElements;
 	isArray_ = isArray;
+	numElements_ = numElements;
 
-	std::size_t size = elementSize * numElements;
-	void *storage;
-
-	if (size > sizeof(value_)) {
-		storage_ = reinterpret_cast<void *>(new uint8_t[size]);
-		storage = storage_;
-	} else {
-		storage = reinterpret_cast<void *>(&value_);
-	}
+	if (oldSize == newSize)
+		return;
 
-	memcpy(storage, data, size);
+	if (newSize > sizeof(value_))
+		storage_ = reinterpret_cast<void *>(new uint8_t[newSize]);
 }
 
 /**
diff --git a/src/libcamera/include/control_serializer.h b/src/libcamera/include/control_serializer.h
index 70aa28fd5f58..99bacd920fce 100644
--- a/src/libcamera/include/control_serializer.h
+++ b/src/libcamera/include/control_serializer.h
@@ -40,9 +40,6 @@  private:
 	static void store(const ControlValue &value, ByteStreamBuffer &buffer);
 	static void store(const ControlInfo &info, ByteStreamBuffer &buffer);
 
-	template<typename T>
-	ControlValue loadControlValue(ByteStreamBuffer &buffer, bool isArray,
-				      unsigned int count);
 	ControlValue loadControlValue(ControlType type, ByteStreamBuffer &buffer,
 				      bool isArray = false, unsigned int count = 1);
 	ControlInfo loadControlInfo(ControlType type, ByteStreamBuffer &buffer);