Patch Detail
Show a patch.
GET /api/patches/23384/?format=api
{ "id": 23384, "url": "https://patchwork.libcamera.org/api/patches/23384/?format=api", "web_url": "https://patchwork.libcamera.org/patch/23384/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20250519091207.561269-2-barnabas.pocze@ideasonboard.com>", "date": "2025-05-19T09:12:07", "name": "[v2,2/2] Revert \"controls: Add boolean constructors for ControlInfo\"", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "3521159d10882056f80c9baeb135caf6c55b096b", "submitter": { "id": 216, "url": "https://patchwork.libcamera.org/api/people/216/?format=api", "name": "Barnabás Pőcze", "email": "barnabas.pocze@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/23384/mbox/", "series": [ { "id": 5181, "url": "https://patchwork.libcamera.org/api/series/5181/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5181", "date": "2025-05-19T09:12:06", "name": "[v2,1/2] libcamera: converter_v4l2_m2m: Add missing `<set>` include", "version": 2, "mbox": "https://patchwork.libcamera.org/series/5181/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/23384/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/23384/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 18F27BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 19 May 2025 09:12:17 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4D0868D7F;\n\tMon, 19 May 2025 11:12:14 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A595468B66\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 19 May 2025 11:12:11 +0200 (CEST)", "from pb-laptop.local (185.221.142.248.nat.pool.zt.hu\n\t[185.221.142.248])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A3E1E9B;\n\tMon, 19 May 2025 11:11:51 +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=\"h3J0VXJx\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747645911;\n\tbh=jQdQTiLgCys8qV56C4vVoyRZxZM61gDkTPASkDRjelI=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=h3J0VXJxPhp4SZKrBSnFpmj0iZLeU0dN+DOf/+I/P9FFquvk3iGjJhJLtadvKrQ4U\n\tSxVo7Guy0RsDFOPV6GlnkCAFRII4ZL16c5QP66FC5afufB1fK5NkvHD6bInpEYgFF3\n\ton2lrIPQCSGD1/bwsdB4b3RjZG01t1C1ypk2RQ1A=", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Paul Elder <paul.elder@ideasonboard.com>", "Subject": "[PATCH v2 2/2] Revert \"controls: Add boolean constructors for\n\tControlInfo\"", "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", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "This reverts commit 10cdc914dad282b4ca0ad11067d5c6d446af1fcc.\n\nThe contstructors introduced by that commit are not used anywhere,\nand they do not match the existing practice for boolean controls.\n\nSpecifically, every single boolean control is described by calling\nthe `ControlInfo(ControlValue, ControlValue, ControlValue)`\nconstructor. Crucially, that constructor does not set `values_`,\nwhile the two removed contructors do. And whether or not `values_`\nhas any elements is currently used as an implicit sign to decide\nwhether or not the control is \"enum-like\", and those are assumed\nto have type `int32_t`.\n\nFor example, any boolean control described using any of the two\nremoved constructors would cause an assertion in failure in\n`CameraSession::listControls()` when calling `value.get<int32_t>()`.\n\nSigned-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n---\nchanges in v2:\n * keep tests\n---\n include/libcamera/controls.h | 3 ---\n src/libcamera/controls.cpp | 29 -----------------------------\n test/controls/control_info.cpp | 9 ++++-----\n 3 files changed, 4 insertions(+), 37 deletions(-)\n\n--\n2.49.0", "diff": "diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 2ae4ec3d4..32fb31f78 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -10,7 +10,6 @@\n #include <assert.h>\n #include <map>\n #include <optional>\n-#include <set>\n #include <stdint.h>\n #include <string>\n #include <unordered_map>\n@@ -334,8 +333,6 @@ public:\n \t\t\t const ControlValue &def = {});\n \texplicit ControlInfo(Span<const ControlValue> values,\n \t\t\t const ControlValue &def = {});\n-\texplicit ControlInfo(std::set<bool> values, bool def);\n-\texplicit ControlInfo(bool value);\n\n \tconst ControlValue &min() const { return min_; }\n \tconst ControlValue &max() const { return max_; }\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 70f6f6092..eaa34e70b 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -625,35 +625,6 @@ ControlInfo::ControlInfo(Span<const ControlValue> values,\n \t\tvalues_.push_back(value);\n }\n\n-/**\n- * \\brief Construct a boolean ControlInfo with both boolean values\n- * \\param[in] values The control valid boolean values (both true and false)\n- * \\param[in] def The control default boolean value\n- *\n- * Construct a ControlInfo for a boolean control, where both true and false are\n- * valid values. \\a values must be { false, true } (the order is irrelevant).\n- * The minimum value will always be false, and the maximum always true. The\n- * default value is \\a def.\n- */\n-ControlInfo::ControlInfo(std::set<bool> values, bool def)\n-\t: min_(false), max_(true), def_(def), values_({ false, true })\n-{\n-\tASSERT(values.count(def) && values.size() == 2);\n-}\n-\n-/**\n- * \\brief Construct a boolean ControlInfo with only one valid value\n- * \\param[in] value The control valid boolean value\n- *\n- * Construct a ControlInfo for a boolean control, where there is only valid\n- * value. The minimum, maximum, and default values will all be \\a value.\n- */\n-ControlInfo::ControlInfo(bool value)\n-\t: min_(value), max_(value), def_(value)\n-{\n-\tvalues_ = { value };\n-}\n-\n /**\n * \\fn ControlInfo::min()\n * \\brief Retrieve the minimum value of the control\ndiff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp\nindex e1bb43f0e..905d92dd5 100644\n--- a/test/controls/control_info.cpp\n+++ b/test/controls/control_info.cpp\n@@ -50,7 +50,7 @@ protected:\n \t\t * Test information retrieval from a control with boolean\n \t\t * values.\n \t\t */\n-\t\tControlInfo aeEnable({ false, true }, false);\n+\t\tControlInfo aeEnable(false, true, false);\n\n \t\tif (aeEnable.min().get<bool>() != false ||\n \t\t aeEnable.def().get<bool>() != false ||\n@@ -59,13 +59,12 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n\n-\t\tif (aeEnable.values()[0].get<bool>() != false ||\n-\t\t aeEnable.values()[1].get<bool>() != true) {\n+\t\tif (!aeEnable.values().empty()) {\n \t\t\tcout << \"Invalid control values for AeEnable\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n\n-\t\tControlInfo awbEnable(true);\n+\t\tControlInfo awbEnable(true, true, true);\n\n \t\tif (awbEnable.min().get<bool>() != true ||\n \t\t awbEnable.def().get<bool>() != true ||\n@@ -74,7 +73,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n\n-\t\tif (awbEnable.values()[0].get<bool>() != true) {\n+\t\tif (!awbEnable.values().empty()) {\n \t\t\tcout << \"Invalid control values for AwbEnable\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n", "prefixes": [ "v2", "2/2" ] }