[RFC,7/8,DNI] libcamera: Use std::string_view in controls
diff mbox series

Message ID 20241215230206.11002-8-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • libcamera: Use std::string_view
Related show

Commit Message

Laurent Pinchart Dec. 15, 2024, 11:02 p.m. UTC
This showcases potential code improvements by using std::string_view in
control-related classes. As the std::string_view usage guidelines forbid
its usage in the libcamera public API, this patch must not be
integrated.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/control_ids.h.in         |  3 ++-
 include/libcamera/controls.h               | 21 +++++++++++----------
 src/apps/cam/capture_script.h              |  3 ++-
 src/gstreamer/gstlibcamera-controls.cpp.in | 21 ++++++++++++---------
 src/libcamera/control_ids.cpp.in           |  4 ++--
 src/libcamera/controls.cpp                 | 15 +++++++--------
 6 files changed, 36 insertions(+), 31 deletions(-)

Patch
diff mbox series

diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in
index 5d0594c687f8eb01..5317368839ffcfc7 100644
--- a/include/libcamera/control_ids.h.in
+++ b/include/libcamera/control_ids.h.in
@@ -13,6 +13,7 @@ 
 #include <map>
 #include <stdint.h>
 #include <string>
+#include <string_view>
 
 #include <libcamera/controls.h>
 
@@ -46,7 +47,7 @@  enum {{ctrl.name}}Enum {
 {%- endfor %}
 };
 extern const std::array<const ControlValue, {{ctrl.enum_values_count}}> {{ctrl.name}}Values;
-extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap;
+extern const std::map<std::string_view, {{ctrl.type}}> {{ctrl.name}}NameValueMap;
 {% endif -%}
 extern const Control<{{ctrl.type}}> {{ctrl.name}};
 {% endfor -%}
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index b24336cc280f51db..636f5b980b688e16 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -13,6 +13,7 @@ 
 #include <set>
 #include <stdint.h>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 #include <vector>
 
@@ -249,28 +250,28 @@  private:
 class ControlId
 {
 public:
-	ControlId(unsigned int id, const std::string &name, const std::string &vendor,
+	ControlId(unsigned int id, std::string_view name, std::string_view vendor,
 		  ControlType type, std::size_t size = 0,
-		  const std::map<std::string, int32_t> &enumStrMap = {});
+		  const std::map<std::string_view, int32_t> &enumStrMap = {});
 
 	unsigned int id() const { return id_; }
-	const std::string &name() const { return name_; }
-	const std::string &vendor() const { return vendor_; }
+	std::string_view name() const { return name_; }
+	std::string_view vendor() const { return vendor_; }
 	ControlType type() const { return type_; }
 	bool isArray() const { return size_ > 0; }
 	std::size_t size() const { return size_; }
-	const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; }
+	const std::map<int32_t, std::string_view> &enumerators() const { return reverseMap_; }
 
 private:
 	LIBCAMERA_DISABLE_COPY_AND_MOVE(ControlId)
 
 	unsigned int id_;
-	std::string name_;
-	std::string vendor_;
+	std::string_view name_;
+	std::string_view vendor_;
 	ControlType type_;
 	std::size_t size_;
-	std::map<std::string, int32_t> enumStrMap_;
-	std::map<int32_t, std::string> reverseMap_;
+	std::map<std::string_view, int32_t> enumStrMap_;
+	std::map<int32_t, std::string_view> reverseMap_;
 };
 
 static inline bool operator==(unsigned int lhs, const ControlId &rhs)
@@ -300,7 +301,7 @@  public:
 	using type = T;
 
 	Control(unsigned int id, const char *name, const char *vendor,
-		const std::map<std::string, int32_t> &enumStrMap = {})
+		const std::map<std::string_view, int32_t> &enumStrMap = {})
 		: ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value,
 			    details::control_type<std::remove_cv_t<T>>::size, enumStrMap)
 	{
diff --git a/src/apps/cam/capture_script.h b/src/apps/cam/capture_script.h
index 294b920363bae01b..254768c259b9e246 100644
--- a/src/apps/cam/capture_script.h
+++ b/src/apps/cam/capture_script.h
@@ -10,6 +10,7 @@ 
 #include <map>
 #include <memory>
 #include <string>
+#include <string_view>
 
 #include <libcamera/camera.h>
 #include <libcamera/controls.h>
@@ -36,7 +37,7 @@  private:
 	};
 	using EventPtr = std::unique_ptr<yaml_event_t, EventDeleter>;
 
-	std::map<std::string, const libcamera::ControlId *> controls_;
+	std::map<std::string_view, const libcamera::ControlId *> controls_;
 	std::map<unsigned int, libcamera::ControlList> frameControls_;
 	std::shared_ptr<libcamera::Camera> camera_;
 	yaml_parser_t parser_;
diff --git a/src/gstreamer/gstlibcamera-controls.cpp.in b/src/gstreamer/gstlibcamera-controls.cpp.in
index ace36b7106867736..31ca6462748d17a1 100644
--- a/src/gstreamer/gstlibcamera-controls.cpp.in
+++ b/src/gstreamer/gstlibcamera-controls.cpp.in
@@ -154,9 +154,9 @@  bool GstCameraControls::getProperty(guint propId, GValue *value,
 				    [[maybe_unused]] GParamSpec *pspec)
 {
 	if (!controls_acc_.contains(propId)) {
-		GST_WARNING("Control '%s' is not available, default value will "
-			    "be returned",
-			    controls::controls.at(propId)->name().c_str());
+		const std::string_view name = controls::controls.at(propId)->name();
+		GST_WARNING("Control '%.*s' is not available, default value will be returned",
+			    static_cast<int>(name.size()), name.data());
 		return true;
 	}
 	const ControlValue &cv = controls_acc_.get(propId);
@@ -211,9 +211,10 @@  bool GstCameraControls::setProperty(guint propId, const GValue *value,
 		auto info = capabilities_.find(cid);
 
 		if (info == capabilities_.end()) {
-			GST_WARNING("Control '%s' is not supported by the "
+			const std::string_view name = cid->name();
+			GST_WARNING("Control '%.*s' is not supported by the "
 				    "camera and will be ignored",
-				    cid->name().c_str());
+				    static_cast<int>(name.size()), name.data());
 			return true;
 		}
 	}
@@ -307,12 +308,14 @@  void GstCameraControls::setCamera(const std::shared_ptr<libcamera::Camera> &cam)
 		auto info = capabilities_.find(cid);
 
 		/* Only add controls which are supported. */
-		if (info != capabilities_.end())
+		if (info != capabilities_.end()) {
 			new_controls.set(id, value);
-		else
-			GST_WARNING("Control '%s' is not supported by the "
+		} else {
+			const std::string_view name = cid->name();
+			GST_WARNING("Control '%.*s' is not supported by the "
 				    "camera and will be ignored",
-				    cid->name().c_str());
+				    static_cast<int>(name.size()), name.data());
+		}
 	}
 
 	controls_acc_ = new_controls;
diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
index afe9e2c9661041f6..caa1cee165d5c99b 100644
--- a/src/libcamera/control_ids.cpp.in
+++ b/src/libcamera/control_ids.cpp.in
@@ -51,7 +51,7 @@  namespace {{vendor}} {
 
 /**
  * \var {{ctrl.name}}NameValueMap
- * \brief Map of all {{ctrl.name}} supported value names (in std::string format) to value
+ * \brief Map of all {{ctrl.name}} supported value names (in std::string_view format) to value
  */
 
 {% endif -%}
@@ -84,7 +84,7 @@  extern const std::array<const ControlValue, {{ctrl.enum_values_count}}> {{ctrl.n
 	static_cast<{{ctrl.type}}>({{enum.name}}),
 {%- endfor %}
 };
-extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {
+extern const std::map<std::string_view, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {
 {%- for enum in ctrl.enum_values %}
 	{ "{{enum.name}}", {{enum.name}} },
 {%- endfor %}
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 65eeef2db9c2398f..8c1e61b23c2abcfd 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -406,7 +406,6 @@  void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
  */
 
 /**
- * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)
  * \brief Construct a ControlId instance
  * \param[in] id The control numerical ID
  * \param[in] name The control name
@@ -415,10 +414,10 @@  void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
  * \param[in] size The size of the array control, or 0 if scalar control
  * \param[in] enumStrMap The map from enum names to values (optional)
  */
-ControlId::ControlId(unsigned int id, const std::string &name,
-		     const std::string &vendor, ControlType type,
+ControlId::ControlId(unsigned int id, std::string_view name,
+		     std::string_view vendor, ControlType type,
 		     std::size_t size,
-		     const std::map<std::string, int32_t> &enumStrMap)
+		     const std::map<std::string_view, int32_t> &enumStrMap)
 	: id_(id), name_(name), vendor_(vendor), type_(type), size_(size),
 	  enumStrMap_(enumStrMap)
 {
@@ -433,15 +432,15 @@  ControlId::ControlId(unsigned int id, const std::string &name,
  */
 
 /**
- * \fn const char *ControlId::name() const
+ * \fn std::string_view ControlId::name() const
  * \brief Retrieve the control name
  * \return The control name
  */
 
 /**
- * \fn const std::string &ControlId::vendor() const
+ * \fn std::string_view ControlId::vendor() const
  * \brief Retrieve the vendor name
- * \return The vendor name, as a string
+ * \return The vendor name
  */
 
 /**
@@ -464,7 +463,7 @@  ControlId::ControlId(unsigned int id, const std::string &name,
  */
 
 /**
- * \fn const std::map<int32_t, std::string> &ControlId::enumerators() const
+ * \fn const std::map<int32_t, std::string_view> &ControlId::enumerators() const
  * \brief Retrieve the map of enum values to enum names
  * \return The map of enum values to enum names
  */