[libcamera-devel] [PATCH v3 3/5] libcamera: controls: Use ControlIdMap in deserialization

[Jacopo Mondi]

Introduce a new field in the controls serialization protocol to
allow discerning which ControlIdMap a ControlInfoMap refers to.

The newly introduced IdMapType enumeration describes the possible
info maps:
- Either the globally available controls::controls and
  properties::properties maps, which are valid across IPC boundaries
- A ControlIdMap created locally by the V4L2 device, which is not valid
  across the IPC boundaries

At de-serialization time the idMapType filed is inspected and
- If the idmap is a globally available one, there's no need to create
  new ControlId instances when populating the de-serialized
  ControlInfoMap. Use the globally available map to retrieve the
  ControlId reference and use it
- If the idmap is a map only available locally, create a new ControlId
  as it used to happen before this patch.

As a direct consequence, this change allows us to perform lookup by
ControlId reference on de-serialized ControlIdMap that refers to the
libcamera defined controls::controls and properties::properties.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
---
 include/libcamera/ipa/ipa_controls.h |  9 +++-
 src/libcamera/control_serializer.cpp | 65 +++++++++++++++++++++++-----
 src/libcamera/ipa_controls.cpp       | 29 +++++++++++++
 3 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h
index 6d3bf279c22d..f4bb7b77a974 100644
--- a/include/libcamera/ipa/ipa_controls.h
+++ b/include/libcamera/ipa/ipa_controls.h
@@ -15,13 +15,20 @@ extern "C" {
 
 #define IPA_CONTROLS_FORMAT_VERSION	1
 
+enum ipa_controls_id_map_type {
+	IPA_CONTROL_ID_MAP_CONTROLS,
+	IPA_CONTROL_ID_MAP_PROPERTIES,
+	IPA_CONTROL_ID_MAP_V4L2,
+};
+
 struct ipa_controls_header {
 	uint32_t version;
 	uint32_t handle;
 	uint32_t entries;
 	uint32_t size;
 	uint32_t data_offset;
-	uint32_t reserved[3];
+	ipa_controls_id_map_type id_map_type;
+	uint32_t reserved[2];
 };
 
 struct ipa_control_value_entry {
diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
index df6ed93f477e..9127678f25f6 100644
--- a/src/libcamera/control_serializer.cpp
+++ b/src/libcamera/control_serializer.cpp
@@ -17,6 +17,7 @@
 #include <libcamera/control_ids.h>
 #include <libcamera/controls.h>
 #include <libcamera/ipa/ipa_controls.h>
+#include <libcamera/property_ids.h>
 
 #include "libcamera/internal/byte_stream_buffer.h"
 
@@ -188,6 +189,15 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
 	for (const auto &ctrl : infoMap)
 		valuesSize += binarySize(ctrl.second);
 
+	const ControlIdMap *idmap = &infoMap.idmap();
+	enum ipa_controls_id_map_type idMapType;
+	if (idmap == &controls::controls)
+		idMapType = IPA_CONTROL_ID_MAP_CONTROLS;
+	else if (idmap == &properties::properties)
+		idMapType = IPA_CONTROL_ID_MAP_PROPERTIES;
+	else
+		idMapType = IPA_CONTROL_ID_MAP_V4L2;
+
 	/* Prepare the packet header, assign a handle to the ControlInfoMap. */
 	struct ipa_controls_header hdr;
 	hdr.version = IPA_CONTROLS_FORMAT_VERSION;
@@ -195,6 +205,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
 	hdr.entries = infoMap.size();
 	hdr.size = sizeof(hdr) + entriesSize + valuesSize;
 	hdr.data_offset = sizeof(hdr) + entriesSize;
+	hdr.id_map_type = idMapType;
 
 	buffer.write(&hdr);
 
@@ -368,6 +379,33 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
 		return {};
 	}
 
+	/*
+         * Use the ControlIdMap corresponding to the id map type. If the type
+         * references a globally defined id map (such as controls::controls
+         * or properties::properties), use it. Otherwise, create a local id map
+         * that will be populated with dynamically created ControlId instances
+         * when deserializing individual ControlInfoMap entries.
+	 */
+	const ControlIdMap *idMap = nullptr;
+	ControlIdMap *localIdMap = nullptr;
+	switch (hdr->id_map_type) {
+	case IPA_CONTROL_ID_MAP_CONTROLS:
+		idMap = &controls::controls;
+		break;
+	case IPA_CONTROL_ID_MAP_PROPERTIES:
+		idMap = &properties::properties;
+		break;
+	case IPA_CONTROL_ID_MAP_V4L2:
+		controlIdMaps_.emplace_back(std::make_unique<ControlIdMap>());
+		localIdMap = controlIdMaps_.back().get();
+		idMap = localIdMap;
+		break;
+	default:
+		LOG(Serializer, Error)
+			<< "Unknown id map type: " << hdr->id_map_type;
+		return {};
+	}
+
 	ByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr));
 	ByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset);
 
@@ -377,9 +415,6 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
 	}
 
 	ControlInfoMap::Map ctrls;
-	controlIdMaps_.emplace_back(std::make_unique<ControlIdMap>());
-	ControlIdMap *localIdMap = controlIdMaps_.back().get();
-
 	for (unsigned int i = 0; i < hdr->entries; ++i) {
 		const struct ipa_control_info_entry *entry =
 			entries.read<decltype(*entry)>();
@@ -388,15 +423,21 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
 			return {};
 		}
 
-		/* Create and cache the individual ControlId. */
 		ControlType type = static_cast<ControlType>(entry->type);
-		/**
-		 * \todo Find a way to preserve the control name for debugging
-		 * purpose.
-		 */
-		controlIds_.emplace_back(std::make_unique<ControlId>(entry->id, "", type));
-		ControlId *controlId = controlIds_.back().get();
-		(*localIdMap)[entry->id] = controlId;
+
+		/* If we're using a local id map, populate it. */
+		if (localIdMap) {
+			/**
+			 * \todo Find a way to preserve the control name for
+			 * debugging purpose.
+			 */
+			controlIds_.emplace_back(std::make_unique<ControlId>(entry->id,
+									     "", type));
+			(*localIdMap)[entry->id] = controlIds_.back().get();
+		}
+
+		const ControlId *controlId = idMap->at(entry->id);
+		ASSERT(controlId);
 
 		if (entry->offset != values.offset()) {
 			LOG(Serializer, Error)
@@ -413,7 +454,7 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
 	 * Create the ControlInfoMap in the cache, and store the map to handle
 	 * association.
 	 */
-	infoMaps_[hdr->handle] = ControlInfoMap(std::move(ctrls), *localIdMap);
+	infoMaps_[hdr->handle] = ControlInfoMap(std::move(ctrls), *idMap);
 	ControlInfoMap &map = infoMaps_[hdr->handle];
 	infoMapHandles_[&map] = hdr->handle;
 
diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp
index 8fd726513182..fb98cda30ede 100644
--- a/src/libcamera/ipa_controls.cpp
+++ b/src/libcamera/ipa_controls.cpp
@@ -134,6 +134,33 @@
  * \brief The current control serialization format version
  */
 
+/**
+ * \var ipa_controls_id_map_type
+ * \brief Enumerates the different control id map types
+ *
+ * Each ControlInfoMap and ControlList refers to a control id map that
+ * associates the ControlId references to a numerical identifier.
+ * During the serialization procedure the raw pointers to the ControlId
+ * instances cannot be transported on the wire, hence their numerical id is
+ * used to identify them in the serialized data buffer. At deserialization time
+ * it is required to associate back to the numerical id the ControlId instance
+ * it represents. This enumeration describes which ControlIdMap should be
+ * used to perform such operation.
+ *
+ * \var ipa_controls_id_map_type::IPA_CONTROL_ID_MAP_CONTROLS
+ * \brief The numerical control identifier are resolved to a ControlId * using
+ * the global controls::controls id map
+ * \var ipa_controls_id_map_type::IPA_CONTROL_ID_MAP_PROPERTIES
+ * \brief The numerical control identifier are resolved to a ControlId * using
+ * the global properties::properties id map
+ * \var ipa_controls_id_map_type::IPA_CONTROL_ID_MAP_V4L2
+ * \brief ControlId for V4L2 defined controls are created by the video device
+ * that enumerates them, and are not available across the IPC boundaries. The
+ * deserializer shall create new ControlId instances for them as well as store
+ * them in a dedicated ControlIdMap. Only lookup by numerical id can be
+ * performed on de-serialized ControlInfoMap that represents V4L2 controls.
+ */
+
 /**
  * \struct ipa_controls_header
  * \brief Serialized control packet header
@@ -149,6 +176,8 @@
  * The total packet size in bytes
  * \var ipa_controls_header::data_offset
  * Offset in bytes from the beginning of the packet of the data section start
+ * \var ipa_controls_header::id_map_type
+ * The id map type as defined by the ipa_controls_id_map_type enumeration
  * \var ipa_controls_header::reserved
  * Reserved for future extensions
  */
-- 
2.32.0