From patchwork Tue Apr 22 12:47:50 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23205 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 99772BE08B for ; Tue, 22 Apr 2025 12:48:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7DCB468AD0; Tue, 22 Apr 2025 14:47:59 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vMEsurTH"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F64E617E6 for ; Tue, 22 Apr 2025 14:47:56 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C529666 for ; Tue, 22 Apr 2025 14:45:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745325948; bh=q0h9SLaAylBdoyOGnrcJVJvmydinzayn/dBVekx7lwo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=vMEsurTHVU2Ba4old+Z69tK8DfPbBnCYtWmWfUYq4Yqs95Qj2jQwMRBgAUvHYTSsp SgS+6HLl0/5EMWtnjhTRRdFGEL8/IslyFbsHHTgAomJ+2ZeLmDIG7k2AHFrq4sWo5T gQHX9Ud7/iPnqNVcR1XFSiUOPYx7uhO1kKmTXScA= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v1 1/2] apps: cam: capture_script: Disallow arrays of strings Date: Tue, 22 Apr 2025 14:47:50 +0200 Message-ID: <20250422124751.30409-2-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250422124751.30409-1-barnabas.pocze@ideasonboard.com> References: <20250422124751.30409-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The current `ControlValue` mechanism does not support arrays of strings, the assignment in the removed snippet will in fact trigger an assertion failure in `ControlValue::set()` because `sizeof(std::string) != ControlValueSize[ControlTypeString]`. Fixes: b35f04b3c194 ("cam: capture_script: Support parsing array controls") Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham --- src/apps/cam/capture_script.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp index e7e69960e..fdf82efc0 100644 --- a/src/apps/cam/capture_script.cpp +++ b/src/apps/cam/capture_script.cpp @@ -578,10 +578,6 @@ ControlValue CaptureScript::parseArrayControl(const ControlId *id, value = Span(values.data(), values.size()); break; } - case ControlTypeString: { - value = Span(repr.data(), repr.size()); - break; - } default: std::cerr << "Unsupported control type" << std::endl; break; From patchwork Tue Apr 22 12:47:51 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23206 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id F383DC32A2 for ; Tue, 22 Apr 2025 12:48:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3E14868ACE; Tue, 22 Apr 2025 14:48:00 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="SFmu9yHX"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 616A468AC5 for ; Tue, 22 Apr 2025 14:47:56 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DF950B2B for ; Tue, 22 Apr 2025 14:45:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745325949; bh=LgCQkHk/eM9ZLIiAtLYBdLPd5RvD5ABCAxHzrWxXu0w=; h=From:To:Subject:Date:In-Reply-To:References:From; b=SFmu9yHXHTzGG1E+4RBP1+9uB6+lTTAy6FDRSuY49nN3fmzcRu4gAa9ESsKfRQIYD o0pbkNXN/HBlSbw2c4MXpdHxlDIs5sJKK1gxAq5d2OY0DoN35jJwNV/8jw61Ea+Dwe 7jfjRyVlbhtcsdKi2dNV7mN6rhFLVGVex5rz6op4= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v1 2/2] libcamera: controls: Expose string controls as `std::string_view` Date: Tue, 22 Apr 2025 14:47:51 +0200 Message-ID: <20250422124751.30409-3-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250422124751.30409-1-barnabas.pocze@ideasonboard.com> References: <20250422124751.30409-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When retrieving the value from a `ControlValue` usually one of two things happen: a small, trivially copyable object is returned by value; or a view into the internal buffer is provided. This is true for everything except strings, which are returned in `std::string`, incurring the overhead of string construction. To guarantee no potentially "expensive" copies, use `std::string_view` pointing to the internal buffer to return the value. This is similar to how other array-like types are returned with a `Span<>`. This is an API break, but its scope is limited to just `properties::Model`. Bug: https://bugs.libcamera.org/show_bug.cgi?id=256 Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham --- include/libcamera/control_ids.h.in | 1 + include/libcamera/controls.h | 15 ++++++++------- src/apps/cam/capture_script.cpp | 2 +- src/apps/cam/main.cpp | 2 +- src/apps/common/dng_writer.cpp | 2 +- src/apps/qcam/cam_select_dialog.cpp | 6 +++--- src/py/libcamera/py_helpers.cpp | 4 ++-- test/controls/control_value.cpp | 4 ++-- utils/codegen/controls.py | 2 +- 9 files changed, 20 insertions(+), 18 deletions(-) diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in index 5d0594c68..41997f79f 100644 --- a/include/libcamera/control_ids.h.in +++ b/include/libcamera/control_ids.h.in @@ -13,6 +13,7 @@ #include #include #include +#include #include diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 4bfe9615c..275f45200 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -96,7 +97,7 @@ struct control_type { }; template<> -struct control_type { +struct control_type { static constexpr ControlType value = ControlTypeString; static constexpr std::size_t size = 0; }; @@ -138,7 +139,7 @@ public: #ifndef __DOXYGEN__ template::value && details::control_type::value && - !std::is_same>::value, + !std::is_same>::value, std::nullptr_t> = nullptr> ControlValue(const T &value) : type_(ControlTypeNone), numElements_(0) @@ -148,7 +149,7 @@ public: } template::value || - std::is_same>::value, + std::is_same>::value, std::nullptr_t> = nullptr> #else template @@ -182,7 +183,7 @@ public: #ifndef __DOXYGEN__ template::value && - !std::is_same>::value, + !std::is_same>::value, std::nullptr_t> = nullptr> T get() const { @@ -193,7 +194,7 @@ public: } template::value || - std::is_same>::value, + std::is_same>::value, std::nullptr_t> = nullptr> #else template @@ -210,7 +211,7 @@ public: #ifndef __DOXYGEN__ template::value && - !std::is_same>::value, + !std::is_same>::value, std::nullptr_t> = nullptr> void set(const T &value) { @@ -219,7 +220,7 @@ public: } template::value || - std::is_same>::value, + std::is_same>::value, std::nullptr_t> = nullptr> #else template diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp index fdf82efc0..bc4c7c3a0 100644 --- a/src/apps/cam/capture_script.cpp +++ b/src/apps/cam/capture_script.cpp @@ -502,7 +502,7 @@ ControlValue CaptureScript::parseScalarControl(const ControlId *id, break; } case ControlTypeString: { - value.set(repr); + value.set(repr); break; } default: diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp index fa266eca6..157d238d7 100644 --- a/src/apps/cam/main.cpp +++ b/src/apps/cam/main.cpp @@ -337,7 +337,7 @@ std::string CamApp::cameraName(const Camera *camera) */ const auto &model = props.get(properties::Model); if (model) - name = "'" + *model + "' "; + name.append("'").append(*model).append("' "); } name += "(" + camera->id() + ")"; diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp index ac4619511..8d57023e1 100644 --- a/src/apps/common/dng_writer.cpp +++ b/src/apps/common/dng_writer.cpp @@ -564,7 +564,7 @@ int DNGWriter::write(const char *filename, const Camera *camera, const auto &model = cameraProperties.get(properties::Model); if (model) { - TIFFSetField(tif, TIFFTAG_MODEL, model->c_str()); + TIFFSetField(tif, TIFFTAG_MODEL, std::string(*model).c_str()); /* \todo set TIFFTAG_UNIQUECAMERAMODEL. */ } diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp index 6b6d0713c..9f25ba091 100644 --- a/src/apps/qcam/cam_select_dialog.cpp +++ b/src/apps/qcam/cam_select_dialog.cpp @@ -114,8 +114,8 @@ void CameraSelectorDialog::updateCameraInfo(QString cameraId) cameraLocation_->setText("Unknown"); } - const auto &model = properties.get(libcamera::properties::Model) - .value_or("Unknown"); + const auto model = properties.get(libcamera::properties::Model) + .value_or("Unknown"); - cameraModel_->setText(QString::fromStdString(model)); + cameraModel_->setText(QString::fromUtf8(model.data(), model.length())); } diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp index 1ad1d4c1a..8c55ef845 100644 --- a/src/py/libcamera/py_helpers.cpp +++ b/src/py/libcamera/py_helpers.cpp @@ -47,7 +47,7 @@ py::object controlValueToPy(const ControlValue &cv) case ControlTypeFloat: return valueOrTuple(cv); case ControlTypeString: - return py::cast(cv.get()); + return py::cast(cv.get()); case ControlTypeSize: { const Size *v = reinterpret_cast(cv.data().data()); return py::cast(v); @@ -88,7 +88,7 @@ ControlValue pyToControlValue(const py::object &ob, ControlType type) case ControlTypeFloat: return controlValueMaybeArray(ob); case ControlTypeString: - return ControlValue(ob.cast()); + return ControlValue(ob.cast()); case ControlTypeRectangle: return controlValueMaybeArray(ob); case ControlTypeSize: diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp index 5084fd0cf..032050a77 100644 --- a/test/controls/control_value.cpp +++ b/test/controls/control_value.cpp @@ -318,7 +318,7 @@ protected: /* * String type. */ - std::string string{ "libcamera" }; + std::string_view string{ "libcamera" }; value.set(string); if (value.isNone() || !value.isArray() || value.type() != ControlTypeString || @@ -327,7 +327,7 @@ protected: return TestFail; } - if (value.get() != string) { + if (value.get() != string) { cerr << "Control value mismatch after setting to string" << endl; return TestFail; } diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py index e51610481..9399727bd 100644 --- a/utils/codegen/controls.py +++ b/utils/codegen/controls.py @@ -111,7 +111,7 @@ class Control(object): size = self.__data.get('size') if typ == 'string': - return 'std::string' + return 'std::string_view' if self.__size is None: return typ