[v1,2/2] libcamera: controls: Expose string controls as `std::string_view`
diff mbox series

Message ID 20250422124751.30409-3-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: controls: String controls as `std::string_view`
Related show

Commit Message

Barnabás Pőcze April 22, 2025, 12:47 p.m. UTC 
When retrieving the value from a `ControlValue` usually one of two
things happen: a small, trivially copyable object is returned by
value; or a view into the internal buffer is provided. This is true
for everything except strings, which are returned in `std::string`,
incurring the overhead of string construction.

To guarantee no potentially "expensive" copies, use `std::string_view`
pointing to the internal buffer to return the value. This is similar
to how other array-like types are returned with a `Span<>`.

This is an API break, but its scope is limited to just `properties::Model`.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=256
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/control_ids.h.in  |  1 +
 include/libcamera/controls.h        | 15 ++++++++-------
 src/apps/cam/capture_script.cpp     |  2 +-
 src/apps/cam/main.cpp               |  2 +-
 src/apps/common/dng_writer.cpp      |  2 +-
 src/apps/qcam/cam_select_dialog.cpp |  6 +++---
 src/py/libcamera/py_helpers.cpp     |  4 ++--
 test/controls/control_value.cpp     |  4 ++--
 utils/codegen/controls.py           |  2 +-
 9 files changed, 20 insertions(+), 18 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in
index 5d0594c68..41997f79f 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>
 
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index 4bfe9615c..275f45200 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>
 
@@ -96,7 +97,7 @@  struct control_type<float> {
 };
 
 template<>
-struct control_type<std::string> {
+struct control_type<std::string_view> {
 	static constexpr ControlType value = ControlTypeString;
 	static constexpr std::size_t size = 0;
 };
@@ -138,7 +139,7 @@  public:
 #ifndef __DOXYGEN__
 	template<typename T, std::enable_if_t<!details::is_span<T>::value &&
 					      details::control_type<T>::value &&
-					      !std::is_same<std::string, std::remove_cv_t<T>>::value,
+					      !std::is_same<std::string_view, std::remove_cv_t<T>>::value,
 					      std::nullptr_t> = nullptr>
 	ControlValue(const T &value)
 		: type_(ControlTypeNone), numElements_(0)
@@ -148,7 +149,7 @@  public:
 	}
 
 	template<typename T, std::enable_if_t<details::is_span<T>::value ||
-					      std::is_same<std::string, std::remove_cv_t<T>>::value,
+					      std::is_same<std::string_view, std::remove_cv_t<T>>::value,
 					      std::nullptr_t> = nullptr>
 #else
 	template<typename T>
@@ -182,7 +183,7 @@  public:
 
 #ifndef __DOXYGEN__
 	template<typename T, std::enable_if_t<!details::is_span<T>::value &&
-					      !std::is_same<std::string, std::remove_cv_t<T>>::value,
+					      !std::is_same<std::string_view, std::remove_cv_t<T>>::value,
 					      std::nullptr_t> = nullptr>
 	T get() const
 	{
@@ -193,7 +194,7 @@  public:
 	}
 
 	template<typename T, std::enable_if_t<details::is_span<T>::value ||
-					      std::is_same<std::string, std::remove_cv_t<T>>::value,
+					      std::is_same<std::string_view, std::remove_cv_t<T>>::value,
 					      std::nullptr_t> = nullptr>
 #else
 	template<typename T>
@@ -210,7 +211,7 @@  public:
 
 #ifndef __DOXYGEN__
 	template<typename T, std::enable_if_t<!details::is_span<T>::value &&
-					      !std::is_same<std::string, std::remove_cv_t<T>>::value,
+					      !std::is_same<std::string_view, std::remove_cv_t<T>>::value,
 					      std::nullptr_t> = nullptr>
 	void set(const T &value)
 	{
@@ -219,7 +220,7 @@  public:
 	}
 
 	template<typename T, std::enable_if_t<details::is_span<T>::value ||
-					      std::is_same<std::string, std::remove_cv_t<T>>::value,
+					      std::is_same<std::string_view, std::remove_cv_t<T>>::value,
 					      std::nullptr_t> = nullptr>
 #else
 	template<typename T>
diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp
index fdf82efc0..bc4c7c3a0 100644
--- a/src/apps/cam/capture_script.cpp
+++ b/src/apps/cam/capture_script.cpp
@@ -502,7 +502,7 @@  ControlValue CaptureScript::parseScalarControl(const ControlId *id,
 		break;
 	}
 	case ControlTypeString: {
-		value.set<std::string>(repr);
+		value.set<std::string_view>(repr);
 		break;
 	}
 	default:
diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp
index fa266eca6..157d238d7 100644
--- a/src/apps/cam/main.cpp
+++ b/src/apps/cam/main.cpp
@@ -337,7 +337,7 @@  std::string CamApp::cameraName(const Camera *camera)
 		 */
 		const auto &model = props.get(properties::Model);
 		if (model)
-			name = "'" + *model + "' ";
+			name.append("'").append(*model).append("' ");
 	}
 
 	name += "(" + camera->id() + ")";
diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp
index ac4619511..8d57023e1 100644
--- a/src/apps/common/dng_writer.cpp
+++ b/src/apps/common/dng_writer.cpp
@@ -564,7 +564,7 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 
 	const auto &model = cameraProperties.get(properties::Model);
 	if (model) {
-		TIFFSetField(tif, TIFFTAG_MODEL, model->c_str());
+		TIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());
 		/* \todo set TIFFTAG_UNIQUECAMERAMODEL. */
 	}
 
diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp
index 6b6d0713c..9f25ba091 100644
--- a/src/apps/qcam/cam_select_dialog.cpp
+++ b/src/apps/qcam/cam_select_dialog.cpp
@@ -114,8 +114,8 @@  void CameraSelectorDialog::updateCameraInfo(QString cameraId)
 		cameraLocation_->setText("Unknown");
 	}
 
-	const auto &model = properties.get(libcamera::properties::Model)
-				    .value_or("Unknown");
+	const auto model = properties.get(libcamera::properties::Model)
+				     .value_or("Unknown");
 
-	cameraModel_->setText(QString::fromStdString(model));
+	cameraModel_->setText(QString::fromUtf8(model.data(), model.length()));
 }
diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp
index 1ad1d4c1a..8c55ef845 100644
--- a/src/py/libcamera/py_helpers.cpp
+++ b/src/py/libcamera/py_helpers.cpp
@@ -47,7 +47,7 @@  py::object controlValueToPy(const ControlValue &cv)
 	case ControlTypeFloat:
 		return valueOrTuple<float>(cv);
 	case ControlTypeString:
-		return py::cast(cv.get<std::string>());
+		return py::cast(cv.get<std::string_view>());
 	case ControlTypeSize: {
 		const Size *v = reinterpret_cast<const Size *>(cv.data().data());
 		return py::cast(v);
@@ -88,7 +88,7 @@  ControlValue pyToControlValue(const py::object &ob, ControlType type)
 	case ControlTypeFloat:
 		return controlValueMaybeArray<float>(ob);
 	case ControlTypeString:
-		return ControlValue(ob.cast<std::string>());
+		return ControlValue(ob.cast<std::string_view>());
 	case ControlTypeRectangle:
 		return controlValueMaybeArray<Rectangle>(ob);
 	case ControlTypeSize:
diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
index 5084fd0cf..032050a77 100644
--- a/test/controls/control_value.cpp
+++ b/test/controls/control_value.cpp
@@ -318,7 +318,7 @@  protected:
 		/*
 		 * String type.
 		 */
-		std::string string{ "libcamera" };
+		std::string_view string{ "libcamera" };
 		value.set(string);
 		if (value.isNone() || !value.isArray() ||
 		    value.type() != ControlTypeString ||
@@ -327,7 +327,7 @@  protected:
 			return TestFail;
 		}
 
-		if (value.get<std::string>() != string) {
+		if (value.get<std::string_view>() != string) {
 			cerr << "Control value mismatch after setting to string" << endl;
 			return TestFail;
 		}
diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py
index e51610481..9399727bd 100644
--- a/utils/codegen/controls.py
+++ b/utils/codegen/controls.py
@@ -111,7 +111,7 @@  class Control(object):
         size = self.__data.get('size')
 
         if typ == 'string':
-            return 'std::string'
+            return 'std::string_view'
 
         if self.__size is None:
             return typ