From patchwork Mon May 19 09:12:07 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23384 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 18F27BD78E for ; Mon, 19 May 2025 09:12:17 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C4D0868D7F; Mon, 19 May 2025 11:12:14 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="h3J0VXJx"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A595468B66 for ; Mon, 19 May 2025 11:12:11 +0200 (CEST) Received: from pb-laptop.local (185.221.142.248.nat.pool.zt.hu [185.221.142.248]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A3E1E9B; Mon, 19 May 2025 11:11:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1747645911; bh=jQdQTiLgCys8qV56C4vVoyRZxZM61gDkTPASkDRjelI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=h3J0VXJxPhp4SZKrBSnFpmj0iZLeU0dN+DOf/+I/P9FFquvk3iGjJhJLtadvKrQ4U SxVo7Guy0RsDFOPV6GlnkCAFRII4ZL16c5QP66FC5afufB1fK5NkvHD6bInpEYgFF3 on2lrIPQCSGD1/bwsdB4b3RjZG01t1C1ypk2RQ1A= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Paul Elder Subject: [PATCH v2 2/2] Revert "controls: Add boolean constructors for ControlInfo" Date: Mon, 19 May 2025 11:12:07 +0200 Message-ID: <20250519091207.561269-2-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250519091207.561269-1-barnabas.pocze@ideasonboard.com> References: <20250519091207.561269-1-barnabas.pocze@ideasonboard.com> 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" This reverts commit 10cdc914dad282b4ca0ad11067d5c6d446af1fcc. The contstructors introduced by that commit are not used anywhere, and they do not match the existing practice for boolean controls. Specifically, every single boolean control is described by calling the `ControlInfo(ControlValue, ControlValue, ControlValue)` constructor. Crucially, that constructor does not set `values_`, while the two removed contructors do. And whether or not `values_` has any elements is currently used as an implicit sign to decide whether or not the control is "enum-like", and those are assumed to have type `int32_t`. For example, any boolean control described using any of the two removed constructors would cause an assertion in failure in `CameraSession::listControls()` when calling `value.get()`. Signed-off-by: Barnabás Pőcze Reviewed-by: Paul Elder Reviewed-by: Stefan Klug --- changes in v2: * keep tests --- include/libcamera/controls.h | 3 --- src/libcamera/controls.cpp | 29 ----------------------------- test/controls/control_info.cpp | 9 ++++----- 3 files changed, 4 insertions(+), 37 deletions(-) -- 2.49.0 diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 2ae4ec3d4..32fb31f78 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -334,8 +333,6 @@ public: const ControlValue &def = {}); explicit ControlInfo(Span values, const ControlValue &def = {}); - explicit ControlInfo(std::set values, bool def); - explicit ControlInfo(bool value); const ControlValue &min() const { return min_; } const ControlValue &max() const { return max_; } diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 70f6f6092..eaa34e70b 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -625,35 +625,6 @@ ControlInfo::ControlInfo(Span values, values_.push_back(value); } -/** - * \brief Construct a boolean ControlInfo with both boolean values - * \param[in] values The control valid boolean values (both true and false) - * \param[in] def The control default boolean value - * - * Construct a ControlInfo for a boolean control, where both true and false are - * valid values. \a values must be { false, true } (the order is irrelevant). - * The minimum value will always be false, and the maximum always true. The - * default value is \a def. - */ -ControlInfo::ControlInfo(std::set values, bool def) - : min_(false), max_(true), def_(def), values_({ false, true }) -{ - ASSERT(values.count(def) && values.size() == 2); -} - -/** - * \brief Construct a boolean ControlInfo with only one valid value - * \param[in] value The control valid boolean value - * - * Construct a ControlInfo for a boolean control, where there is only valid - * value. The minimum, maximum, and default values will all be \a value. - */ -ControlInfo::ControlInfo(bool value) - : min_(value), max_(value), def_(value) -{ - values_ = { value }; -} - /** * \fn ControlInfo::min() * \brief Retrieve the minimum value of the control diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index e1bb43f0e..905d92dd5 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -50,7 +50,7 @@ protected: * Test information retrieval from a control with boolean * values. */ - ControlInfo aeEnable({ false, true }, false); + ControlInfo aeEnable(false, true, false); if (aeEnable.min().get() != false || aeEnable.def().get() != false || @@ -59,13 +59,12 @@ protected: return TestFail; } - if (aeEnable.values()[0].get() != false || - aeEnable.values()[1].get() != true) { + if (!aeEnable.values().empty()) { cout << "Invalid control values for AeEnable" << endl; return TestFail; } - ControlInfo awbEnable(true); + ControlInfo awbEnable(true, true, true); if (awbEnable.min().get() != true || awbEnable.def().get() != true || @@ -74,7 +73,7 @@ protected: return TestFail; } - if (awbEnable.values()[0].get() != true) { + if (!awbEnable.values().empty()) { cout << "Invalid control values for AwbEnable" << endl; return TestFail; }