[libcamera-devel] [PATCH] libcamera: controls: Remove merge assertion

Kieran Bingham kieran.bingham at ideasonboard.com
Fri May 7 14:44:44 CEST 2021


The ControlList merge operation is protected with an ASSERT to guarantee
that the two lists are compatible.

Unfortunately this assertion fails when we run IPAs in an isolated case
as while the lists are compatible, the isolated IPA has a unique
instance of the id map. This breaks the pointer comparison, and the
assertion fails with a false positive.

Remove the assertion, leaving only a todo in it's place as this breaks
active users of the library.

Bugzilla: https://bugs.libcamera.org/show_bug.cgi?id=31
Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
---
 src/libcamera/controls.cpp | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index b763148d4391..5aef4e7145bd 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -890,7 +890,17 @@ ControlList::ControlList(const ControlInfoMap &infoMap, ControlValidator *valida
  */
 void ControlList::merge(const ControlList &source)
 {
-	ASSERT(idmap_ == source.idmap_);
+	/**
+	 * \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
+	 */
 
 	for (const auto &ctrl : source) {
 		if (contains(ctrl.first)) {
-- 
2.25.1



More information about the libcamera-devel mailing list