Patch Detail
Show a patch.
GET /api/1.1/patches/22327/?format=api
{ "id": 22327, "url": "https://patchwork.libcamera.org/api/1.1/patches/22327/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22327/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20241215230206.11002-8-laurent.pinchart@ideasonboard.com>", "date": "2024-12-15T23:02:05", "name": "[RFC,7/8,DNI] libcamera: Use std::string_view in controls", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "5ff98fe29d4d0ea4f02f40d18e4acdc9e8c23a1d", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22327/mbox/", "series": [ { "id": 4889, "url": "https://patchwork.libcamera.org/api/1.1/series/4889/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4889", "date": "2024-12-15T23:01:58", "name": "libcamera: Use std::string_view", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4889/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22327/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22327/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 7D6C2C32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 15 Dec 2024 23:02:42 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 89DFB67F47;\n\tMon, 16 Dec 2024 00:02:41 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 613BE67F38\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 16 Dec 2024 00:02:32 +0100 (CET)", "from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F18C2C6;\n\tMon, 16 Dec 2024 00:01:56 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fKLOEEVt\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734303716;\n\tbh=oCiYSbfCI97Mme8FpH/9YtMqHWwcNxZWhtoO3dbgo3o=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=fKLOEEVtTjQmxfb24TbaUks3n6OqzfYmGmtPSlPqCItByWZOzvBK8ErMBoQ1iHcQq\n\tz97QRxDisoCehHBhFd3biTHcLwkpML0X1djHyPkYhwfcR/V+HYfjRInjuYUgA/OkQv\n\tXZkyF+HEGq5AZRJ0QeCtgOl0v4gEFJSuE/ZYFuAU=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>", "Subject": "[RFC PATCH 7/8] [DNI] libcamera: Use std::string_view in controls", "Date": "Mon, 16 Dec 2024 01:02:05 +0200", "Message-ID": "<20241215230206.11002-8-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.45.2", "In-Reply-To": "<20241215230206.11002-1-laurent.pinchart@ideasonboard.com>", "References": "<20241215230206.11002-1-laurent.pinchart@ideasonboard.com>", "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": "<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 showcases potential code improvements by using std::string_view in\ncontrol-related classes. As the std::string_view usage guidelines forbid\nits usage in the libcamera public API, this patch must not be\nintegrated.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/control_ids.h.in | 3 ++-\n include/libcamera/controls.h | 21 +++++++++++----------\n src/apps/cam/capture_script.h | 3 ++-\n src/gstreamer/gstlibcamera-controls.cpp.in | 21 ++++++++++++---------\n src/libcamera/control_ids.cpp.in | 4 ++--\n src/libcamera/controls.cpp | 15 +++++++--------\n 6 files changed, 36 insertions(+), 31 deletions(-)", "diff": "diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\nindex 5d0594c687f8eb01..5317368839ffcfc7 100644\n--- a/include/libcamera/control_ids.h.in\n+++ b/include/libcamera/control_ids.h.in\n@@ -13,6 +13,7 @@\n #include <map>\n #include <stdint.h>\n #include <string>\n+#include <string_view>\n \n #include <libcamera/controls.h>\n \n@@ -46,7 +47,7 @@ enum {{ctrl.name}}Enum {\n {%- endfor %}\n };\n extern const std::array<const ControlValue, {{ctrl.enum_values_count}}> {{ctrl.name}}Values;\n-extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap;\n+extern const std::map<std::string_view, {{ctrl.type}}> {{ctrl.name}}NameValueMap;\n {% endif -%}\n extern const Control<{{ctrl.type}}> {{ctrl.name}};\n {% endfor -%}\ndiff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex b24336cc280f51db..636f5b980b688e16 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -13,6 +13,7 @@\n #include <set>\n #include <stdint.h>\n #include <string>\n+#include <string_view>\n #include <unordered_map>\n #include <vector>\n \n@@ -249,28 +250,28 @@ private:\n class ControlId\n {\n public:\n-\tControlId(unsigned int id, const std::string &name, const std::string &vendor,\n+\tControlId(unsigned int id, std::string_view name, std::string_view vendor,\n \t\t ControlType type, std::size_t size = 0,\n-\t\t const std::map<std::string, int32_t> &enumStrMap = {});\n+\t\t const std::map<std::string_view, int32_t> &enumStrMap = {});\n \n \tunsigned int id() const { return id_; }\n-\tconst std::string &name() const { return name_; }\n-\tconst std::string &vendor() const { return vendor_; }\n+\tstd::string_view name() const { return name_; }\n+\tstd::string_view vendor() const { return vendor_; }\n \tControlType type() const { return type_; }\n \tbool isArray() const { return size_ > 0; }\n \tstd::size_t size() const { return size_; }\n-\tconst std::map<int32_t, std::string> &enumerators() const { return reverseMap_; }\n+\tconst std::map<int32_t, std::string_view> &enumerators() const { return reverseMap_; }\n \n private:\n \tLIBCAMERA_DISABLE_COPY_AND_MOVE(ControlId)\n \n \tunsigned int id_;\n-\tstd::string name_;\n-\tstd::string vendor_;\n+\tstd::string_view name_;\n+\tstd::string_view vendor_;\n \tControlType type_;\n \tstd::size_t size_;\n-\tstd::map<std::string, int32_t> enumStrMap_;\n-\tstd::map<int32_t, std::string> reverseMap_;\n+\tstd::map<std::string_view, int32_t> enumStrMap_;\n+\tstd::map<int32_t, std::string_view> reverseMap_;\n };\n \n static inline bool operator==(unsigned int lhs, const ControlId &rhs)\n@@ -300,7 +301,7 @@ public:\n \tusing type = T;\n \n \tControl(unsigned int id, const char *name, const char *vendor,\n-\t\tconst std::map<std::string, int32_t> &enumStrMap = {})\n+\t\tconst std::map<std::string_view, int32_t> &enumStrMap = {})\n \t\t: ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value,\n \t\t\t details::control_type<std::remove_cv_t<T>>::size, enumStrMap)\n \t{\ndiff --git a/src/apps/cam/capture_script.h b/src/apps/cam/capture_script.h\nindex 294b920363bae01b..254768c259b9e246 100644\n--- a/src/apps/cam/capture_script.h\n+++ b/src/apps/cam/capture_script.h\n@@ -10,6 +10,7 @@\n #include <map>\n #include <memory>\n #include <string>\n+#include <string_view>\n \n #include <libcamera/camera.h>\n #include <libcamera/controls.h>\n@@ -36,7 +37,7 @@ private:\n \t};\n \tusing EventPtr = std::unique_ptr<yaml_event_t, EventDeleter>;\n \n-\tstd::map<std::string, const libcamera::ControlId *> controls_;\n+\tstd::map<std::string_view, const libcamera::ControlId *> controls_;\n \tstd::map<unsigned int, libcamera::ControlList> frameControls_;\n \tstd::shared_ptr<libcamera::Camera> camera_;\n \tyaml_parser_t parser_;\ndiff --git a/src/gstreamer/gstlibcamera-controls.cpp.in b/src/gstreamer/gstlibcamera-controls.cpp.in\nindex ace36b7106867736..31ca6462748d17a1 100644\n--- a/src/gstreamer/gstlibcamera-controls.cpp.in\n+++ b/src/gstreamer/gstlibcamera-controls.cpp.in\n@@ -154,9 +154,9 @@ bool GstCameraControls::getProperty(guint propId, GValue *value,\n \t\t\t\t [[maybe_unused]] GParamSpec *pspec)\n {\n \tif (!controls_acc_.contains(propId)) {\n-\t\tGST_WARNING(\"Control '%s' is not available, default value will \"\n-\t\t\t \"be returned\",\n-\t\t\t controls::controls.at(propId)->name().c_str());\n+\t\tconst std::string_view name = controls::controls.at(propId)->name();\n+\t\tGST_WARNING(\"Control '%.*s' is not available, default value will be returned\",\n+\t\t\t static_cast<int>(name.size()), name.data());\n \t\treturn true;\n \t}\n \tconst ControlValue &cv = controls_acc_.get(propId);\n@@ -211,9 +211,10 @@ bool GstCameraControls::setProperty(guint propId, const GValue *value,\n \t\tauto info = capabilities_.find(cid);\n \n \t\tif (info == capabilities_.end()) {\n-\t\t\tGST_WARNING(\"Control '%s' is not supported by the \"\n+\t\t\tconst std::string_view name = cid->name();\n+\t\t\tGST_WARNING(\"Control '%.*s' is not supported by the \"\n \t\t\t\t \"camera and will be ignored\",\n-\t\t\t\t cid->name().c_str());\n+\t\t\t\t static_cast<int>(name.size()), name.data());\n \t\t\treturn true;\n \t\t}\n \t}\n@@ -307,12 +308,14 @@ void GstCameraControls::setCamera(const std::shared_ptr<libcamera::Camera> &cam)\n \t\tauto info = capabilities_.find(cid);\n \n \t\t/* Only add controls which are supported. */\n-\t\tif (info != capabilities_.end())\n+\t\tif (info != capabilities_.end()) {\n \t\t\tnew_controls.set(id, value);\n-\t\telse\n-\t\t\tGST_WARNING(\"Control '%s' is not supported by the \"\n+\t\t} else {\n+\t\t\tconst std::string_view name = cid->name();\n+\t\t\tGST_WARNING(\"Control '%.*s' is not supported by the \"\n \t\t\t\t \"camera and will be ignored\",\n-\t\t\t\t cid->name().c_str());\n+\t\t\t\t static_cast<int>(name.size()), name.data());\n+\t\t}\n \t}\n \n \tcontrols_acc_ = new_controls;\ndiff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\nindex afe9e2c9661041f6..caa1cee165d5c99b 100644\n--- a/src/libcamera/control_ids.cpp.in\n+++ b/src/libcamera/control_ids.cpp.in\n@@ -51,7 +51,7 @@ namespace {{vendor}} {\n \n /**\n * \\var {{ctrl.name}}NameValueMap\n- * \\brief Map of all {{ctrl.name}} supported value names (in std::string format) to value\n+ * \\brief Map of all {{ctrl.name}} supported value names (in std::string_view format) to value\n */\n \n {% endif -%}\n@@ -84,7 +84,7 @@ extern const std::array<const ControlValue, {{ctrl.enum_values_count}}> {{ctrl.n\n \tstatic_cast<{{ctrl.type}}>({{enum.name}}),\n {%- endfor %}\n };\n-extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {\n+extern const std::map<std::string_view, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {\n {%- for enum in ctrl.enum_values %}\n \t{ \"{{enum.name}}\", {{enum.name}} },\n {%- endfor %}\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 65eeef2db9c2398f..8c1e61b23c2abcfd 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -406,7 +406,6 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n */\n \n /**\n- * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n * \\brief Construct a ControlId instance\n * \\param[in] id The control numerical ID\n * \\param[in] name The control name\n@@ -415,10 +414,10 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n * \\param[in] size The size of the array control, or 0 if scalar control\n * \\param[in] enumStrMap The map from enum names to values (optional)\n */\n-ControlId::ControlId(unsigned int id, const std::string &name,\n-\t\t const std::string &vendor, ControlType type,\n+ControlId::ControlId(unsigned int id, std::string_view name,\n+\t\t std::string_view vendor, ControlType type,\n \t\t std::size_t size,\n-\t\t const std::map<std::string, int32_t> &enumStrMap)\n+\t\t const std::map<std::string_view, int32_t> &enumStrMap)\n \t: id_(id), name_(name), vendor_(vendor), type_(type), size_(size),\n \t enumStrMap_(enumStrMap)\n {\n@@ -433,15 +432,15 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n */\n \n /**\n- * \\fn const char *ControlId::name() const\n+ * \\fn std::string_view ControlId::name() const\n * \\brief Retrieve the control name\n * \\return The control name\n */\n \n /**\n- * \\fn const std::string &ControlId::vendor() const\n+ * \\fn std::string_view ControlId::vendor() const\n * \\brief Retrieve the vendor name\n- * \\return The vendor name, as a string\n+ * \\return The vendor name\n */\n \n /**\n@@ -464,7 +463,7 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n */\n \n /**\n- * \\fn const std::map<int32_t, std::string> &ControlId::enumerators() const\n+ * \\fn const std::map<int32_t, std::string_view> &ControlId::enumerators() const\n * \\brief Retrieve the map of enum values to enum names\n * \\return The map of enum values to enum names\n */\n", "prefixes": [ "RFC", "7/8", "DNI" ] }