{"id":21562,"url":"https://patchwork.libcamera.org/api/patches/21562/?format=json","web_url":"https://patchwork.libcamera.org/patch/21562/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20241009131109.547742-1-stefan.klug@ideasonboard.com>","date":"2024-10-09T13:10:53","name":"[v1] libcamera: controls: Add ControlList::merge overload that moves entries","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"ccd7fb8cc8d692b9c4403b1da11d59c560c3d7ab","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/?format=json","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/21562/mbox/","series":[{"id":4674,"url":"https://patchwork.libcamera.org/api/series/4674/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4674","date":"2024-10-09T13:10:53","name":"[v1] libcamera: controls: Add ControlList::merge overload that moves entries","version":1,"mbox":"https://patchwork.libcamera.org/series/4674/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/21562/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/21562/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 117CFC32E0\n\tfor ;\n\tWed, 9 Oct 2024 13:11:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2239063538;\n\tWed, 9 Oct 2024 15:11:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C9BF618C5\n\tfor ;\n\tWed, 9 Oct 2024 15:11:14 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:749:b3f1:dbb0:6c33])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C5A32594;\n\tWed, 9 Oct 2024 15:09:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hWa8AirV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728479376;\n\tbh=TN8oLvmY7bPuRsbq82NhnPUgP/5DXsijY+6Z1PDWEUg=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=hWa8AirVuq/i3cqoNrOJQZ71aM+Me24FA/yaTWCUeJy2eFbsGfc57/vE4G4q1N5dw\n\tA2TPOoxNPQdlNWYOIKKaT8h6Rk5P1VoC3L1rIyq1cD/609JM+kqSewtiWZDlqLlbU4\n\tj9ECobBup3KjdI8C/H16ZwSIcbi1HIMduDtLyYWU=","From":"Stefan Klug ","To":"libcamera-devel@lists.libcamera.org","Cc":"Stefan Klug ","Subject":"[PATCH v1] libcamera: controls: Add ControlList::merge overload that\n\tmoves entries","Date":"Wed, 9 Oct 2024 15:10:53 +0200","Message-ID":"<20241009131109.547742-1-stefan.klug@ideasonboard.com>","X-Mailer":"git-send-email 2.43.0","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"When merging two control lists, copies of the entries are made. Add a\noverload of the merge function that accepts a non-const source and\nprovides a merge without copy. This was deemed important in the light of\ndebug metadata were the data should not be copied more often than\nnecessary.\n\nThis change does not break the API, but it changes the behavior of\napplications because the source list now gets modified if it is\nnon-const (which might be a common case).\n\nSigned-off-by: Stefan Klug \n\n---\n\nTodo: After we agreed on a approach, the tests need to be adapted\naccordingly.\n---\n include/libcamera/controls.h | 1 +\n src/libcamera/controls.cpp | 32 +++++++++++++++++++++++++++++---\n 2 files changed, 30 insertions(+), 3 deletions(-)","diff":"diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex ca60bbacad17..2c28ab9447a8 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -401,6 +401,7 @@ public:\n \n \tvoid clear() { controls_.clear(); }\n \tvoid merge(const ControlList &source, MergePolicy policy = MergePolicy::KeepExisting);\n+\tvoid merge(ControlList &source, MergePolicy policy = MergePolicy::KeepExisting);\n \n \tbool contains(unsigned int id) const;\n \ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 62185d643ecd..cb977e1212d8 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -963,11 +963,13 @@ ControlList::ControlList(const ControlInfoMap &infoMap,\n * present in *this, then those elements are only overwritten if\n * \\a policy is MergePolicy::OverwriteExisting.\n *\n+ * If \\a source is non-const, it will contain the dropped entries after the\n+ * merge. These are either the overwritten ones if policy was\n+ * MergePolicy::OverwriteExisting or the skipped ones if policy was\n+ * MergePolicy::KeepExisting\n+ *\n * Only control lists created from the same ControlIdMap or ControlInfoMap may\n * be merged. Attempting to do otherwise results in undefined behaviour.\n- *\n- * \\todo Reimplement or implement an overloaded version which internally uses\n- * std::unordered_map::merge() and accepts a non-const argument.\n */\n void ControlList::merge(const ControlList &source, MergePolicy policy)\n {\n@@ -995,6 +997,30 @@ void ControlList::merge(const ControlList &source, MergePolicy policy)\n \t}\n }\n \n+/**\n+ * \\copydoc ControlList::merge(const ControlList &source, MergePolicy policy)\n+ */\n+void ControlList::merge(ControlList &source, MergePolicy policy)\n+{\n+\t/**\n+\t * \\todo ASSERT that the current and source ControlList are derived\n+\t * from a compatible ControlIdMap, to prevent undefined behaviour due to\n+\t * id collisions.\n+\t *\n+\t * This can not currently be a direct pointer comparison due to the\n+\t * duplication of the ControlIdMaps in the isolated IPA use cases.\n+\t * Furthermore, manually checking each entry of the id map is identical\n+\t * is expensive.\n+\t * See https://bugs.libcamera.org/show_bug.cgi?id=31 for further details\n+\t */\n+\n+\tif (policy == MergePolicy::OverwriteExisting) {\n+\t\tsource.controls_.merge(controls_);\n+\t\tsource.controls_.swap(controls_);\n+\t} else\n+\t\tcontrols_.merge(source.controls_);\n+}\n+\n /**\n * \\brief Check if the list contains a control with the specified \\a id\n * \\param[in] id The control numerical ID\n","prefixes":["v1"]}