[PATCH v1] libcamera: controls: Add ControlList::merge overload that moves entries

[Stefan Klug]

When merging two control lists, copies of the entries are made. Add a
overload of the merge function that accepts a non-const source and
provides a merge without copy. This was deemed important in the light of
debug metadata were the data should not be copied more often than
necessary.

This change does not break the API, but it changes the behavior of
applications because the source list now gets modified if it is
non-const (which might be a common case).

Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>

---

Todo: After we agreed on a approach, the tests need to be adapted
accordingly.
---
 include/libcamera/controls.h |  1 +
 src/libcamera/controls.cpp   | 32 +++++++++++++++++++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index ca60bbacad17..2c28ab9447a8 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -401,6 +401,7 @@ public:
 
 	void clear() { controls_.clear(); }
 	void merge(const ControlList &source, MergePolicy policy = MergePolicy::KeepExisting);
+	void merge(ControlList &source, MergePolicy policy = MergePolicy::KeepExisting);
 
 	bool contains(unsigned int id) const;
 
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 62185d643ecd..cb977e1212d8 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -963,11 +963,13 @@ ControlList::ControlList(const ControlInfoMap &infoMap,
  * present in *this, then those elements are only overwritten if
  * \a policy is MergePolicy::OverwriteExisting.
  *
+ * If \a source is non-const, it will contain the dropped entries after the
+ * merge. These are either the overwritten ones if policy was
+ * MergePolicy::OverwriteExisting or the skipped ones if policy was
+ * MergePolicy::KeepExisting
+ *
  * Only control lists created from the same ControlIdMap or ControlInfoMap may
  * be merged. Attempting to do otherwise results in undefined behaviour.
- *
- * \todo Reimplement or implement an overloaded version which internally uses
- * std::unordered_map::merge() and accepts a non-const argument.
  */
 void ControlList::merge(const ControlList &source, MergePolicy policy)
 {
@@ -995,6 +997,30 @@ void ControlList::merge(const ControlList &source, MergePolicy policy)
 	}
 }
 
+/**
+ * \copydoc ControlList::merge(const ControlList &source, MergePolicy policy)
+ */
+void ControlList::merge(ControlList &source, MergePolicy policy)
+{
+	/**
+	 * \todo ASSERT that the current and source ControlList are derived
+	 * from a compatible ControlIdMap, to prevent undefined behaviour due to
+	 * id collisions.
+	 *
+	 * This can not currently be a direct pointer comparison due to the
+	 * duplication of the ControlIdMaps in the isolated IPA use cases.
+	 * Furthermore, manually checking each entry of the id map is identical
+	 * is expensive.
+	 * See https://bugs.libcamera.org/show_bug.cgi?id=31 for further details
+	 */
+
+	if (policy == MergePolicy::OverwriteExisting) {
+		source.controls_.merge(controls_);
+		source.controls_.swap(controls_);
+	} else
+		controls_.merge(source.controls_);
+}
+
 /**
  * \brief Check if the list contains a control with the specified \a id
  * \param[in] id The control numerical ID
-- 
2.43.0