[{"id":34109,"web_url":"https://patchwork.libcamera.org/comment/34109/","msgid":"","date":"2025-05-02T10:11:49","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n> When retrieving the value from a `ControlValue` usually one of two\n> things happen: a small, trivially copyable object is returned by\n> value; or a view into the internal buffer is provided. This is true\n> for everything except strings, which are returned in `std::string`,\n> incurring the overhead of string construction.\n>\n> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n> pointing to the internal buffer to return the value. This is similar\n> to how other array-like types are returned with a `Span<>`.\n>\n> This is an API break, but its scope is limited to just `properties::Model`.\n\nI've been a bit in two minds about this, until I realized we already\nrequire c++17 for applications.\n\nMy understanding is that using string_view is no different than\nreturning a Span<> when it comes to lifetime of the referenced data,\nso if we're fine with Span<> we're fine with string_view.\n\nAnd as pointed out by Barnabas, if a user wants a string, it can be\nconstructed easily with the same cost as we have today.\n\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/control_ids.h.in | 1 +\n> include/libcamera/controls.h | 15 ++++++++-------\n> src/apps/cam/capture_script.cpp | 2 +-\n> src/apps/cam/main.cpp | 2 +-\n> src/apps/common/dng_writer.cpp | 2 +-\n> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n> src/py/libcamera/py_helpers.cpp | 4 ++--\n> test/controls/control_value.cpp | 4 ++--\n> utils/codegen/controls.py | 2 +-\n> 9 files changed, 20 insertions(+), 18 deletions(-)\n>\n> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> index 5d0594c68..41997f79f 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> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 2ae4ec3d4..5a707a387 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> @@ -96,7 +97,7 @@ struct control_type {\n> };\n>\n> template<>\n> -struct control_type {\n> +struct control_type {\n> \tstatic constexpr ControlType value = ControlTypeString;\n> \tstatic constexpr std::size_t size = 0;\n> };\n> @@ -138,7 +139,7 @@ public:\n> #ifndef __DOXYGEN__\n> \ttemplate::value &&\n> \t\t\t\t\t details::control_type::value &&\n> -\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t !std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> \tControlValue(const T &value)\n> \t\t: type_(ControlTypeNone), numElements_(0)\n> @@ -148,7 +149,7 @@ public:\n> \t}\n>\n> \ttemplate::value ||\n> -\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> @@ -182,7 +183,7 @@ public:\n>\n> #ifndef __DOXYGEN__\n> \ttemplate::value &&\n> -\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t !std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> \tT get() const\n> \t{\n> @@ -193,7 +194,7 @@ public:\n> \t}\n>\n> \ttemplate::value ||\n> -\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> @@ -210,7 +211,7 @@ public:\n>\n> #ifndef __DOXYGEN__\n> \ttemplate::value &&\n> -\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t !std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> \tvoid set(const T &value)\n> \t{\n> @@ -219,7 +220,7 @@ public:\n> \t}\n>\n> \ttemplate::value ||\n> -\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> index fdf82efc0..bc4c7c3a0 100644\n> --- a/src/apps/cam/capture_script.cpp\n> +++ b/src/apps/cam/capture_script.cpp\n> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n> \t\tbreak;\n> \t}\n> \tcase ControlTypeString: {\n> -\t\tvalue.set(repr);\n> +\t\tvalue.set(repr);\n> \t\tbreak;\n> \t}\n> \tdefault:\n> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> index fa266eca6..157d238d7 100644\n> --- a/src/apps/cam/main.cpp\n> +++ b/src/apps/cam/main.cpp\n> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> \t\t */\n> \t\tconst auto &model = props.get(properties::Model);\n\nDoes this need to be a reference now ?\n\n> \t\tif (model)\n> -\t\t\tname = \"'\" + *model + \"' \";\n> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n> \t}\n>\n> \tname += \"(\" + camera->id() + \")\";\n> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> index ac4619511..8d57023e1 100644\n> --- a/src/apps/common/dng_writer.cpp\n> +++ b/src/apps/common/dng_writer.cpp\n> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>\n> \tconst auto &model = cameraProperties.get(properties::Model);\n\nlikewise\n\n> \tif (model) {\n> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> \t}\n>\n> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n> index 6b6d0713c..9f25ba091 100644\n> --- a/src/apps/qcam/cam_select_dialog.cpp\n> +++ b/src/apps/qcam/cam_select_dialog.cpp\n> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n> \t\tcameraLocation_->setText(\"Unknown\");\n> \t}\n>\n> -\tconst auto &model = properties.get(libcamera::properties::Model)\n> -\t\t\t\t .value_or(\"Unknown\");\n> +\tconst auto model = properties.get(libcamera::properties::Model)\n> +\t\t\t\t .value_or(\"Unknown\");\n>\n> -\tcameraModel_->setText(QString::fromStdString(model));\n> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n> }\n> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n> index 1ad1d4c1a..8c55ef845 100644\n> --- a/src/py/libcamera/py_helpers.cpp\n> +++ b/src/py/libcamera/py_helpers.cpp\n> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n> \tcase ControlTypeFloat:\n> \t\treturn valueOrTuple(cv);\n> \tcase ControlTypeString:\n> -\t\treturn py::cast(cv.get());\n> +\t\treturn py::cast(cv.get());\n> \tcase ControlTypeSize: {\n> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n> \t\treturn py::cast(v);\n> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> \tcase ControlTypeFloat:\n> \t\treturn controlValueMaybeArray(ob);\n> \tcase ControlTypeString:\n> -\t\treturn ControlValue(ob.cast());\n> +\t\treturn ControlValue(ob.cast());\n> \tcase ControlTypeRectangle:\n> \t\treturn controlValueMaybeArray(ob);\n> \tcase ControlTypeSize:\n> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> index 5084fd0cf..032050a77 100644\n> --- a/test/controls/control_value.cpp\n> +++ b/test/controls/control_value.cpp\n> @@ -318,7 +318,7 @@ protected:\n> \t\t/*\n> \t\t * String type.\n> \t\t */\n> -\t\tstd::string string{ \"libcamera\" };\n> +\t\tstd::string_view string{ \"libcamera\" };\n> \t\tvalue.set(string);\n\nHere's one thing: we now require users to use string_view when setting\na control, and we anyway end up copying it's content into the\nControlValue.\n\nShould we provide a set() overload for std::string ? Is it worth it ?\n\n> \t\tif (value.isNone() || !value.isArray() ||\n> \t\t value.type() != ControlTypeString ||\n> @@ -327,7 +327,7 @@ protected:\n> \t\t\treturn TestFail;\n> \t\t}\n>\n> -\t\tif (value.get() != string) {\n> +\t\tif (value.get() != string) {\n\nFor get() instead I think it's fine always returning a view\n\n> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> index e51610481..9399727bd 100644\n> --- a/utils/codegen/controls.py\n> +++ b/utils/codegen/controls.py\n> @@ -111,7 +111,7 @@ class Control(object):\n> size = self.__data.get('size')\n>\n> if typ == 'string':\n> - return 'std::string'\n> + return 'std::string_view'\n>\n> if self.__size is None:\n> return typ\n> --\n> 2.49.0\n>","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 0727DBE08B\n\tfor ;\n\tFri, 2 May 2025 10:11:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B55B68ADC;\n\tFri, 2 May 2025 12:11:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F91968AD3\n\tfor ;\n\tFri, 2 May 2025 12:11:53 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 848E6353;\n\tFri, 2 May 2025 12:11:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BWp6+68E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746180705;\n\tbh=Ws9JZNQTE4ZuEekthR7D31pwEPFpQjBB6WnKwbnd8io=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BWp6+68Emq68ItajPVpI7orJ9dCtuiJtxyyxaxIBpDMZfSaGYk9GZAUc1E3jnqr5A\n\tmRVSh+iNggkW3hXpo1PYZflJcj9QoX5vFxQGby6l7WUulLbMJK4QM+v+pGMOO2GJEP\n\tTzcKvgs0/viXB6M0CCax305ABQpN6Zwqf+3nrCgs=","Date":"Fri, 2 May 2025 12:11:49 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","Message-ID":"","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>","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\" "}},{"id":34114,"web_url":"https://patchwork.libcamera.org/comment/34114/","msgid":"","date":"2025-05-02T10:22:13","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 05. 02. 12:11 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n>> When retrieving the value from a `ControlValue` usually one of two\n>> things happen: a small, trivially copyable object is returned by\n>> value; or a view into the internal buffer is provided. This is true\n>> for everything except strings, which are returned in `std::string`,\n>> incurring the overhead of string construction.\n>>\n>> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n>> pointing to the internal buffer to return the value. This is similar\n>> to how other array-like types are returned with a `Span<>`.\n>>\n>> This is an API break, but its scope is limited to just `properties::Model`.\n> \n> I've been a bit in two minds about this, until I realized we already\n> require c++17 for applications.\n> \n> My understanding is that using string_view is no different than\n> returning a Span<> when it comes to lifetime of the referenced data,\n> so if we're fine with Span<> we're fine with string_view.\n> \n> And as pointed out by Barnabas, if a user wants a string, it can be\n> constructed easily with the same cost as we have today.\n> \n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> include/libcamera/control_ids.h.in | 1 +\n>> include/libcamera/controls.h | 15 ++++++++-------\n>> src/apps/cam/capture_script.cpp | 2 +-\n>> src/apps/cam/main.cpp | 2 +-\n>> src/apps/common/dng_writer.cpp | 2 +-\n>> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n>> src/py/libcamera/py_helpers.cpp | 4 ++--\n>> test/controls/control_value.cpp | 4 ++--\n>> utils/codegen/controls.py | 2 +-\n>> 9 files changed, 20 insertions(+), 18 deletions(-)\n>>\n>> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n>> index 5d0594c68..41997f79f 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>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index 2ae4ec3d4..5a707a387 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>> @@ -96,7 +97,7 @@ struct control_type {\n>> };\n>>\n>> template<>\n>> -struct control_type {\n>> +struct control_type {\n>> \tstatic constexpr ControlType value = ControlTypeString;\n>> \tstatic constexpr std::size_t size = 0;\n>> };\n>> @@ -138,7 +139,7 @@ public:\n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> \t\t\t\t\t details::control_type::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tControlValue(const T &value)\n>> \t\t: type_(ControlTypeNone), numElements_(0)\n>> @@ -148,7 +149,7 @@ public:\n>> \t}\n>>\n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> @@ -182,7 +183,7 @@ public:\n>>\n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tT get() const\n>> \t{\n>> @@ -193,7 +194,7 @@ public:\n>> \t}\n>>\n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> @@ -210,7 +211,7 @@ public:\n>>\n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tvoid set(const T &value)\n>> \t{\n>> @@ -219,7 +220,7 @@ public:\n>> \t}\n>>\n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n>> index fdf82efc0..bc4c7c3a0 100644\n>> --- a/src/apps/cam/capture_script.cpp\n>> +++ b/src/apps/cam/capture_script.cpp\n>> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n>> \t\tbreak;\n>> \t}\n>> \tcase ControlTypeString: {\n>> -\t\tvalue.set(repr);\n>> +\t\tvalue.set(repr);\n>> \t\tbreak;\n>> \t}\n>> \tdefault:\n>> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n>> index fa266eca6..157d238d7 100644\n>> --- a/src/apps/cam/main.cpp\n>> +++ b/src/apps/cam/main.cpp\n>> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n>> \t\t */\n>> \t\tconst auto &model = props.get(properties::Model);\n> \n> Does this need to be a reference now ?\n> \n>> \t\tif (model)\n>> -\t\t\tname = \"'\" + *model + \"' \";\n>> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n>> \t}\n>>\n>> \tname += \"(\" + camera->id() + \")\";\n>> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n>> index ac4619511..8d57023e1 100644\n>> --- a/src/apps/common/dng_writer.cpp\n>> +++ b/src/apps/common/dng_writer.cpp\n>> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>\n>> \tconst auto &model = cameraProperties.get(properties::Model);\n> \n> likewise\n> \n>> \tif (model) {\n>> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n>> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n>> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n>> \t}\n>>\n>> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n>> index 6b6d0713c..9f25ba091 100644\n>> --- a/src/apps/qcam/cam_select_dialog.cpp\n>> +++ b/src/apps/qcam/cam_select_dialog.cpp\n>> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n>> \t\tcameraLocation_->setText(\"Unknown\");\n>> \t}\n>>\n>> -\tconst auto &model = properties.get(libcamera::properties::Model)\n>> -\t\t\t\t .value_or(\"Unknown\");\n>> +\tconst auto model = properties.get(libcamera::properties::Model)\n>> +\t\t\t\t .value_or(\"Unknown\");\n>>\n>> -\tcameraModel_->setText(QString::fromStdString(model));\n>> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n>> }\n>> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n>> index 1ad1d4c1a..8c55ef845 100644\n>> --- a/src/py/libcamera/py_helpers.cpp\n>> +++ b/src/py/libcamera/py_helpers.cpp\n>> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n>> \tcase ControlTypeFloat:\n>> \t\treturn valueOrTuple(cv);\n>> \tcase ControlTypeString:\n>> -\t\treturn py::cast(cv.get());\n>> +\t\treturn py::cast(cv.get());\n>> \tcase ControlTypeSize: {\n>> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n>> \t\treturn py::cast(v);\n>> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n>> \tcase ControlTypeFloat:\n>> \t\treturn controlValueMaybeArray(ob);\n>> \tcase ControlTypeString:\n>> -\t\treturn ControlValue(ob.cast());\n>> +\t\treturn ControlValue(ob.cast());\n>> \tcase ControlTypeRectangle:\n>> \t\treturn controlValueMaybeArray(ob);\n>> \tcase ControlTypeSize:\n>> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n>> index 5084fd0cf..032050a77 100644\n>> --- a/test/controls/control_value.cpp\n>> +++ b/test/controls/control_value.cpp\n>> @@ -318,7 +318,7 @@ protected:\n>> \t\t/*\n>> \t\t * String type.\n>> \t\t */\n>> -\t\tstd::string string{ \"libcamera\" };\n>> +\t\tstd::string_view string{ \"libcamera\" };\n>> \t\tvalue.set(string);\n> \n> Here's one thing: we now require users to use string_view when setting\n> a control, and we anyway end up copying it's content into the\n> ControlValue.\n> \n> Should we provide a set() overload for std::string ? Is it worth it ?\n\nI think that could be useful, although I would defer it because there is only\na single string-valued control (properties::Model). And if we want to support\nmultiple \"input\" types for a single control types, then I think there are\nbetter ways but those need potentially more changes.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>> \t\tif (value.isNone() || !value.isArray() ||\n>> \t\t value.type() != ControlTypeString ||\n>> @@ -327,7 +327,7 @@ protected:\n>> \t\t\treturn TestFail;\n>> \t\t}\n>>\n>> -\t\tif (value.get() != string) {\n>> +\t\tif (value.get() != string) {\n> \n> For get() instead I think it's fine always returning a view\n> \n>> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n>> \t\t\treturn TestFail;\n>> \t\t}\n>> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n>> index e51610481..9399727bd 100644\n>> --- a/utils/codegen/controls.py\n>> +++ b/utils/codegen/controls.py\n>> @@ -111,7 +111,7 @@ class Control(object):\n>> size = self.__data.get('size')\n>>\n>> if typ == 'string':\n>> - return 'std::string'\n>> + return 'std::string_view'\n>>\n>> if self.__size is None:\n>> return typ\n>> --\n>> 2.49.0\n>>","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 34FF8BE08B\n\tfor ;\n\tFri, 2 May 2025 10:22:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4006168B25;\n\tFri, 2 May 2025 12:22:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C79368AD3\n\tfor ;\n\tFri, 2 May 2025 12:22:23 +0200 (CEST)","from [192.168.33.18] (185.221.143.50.nat.pool.zt.hu\n\t[185.221.143.50])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A809F353;\n\tFri, 2 May 2025 12:22:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DMuvnhQS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746181335;\n\tbh=oECIwKtOvFimqBp9x8G7pFLt+Ry8l7AgDLZPdTkzyeM=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=DMuvnhQSdLadZBn5vg00ltLL2CpIOJwH2Nwj/X1jR5DOpSDHBdR+vDRCtk+pCVZOn\n\thVpEEyLNtqVckOqUV3jwt7TtDutQ5TRaEo0Guj+z9Dk+/3VcJmrLRMJCyytHDQGJmD\n\tOcFodCJE5bx4oVdEOQ0iDZolF5bSggLsnbqbAhfE=","Message-ID":"","Date":"Fri, 2 May 2025 12:22:13 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","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\" "}},{"id":34115,"web_url":"https://patchwork.libcamera.org/comment/34115/","msgid":"","date":"2025-05-02T11:19:05","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Fri, May 02, 2025 at 12:22:13PM +0200, Barnabás Pőcze wrote:\n> Hi\n>\n> 2025. 05. 02. 12:11 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n> > > When retrieving the value from a `ControlValue` usually one of two\n> > > things happen: a small, trivially copyable object is returned by\n> > > value; or a view into the internal buffer is provided. This is true\n> > > for everything except strings, which are returned in `std::string`,\n> > > incurring the overhead of string construction.\n> > >\n> > > To guarantee no potentially \"expensive\" copies, use `std::string_view`\n> > > pointing to the internal buffer to return the value. This is similar\n> > > to how other array-like types are returned with a `Span<>`.\n> > >\n> > > This is an API break, but its scope is limited to just `properties::Model`.\n> >\n> > I've been a bit in two minds about this, until I realized we already\n> > require c++17 for applications.\n> >\n> > My understanding is that using string_view is no different than\n> > returning a Span<> when it comes to lifetime of the referenced data,\n> > so if we're fine with Span<> we're fine with string_view.\n> >\n> > And as pointed out by Barnabas, if a user wants a string, it can be\n> > constructed easily with the same cost as we have today.\n> >\n> > >\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > include/libcamera/control_ids.h.in | 1 +\n> > > include/libcamera/controls.h | 15 ++++++++-------\n> > > src/apps/cam/capture_script.cpp | 2 +-\n> > > src/apps/cam/main.cpp | 2 +-\n> > > src/apps/common/dng_writer.cpp | 2 +-\n> > > src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n> > > src/py/libcamera/py_helpers.cpp | 4 ++--\n> > > test/controls/control_value.cpp | 4 ++--\n> > > utils/codegen/controls.py | 2 +-\n> > > 9 files changed, 20 insertions(+), 18 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> > > index 5d0594c68..41997f79f 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> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > index 2ae4ec3d4..5a707a387 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> > > @@ -96,7 +97,7 @@ struct control_type {\n> > > };\n> > >\n> > > template<>\n> > > -struct control_type {\n> > > +struct control_type {\n> > > \tstatic constexpr ControlType value = ControlTypeString;\n> > > \tstatic constexpr std::size_t size = 0;\n> > > };\n> > > @@ -138,7 +139,7 @@ public:\n> > > #ifndef __DOXYGEN__\n> > > \ttemplate::value &&\n> > > \t\t\t\t\t details::control_type::value &&\n> > > -\t\t\t\t\t !std::is_same>::value,\n> > > +\t\t\t\t\t !std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > \tControlValue(const T &value)\n> > > \t\t: type_(ControlTypeNone), numElements_(0)\n> > > @@ -148,7 +149,7 @@ public:\n> > > \t}\n> > >\n> > > \ttemplate::value ||\n> > > -\t\t\t\t\t std::is_same>::value,\n> > > +\t\t\t\t\t std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > #else\n> > > \ttemplate\n> > > @@ -182,7 +183,7 @@ public:\n> > >\n> > > #ifndef __DOXYGEN__\n> > > \ttemplate::value &&\n> > > -\t\t\t\t\t !std::is_same>::value,\n> > > +\t\t\t\t\t !std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > \tT get() const\n> > > \t{\n> > > @@ -193,7 +194,7 @@ public:\n> > > \t}\n> > >\n> > > \ttemplate::value ||\n> > > -\t\t\t\t\t std::is_same>::value,\n> > > +\t\t\t\t\t std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > #else\n> > > \ttemplate\n> > > @@ -210,7 +211,7 @@ public:\n> > >\n> > > #ifndef __DOXYGEN__\n> > > \ttemplate::value &&\n> > > -\t\t\t\t\t !std::is_same>::value,\n> > > +\t\t\t\t\t !std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > \tvoid set(const T &value)\n> > > \t{\n> > > @@ -219,7 +220,7 @@ public:\n> > > \t}\n> > >\n> > > \ttemplate::value ||\n> > > -\t\t\t\t\t std::is_same>::value,\n> > > +\t\t\t\t\t std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > #else\n> > > \ttemplate\n> > > diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> > > index fdf82efc0..bc4c7c3a0 100644\n> > > --- a/src/apps/cam/capture_script.cpp\n> > > +++ b/src/apps/cam/capture_script.cpp\n> > > @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n> > > \t\tbreak;\n> > > \t}\n> > > \tcase ControlTypeString: {\n> > > -\t\tvalue.set(repr);\n> > > +\t\tvalue.set(repr);\n> > > \t\tbreak;\n> > > \t}\n> > > \tdefault:\n> > > diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> > > index fa266eca6..157d238d7 100644\n> > > --- a/src/apps/cam/main.cpp\n> > > +++ b/src/apps/cam/main.cpp\n> > > @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> > > \t\t */\n> > > \t\tconst auto &model = props.get(properties::Model);\n> >\n> > Does this need to be a reference now ?\n> >\n> > > \t\tif (model)\n> > > -\t\t\tname = \"'\" + *model + \"' \";\n> > > +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n> > > \t}\n> > >\n> > > \tname += \"(\" + camera->id() + \")\";\n> > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> > > index ac4619511..8d57023e1 100644\n> > > --- a/src/apps/common/dng_writer.cpp\n> > > +++ b/src/apps/common/dng_writer.cpp\n> > > @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > >\n> > > \tconst auto &model = cameraProperties.get(properties::Model);\n> >\n> > likewise\n> >\n> > > \tif (model) {\n> > > -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> > > +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n> > > \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> > > \t}\n> > >\n> > > diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n> > > index 6b6d0713c..9f25ba091 100644\n> > > --- a/src/apps/qcam/cam_select_dialog.cpp\n> > > +++ b/src/apps/qcam/cam_select_dialog.cpp\n> > > @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n> > > \t\tcameraLocation_->setText(\"Unknown\");\n> > > \t}\n> > >\n> > > -\tconst auto &model = properties.get(libcamera::properties::Model)\n> > > -\t\t\t\t .value_or(\"Unknown\");\n> > > +\tconst auto model = properties.get(libcamera::properties::Model)\n> > > +\t\t\t\t .value_or(\"Unknown\");\n> > >\n> > > -\tcameraModel_->setText(QString::fromStdString(model));\n> > > +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n> > > }\n> > > diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n> > > index 1ad1d4c1a..8c55ef845 100644\n> > > --- a/src/py/libcamera/py_helpers.cpp\n> > > +++ b/src/py/libcamera/py_helpers.cpp\n> > > @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n> > > \tcase ControlTypeFloat:\n> > > \t\treturn valueOrTuple(cv);\n> > > \tcase ControlTypeString:\n> > > -\t\treturn py::cast(cv.get());\n> > > +\t\treturn py::cast(cv.get());\n> > > \tcase ControlTypeSize: {\n> > > \t\tconst Size *v = reinterpret_cast(cv.data().data());\n> > > \t\treturn py::cast(v);\n> > > @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> > > \tcase ControlTypeFloat:\n> > > \t\treturn controlValueMaybeArray(ob);\n> > > \tcase ControlTypeString:\n> > > -\t\treturn ControlValue(ob.cast());\n> > > +\t\treturn ControlValue(ob.cast());\n> > > \tcase ControlTypeRectangle:\n> > > \t\treturn controlValueMaybeArray(ob);\n> > > \tcase ControlTypeSize:\n> > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > > index 5084fd0cf..032050a77 100644\n> > > --- a/test/controls/control_value.cpp\n> > > +++ b/test/controls/control_value.cpp\n> > > @@ -318,7 +318,7 @@ protected:\n> > > \t\t/*\n> > > \t\t * String type.\n> > > \t\t */\n> > > -\t\tstd::string string{ \"libcamera\" };\n> > > +\t\tstd::string_view string{ \"libcamera\" };\n> > > \t\tvalue.set(string);\n> >\n> > Here's one thing: we now require users to use string_view when setting\n> > a control, and we anyway end up copying it's content into the\n> > ControlValue.\n> >\n> > Should we provide a set() overload for std::string ? Is it worth it ?\n>\n> I think that could be useful, although I would defer it because there is only\n> a single string-valued control (properties::Model). And if we want to support\n> multiple \"input\" types for a single control types, then I think there are\n> better ways but those need potentially more changes.\n>\n\nAlso, it's a property, so no way one can set it. Fine then, I would\nrecord a \\todo so that we don't forget about this when we'll add a\nstring control, but up to you.\n\n>\n> Regards,\n> Barnabás Pőcze\n>\n> >\n> > > \t\tif (value.isNone() || !value.isArray() ||\n> > > \t\t value.type() != ControlTypeString ||\n> > > @@ -327,7 +327,7 @@ protected:\n> > > \t\t\treturn TestFail;\n> > > \t\t}\n> > >\n> > > -\t\tif (value.get() != string) {\n> > > +\t\tif (value.get() != string) {\n> >\n> > For get() instead I think it's fine always returning a view\n> >\n> > > \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n> > > \t\t\treturn TestFail;\n> > > \t\t}\n> > > diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> > > index e51610481..9399727bd 100644\n> > > --- a/utils/codegen/controls.py\n> > > +++ b/utils/codegen/controls.py\n> > > @@ -111,7 +111,7 @@ class Control(object):\n> > > size = self.__data.get('size')\n> > >\n> > > if typ == 'string':\n> > > - return 'std::string'\n> > > + return 'std::string_view'\n> > >\n> > > if self.__size is None:\n> > > return typ\n> > > --\n> > > 2.49.0\n> > >\n>","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 68E4EBE08B\n\tfor ;\n\tFri, 2 May 2025 11:19:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 243BD68ADC;\n\tFri, 2 May 2025 13:19:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5986A68AD3\n\tfor ;\n\tFri, 2 May 2025 13:19:08 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5CBA555;\n\tFri, 2 May 2025 13:19:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"O+itq3HM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746184740;\n\tbh=yEePU6kvHyTYmM5im/RMt5YKi8M9WVLjKh9uiQQtKh8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O+itq3HMFDY/KDSVEumO4IKoaeeRqcg8e7T6KB9is2RG6F36YKXpB/Zy6SYMgoTeO\n\tyEVZHje8WzhA2IfegO7PkRUwW3jqQKY5lKFZ3TF4ki/JmNjyDQj6s4kIwjL9Q3g9kY\n\t6Wv5YhKwbo8ky4tBQ52eVvZQt4X1mL484FfVb/HQ=","Date":"Fri, 2 May 2025 13:19:05 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","Message-ID":"","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}},{"id":34116,"web_url":"https://patchwork.libcamera.org/comment/34116/","msgid":"","date":"2025-05-02T12:17:17","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 05. 02. 12:11 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n>> When retrieving the value from a `ControlValue` usually one of two\n>> things happen: a small, trivially copyable object is returned by\n>> value; or a view into the internal buffer is provided. This is true\n>> for everything except strings, which are returned in `std::string`,\n>> incurring the overhead of string construction.\n>>\n>> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n>> pointing to the internal buffer to return the value. This is similar\n>> to how other array-like types are returned with a `Span<>`.\n>>\n>> This is an API break, but its scope is limited to just `properties::Model`.\n> \n> I've been a bit in two minds about this, until I realized we already\n> require c++17 for applications.\n> \n> My understanding is that using string_view is no different than\n> returning a Span<> when it comes to lifetime of the referenced data,\n> so if we're fine with Span<> we're fine with string_view.\n> \n> And as pointed out by Barnabas, if a user wants a string, it can be\n> constructed easily with the same cost as we have today.\n> \n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> include/libcamera/control_ids.h.in | 1 +\n>> include/libcamera/controls.h | 15 ++++++++-------\n>> src/apps/cam/capture_script.cpp | 2 +-\n>> src/apps/cam/main.cpp | 2 +-\n>> src/apps/common/dng_writer.cpp | 2 +-\n>> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n>> src/py/libcamera/py_helpers.cpp | 4 ++--\n>> test/controls/control_value.cpp | 4 ++--\n>> utils/codegen/controls.py | 2 +-\n>> 9 files changed, 20 insertions(+), 18 deletions(-)\n>>\n>> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n>> index 5d0594c68..41997f79f 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>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index 2ae4ec3d4..5a707a387 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>> @@ -96,7 +97,7 @@ struct control_type {\n>> };\n>>\n>> template<>\n>> -struct control_type {\n>> +struct control_type {\n>> \tstatic constexpr ControlType value = ControlTypeString;\n>> \tstatic constexpr std::size_t size = 0;\n>> };\n>> @@ -138,7 +139,7 @@ public:\n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> \t\t\t\t\t details::control_type::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tControlValue(const T &value)\n>> \t\t: type_(ControlTypeNone), numElements_(0)\n>> @@ -148,7 +149,7 @@ public:\n>> \t}\n>>\n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> @@ -182,7 +183,7 @@ public:\n>>\n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tT get() const\n>> \t{\n>> @@ -193,7 +194,7 @@ public:\n>> \t}\n>>\n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> @@ -210,7 +211,7 @@ public:\n>>\n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tvoid set(const T &value)\n>> \t{\n>> @@ -219,7 +220,7 @@ public:\n>> \t}\n>>\n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n>> index fdf82efc0..bc4c7c3a0 100644\n>> --- a/src/apps/cam/capture_script.cpp\n>> +++ b/src/apps/cam/capture_script.cpp\n>> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n>> \t\tbreak;\n>> \t}\n>> \tcase ControlTypeString: {\n>> -\t\tvalue.set(repr);\n>> +\t\tvalue.set(repr);\n>> \t\tbreak;\n>> \t}\n>> \tdefault:\n>> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n>> index fa266eca6..157d238d7 100644\n>> --- a/src/apps/cam/main.cpp\n>> +++ b/src/apps/cam/main.cpp\n>> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n>> \t\t */\n>> \t\tconst auto &model = props.get(properties::Model);\n> \n> Does this need to be a reference now ?\n\nThere isn't any real difference because it's an `std::optional` in both cases\n(just with different value type), so I decided not to change it.\n\n\n> \n>> \t\tif (model)\n>> -\t\t\tname = \"'\" + *model + \"' \";\n>> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n>> \t}\n>>\n>> \tname += \"(\" + camera->id() + \")\";\n>> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n>> index ac4619511..8d57023e1 100644\n>> --- a/src/apps/common/dng_writer.cpp\n>> +++ b/src/apps/common/dng_writer.cpp\n>> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>\n>> \tconst auto &model = cameraProperties.get(properties::Model);\n> \n> likewise\n\nSame here.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>> \tif (model) {\n>> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n>> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n>> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n>> \t}\n>>\n>> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n>> index 6b6d0713c..9f25ba091 100644\n>> --- a/src/apps/qcam/cam_select_dialog.cpp\n>> +++ b/src/apps/qcam/cam_select_dialog.cpp\n>> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n>> \t\tcameraLocation_->setText(\"Unknown\");\n>> \t}\n>>\n>> -\tconst auto &model = properties.get(libcamera::properties::Model)\n>> -\t\t\t\t .value_or(\"Unknown\");\n>> +\tconst auto model = properties.get(libcamera::properties::Model)\n>> +\t\t\t\t .value_or(\"Unknown\");\n>>\n>> -\tcameraModel_->setText(QString::fromStdString(model));\n>> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n>> }\n>> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n>> index 1ad1d4c1a..8c55ef845 100644\n>> --- a/src/py/libcamera/py_helpers.cpp\n>> +++ b/src/py/libcamera/py_helpers.cpp\n>> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n>> \tcase ControlTypeFloat:\n>> \t\treturn valueOrTuple(cv);\n>> \tcase ControlTypeString:\n>> -\t\treturn py::cast(cv.get());\n>> +\t\treturn py::cast(cv.get());\n>> \tcase ControlTypeSize: {\n>> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n>> \t\treturn py::cast(v);\n>> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n>> \tcase ControlTypeFloat:\n>> \t\treturn controlValueMaybeArray(ob);\n>> \tcase ControlTypeString:\n>> -\t\treturn ControlValue(ob.cast());\n>> +\t\treturn ControlValue(ob.cast());\n>> \tcase ControlTypeRectangle:\n>> \t\treturn controlValueMaybeArray(ob);\n>> \tcase ControlTypeSize:\n>> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n>> index 5084fd0cf..032050a77 100644\n>> --- a/test/controls/control_value.cpp\n>> +++ b/test/controls/control_value.cpp\n>> @@ -318,7 +318,7 @@ protected:\n>> \t\t/*\n>> \t\t * String type.\n>> \t\t */\n>> -\t\tstd::string string{ \"libcamera\" };\n>> +\t\tstd::string_view string{ \"libcamera\" };\n>> \t\tvalue.set(string);\n> \n> Here's one thing: we now require users to use string_view when setting\n> a control, and we anyway end up copying it's content into the\n> ControlValue.\n> \n> Should we provide a set() overload for std::string ? Is it worth it ?\n> \n>> \t\tif (value.isNone() || !value.isArray() ||\n>> \t\t value.type() != ControlTypeString ||\n>> @@ -327,7 +327,7 @@ protected:\n>> \t\t\treturn TestFail;\n>> \t\t}\n>>\n>> -\t\tif (value.get() != string) {\n>> +\t\tif (value.get() != string) {\n> \n> For get() instead I think it's fine always returning a view\n> \n>> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n>> \t\t\treturn TestFail;\n>> \t\t}\n>> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n>> index e51610481..9399727bd 100644\n>> --- a/utils/codegen/controls.py\n>> +++ b/utils/codegen/controls.py\n>> @@ -111,7 +111,7 @@ class Control(object):\n>> size = self.__data.get('size')\n>>\n>> if typ == 'string':\n>> - return 'std::string'\n>> + return 'std::string_view'\n>>\n>> if self.__size is None:\n>> return typ\n>> --\n>> 2.49.0\n>>","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 E13C7C327D\n\tfor ;\n\tFri, 2 May 2025 12:17:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E395F68ADC;\n\tFri, 2 May 2025 14:17:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 694FF68AD3\n\tfor ;\n\tFri, 2 May 2025 14:17:21 +0200 (CEST)","from [192.168.33.18] (185.221.143.50.nat.pool.zt.hu\n\t[185.221.143.50])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5C31DAF;\n\tFri, 2 May 2025 14:17:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FKqTGMG0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746188233;\n\tbh=SoRW4QVcoO1LmmjV1lKsaRFRR96mQSH3cAZw7dA52fc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=FKqTGMG0SlJI24RsCmN2P8KBha2OWSNScCO2Rt181HG52gWf1l/gr4APDkZ30Fo3M\n\tRHuJQcNT35PQHCYDFzfWfy8ybBt0iCpmZxf/Qd5qxHg2n1N4u1Nv5VECDj5jQfxZcz\n\tLjluk8xxEzDkE+TkDz5B3eejF0B65kLNcpHjquqs=","Message-ID":"","Date":"Fri, 2 May 2025 14:17:17 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","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\" "}},{"id":34117,"web_url":"https://patchwork.libcamera.org/comment/34117/","msgid":"","date":"2025-05-02T13:07:41","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Fri, May 02, 2025 at 02:17:17PM +0200, Barnabás Pőcze wrote:\n> Hi\n>\n>\n> 2025. 05. 02. 12:11 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n> > > When retrieving the value from a `ControlValue` usually one of two\n> > > things happen: a small, trivially copyable object is returned by\n> > > value; or a view into the internal buffer is provided. This is true\n> > > for everything except strings, which are returned in `std::string`,\n> > > incurring the overhead of string construction.\n> > >\n> > > To guarantee no potentially \"expensive\" copies, use `std::string_view`\n> > > pointing to the internal buffer to return the value. This is similar\n> > > to how other array-like types are returned with a `Span<>`.\n> > >\n> > > This is an API break, but its scope is limited to just `properties::Model`.\n> >\n> > I've been a bit in two minds about this, until I realized we already\n> > require c++17 for applications.\n> >\n> > My understanding is that using string_view is no different than\n> > returning a Span<> when it comes to lifetime of the referenced data,\n> > so if we're fine with Span<> we're fine with string_view.\n> >\n> > And as pointed out by Barnabas, if a user wants a string, it can be\n> > constructed easily with the same cost as we have today.\n> >\n> > >\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > include/libcamera/control_ids.h.in | 1 +\n> > > include/libcamera/controls.h | 15 ++++++++-------\n> > > src/apps/cam/capture_script.cpp | 2 +-\n> > > src/apps/cam/main.cpp | 2 +-\n> > > src/apps/common/dng_writer.cpp | 2 +-\n> > > src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n> > > src/py/libcamera/py_helpers.cpp | 4 ++--\n> > > test/controls/control_value.cpp | 4 ++--\n> > > utils/codegen/controls.py | 2 +-\n> > > 9 files changed, 20 insertions(+), 18 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> > > index 5d0594c68..41997f79f 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> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > index 2ae4ec3d4..5a707a387 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> > > @@ -96,7 +97,7 @@ struct control_type {\n> > > };\n> > >\n> > > template<>\n> > > -struct control_type {\n> > > +struct control_type {\n> > > \tstatic constexpr ControlType value = ControlTypeString;\n> > > \tstatic constexpr std::size_t size = 0;\n> > > };\n> > > @@ -138,7 +139,7 @@ public:\n> > > #ifndef __DOXYGEN__\n> > > \ttemplate::value &&\n> > > \t\t\t\t\t details::control_type::value &&\n> > > -\t\t\t\t\t !std::is_same>::value,\n> > > +\t\t\t\t\t !std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > \tControlValue(const T &value)\n> > > \t\t: type_(ControlTypeNone), numElements_(0)\n> > > @@ -148,7 +149,7 @@ public:\n> > > \t}\n> > >\n> > > \ttemplate::value ||\n> > > -\t\t\t\t\t std::is_same>::value,\n> > > +\t\t\t\t\t std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > #else\n> > > \ttemplate\n> > > @@ -182,7 +183,7 @@ public:\n> > >\n> > > #ifndef __DOXYGEN__\n> > > \ttemplate::value &&\n> > > -\t\t\t\t\t !std::is_same>::value,\n> > > +\t\t\t\t\t !std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > \tT get() const\n> > > \t{\n> > > @@ -193,7 +194,7 @@ public:\n> > > \t}\n> > >\n> > > \ttemplate::value ||\n> > > -\t\t\t\t\t std::is_same>::value,\n> > > +\t\t\t\t\t std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > #else\n> > > \ttemplate\n> > > @@ -210,7 +211,7 @@ public:\n> > >\n> > > #ifndef __DOXYGEN__\n> > > \ttemplate::value &&\n> > > -\t\t\t\t\t !std::is_same>::value,\n> > > +\t\t\t\t\t !std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > \tvoid set(const T &value)\n> > > \t{\n> > > @@ -219,7 +220,7 @@ public:\n> > > \t}\n> > >\n> > > \ttemplate::value ||\n> > > -\t\t\t\t\t std::is_same>::value,\n> > > +\t\t\t\t\t std::is_same>::value,\n> > > \t\t\t\t\t std::nullptr_t> = nullptr>\n> > > #else\n> > > \ttemplate\n> > > diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> > > index fdf82efc0..bc4c7c3a0 100644\n> > > --- a/src/apps/cam/capture_script.cpp\n> > > +++ b/src/apps/cam/capture_script.cpp\n> > > @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n> > > \t\tbreak;\n> > > \t}\n> > > \tcase ControlTypeString: {\n> > > -\t\tvalue.set(repr);\n> > > +\t\tvalue.set(repr);\n> > > \t\tbreak;\n> > > \t}\n> > > \tdefault:\n> > > diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> > > index fa266eca6..157d238d7 100644\n> > > --- a/src/apps/cam/main.cpp\n> > > +++ b/src/apps/cam/main.cpp\n> > > @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> > > \t\t */\n> > > \t\tconst auto &model = props.get(properties::Model);\n> >\n> > Does this need to be a reference now ?\n>\n> There isn't any real difference because it's an `std::optional` in both cases\n> (just with different value type), so I decided not to change it.\n>\n>\n> >\n> > > \t\tif (model)\n> > > -\t\t\tname = \"'\" + *model + \"' \";\n> > > +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n> > > \t}\n> > >\n> > > \tname += \"(\" + camera->id() + \")\";\n> > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> > > index ac4619511..8d57023e1 100644\n> > > --- a/src/apps/common/dng_writer.cpp\n> > > +++ b/src/apps/common/dng_writer.cpp\n> > > @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > >\n> > > \tconst auto &model = cameraProperties.get(properties::Model);\n> >\n> > likewise\n>\n> Same here.\n\nAck, thanks for clarifying.\n\nPlease add\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n> >\n> > > \tif (model) {\n> > > -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> > > +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n> > > \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> > > \t}\n> > >\n> > > diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n> > > index 6b6d0713c..9f25ba091 100644\n> > > --- a/src/apps/qcam/cam_select_dialog.cpp\n> > > +++ b/src/apps/qcam/cam_select_dialog.cpp\n> > > @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n> > > \t\tcameraLocation_->setText(\"Unknown\");\n> > > \t}\n> > >\n> > > -\tconst auto &model = properties.get(libcamera::properties::Model)\n> > > -\t\t\t\t .value_or(\"Unknown\");\n> > > +\tconst auto model = properties.get(libcamera::properties::Model)\n> > > +\t\t\t\t .value_or(\"Unknown\");\n> > >\n> > > -\tcameraModel_->setText(QString::fromStdString(model));\n> > > +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n> > > }\n> > > diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n> > > index 1ad1d4c1a..8c55ef845 100644\n> > > --- a/src/py/libcamera/py_helpers.cpp\n> > > +++ b/src/py/libcamera/py_helpers.cpp\n> > > @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n> > > \tcase ControlTypeFloat:\n> > > \t\treturn valueOrTuple(cv);\n> > > \tcase ControlTypeString:\n> > > -\t\treturn py::cast(cv.get());\n> > > +\t\treturn py::cast(cv.get());\n> > > \tcase ControlTypeSize: {\n> > > \t\tconst Size *v = reinterpret_cast(cv.data().data());\n> > > \t\treturn py::cast(v);\n> > > @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> > > \tcase ControlTypeFloat:\n> > > \t\treturn controlValueMaybeArray(ob);\n> > > \tcase ControlTypeString:\n> > > -\t\treturn ControlValue(ob.cast());\n> > > +\t\treturn ControlValue(ob.cast());\n> > > \tcase ControlTypeRectangle:\n> > > \t\treturn controlValueMaybeArray(ob);\n> > > \tcase ControlTypeSize:\n> > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > > index 5084fd0cf..032050a77 100644\n> > > --- a/test/controls/control_value.cpp\n> > > +++ b/test/controls/control_value.cpp\n> > > @@ -318,7 +318,7 @@ protected:\n> > > \t\t/*\n> > > \t\t * String type.\n> > > \t\t */\n> > > -\t\tstd::string string{ \"libcamera\" };\n> > > +\t\tstd::string_view string{ \"libcamera\" };\n> > > \t\tvalue.set(string);\n> >\n> > Here's one thing: we now require users to use string_view when setting\n> > a control, and we anyway end up copying it's content into the\n> > ControlValue.\n> >\n> > Should we provide a set() overload for std::string ? Is it worth it ?\n> >\n> > > \t\tif (value.isNone() || !value.isArray() ||\n> > > \t\t value.type() != ControlTypeString ||\n> > > @@ -327,7 +327,7 @@ protected:\n> > > \t\t\treturn TestFail;\n> > > \t\t}\n> > >\n> > > -\t\tif (value.get() != string) {\n> > > +\t\tif (value.get() != string) {\n> >\n> > For get() instead I think it's fine always returning a view\n> >\n> > > \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n> > > \t\t\treturn TestFail;\n> > > \t\t}\n> > > diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> > > index e51610481..9399727bd 100644\n> > > --- a/utils/codegen/controls.py\n> > > +++ b/utils/codegen/controls.py\n> > > @@ -111,7 +111,7 @@ class Control(object):\n> > > size = self.__data.get('size')\n> > >\n> > > if typ == 'string':\n> > > - return 'std::string'\n> > > + return 'std::string_view'\n> > >\n> > > if self.__size is None:\n> > > return typ\n> > > --\n> > > 2.49.0\n> > >\n>","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 64BAABE08B\n\tfor ;\n\tFri, 2 May 2025 13:07:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B37D68ADC;\n\tFri, 2 May 2025 15:07:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CE2A68AD3\n\tfor ;\n\tFri, 2 May 2025 15:07:44 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7E120353;\n\tFri, 2 May 2025 15:07:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"b46jl4S8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746191256;\n\tbh=IRuoAc4e9BsAPtcYrEB2PHkh+hSsdupmxmeeF7ak84o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b46jl4S8iFEHKfnrm5UXXUEfKOUvXn6Si4PvWrcw2M18mJOBo9UjrZoGQvy2RBv4Q\n\tRETk3mXxCDsYEufjUtg4KwGM7rmKsweGlczBQFS5xou1tIQiWXwTx2F2a/xJJLrUj0\n\tTfSDNrYfxU1R3c3MoDGavqGfmfc71hbkhZ+2D/3k=","Date":"Fri, 2 May 2025 15:07:41 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","Message-ID":"","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}},{"id":34230,"web_url":"https://patchwork.libcamera.org/comment/34230/","msgid":"<174714997060.233090.10777738173739600758@pyrite.rasen.tech>","date":"2025-05-13T15:26:10","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Barnabás,\n\nThanks for the patch.\n\nQuoting Barnabás Pőcze (2025-05-01 11:58:18)\n> When retrieving the value from a `ControlValue` usually one of two\n> things happen: a small, trivially copyable object is returned by\n> value; or a view into the internal buffer is provided. This is true\n> for everything except strings, which are returned in `std::string`,\n> incurring the overhead of string construction.\n> \n> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n> pointing to the internal buffer to return the value. This is similar\n> to how other array-like types are returned with a `Span<>`.\n> \n> This is an API break, but its scope is limited to just `properties::Model`.\n\nCreating compatibility report ...\nBinary compatibility: 100%\nSource compatibility: 99.8%\nTotal binary compatibility problems: 0, warnings: 1\nTotal source compatibility problems: 2, warnings: 1\n\nNot bad.\n\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n> Signed-off-by: Barnabás Pőcze \n\nLooks good to me.\n\nReviewed-by: Paul Elder \n\n> ---\n> include/libcamera/control_ids.h.in | 1 +\n> include/libcamera/controls.h | 15 ++++++++-------\n> src/apps/cam/capture_script.cpp | 2 +-\n> src/apps/cam/main.cpp | 2 +-\n> src/apps/common/dng_writer.cpp | 2 +-\n> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n> src/py/libcamera/py_helpers.cpp | 4 ++--\n> test/controls/control_value.cpp | 4 ++--\n> utils/codegen/controls.py | 2 +-\n> 9 files changed, 20 insertions(+), 18 deletions(-)\n> \n> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> index 5d0594c68..41997f79f 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> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 2ae4ec3d4..5a707a387 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> @@ -96,7 +97,7 @@ struct control_type {\n> };\n> \n> template<>\n> -struct control_type {\n> +struct control_type {\n> static constexpr ControlType value = ControlTypeString;\n> static constexpr std::size_t size = 0;\n> };\n> @@ -138,7 +139,7 @@ public:\n> #ifndef __DOXYGEN__\n> template::value &&\n> details::control_type::value &&\n> - !std::is_same>::value,\n> + !std::is_same>::value,\n> std::nullptr_t> = nullptr>\n> ControlValue(const T &value)\n> : type_(ControlTypeNone), numElements_(0)\n> @@ -148,7 +149,7 @@ public:\n> }\n> \n> template::value ||\n> - std::is_same>::value,\n> + std::is_same>::value,\n> std::nullptr_t> = nullptr>\n> #else\n> template\n> @@ -182,7 +183,7 @@ public:\n> \n> #ifndef __DOXYGEN__\n> template::value &&\n> - !std::is_same>::value,\n> + !std::is_same>::value,\n> std::nullptr_t> = nullptr>\n> T get() const\n> {\n> @@ -193,7 +194,7 @@ public:\n> }\n> \n> template::value ||\n> - std::is_same>::value,\n> + std::is_same>::value,\n> std::nullptr_t> = nullptr>\n> #else\n> template\n> @@ -210,7 +211,7 @@ public:\n> \n> #ifndef __DOXYGEN__\n> template::value &&\n> - !std::is_same>::value,\n> + !std::is_same>::value,\n> std::nullptr_t> = nullptr>\n> void set(const T &value)\n> {\n> @@ -219,7 +220,7 @@ public:\n> }\n> \n> template::value ||\n> - std::is_same>::value,\n> + std::is_same>::value,\n> std::nullptr_t> = nullptr>\n> #else\n> template\n> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> index fdf82efc0..bc4c7c3a0 100644\n> --- a/src/apps/cam/capture_script.cpp\n> +++ b/src/apps/cam/capture_script.cpp\n> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n> break;\n> }\n> case ControlTypeString: {\n> - value.set(repr);\n> + value.set(repr);\n> break;\n> }\n> default:\n> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> index fa266eca6..157d238d7 100644\n> --- a/src/apps/cam/main.cpp\n> +++ b/src/apps/cam/main.cpp\n> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> */\n> const auto &model = props.get(properties::Model);\n> if (model)\n> - name = \"'\" + *model + \"' \";\n> + name.append(\"'\").append(*model).append(\"' \");\n> }\n> \n> name += \"(\" + camera->id() + \")\";\n> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> index ac4619511..8d57023e1 100644\n> --- a/src/apps/common/dng_writer.cpp\n> +++ b/src/apps/common/dng_writer.cpp\n> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \n> const auto &model = cameraProperties.get(properties::Model);\n> if (model) {\n> - TIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> + TIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n> /* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> }\n> \n> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n> index 6b6d0713c..9f25ba091 100644\n> --- a/src/apps/qcam/cam_select_dialog.cpp\n> +++ b/src/apps/qcam/cam_select_dialog.cpp\n> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n> cameraLocation_->setText(\"Unknown\");\n> }\n> \n> - const auto &model = properties.get(libcamera::properties::Model)\n> - .value_or(\"Unknown\");\n> + const auto model = properties.get(libcamera::properties::Model)\n> + .value_or(\"Unknown\");\n> \n> - cameraModel_->setText(QString::fromStdString(model));\n> + cameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n> }\n> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n> index 1ad1d4c1a..8c55ef845 100644\n> --- a/src/py/libcamera/py_helpers.cpp\n> +++ b/src/py/libcamera/py_helpers.cpp\n> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n> case ControlTypeFloat:\n> return valueOrTuple(cv);\n> case ControlTypeString:\n> - return py::cast(cv.get());\n> + return py::cast(cv.get());\n> case ControlTypeSize: {\n> const Size *v = reinterpret_cast(cv.data().data());\n> return py::cast(v);\n> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> case ControlTypeFloat:\n> return controlValueMaybeArray(ob);\n> case ControlTypeString:\n> - return ControlValue(ob.cast());\n> + return ControlValue(ob.cast());\n> case ControlTypeRectangle:\n> return controlValueMaybeArray(ob);\n> case ControlTypeSize:\n> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> index 5084fd0cf..032050a77 100644\n> --- a/test/controls/control_value.cpp\n> +++ b/test/controls/control_value.cpp\n> @@ -318,7 +318,7 @@ protected:\n> /*\n> * String type.\n> */\n> - std::string string{ \"libcamera\" };\n> + std::string_view string{ \"libcamera\" };\n> value.set(string);\n> if (value.isNone() || !value.isArray() ||\n> value.type() != ControlTypeString ||\n> @@ -327,7 +327,7 @@ protected:\n> return TestFail;\n> }\n> \n> - if (value.get() != string) {\n> + if (value.get() != string) {\n> cerr << \"Control value mismatch after setting to string\" << endl;\n> return TestFail;\n> }\n> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> index e51610481..9399727bd 100644\n> --- a/utils/codegen/controls.py\n> +++ b/utils/codegen/controls.py\n> @@ -111,7 +111,7 @@ class Control(object):\n> size = self.__data.get('size')\n> \n> if typ == 'string':\n> - return 'std::string'\n> + return 'std::string_view'\n> \n> if self.__size is None:\n> return typ\n> -- \n> 2.49.0\n>","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 6AE74C3200\n\tfor ;\n\tTue, 13 May 2025 15:26:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B15F68B61;\n\tTue, 13 May 2025 17:26:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A0FE768B40\n\tfor ;\n\tTue, 13 May 2025 17:26:13 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2001:861:3a80:3300:4f2f:8c2c:b3ef:17d4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CA4A24C9;\n\tTue, 13 May 2025 17:25:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bi4l0ZMl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747149957;\n\tbh=ub1W7WAGov1KCvu2IB5lshLqNKoIsNXCQ12U8nzG1aw=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=bi4l0ZMlBwU0c6yo2c//x4KJOp2vWzvtoYuGm/0w7oAas/IPvKSTlSZsZbHd6lraZ\n\tqMVMNECaBkPwNjHZCe/453zJiZzCfBna/sNkxtGzAgQObO2l+063Cwtyi7ZIawkOXn\n\tFSVRwc1QHl8Ln12+GPEwinit6kddyGyX+z7yrFag=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","From":"Paul Elder ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 13 May 2025 17:26:10 +0200","Message-ID":"<174714997060.233090.10777738173739600758@pyrite.rasen.tech>","User-Agent":"alot/0.0.0","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\" "}},{"id":36087,"web_url":"https://patchwork.libcamera.org/comment/36087/","msgid":"<20251002182700.GF10198@pendragon.ideasonboard.com>","date":"2025-10-02T18:27:00","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n> When retrieving the value from a `ControlValue` usually one of two\n> things happen: a small, trivially copyable object is returned by\n> value; or a view into the internal buffer is provided. This is true\n> for everything except strings, which are returned in `std::string`,\n> incurring the overhead of string construction.\n> \n> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n> pointing to the internal buffer to return the value. This is similar\n> to how other array-like types are returned with a `Span<>`.\n> \n> This is an API break, but its scope is limited to just `properties::Model`.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/control_ids.h.in | 1 +\n> include/libcamera/controls.h | 15 ++++++++-------\n> src/apps/cam/capture_script.cpp | 2 +-\n> src/apps/cam/main.cpp | 2 +-\n> src/apps/common/dng_writer.cpp | 2 +-\n> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n> src/py/libcamera/py_helpers.cpp | 4 ++--\n> test/controls/control_value.cpp | 4 ++--\n> utils/codegen/controls.py | 2 +-\n> 9 files changed, 20 insertions(+), 18 deletions(-)\n> \n> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> index 5d0594c68..41997f79f 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> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 2ae4ec3d4..5a707a387 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> @@ -96,7 +97,7 @@ struct control_type {\n> };\n> \n> template<>\n> -struct control_type {\n> +struct control_type {\n> \tstatic constexpr ControlType value = ControlTypeString;\n> \tstatic constexpr std::size_t size = 0;\n> };\n> @@ -138,7 +139,7 @@ public:\n> #ifndef __DOXYGEN__\n> \ttemplate::value &&\n> \t\t\t\t\t details::control_type::value &&\n> -\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t !std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> \tControlValue(const T &value)\n> \t\t: type_(ControlTypeNone), numElements_(0)\n> @@ -148,7 +149,7 @@ public:\n> \t}\n> \n> \ttemplate::value ||\n> -\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> @@ -182,7 +183,7 @@ public:\n> \n> #ifndef __DOXYGEN__\n> \ttemplate::value &&\n> -\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t !std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> \tT get() const\n> \t{\n> @@ -193,7 +194,7 @@ public:\n> \t}\n> \n> \ttemplate::value ||\n> -\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> @@ -210,7 +211,7 @@ public:\n> \n> #ifndef __DOXYGEN__\n> \ttemplate::value &&\n> -\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t !std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> \tvoid set(const T &value)\n> \t{\n> @@ -219,7 +220,7 @@ public:\n> \t}\n> \n> \ttemplate::value ||\n> -\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t std::is_same>::value,\n> \t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> index fdf82efc0..bc4c7c3a0 100644\n> --- a/src/apps/cam/capture_script.cpp\n> +++ b/src/apps/cam/capture_script.cpp\n> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n> \t\tbreak;\n> \t}\n> \tcase ControlTypeString: {\n> -\t\tvalue.set(repr);\n> +\t\tvalue.set(repr);\n> \t\tbreak;\n> \t}\n> \tdefault:\n> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> index fa266eca6..157d238d7 100644\n> --- a/src/apps/cam/main.cpp\n> +++ b/src/apps/cam/main.cpp\n> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> \t\t */\n> \t\tconst auto &model = props.get(properties::Model);\n> \t\tif (model)\n> -\t\t\tname = \"'\" + *model + \"' \";\n> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n> \t}\n> \n> \tname += \"(\" + camera->id() + \")\";\n> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> index ac4619511..8d57023e1 100644\n> --- a/src/apps/common/dng_writer.cpp\n> +++ b/src/apps/common/dng_writer.cpp\n> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \n> \tconst auto &model = cameraProperties.get(properties::Model);\n> \tif (model) {\n> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n\nSlightly annoying, but that's an issue with the TIFF library API, it's\nnot our fault.\n\n> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> \t}\n> \n> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n> index 6b6d0713c..9f25ba091 100644\n> --- a/src/apps/qcam/cam_select_dialog.cpp\n> +++ b/src/apps/qcam/cam_select_dialog.cpp\n> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n> \t\tcameraLocation_->setText(\"Unknown\");\n> \t}\n> \n> -\tconst auto &model = properties.get(libcamera::properties::Model)\n> -\t\t\t\t .value_or(\"Unknown\");\n> +\tconst auto model = properties.get(libcamera::properties::Model)\n> +\t\t\t\t .value_or(\"Unknown\");\n\nI was concerned that this could result in a use-after-free if the\nargument to value_or() is a temporary std::string, but I think the\nlifetime of the temporary will be extended to cover the lifetime of\nmodel.\n\nReviewed-by: Laurent Pinchart \n\n> \n> -\tcameraModel_->setText(QString::fromStdString(model));\n> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n> }\n> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n> index 1ad1d4c1a..8c55ef845 100644\n> --- a/src/py/libcamera/py_helpers.cpp\n> +++ b/src/py/libcamera/py_helpers.cpp\n> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n> \tcase ControlTypeFloat:\n> \t\treturn valueOrTuple(cv);\n> \tcase ControlTypeString:\n> -\t\treturn py::cast(cv.get());\n> +\t\treturn py::cast(cv.get());\n> \tcase ControlTypeSize: {\n> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n> \t\treturn py::cast(v);\n> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> \tcase ControlTypeFloat:\n> \t\treturn controlValueMaybeArray(ob);\n> \tcase ControlTypeString:\n> -\t\treturn ControlValue(ob.cast());\n> +\t\treturn ControlValue(ob.cast());\n> \tcase ControlTypeRectangle:\n> \t\treturn controlValueMaybeArray(ob);\n> \tcase ControlTypeSize:\n> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> index 5084fd0cf..032050a77 100644\n> --- a/test/controls/control_value.cpp\n> +++ b/test/controls/control_value.cpp\n> @@ -318,7 +318,7 @@ protected:\n> \t\t/*\n> \t\t * String type.\n> \t\t */\n> -\t\tstd::string string{ \"libcamera\" };\n> +\t\tstd::string_view string{ \"libcamera\" };\n> \t\tvalue.set(string);\n> \t\tif (value.isNone() || !value.isArray() ||\n> \t\t value.type() != ControlTypeString ||\n> @@ -327,7 +327,7 @@ protected:\n> \t\t\treturn TestFail;\n> \t\t}\n> \n> -\t\tif (value.get() != string) {\n> +\t\tif (value.get() != string) {\n> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> index e51610481..9399727bd 100644\n> --- a/utils/codegen/controls.py\n> +++ b/utils/codegen/controls.py\n> @@ -111,7 +111,7 @@ class Control(object):\n> size = self.__data.get('size')\n> \n> if typ == 'string':\n> - return 'std::string'\n> + return 'std::string_view'\n> \n> if self.__size is None:\n> return typ","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 261BEC324C\n\tfor ;\n\tThu, 2 Oct 2025 18:27:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2FFF46B5F3;\n\tThu, 2 Oct 2025 20:27:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65BE46B5A2\n\tfor ;\n\tThu, 2 Oct 2025 20:27:06 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 5E661CF;\n\tThu, 2 Oct 2025 20:25:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UKmPqBzj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759429536;\n\tbh=yLhwzRUcXuYr5Dy1nCsqEwKEcUnEWF6ncU91hfxkGto=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UKmPqBzjNYrU58iab6//9gtWDPrzkzze9YJ7e8NVk1eoCKiAz49t8mlMZIJ5qq7JV\n\tiiHbViA84HNvyAcqyeg1g5aCAqukfducbwphUAhzoIq/iGrx7l339oLlWhd0bHe52p\n\tBmKfRCvKNXTBdISkapjw3l6HL4xgBi5T+VOk/nYI=","Date":"Thu, 2 Oct 2025 21:27:00 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","Message-ID":"<20251002182700.GF10198@pendragon.ideasonboard.com>","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>","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\" "}},{"id":36106,"web_url":"https://patchwork.libcamera.org/comment/36106/","msgid":"","date":"2025-10-03T13:30:21","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 02. 20:27 keltezéssel, Laurent Pinchart írta:\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n>> When retrieving the value from a `ControlValue` usually one of two\n>> things happen: a small, trivially copyable object is returned by\n>> value; or a view into the internal buffer is provided. This is true\n>> for everything except strings, which are returned in `std::string`,\n>> incurring the overhead of string construction.\n>>\n>> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n>> pointing to the internal buffer to return the value. This is similar\n>> to how other array-like types are returned with a `Span<>`.\n>>\n>> This is an API break, but its scope is limited to just `properties::Model`.\n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> include/libcamera/control_ids.h.in | 1 +\n>> include/libcamera/controls.h | 15 ++++++++-------\n>> src/apps/cam/capture_script.cpp | 2 +-\n>> src/apps/cam/main.cpp | 2 +-\n>> src/apps/common/dng_writer.cpp | 2 +-\n>> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n>> src/py/libcamera/py_helpers.cpp | 4 ++--\n>> test/controls/control_value.cpp | 4 ++--\n>> utils/codegen/controls.py | 2 +-\n>> 9 files changed, 20 insertions(+), 18 deletions(-)\n>>\n>> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n>> index 5d0594c68..41997f79f 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>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index 2ae4ec3d4..5a707a387 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>> @@ -96,7 +97,7 @@ struct control_type {\n>> };\n>> \n>> template<>\n>> -struct control_type {\n>> +struct control_type {\n>> \tstatic constexpr ControlType value = ControlTypeString;\n>> \tstatic constexpr std::size_t size = 0;\n>> };\n>> @@ -138,7 +139,7 @@ public:\n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> \t\t\t\t\t details::control_type::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tControlValue(const T &value)\n>> \t\t: type_(ControlTypeNone), numElements_(0)\n>> @@ -148,7 +149,7 @@ public:\n>> \t}\n>> \n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> @@ -182,7 +183,7 @@ public:\n>> \n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tT get() const\n>> \t{\n>> @@ -193,7 +194,7 @@ public:\n>> \t}\n>> \n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> @@ -210,7 +211,7 @@ public:\n>> \n>> #ifndef __DOXYGEN__\n>> \ttemplate::value &&\n>> -\t\t\t\t\t !std::is_same>::value,\n>> +\t\t\t\t\t !std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> \tvoid set(const T &value)\n>> \t{\n>> @@ -219,7 +220,7 @@ public:\n>> \t}\n>> \n>> \ttemplate::value ||\n>> -\t\t\t\t\t std::is_same>::value,\n>> +\t\t\t\t\t std::is_same>::value,\n>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>> #else\n>> \ttemplate\n>> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n>> index fdf82efc0..bc4c7c3a0 100644\n>> --- a/src/apps/cam/capture_script.cpp\n>> +++ b/src/apps/cam/capture_script.cpp\n>> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n>> \t\tbreak;\n>> \t}\n>> \tcase ControlTypeString: {\n>> -\t\tvalue.set(repr);\n>> +\t\tvalue.set(repr);\n>> \t\tbreak;\n>> \t}\n>> \tdefault:\n>> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n>> index fa266eca6..157d238d7 100644\n>> --- a/src/apps/cam/main.cpp\n>> +++ b/src/apps/cam/main.cpp\n>> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n>> \t\t */\n>> \t\tconst auto &model = props.get(properties::Model);\n>> \t\tif (model)\n>> -\t\t\tname = \"'\" + *model + \"' \";\n>> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n>> \t}\n>> \n>> \tname += \"(\" + camera->id() + \")\";\n>> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n>> index ac4619511..8d57023e1 100644\n>> --- a/src/apps/common/dng_writer.cpp\n>> +++ b/src/apps/common/dng_writer.cpp\n>> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>> \n>> \tconst auto &model = cameraProperties.get(properties::Model);\n>> \tif (model) {\n>> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n>> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n> \n> Slightly annoying, but that's an issue with the TIFF library API, it's\n> not our fault.\n> \n>> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n>> \t}\n>> \n>> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n>> index 6b6d0713c..9f25ba091 100644\n>> --- a/src/apps/qcam/cam_select_dialog.cpp\n>> +++ b/src/apps/qcam/cam_select_dialog.cpp\n>> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n>> \t\tcameraLocation_->setText(\"Unknown\");\n>> \t}\n>> \n>> -\tconst auto &model = properties.get(libcamera::properties::Model)\n>> -\t\t\t\t .value_or(\"Unknown\");\n>> +\tconst auto model = properties.get(libcamera::properties::Model)\n>> +\t\t\t\t .value_or(\"Unknown\");\n> \n> I was concerned that this could result in a use-after-free if the\n> argument to value_or() is a temporary std::string, but I think the\n\nI am afraid that's exactly what would happen, an `std::string_view` is\nreturned to the temporary `std::string`. So that would be a use-after-free\nin some cases.\n\n\n> lifetime of the temporary will be extended to cover the lifetime of\n> model.\n> \n> Reviewed-by: Laurent Pinchart \n> \n>> \n>> -\tcameraModel_->setText(QString::fromStdString(model));\n>> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n>> }\n>> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n>> index 1ad1d4c1a..8c55ef845 100644\n>> --- a/src/py/libcamera/py_helpers.cpp\n>> +++ b/src/py/libcamera/py_helpers.cpp\n>> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n>> \tcase ControlTypeFloat:\n>> \t\treturn valueOrTuple(cv);\n>> \tcase ControlTypeString:\n>> -\t\treturn py::cast(cv.get());\n>> +\t\treturn py::cast(cv.get());\n>> \tcase ControlTypeSize: {\n>> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n>> \t\treturn py::cast(v);\n>> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n>> \tcase ControlTypeFloat:\n>> \t\treturn controlValueMaybeArray(ob);\n>> \tcase ControlTypeString:\n>> -\t\treturn ControlValue(ob.cast());\n>> +\t\treturn ControlValue(ob.cast());\n>> \tcase ControlTypeRectangle:\n>> \t\treturn controlValueMaybeArray(ob);\n>> \tcase ControlTypeSize:\n>> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n>> index 5084fd0cf..032050a77 100644\n>> --- a/test/controls/control_value.cpp\n>> +++ b/test/controls/control_value.cpp\n>> @@ -318,7 +318,7 @@ protected:\n>> \t\t/*\n>> \t\t * String type.\n>> \t\t */\n>> -\t\tstd::string string{ \"libcamera\" };\n>> +\t\tstd::string_view string{ \"libcamera\" };\n>> \t\tvalue.set(string);\n>> \t\tif (value.isNone() || !value.isArray() ||\n>> \t\t value.type() != ControlTypeString ||\n>> @@ -327,7 +327,7 @@ protected:\n>> \t\t\treturn TestFail;\n>> \t\t}\n>> \n>> -\t\tif (value.get() != string) {\n>> +\t\tif (value.get() != string) {\n>> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n>> \t\t\treturn TestFail;\n>> \t\t}\n>> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n>> index e51610481..9399727bd 100644\n>> --- a/utils/codegen/controls.py\n>> +++ b/utils/codegen/controls.py\n>> @@ -111,7 +111,7 @@ class Control(object):\n>> size = self.__data.get('size')\n>> \n>> if typ == 'string':\n>> - return 'std::string'\n>> + return 'std::string_view'\n>> \n>> if self.__size is None:\n>> return typ\n>","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 723D6C324C\n\tfor ;\n\tFri, 3 Oct 2025 13:30:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 236B46B5F3;\n\tFri, 3 Oct 2025 15:30:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 919BE69318\n\tfor ;\n\tFri, 3 Oct 2025 15:30:24 +0200 (CEST)","from [192.168.33.17] (185.182.214.142.nat.pool.zt.hu\n\t[185.182.214.142])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AFC4EC7A;\n\tFri, 3 Oct 2025 15:28:53 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bxcWEMNg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759498133;\n\tbh=+cafmqMmsV58wKh4CM4dF7qnGQ4to4W3XAtSIKunvlM=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=bxcWEMNgvC219+ovb2NaM+Vh2VfEAxj2+LTk7vyfjPI77cClPAVOes6RzLUDVvgUu\n\teURfGys1bFkuCEhnDVruZKpZjrjrIZItXh8Gbo2FsUwDBDuo+aEnDvGDmnSbMtccRx\n\t7EK0yhlpbAG2rytjzODuADaLx6vwVD8pCiNL+ErM=","Message-ID":"","Date":"Fri, 3 Oct 2025 15:30:21 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t<20251002182700.GF10198@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251002182700.GF10198@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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\" "}},{"id":36120,"web_url":"https://patchwork.libcamera.org/comment/36120/","msgid":"<20251003165818.GA20317@pendragon.ideasonboard.com>","date":"2025-10-03T16:58:18","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Oct 03, 2025 at 03:30:21PM +0200, Barnabás Pőcze wrote:\n> 2025. 10. 02. 20:27 keltezéssel, Laurent Pinchart írta:\n> > On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n> >> When retrieving the value from a `ControlValue` usually one of two\n> >> things happen: a small, trivially copyable object is returned by\n> >> value; or a view into the internal buffer is provided. This is true\n> >> for everything except strings, which are returned in `std::string`,\n> >> incurring the overhead of string construction.\n> >>\n> >> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n> >> pointing to the internal buffer to return the value. This is similar\n> >> to how other array-like types are returned with a `Span<>`.\n> >>\n> >> This is an API break, but its scope is limited to just `properties::Model`.\n> >>\n> >> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n> >> Signed-off-by: Barnabás Pőcze \n> >> ---\n> >> include/libcamera/control_ids.h.in | 1 +\n> >> include/libcamera/controls.h | 15 ++++++++-------\n> >> src/apps/cam/capture_script.cpp | 2 +-\n> >> src/apps/cam/main.cpp | 2 +-\n> >> src/apps/common/dng_writer.cpp | 2 +-\n> >> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n> >> src/py/libcamera/py_helpers.cpp | 4 ++--\n> >> test/controls/control_value.cpp | 4 ++--\n> >> utils/codegen/controls.py | 2 +-\n> >> 9 files changed, 20 insertions(+), 18 deletions(-)\n> >>\n> >> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> >> index 5d0594c68..41997f79f 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> >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> >> index 2ae4ec3d4..5a707a387 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> >> @@ -96,7 +97,7 @@ struct control_type {\n> >> };\n> >> \n> >> template<>\n> >> -struct control_type {\n> >> +struct control_type {\n> >> \tstatic constexpr ControlType value = ControlTypeString;\n> >> \tstatic constexpr std::size_t size = 0;\n> >> };\n> >> @@ -138,7 +139,7 @@ public:\n> >> #ifndef __DOXYGEN__\n> >> \ttemplate::value &&\n> >> \t\t\t\t\t details::control_type::value &&\n> >> -\t\t\t\t\t !std::is_same>::value,\n> >> +\t\t\t\t\t !std::is_same>::value,\n> >> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >> \tControlValue(const T &value)\n> >> \t\t: type_(ControlTypeNone), numElements_(0)\n> >> @@ -148,7 +149,7 @@ public:\n> >> \t}\n> >> \n> >> \ttemplate::value ||\n> >> -\t\t\t\t\t std::is_same>::value,\n> >> +\t\t\t\t\t std::is_same>::value,\n> >> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >> #else\n> >> \ttemplate\n> >> @@ -182,7 +183,7 @@ public:\n> >> \n> >> #ifndef __DOXYGEN__\n> >> \ttemplate::value &&\n> >> -\t\t\t\t\t !std::is_same>::value,\n> >> +\t\t\t\t\t !std::is_same>::value,\n> >> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >> \tT get() const\n> >> \t{\n> >> @@ -193,7 +194,7 @@ public:\n> >> \t}\n> >> \n> >> \ttemplate::value ||\n> >> -\t\t\t\t\t std::is_same>::value,\n> >> +\t\t\t\t\t std::is_same>::value,\n> >> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >> #else\n> >> \ttemplate\n> >> @@ -210,7 +211,7 @@ public:\n> >> \n> >> #ifndef __DOXYGEN__\n> >> \ttemplate::value &&\n> >> -\t\t\t\t\t !std::is_same>::value,\n> >> +\t\t\t\t\t !std::is_same>::value,\n> >> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >> \tvoid set(const T &value)\n> >> \t{\n> >> @@ -219,7 +220,7 @@ public:\n> >> \t}\n> >> \n> >> \ttemplate::value ||\n> >> -\t\t\t\t\t std::is_same>::value,\n> >> +\t\t\t\t\t std::is_same>::value,\n> >> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >> #else\n> >> \ttemplate\n> >> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> >> index fdf82efc0..bc4c7c3a0 100644\n> >> --- a/src/apps/cam/capture_script.cpp\n> >> +++ b/src/apps/cam/capture_script.cpp\n> >> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n> >> \t\tbreak;\n> >> \t}\n> >> \tcase ControlTypeString: {\n> >> -\t\tvalue.set(repr);\n> >> +\t\tvalue.set(repr);\n> >> \t\tbreak;\n> >> \t}\n> >> \tdefault:\n> >> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> >> index fa266eca6..157d238d7 100644\n> >> --- a/src/apps/cam/main.cpp\n> >> +++ b/src/apps/cam/main.cpp\n> >> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> >> \t\t */\n> >> \t\tconst auto &model = props.get(properties::Model);\n> >> \t\tif (model)\n> >> -\t\t\tname = \"'\" + *model + \"' \";\n> >> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n> >> \t}\n> >> \n> >> \tname += \"(\" + camera->id() + \")\";\n> >> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> >> index ac4619511..8d57023e1 100644\n> >> --- a/src/apps/common/dng_writer.cpp\n> >> +++ b/src/apps/common/dng_writer.cpp\n> >> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >> \n> >> \tconst auto &model = cameraProperties.get(properties::Model);\n> >> \tif (model) {\n> >> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> >> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n> > \n> > Slightly annoying, but that's an issue with the TIFF library API, it's\n> > not our fault.\n> > \n> >> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> >> \t}\n> >> \n> >> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n> >> index 6b6d0713c..9f25ba091 100644\n> >> --- a/src/apps/qcam/cam_select_dialog.cpp\n> >> +++ b/src/apps/qcam/cam_select_dialog.cpp\n> >> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n> >> \t\tcameraLocation_->setText(\"Unknown\");\n> >> \t}\n> >> \n> >> -\tconst auto &model = properties.get(libcamera::properties::Model)\n> >> -\t\t\t\t .value_or(\"Unknown\");\n> >> +\tconst auto model = properties.get(libcamera::properties::Model)\n> >> +\t\t\t\t .value_or(\"Unknown\");\n> > \n> > I was concerned that this could result in a use-after-free if the\n> > argument to value_or() is a temporary std::string, but I think the\n> \n> I am afraid that's exactly what would happen, an `std::string_view` is\n> returned to the temporary `std::string`. So that would be a use-after-free\n> in some cases.\n\nvalgrind didn't catch any issue. Isn't the lifetime of temporary objects\nextended in this case ?\n\n> > lifetime of the temporary will be extended to cover the lifetime of\n> > model.\n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> >> \n> >> -\tcameraModel_->setText(QString::fromStdString(model));\n> >> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n> >> }\n> >> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n> >> index 1ad1d4c1a..8c55ef845 100644\n> >> --- a/src/py/libcamera/py_helpers.cpp\n> >> +++ b/src/py/libcamera/py_helpers.cpp\n> >> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n> >> \tcase ControlTypeFloat:\n> >> \t\treturn valueOrTuple(cv);\n> >> \tcase ControlTypeString:\n> >> -\t\treturn py::cast(cv.get());\n> >> +\t\treturn py::cast(cv.get());\n> >> \tcase ControlTypeSize: {\n> >> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n> >> \t\treturn py::cast(v);\n> >> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> >> \tcase ControlTypeFloat:\n> >> \t\treturn controlValueMaybeArray(ob);\n> >> \tcase ControlTypeString:\n> >> -\t\treturn ControlValue(ob.cast());\n> >> +\t\treturn ControlValue(ob.cast());\n> >> \tcase ControlTypeRectangle:\n> >> \t\treturn controlValueMaybeArray(ob);\n> >> \tcase ControlTypeSize:\n> >> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> >> index 5084fd0cf..032050a77 100644\n> >> --- a/test/controls/control_value.cpp\n> >> +++ b/test/controls/control_value.cpp\n> >> @@ -318,7 +318,7 @@ protected:\n> >> \t\t/*\n> >> \t\t * String type.\n> >> \t\t */\n> >> -\t\tstd::string string{ \"libcamera\" };\n> >> +\t\tstd::string_view string{ \"libcamera\" };\n> >> \t\tvalue.set(string);\n> >> \t\tif (value.isNone() || !value.isArray() ||\n> >> \t\t value.type() != ControlTypeString ||\n> >> @@ -327,7 +327,7 @@ protected:\n> >> \t\t\treturn TestFail;\n> >> \t\t}\n> >> \n> >> -\t\tif (value.get() != string) {\n> >> +\t\tif (value.get() != string) {\n> >> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n> >> \t\t\treturn TestFail;\n> >> \t\t}\n> >> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> >> index e51610481..9399727bd 100644\n> >> --- a/utils/codegen/controls.py\n> >> +++ b/utils/codegen/controls.py\n> >> @@ -111,7 +111,7 @@ class Control(object):\n> >> size = self.__data.get('size')\n> >> \n> >> if typ == 'string':\n> >> - return 'std::string'\n> >> + return 'std::string_view'\n> >> \n> >> if self.__size is None:\n> >> return typ","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 EEDDDBF415\n\tfor ;\n\tFri, 3 Oct 2025 16:58:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1DFBF6B5F3;\n\tFri, 3 Oct 2025 18:58:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B496D69318\n\tfor ;\n\tFri, 3 Oct 2025 18:58:24 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id EB3711F16; \n\tFri, 3 Oct 2025 18:56:53 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"c5EKLET4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759510614;\n\tbh=2FYa/mjJMXQgozcyhyCtejQ75yTcVdPyjXcpsZG4ncQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=c5EKLET4gG4aagE1bK9cCcivCs34N9cWFHa59mqTKU4/UH4OFFcUvObuePkoXAgRm\n\ta0zXXzrDXacomVG8Uo8OroRLW6s+cdxR3KtNHZhJbSRQv8+jiTKmsOyOy1/H9w8YR5\n\t2hctqy2+JEo3Vjm2TaZxAVqkMWxHsT151t3Wm6DI=","Date":"Fri, 3 Oct 2025 19:58:18 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","Message-ID":"<20251003165818.GA20317@pendragon.ideasonboard.com>","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t<20251002182700.GF10198@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}},{"id":36121,"web_url":"https://patchwork.libcamera.org/comment/36121/","msgid":"<73167cdc-f279-47a0-8f75-ac8dc7441589@ideasonboard.com>","date":"2025-10-03T17:08:35","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 03. 18:58 keltezéssel, Laurent Pinchart írta:\n> On Fri, Oct 03, 2025 at 03:30:21PM +0200, Barnabás Pőcze wrote:\n>> 2025. 10. 02. 20:27 keltezéssel, Laurent Pinchart írta:\n>>> On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n>>>> When retrieving the value from a `ControlValue` usually one of two\n>>>> things happen: a small, trivially copyable object is returned by\n>>>> value; or a view into the internal buffer is provided. This is true\n>>>> for everything except strings, which are returned in `std::string`,\n>>>> incurring the overhead of string construction.\n>>>>\n>>>> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n>>>> pointing to the internal buffer to return the value. This is similar\n>>>> to how other array-like types are returned with a `Span<>`.\n>>>>\n>>>> This is an API break, but its scope is limited to just `properties::Model`.\n>>>>\n>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n>>>> Signed-off-by: Barnabás Pőcze \n>>>> ---\n>>>> include/libcamera/control_ids.h.in | 1 +\n>>>> include/libcamera/controls.h | 15 ++++++++-------\n>>>> src/apps/cam/capture_script.cpp | 2 +-\n>>>> src/apps/cam/main.cpp | 2 +-\n>>>> src/apps/common/dng_writer.cpp | 2 +-\n>>>> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n>>>> src/py/libcamera/py_helpers.cpp | 4 ++--\n>>>> test/controls/control_value.cpp | 4 ++--\n>>>> utils/codegen/controls.py | 2 +-\n>>>> 9 files changed, 20 insertions(+), 18 deletions(-)\n>>>>\n>>>> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n>>>> index 5d0594c68..41997f79f 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>>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>>>> index 2ae4ec3d4..5a707a387 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>>>> @@ -96,7 +97,7 @@ struct control_type {\n>>>> };\n>>>> \n>>>> template<>\n>>>> -struct control_type {\n>>>> +struct control_type {\n>>>> \tstatic constexpr ControlType value = ControlTypeString;\n>>>> \tstatic constexpr std::size_t size = 0;\n>>>> };\n>>>> @@ -138,7 +139,7 @@ public:\n>>>> #ifndef __DOXYGEN__\n>>>> \ttemplate::value &&\n>>>> \t\t\t\t\t details::control_type::value &&\n>>>> -\t\t\t\t\t !std::is_same>::value,\n>>>> +\t\t\t\t\t !std::is_same>::value,\n>>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>>>> \tControlValue(const T &value)\n>>>> \t\t: type_(ControlTypeNone), numElements_(0)\n>>>> @@ -148,7 +149,7 @@ public:\n>>>> \t}\n>>>> \n>>>> \ttemplate::value ||\n>>>> -\t\t\t\t\t std::is_same>::value,\n>>>> +\t\t\t\t\t std::is_same>::value,\n>>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>>>> #else\n>>>> \ttemplate\n>>>> @@ -182,7 +183,7 @@ public:\n>>>> \n>>>> #ifndef __DOXYGEN__\n>>>> \ttemplate::value &&\n>>>> -\t\t\t\t\t !std::is_same>::value,\n>>>> +\t\t\t\t\t !std::is_same>::value,\n>>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>>>> \tT get() const\n>>>> \t{\n>>>> @@ -193,7 +194,7 @@ public:\n>>>> \t}\n>>>> \n>>>> \ttemplate::value ||\n>>>> -\t\t\t\t\t std::is_same>::value,\n>>>> +\t\t\t\t\t std::is_same>::value,\n>>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>>>> #else\n>>>> \ttemplate\n>>>> @@ -210,7 +211,7 @@ public:\n>>>> \n>>>> #ifndef __DOXYGEN__\n>>>> \ttemplate::value &&\n>>>> -\t\t\t\t\t !std::is_same>::value,\n>>>> +\t\t\t\t\t !std::is_same>::value,\n>>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>>>> \tvoid set(const T &value)\n>>>> \t{\n>>>> @@ -219,7 +220,7 @@ public:\n>>>> \t}\n>>>> \n>>>> \ttemplate::value ||\n>>>> -\t\t\t\t\t std::is_same>::value,\n>>>> +\t\t\t\t\t std::is_same>::value,\n>>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n>>>> #else\n>>>> \ttemplate\n>>>> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n>>>> index fdf82efc0..bc4c7c3a0 100644\n>>>> --- a/src/apps/cam/capture_script.cpp\n>>>> +++ b/src/apps/cam/capture_script.cpp\n>>>> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n>>>> \t\tbreak;\n>>>> \t}\n>>>> \tcase ControlTypeString: {\n>>>> -\t\tvalue.set(repr);\n>>>> +\t\tvalue.set(repr);\n>>>> \t\tbreak;\n>>>> \t}\n>>>> \tdefault:\n>>>> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n>>>> index fa266eca6..157d238d7 100644\n>>>> --- a/src/apps/cam/main.cpp\n>>>> +++ b/src/apps/cam/main.cpp\n>>>> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n>>>> \t\t */\n>>>> \t\tconst auto &model = props.get(properties::Model);\n>>>> \t\tif (model)\n>>>> -\t\t\tname = \"'\" + *model + \"' \";\n>>>> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n>>>> \t}\n>>>> \n>>>> \tname += \"(\" + camera->id() + \")\";\n>>>> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n>>>> index ac4619511..8d57023e1 100644\n>>>> --- a/src/apps/common/dng_writer.cpp\n>>>> +++ b/src/apps/common/dng_writer.cpp\n>>>> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>>> \n>>>> \tconst auto &model = cameraProperties.get(properties::Model);\n>>>> \tif (model) {\n>>>> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n>>>> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n>>>\n>>> Slightly annoying, but that's an issue with the TIFF library API, it's\n>>> not our fault.\n>>>\n>>>> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n>>>> \t}\n>>>> \n>>>> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n>>>> index 6b6d0713c..9f25ba091 100644\n>>>> --- a/src/apps/qcam/cam_select_dialog.cpp\n>>>> +++ b/src/apps/qcam/cam_select_dialog.cpp\n>>>> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n>>>> \t\tcameraLocation_->setText(\"Unknown\");\n>>>> \t}\n>>>> \n>>>> -\tconst auto &model = properties.get(libcamera::properties::Model)\n>>>> -\t\t\t\t .value_or(\"Unknown\");\n>>>> +\tconst auto model = properties.get(libcamera::properties::Model)\n>>>> +\t\t\t\t .value_or(\"Unknown\");\n>>>\n>>> I was concerned that this could result in a use-after-free if the\n>>> argument to value_or() is a temporary std::string, but I think the\n>>\n>> I am afraid that's exactly what would happen, an `std::string_view` is\n>> returned to the temporary `std::string`. So that would be a use-after-free\n>> in some cases.\n> \n> valgrind didn't catch any issue. Isn't the lifetime of temporary objects\n> extended in this case ?\n\nThe return type of this `value_or()` call will be `std::string_view`,\nthe underlying temporary object passed as argument will be destroyed.\n\nhttps://gcc.godbolt.org/z/Mb3qKT1jK\n\nThis issue is generally present for array like controls:\n\n auto x = request->metadata().get(controls::ColourGains).value_or(std::vector { 1.f, 1.f });\n x[0] // UAF if `controls::ColourGains` is not in metadata\n\nor similar.\n\n> \n>>> lifetime of the temporary will be extended to cover the lifetime of\n>>> model.\n>>>\n>>> Reviewed-by: Laurent Pinchart \n>>>\n>>>> \n>>>> -\tcameraModel_->setText(QString::fromStdString(model));\n>>>> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n>>>> }\n>>>> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n>>>> index 1ad1d4c1a..8c55ef845 100644\n>>>> --- a/src/py/libcamera/py_helpers.cpp\n>>>> +++ b/src/py/libcamera/py_helpers.cpp\n>>>> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n>>>> \tcase ControlTypeFloat:\n>>>> \t\treturn valueOrTuple(cv);\n>>>> \tcase ControlTypeString:\n>>>> -\t\treturn py::cast(cv.get());\n>>>> +\t\treturn py::cast(cv.get());\n>>>> \tcase ControlTypeSize: {\n>>>> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n>>>> \t\treturn py::cast(v);\n>>>> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n>>>> \tcase ControlTypeFloat:\n>>>> \t\treturn controlValueMaybeArray(ob);\n>>>> \tcase ControlTypeString:\n>>>> -\t\treturn ControlValue(ob.cast());\n>>>> +\t\treturn ControlValue(ob.cast());\n>>>> \tcase ControlTypeRectangle:\n>>>> \t\treturn controlValueMaybeArray(ob);\n>>>> \tcase ControlTypeSize:\n>>>> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n>>>> index 5084fd0cf..032050a77 100644\n>>>> --- a/test/controls/control_value.cpp\n>>>> +++ b/test/controls/control_value.cpp\n>>>> @@ -318,7 +318,7 @@ protected:\n>>>> \t\t/*\n>>>> \t\t * String type.\n>>>> \t\t */\n>>>> -\t\tstd::string string{ \"libcamera\" };\n>>>> +\t\tstd::string_view string{ \"libcamera\" };\n>>>> \t\tvalue.set(string);\n>>>> \t\tif (value.isNone() || !value.isArray() ||\n>>>> \t\t value.type() != ControlTypeString ||\n>>>> @@ -327,7 +327,7 @@ protected:\n>>>> \t\t\treturn TestFail;\n>>>> \t\t}\n>>>> \n>>>> -\t\tif (value.get() != string) {\n>>>> +\t\tif (value.get() != string) {\n>>>> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n>>>> \t\t\treturn TestFail;\n>>>> \t\t}\n>>>> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n>>>> index e51610481..9399727bd 100644\n>>>> --- a/utils/codegen/controls.py\n>>>> +++ b/utils/codegen/controls.py\n>>>> @@ -111,7 +111,7 @@ class Control(object):\n>>>> size = self.__data.get('size')\n>>>> \n>>>> if typ == 'string':\n>>>> - return 'std::string'\n>>>> + return 'std::string_view'\n>>>> \n>>>> if self.__size is None:\n>>>> return typ\n>","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 4809DC324C\n\tfor ;\n\tFri, 3 Oct 2025 17:08:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5CE876B5F3;\n\tFri, 3 Oct 2025 19:08:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A2A069318\n\tfor ;\n\tFri, 3 Oct 2025 19:08:38 +0200 (CEST)","from [192.168.33.17] (185.182.214.142.nat.pool.zt.hu\n\t[185.182.214.142])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8E81B1F25;\n\tFri, 3 Oct 2025 19:07:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QZr0ocOO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759511227;\n\tbh=L7MZrUx+LrQuI0cfUtof3vI6MHtKY4nl9J9d9vL8ecw=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=QZr0ocOOqa7mAV+2xX5vb4AeSNjFtyM+Cic/2EmdpzlRzbeEES/7l85Hf3JS/TeeY\n\tGb3AfYJVOeLVfKS1yhk60lQ6rcVkAd3qO6EYoSd9YCXMqy/YusNJ6jNYnkIBpU/my6\n\t6HrmpJ1QSDR7JouLUrkXi4+BMfWrXcifaBMhyD9k=","Message-ID":"<73167cdc-f279-47a0-8f75-ac8dc7441589@ideasonboard.com>","Date":"Fri, 3 Oct 2025 19:08:35 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t<20251002182700.GF10198@pendragon.ideasonboard.com>\n\t\n\t<20251003165818.GA20317@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251003165818.GA20317@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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\" "}},{"id":36122,"web_url":"https://patchwork.libcamera.org/comment/36122/","msgid":"<20251003223625.GB20317@pendragon.ideasonboard.com>","date":"2025-10-03T22:36:25","subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Oct 03, 2025 at 07:08:35PM +0200, Barnabás Pőcze wrote:\n> 2025. 10. 03. 18:58 keltezéssel, Laurent Pinchart írta:\n> > On Fri, Oct 03, 2025 at 03:30:21PM +0200, Barnabás Pőcze wrote:\n> >> 2025. 10. 02. 20:27 keltezéssel, Laurent Pinchart írta:\n> >>> On Thu, May 01, 2025 at 11:58:18AM +0200, Barnabás Pőcze wrote:\n> >>>> When retrieving the value from a `ControlValue` usually one of two\n> >>>> things happen: a small, trivially copyable object is returned by\n> >>>> value; or a view into the internal buffer is provided. This is true\n> >>>> for everything except strings, which are returned in `std::string`,\n> >>>> incurring the overhead of string construction.\n> >>>>\n> >>>> To guarantee no potentially \"expensive\" copies, use `std::string_view`\n> >>>> pointing to the internal buffer to return the value. This is similar\n> >>>> to how other array-like types are returned with a `Span<>`.\n> >>>>\n> >>>> This is an API break, but its scope is limited to just `properties::Model`.\n> >>>>\n> >>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=256\n> >>>> Signed-off-by: Barnabás Pőcze \n> >>>> ---\n> >>>> include/libcamera/control_ids.h.in | 1 +\n> >>>> include/libcamera/controls.h | 15 ++++++++-------\n> >>>> src/apps/cam/capture_script.cpp | 2 +-\n> >>>> src/apps/cam/main.cpp | 2 +-\n> >>>> src/apps/common/dng_writer.cpp | 2 +-\n> >>>> src/apps/qcam/cam_select_dialog.cpp | 6 +++---\n> >>>> src/py/libcamera/py_helpers.cpp | 4 ++--\n> >>>> test/controls/control_value.cpp | 4 ++--\n> >>>> utils/codegen/controls.py | 2 +-\n> >>>> 9 files changed, 20 insertions(+), 18 deletions(-)\n> >>>>\n> >>>> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> >>>> index 5d0594c68..41997f79f 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> >>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> >>>> index 2ae4ec3d4..5a707a387 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> >>>> @@ -96,7 +97,7 @@ struct control_type {\n> >>>> };\n> >>>> \n> >>>> template<>\n> >>>> -struct control_type {\n> >>>> +struct control_type {\n> >>>> \tstatic constexpr ControlType value = ControlTypeString;\n> >>>> \tstatic constexpr std::size_t size = 0;\n> >>>> };\n> >>>> @@ -138,7 +139,7 @@ public:\n> >>>> #ifndef __DOXYGEN__\n> >>>> \ttemplate::value &&\n> >>>> \t\t\t\t\t details::control_type::value &&\n> >>>> -\t\t\t\t\t !std::is_same>::value,\n> >>>> +\t\t\t\t\t !std::is_same>::value,\n> >>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >>>> \tControlValue(const T &value)\n> >>>> \t\t: type_(ControlTypeNone), numElements_(0)\n> >>>> @@ -148,7 +149,7 @@ public:\n> >>>> \t}\n> >>>> \n> >>>> \ttemplate::value ||\n> >>>> -\t\t\t\t\t std::is_same>::value,\n> >>>> +\t\t\t\t\t std::is_same>::value,\n> >>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >>>> #else\n> >>>> \ttemplate\n> >>>> @@ -182,7 +183,7 @@ public:\n> >>>> \n> >>>> #ifndef __DOXYGEN__\n> >>>> \ttemplate::value &&\n> >>>> -\t\t\t\t\t !std::is_same>::value,\n> >>>> +\t\t\t\t\t !std::is_same>::value,\n> >>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >>>> \tT get() const\n> >>>> \t{\n> >>>> @@ -193,7 +194,7 @@ public:\n> >>>> \t}\n> >>>> \n> >>>> \ttemplate::value ||\n> >>>> -\t\t\t\t\t std::is_same>::value,\n> >>>> +\t\t\t\t\t std::is_same>::value,\n> >>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >>>> #else\n> >>>> \ttemplate\n> >>>> @@ -210,7 +211,7 @@ public:\n> >>>> \n> >>>> #ifndef __DOXYGEN__\n> >>>> \ttemplate::value &&\n> >>>> -\t\t\t\t\t !std::is_same>::value,\n> >>>> +\t\t\t\t\t !std::is_same>::value,\n> >>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >>>> \tvoid set(const T &value)\n> >>>> \t{\n> >>>> @@ -219,7 +220,7 @@ public:\n> >>>> \t}\n> >>>> \n> >>>> \ttemplate::value ||\n> >>>> -\t\t\t\t\t std::is_same>::value,\n> >>>> +\t\t\t\t\t std::is_same>::value,\n> >>>> \t\t\t\t\t std::nullptr_t> = nullptr>\n> >>>> #else\n> >>>> \ttemplate\n> >>>> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> >>>> index fdf82efc0..bc4c7c3a0 100644\n> >>>> --- a/src/apps/cam/capture_script.cpp\n> >>>> +++ b/src/apps/cam/capture_script.cpp\n> >>>> @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id,\n> >>>> \t\tbreak;\n> >>>> \t}\n> >>>> \tcase ControlTypeString: {\n> >>>> -\t\tvalue.set(repr);\n> >>>> +\t\tvalue.set(repr);\n> >>>> \t\tbreak;\n> >>>> \t}\n> >>>> \tdefault:\n> >>>> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> >>>> index fa266eca6..157d238d7 100644\n> >>>> --- a/src/apps/cam/main.cpp\n> >>>> +++ b/src/apps/cam/main.cpp\n> >>>> @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> >>>> \t\t */\n> >>>> \t\tconst auto &model = props.get(properties::Model);\n> >>>> \t\tif (model)\n> >>>> -\t\t\tname = \"'\" + *model + \"' \";\n> >>>> +\t\t\tname.append(\"'\").append(*model).append(\"' \");\n> >>>> \t}\n> >>>> \n> >>>> \tname += \"(\" + camera->id() + \")\";\n> >>>> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> >>>> index ac4619511..8d57023e1 100644\n> >>>> --- a/src/apps/common/dng_writer.cpp\n> >>>> +++ b/src/apps/common/dng_writer.cpp\n> >>>> @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >>>> \n> >>>> \tconst auto &model = cameraProperties.get(properties::Model);\n> >>>> \tif (model) {\n> >>>> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> >>>> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str());\n> >>>\n> >>> Slightly annoying, but that's an issue with the TIFF library API, it's\n> >>> not our fault.\n> >>>\n> >>>> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> >>>> \t}\n> >>>> \n> >>>> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp\n> >>>> index 6b6d0713c..9f25ba091 100644\n> >>>> --- a/src/apps/qcam/cam_select_dialog.cpp\n> >>>> +++ b/src/apps/qcam/cam_select_dialog.cpp\n> >>>> @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId)\n> >>>> \t\tcameraLocation_->setText(\"Unknown\");\n> >>>> \t}\n> >>>> \n> >>>> -\tconst auto &model = properties.get(libcamera::properties::Model)\n> >>>> -\t\t\t\t .value_or(\"Unknown\");\n> >>>> +\tconst auto model = properties.get(libcamera::properties::Model)\n> >>>> +\t\t\t\t .value_or(\"Unknown\");\n> >>>\n> >>> I was concerned that this could result in a use-after-free if the\n> >>> argument to value_or() is a temporary std::string, but I think the\n> >>\n> >> I am afraid that's exactly what would happen, an `std::string_view` is\n> >> returned to the temporary `std::string`. So that would be a use-after-free\n> >> in some cases.\n> > \n> > valgrind didn't catch any issue. Isn't the lifetime of temporary objects\n> > extended in this case ?\n> \n> The return type of this `value_or()` call will be `std::string_view`,\n> the underlying temporary object passed as argument will be destroyed.\n> \n> https://gcc.godbolt.org/z/Mb3qKT1jK\n> \n> This issue is generally present for array like controls:\n> \n> auto x = request->metadata().get(controls::ColourGains).value_or(std::vector { 1.f, 1.f });\n> x[0] // UAF if `controls::ColourGains` is not in metadata\n> \n> or similar.\n\nYou're right, I somehow extended the lifetime extension rules of\nrange-based for loops to all expressions :-/\n\nIt's not a new issues so I don't think it needs to be solved right now,\nbut it's something to consider for future revisions of this API.\n\n> >>> lifetime of the temporary will be extended to cover the lifetime of\n> >>> model.\n> >>>\n> >>> Reviewed-by: Laurent Pinchart \n> >>>\n> >>>> \n> >>>> -\tcameraModel_->setText(QString::fromStdString(model));\n> >>>> +\tcameraModel_->setText(QString::fromUtf8(model.data(), model.length()));\n> >>>> }\n> >>>> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp\n> >>>> index 1ad1d4c1a..8c55ef845 100644\n> >>>> --- a/src/py/libcamera/py_helpers.cpp\n> >>>> +++ b/src/py/libcamera/py_helpers.cpp\n> >>>> @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv)\n> >>>> \tcase ControlTypeFloat:\n> >>>> \t\treturn valueOrTuple(cv);\n> >>>> \tcase ControlTypeString:\n> >>>> -\t\treturn py::cast(cv.get());\n> >>>> +\t\treturn py::cast(cv.get());\n> >>>> \tcase ControlTypeSize: {\n> >>>> \t\tconst Size *v = reinterpret_cast(cv.data().data());\n> >>>> \t\treturn py::cast(v);\n> >>>> @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> >>>> \tcase ControlTypeFloat:\n> >>>> \t\treturn controlValueMaybeArray(ob);\n> >>>> \tcase ControlTypeString:\n> >>>> -\t\treturn ControlValue(ob.cast());\n> >>>> +\t\treturn ControlValue(ob.cast());\n> >>>> \tcase ControlTypeRectangle:\n> >>>> \t\treturn controlValueMaybeArray(ob);\n> >>>> \tcase ControlTypeSize:\n> >>>> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> >>>> index 5084fd0cf..032050a77 100644\n> >>>> --- a/test/controls/control_value.cpp\n> >>>> +++ b/test/controls/control_value.cpp\n> >>>> @@ -318,7 +318,7 @@ protected:\n> >>>> \t\t/*\n> >>>> \t\t * String type.\n> >>>> \t\t */\n> >>>> -\t\tstd::string string{ \"libcamera\" };\n> >>>> +\t\tstd::string_view string{ \"libcamera\" };\n> >>>> \t\tvalue.set(string);\n> >>>> \t\tif (value.isNone() || !value.isArray() ||\n> >>>> \t\t value.type() != ControlTypeString ||\n> >>>> @@ -327,7 +327,7 @@ protected:\n> >>>> \t\t\treturn TestFail;\n> >>>> \t\t}\n> >>>> \n> >>>> -\t\tif (value.get() != string) {\n> >>>> +\t\tif (value.get() != string) {\n> >>>> \t\t\tcerr << \"Control value mismatch after setting to string\" << endl;\n> >>>> \t\t\treturn TestFail;\n> >>>> \t\t}\n> >>>> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> >>>> index e51610481..9399727bd 100644\n> >>>> --- a/utils/codegen/controls.py\n> >>>> +++ b/utils/codegen/controls.py\n> >>>> @@ -111,7 +111,7 @@ class Control(object):\n> >>>> size = self.__data.get('size')\n> >>>> \n> >>>> if typ == 'string':\n> >>>> - return 'std::string'\n> >>>> + return 'std::string_view'\n> >>>> \n> >>>> if self.__size is None:\n> >>>> return typ","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 E0181BF415\n\tfor ;\n\tFri, 3 Oct 2025 22:36:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C60626B5F3;\n\tSat, 4 Oct 2025 00:36:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F56E6B5A2\n\tfor ;\n\tSat, 4 Oct 2025 00:36:32 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id A2174C6E;\n\tSat, 4 Oct 2025 00:35:01 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uGX63vit\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759530901;\n\tbh=0NzonVm+usNejqU0VS7o0ruOo6/4dqRvZSfJ0l3X2hc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uGX63vit8Sgh3BG+bhFI3hXeTbNMhwXJ9tCynrWUa/rD9uoc6meDNl06+oUwOdCcp\n\tQZm/GGDrxsdlOzy6V+TotpnNe5AYBHiEB+0g3OzJAGhgKqSevKem2688ZGHHCNVTDs\n\tZf+1Jx7cauCpVemLqWNG7SjqBQ2N3LxUmTeldxW4=","Date":"Sat, 4 Oct 2025 01:36:25 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] libcamera: controls: Expose string controls as\n\t`std::string_view`","Message-ID":"<20251003223625.GB20317@pendragon.ideasonboard.com>","References":"<20250501095818.3996419-1-barnabas.pocze@ideasonboard.com>\n\t<20250501095818.3996419-4-barnabas.pocze@ideasonboard.com>\n\t<20251002182700.GF10198@pendragon.ideasonboard.com>\n\t\n\t<20251003165818.GA20317@pendragon.ideasonboard.com>\n\t<73167cdc-f279-47a0-8f75-ac8dc7441589@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<73167cdc-f279-47a0-8f75-ac8dc7441589@ideasonboard.com>","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\" "}}]