{"id":22327,"url":"https://patchwork.libcamera.org/api/1.1/patches/22327/?format=json","web_url":"https://patchwork.libcamera.org/patch/22327/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<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=json","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=json","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":"","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 ;\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 ;\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 ","To":"libcamera-devel@lists.libcamera.org","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","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":"","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":"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 \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 \n #include \n #include \n+#include \n \n #include \n \n@@ -46,7 +47,7 @@ enum {{ctrl.name}}Enum {\n {%- endfor %}\n };\n extern const std::array {{ctrl.name}}Values;\n-extern const std::map {{ctrl.name}}NameValueMap;\n+extern const std::map {{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 \n #include \n #include \n+#include \n #include \n #include \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 &enumStrMap = {});\n+\t\t const std::map &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 &enumerators() const { return reverseMap_; }\n+\tconst std::map &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 enumStrMap_;\n-\tstd::map reverseMap_;\n+\tstd::map enumStrMap_;\n+\tstd::map 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 &enumStrMap = {})\n+\t\tconst std::map &enumStrMap = {})\n \t\t: ControlId(id, name, vendor, details::control_type>::value,\n \t\t\t details::control_type>::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 \n #include \n #include \n+#include \n \n #include \n #include \n@@ -36,7 +37,7 @@ private:\n \t};\n \tusing EventPtr = std::unique_ptr;\n \n-\tstd::map controls_;\n+\tstd::map controls_;\n \tstd::map frameControls_;\n \tstd::shared_ptr 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(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(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 &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(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 {{ctrl.n\n \tstatic_cast<{{ctrl.type}}>({{enum.name}}),\n {%- endfor %}\n };\n-extern const std::map {{ctrl.name}}NameValueMap = {\n+extern const std::map {{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 &enumStrMap)\n+\t\t const std::map &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 &ControlId::enumerators() const\n+ * \\fn const std::map &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"]}