From f84522d7cd208b5123f0bd249ed1e160d35fdfb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Wed, 16 Apr 2025 13:57:06 +0200 Subject: [PATCH] libcamera: controls: Expose string controls as `std::string_view` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Kieran Bingham Reviewed-by: Jacopo Mondi Reviewed-by: Paul Elder Reviewed-by: Laurent Pinchart --- 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 | 4 ++-- src/py/libcamera/py_helpers.cpp | 4 ++-- test/controls/control_value.cpp | 4 ++-- utils/codegen/controls.py | 2 +- 9 files changed, 19 insertions(+), 17 deletions(-) diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in index 6b571233..06525318 100644 --- a/include/libcamera/control_ids.h.in +++ b/include/libcamera/control_ids.h.in @@ -13,6 +13,7 @@ #include #include #include +#include #include diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 32fb31f7..c970e4b7 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -95,7 +96,7 @@ struct control_type { }; template<> -struct control_type { +struct control_type { static constexpr ControlType value = ControlTypeString; static constexpr std::size_t size = 0; }; @@ -137,7 +138,7 @@ public: #ifndef __DOXYGEN__ template::value && details::control_type::value && - !std::is_same>::value, + !std::is_same>::value, std::nullptr_t> = nullptr> ControlValue(const T &value) : type_(ControlTypeNone), numElements_(0) @@ -147,7 +148,7 @@ public: } template::value || - std::is_same>::value, + std::is_same>::value, std::nullptr_t> = nullptr> #else template @@ -181,7 +182,7 @@ public: #ifndef __DOXYGEN__ template::value && - !std::is_same>::value, + !std::is_same>::value, std::nullptr_t> = nullptr> T get() const { @@ -192,7 +193,7 @@ public: } template::value || - std::is_same>::value, + std::is_same>::value, std::nullptr_t> = nullptr> #else template @@ -209,7 +210,7 @@ public: #ifndef __DOXYGEN__ template::value && - !std::is_same>::value, + !std::is_same>::value, std::nullptr_t> = nullptr> void set(const T &value) { @@ -218,7 +219,7 @@ public: } template::value || - std::is_same>::value, + std::is_same>::value, std::nullptr_t> = nullptr> #else template diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp index fdf82efc..bc4c7c3a 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(repr); + value.set(repr); break; } default: diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp index 75d9c99b..029f518f 100644 --- a/src/apps/cam/main.cpp +++ b/src/apps/cam/main.cpp @@ -345,7 +345,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 ac461951..8d57023e 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 6b6d0713..7370567d 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) + 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 1ad1d4c1..8c55ef84 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(cv); case ControlTypeString: - return py::cast(cv.get()); + return py::cast(cv.get()); case ControlTypeSize: { const Size *v = reinterpret_cast(cv.data().data()); return py::cast(v); @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type) case ControlTypeFloat: return controlValueMaybeArray(ob); case ControlTypeString: - return ControlValue(ob.cast()); + return ControlValue(ob.cast()); case ControlTypeRectangle: return controlValueMaybeArray(ob); case ControlTypeSize: diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp index 5084fd0c..032050a7 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() != string) { + if (value.get() != 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 e5161048..9399727b 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