From patchwork Wed Oct 9 13:10:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 21562 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 117CFC32E0 for ; Wed, 9 Oct 2024 13:11:17 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2239063538; Wed, 9 Oct 2024 15:11:16 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="hWa8AirV"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C9BF618C5 for ; Wed, 9 Oct 2024 15:11:14 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:749:b3f1:dbb0:6c33]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C5A32594; Wed, 9 Oct 2024 15:09:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1728479376; bh=TN8oLvmY7bPuRsbq82NhnPUgP/5DXsijY+6Z1PDWEUg=; h=From:To:Cc:Subject:Date:From; b=hWa8AirVuq/i3cqoNrOJQZ71aM+Me24FA/yaTWCUeJy2eFbsGfc57/vE4G4q1N5dw A2TPOoxNPQdlNWYOIKKaT8h6Rk5P1VoC3L1rIyq1cD/609JM+kqSewtiWZDlqLlbU4 j9ECobBup3KjdI8C/H16ZwSIcbi1HIMduDtLyYWU= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v1] libcamera: controls: Add ControlList::merge overload that moves 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 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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