[{"id":35137,"web_url":"https://patchwork.libcamera.org/comment/35137/","msgid":"<6z6linckfhzco5ntwmulglgtrktiemtyozzirmjrivd472at5q@kymf6c3moxgj>","date":"2025-07-25T12:55:16","subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","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 Mon, Jul 21, 2025 at 12:46:02PM +0200, Barnabás Pőcze wrote:\n> Add `ControlValueView`, which is a non-owning read-only handle to a control\n> value with a very similar interface.\n>\n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> ---\n> changes in v2:\n>   * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)\n>     to avoid duplication\n> ---\n>  include/libcamera/controls.h |  68 +++++++++\n>  src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------\n>  2 files changed, 263 insertions(+), 78 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index b170e30cb..259fc913b 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -8,6 +8,7 @@\n>  #pragma once\n>\n>  #include <assert.h>\n> +#include <cstddef>\n>  #include <map>\n>  #include <optional>\n>  #include <set>\n> @@ -25,6 +26,7 @@\n>  namespace libcamera {\n>\n>  class ControlValidator;\n> +class ControlValueView;\n>\n>  enum ControlType {\n>  \tControlTypeNone,\n> @@ -160,6 +162,8 @@ public:\n>  \t\t    value.data(), value.size(), sizeof(typename T::value_type));\n>  \t}\n>\n> +\texplicit ControlValue(const ControlValueView &cvv);\n> +\n>  \t~ControlValue();\n>\n>  \tControlValue(const ControlValue &other);\n> @@ -247,6 +251,70 @@ private:\n>  \t\t std::size_t numElements, std::size_t elementSize);\n>  };\n>\n> +class ControlValueView\n> +{\n> +public:\n> +\tconstexpr ControlValueView() noexcept\n> +\t\t: type_(ControlTypeNone)\n> +\t{\n> +\t}\n> +\n> +\tControlValueView(const ControlValue &cv) noexcept\n> +\t\t: ControlValueView(cv.type(), cv.isArray(), cv.numElements(),\n> +\t\t\t\t   reinterpret_cast<const std::byte *>(cv.data().data()))\n> +\t{\n> +\t}\n> +\n> +#ifndef __DOXYGEN__\n> +\t// TODO: should have restricted access?\n> +\tControlValueView(ControlType type, bool isArray, std::size_t numElements,\n> +\t\t\t const std::byte *data) noexcept\n> +\t\t: type_(type), isArray_(isArray), numElements_(numElements),\n> +\t\t  data_(data)\n\nI'm still a little concerned about simply copying the data address\nhere. If the ControlValue this view has been constructed with goes\naway before its view, we would access invalid memory.\n\nAm I too concerned ?\n\n\n> +\t{\n> +\t\tassert(isArray || numElements == 1);\n> +\t}\n> +#endif\n> +\n> +\t[[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }\n> +\t[[nodiscard]] ControlType type() const { return type_; }\n> +\t[[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }\n> +\t[[nodiscard]] bool isArray() const { return isArray_; }\n> +\t[[nodiscard]] std::size_t numElements() const { return numElements_; }\n> +\t[[nodiscard]] Span<const std::byte> data() const;\n> +\n> +\t[[nodiscard]] bool operator==(const ControlValueView &other) const;\n> +\n> +\t[[nodiscard]] bool operator!=(const ControlValueView &other) const\n> +\t{\n> +\t\treturn !(*this == other);\n> +\t}\n> +\n> +\ttemplate<typename T>\n> +\t[[nodiscard]] auto get() const\n> +\t{\n> +\t\tusing TypeInfo = details::control_type<std::remove_cv_t<T>>;\n> +\n> +\t\tassert(type_ == TypeInfo::value);\n> +\t\tassert(isArray_ == (TypeInfo::size > 0));\n> +\n> +\t\tif constexpr (TypeInfo::size > 0) {\n> +\t\t\treturn T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);\n> +\t\t} else {\n> +\t\t\tassert(numElements_ == 1);\n> +\t\t\treturn *reinterpret_cast<const T *>(data().data());\n> +\t\t}\n> +\t}\n> +\n> +private:\n> +\tControlType type_ : 8;\n> +\tbool isArray_ = false;\n> +\tuint32_t numElements_ = 0;\n> +\tconst std::byte *data_ = nullptr;\n> +};\n> +\n> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v);\n> +\n>  class ControlId\n>  {\n>  public:\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 98fa7583d..a238141a5 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -106,6 +106,16 @@ ControlValue::ControlValue()\n>  {\n>  }\n>\n> +/**\n> + * \\brief Construct a ControlValue from a ControlValueView\n> + */\n> +ControlValue::ControlValue(const ControlValueView &cvv)\n> +\t: ControlValue()\n> +{\n> +\tset(cvv.type(), cvv.isArray(), cvv.data().data(),\n> +\t    cvv.numElements(), ControlValueSize[cvv.type()]);\n> +}\n> +\n>  /**\n>   * \\fn template<typename T> T ControlValue::ControlValue(const T &value)\n>   * \\brief Construct a ControlValue of type T\n> @@ -213,84 +223,7 @@ Span<uint8_t> ControlValue::data()\n>   */\n>  std::string ControlValue::toString() const\n>  {\n> -\tif (type_ == ControlTypeNone)\n> -\t\treturn \"<ValueType Error>\";\n> -\n> -\tconst uint8_t *data = ControlValue::data().data();\n> -\n> -\tif (type_ == ControlTypeString)\n> -\t\treturn std::string(reinterpret_cast<const char *>(data),\n> -\t\t\t\t   numElements_);\n> -\n> -\tstd::string str(isArray_ ? \"[ \" : \"\");\n> -\n> -\tfor (unsigned int i = 0; i < numElements_; ++i) {\n> -\t\tswitch (type_) {\n> -\t\tcase ControlTypeBool: {\n> -\t\t\tconst bool *value = reinterpret_cast<const bool *>(data);\n> -\t\t\tstr += *value ? \"true\" : \"false\";\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeByte: {\n> -\t\t\tconst uint8_t *value = reinterpret_cast<const uint8_t *>(data);\n> -\t\t\tstr += std::to_string(*value);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeUnsigned16: {\n> -\t\t\tconst uint16_t *value = reinterpret_cast<const uint16_t *>(data);\n> -\t\t\tstr += std::to_string(*value);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeUnsigned32: {\n> -\t\t\tconst uint32_t *value = reinterpret_cast<const uint32_t *>(data);\n> -\t\t\tstr += std::to_string(*value);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeInteger32: {\n> -\t\t\tconst int32_t *value = reinterpret_cast<const int32_t *>(data);\n> -\t\t\tstr += std::to_string(*value);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeInteger64: {\n> -\t\t\tconst int64_t *value = reinterpret_cast<const int64_t *>(data);\n> -\t\t\tstr += std::to_string(*value);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeFloat: {\n> -\t\t\tconst float *value = reinterpret_cast<const float *>(data);\n> -\t\t\tstr += std::to_string(*value);\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeRectangle: {\n> -\t\t\tconst Rectangle *value = reinterpret_cast<const Rectangle *>(data);\n> -\t\t\tstr += value->toString();\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeSize: {\n> -\t\t\tconst Size *value = reinterpret_cast<const Size *>(data);\n> -\t\t\tstr += value->toString();\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypePoint: {\n> -\t\t\tconst Point *value = reinterpret_cast<const Point *>(data);\n> -\t\t\tstr += value->toString();\n> -\t\t\tbreak;\n> -\t\t}\n> -\t\tcase ControlTypeNone:\n> -\t\tcase ControlTypeString:\n> -\t\t\tbreak;\n> -\t\t}\n> -\n> -\t\tif (i + 1 != numElements_)\n> -\t\t\tstr += \", \";\n> -\n> -\t\tdata += ControlValueSize[type_];\n> -\t}\n> -\n> -\tif (isArray_)\n> -\t\tstr += \" ]\";\n> -\n> -\treturn str;\n> +\treturn static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();\n>  }\n>\n>  /**\n> @@ -395,6 +328,190 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n>  \t\tstorage_ = reinterpret_cast<void *>(new uint8_t[newSize]);\n>  }\n>\n> +/**\n> + * \\class ControlValueView\n> + * \\brief A non-owning view-like type to the value of a control\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::ControlValueView()\n> + * \\brief Construct an empty view\n> + * \\sa ControlValue::ControlValue()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::ControlValueView(const ControlValue &v)\n> + * \\brief Construct a view referring to \\a v\n> + *\n> + * The constructed view will refer to the value stored by \\a v, and\n> + * thus \\a v must not be modified or destroyed before the view.\n> + *\n> + * \\sa ControlValue::ControlValue()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::operator bool() const\n> + * \\brief Determine if the referred value is valid\n> + * \\sa ControlValueView::isNone()\n> + */\n> +\n> +/**\n> + * \\fn ControlType ControlValueView::type() const\n> + * \\copydoc ControlValue::type()\n> + * \\sa ControlValue::type()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::isNone() const\n> + * \\copydoc ControlValue::isNone()\n> + * \\sa ControlValue::isNone()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::isArray() const\n> + * \\copydoc ControlValue::isArray()\n> + * \\sa ControlValue::isArray()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::numElements() const\n> + * \\copydoc ControlValue::numElements()\n> + * \\sa ControlValue::numElements()\n> + */\n> +\n> +/**\n> + * \\copydoc ControlValue::data()\n> + * \\sa ControlValue::data()\n> + */\n> +Span<const std::byte> ControlValueView::data() const\n> +{\n> +\treturn { data_, numElements_ * ControlValueSize[type_] };\n> +}\n> +\n> +/**\n> + * \\copydoc ControlValue::operator==()\n> + * \\sa ControlValue::operator==()\n> + * \\sa ControlValueView::operator!=()\n> + */\n> +bool ControlValueView::operator==(const ControlValueView &other) const\n> +{\n> +\tif (type_ != other.type_)\n> +\t\treturn false;\n> +\n> +\tif (numElements_ != other.numElements_)\n> +\t\treturn false;\n> +\n> +\tif (isArray_ != other.isArray_)\n> +\t\treturn false;\n> +\n> +\tconst auto d = data();\n> +\n> +\treturn memcmp(d.data(), other.data_, d.size_bytes()) == 0;\n> +}\n> +\n> +/**\n> + * \\fn ControlValueView::operator!=() const\n> + * \\copydoc ControlValue::operator!=()\n> + * \\sa ControlValue::operator!=()\n> + * \\sa ControlValueView::operator==()\n> + */\n> +\n> +/**\n> + * \\fn template<typename T> T ControlValueView::get() const\n> + * \\copydoc ControlValue::get()\n> + * \\sa ControlValue::get()\n> + */\n> +\n> +/**\n> + * \\brief Insert a text representation of a value into an output stream\n> + * \\sa ControlValue::toString()\n> + */\n> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v)\n> +{\n> +\tconst auto type = v.type();\n> +\tif (type == ControlTypeNone)\n> +\t\treturn s << \"None\";\n> +\n> +\tconst auto *data = v.data().data();\n> +\tconst auto numElements = v.numElements();\n> +\n> +\tif (type == ControlTypeString)\n> +\t\treturn s << std::string_view(reinterpret_cast<const char *>(data),\n> +\t\t\t\t\t     numElements);\n> +\n> +\tconst bool isArray = v.isArray();\n> +\tif (isArray)\n> +\t\ts << \"[ \";\n> +\n> +\tfor (std::size_t i = 0; i < numElements; ++i) {\n> +\t\tif (i > 0)\n> +\t\t\ts << \", \";\n> +\n> +\t\tswitch (type) {\n> +\t\tcase ControlTypeBool: {\n> +\t\t\tconst bool *value = reinterpret_cast<const bool *>(data);\n> +\t\t\ts << (*value ? \"true\" : \"false\");\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeByte: {\n> +\t\t\tconst auto *value = reinterpret_cast<const uint8_t *>(data);\n> +\t\t\ts << static_cast<unsigned int>(*value);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeUnsigned16: {\n> +\t\t\tconst auto *value = reinterpret_cast<const uint16_t *>(data);\n> +\t\t\ts << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeUnsigned32: {\n> +\t\t\tconst auto *value = reinterpret_cast<const uint32_t *>(data);\n> +\t\t\ts << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeInteger32: {\n> +\t\t\tconst auto *value = reinterpret_cast<const int32_t *>(data);\n> +\t\t\ts << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeInteger64: {\n> +\t\t\tconst auto *value = reinterpret_cast<const int64_t *>(data);\n> +\t\t\ts << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeFloat: {\n> +\t\t\tconst auto *value = reinterpret_cast<const float *>(data);\n> +\t\t\ts << std::fixed << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeRectangle: {\n> +\t\t\tconst auto *value = reinterpret_cast<const Rectangle *>(data);\n> +\t\t\ts << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeSize: {\n> +\t\t\tconst auto *value = reinterpret_cast<const Size *>(data);\n> +\t\t\ts << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypePoint: {\n> +\t\t\tconst auto *value = reinterpret_cast<const Point *>(data);\n> +\t\t\ts << *value;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase ControlTypeNone:\n> +\t\tcase ControlTypeString:\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tdata += ControlValueSize[type];\n> +\t}\n> +\n> +\tif (isArray)\n> +\t\ts << \" ]\";\n> +\n> +\treturn s;\n> +}\n> +\n>  /**\n>   * \\class ControlId\n>   * \\brief Control static metadata\n> --\n> 2.50.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DB3F2C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 25 Jul 2025 12:55:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B0139690DF;\n\tFri, 25 Jul 2025 14:55:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 91EBD690A6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 25 Jul 2025 14:55:20 +0200 (CEST)","from ideasonboard.com (mob-5-90-139-29.net.vodafone.it\n\t[5.90.139.29])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 59E2E710;\n\tFri, 25 Jul 2025 14:54:40 +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=\"iGT/Htzk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753448080;\n\tbh=C38MOfiyB2FIW4aKzCRmDeTjQC1h2kfA9tP0zfNV3A0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iGT/Htzkbf2iRlO5fOyXRdCWYwxY7I+wWRluBLs/psAN3nChk14KlTb7jmiL7jkqY\n\t/Ghn3GC7I7i4uDxiQIr+fs8/gxLMP1SiYk5dLPYaYXUjg+T1oSZMf3VAw218L5ygW1\n\tyGJ0bppTiWl6ISlwvar5+DmFC6c3UFkIWQXiLvmA=","Date":"Fri, 25 Jul 2025 14:55:16 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","Message-ID":"<6z6linckfhzco5ntwmulglgtrktiemtyozzirmjrivd472at5q@kymf6c3moxgj>","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-3-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":"<20250721104622.1550908-3-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35249,"web_url":"https://patchwork.libcamera.org/comment/35249/","msgid":"<1a831e9f-1951-4af1-ab07-5d988f814eab@ideasonboard.com>","date":"2025-07-30T14:48:33","subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","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. 07. 25. 14:55 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Jul 21, 2025 at 12:46:02PM +0200, Barnabás Pőcze wrote:\n>> Add `ControlValueView`, which is a non-owning read-only handle to a control\n>> value with a very similar interface.\n>>\n>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>> ---\n>> changes in v2:\n>>    * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)\n>>      to avoid duplication\n>> ---\n>>   include/libcamera/controls.h |  68 +++++++++\n>>   src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------\n>>   2 files changed, 263 insertions(+), 78 deletions(-)\n>>\n>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index b170e30cb..259fc913b 100644\n>> --- a/include/libcamera/controls.h\n>> +++ b/include/libcamera/controls.h\n>> @@ -8,6 +8,7 @@\n>>   #pragma once\n>>\n>>   #include <assert.h>\n>> +#include <cstddef>\n>>   #include <map>\n>>   #include <optional>\n>>   #include <set>\n>> @@ -25,6 +26,7 @@\n>>   namespace libcamera {\n>>\n>>   class ControlValidator;\n>> +class ControlValueView;\n>>\n>>   enum ControlType {\n>>   \tControlTypeNone,\n>> @@ -160,6 +162,8 @@ public:\n>>   \t\t    value.data(), value.size(), sizeof(typename T::value_type));\n>>   \t}\n>>\n>> +\texplicit ControlValue(const ControlValueView &cvv);\n>> +\n>>   \t~ControlValue();\n>>\n>>   \tControlValue(const ControlValue &other);\n>> @@ -247,6 +251,70 @@ private:\n>>   \t\t std::size_t numElements, std::size_t elementSize);\n>>   };\n>>\n>> +class ControlValueView\n>> +{\n>> +public:\n>> +\tconstexpr ControlValueView() noexcept\n>> +\t\t: type_(ControlTypeNone)\n>> +\t{\n>> +\t}\n>> +\n>> +\tControlValueView(const ControlValue &cv) noexcept\n>> +\t\t: ControlValueView(cv.type(), cv.isArray(), cv.numElements(),\n>> +\t\t\t\t   reinterpret_cast<const std::byte *>(cv.data().data()))\n>> +\t{\n>> +\t}\n>> +\n>> +#ifndef __DOXYGEN__\n>> +\t// TODO: should have restricted access?\n>> +\tControlValueView(ControlType type, bool isArray, std::size_t numElements,\n>> +\t\t\t const std::byte *data) noexcept\n>> +\t\t: type_(type), isArray_(isArray), numElements_(numElements),\n>> +\t\t  data_(data)\n> \n> I'm still a little concerned about simply copying the data address\n> here. If the ControlValue this view has been constructed with goes\n> away before its view, we would access invalid memory.\n> \n> Am I too concerned ?\n\nThat is true. I am not sure what could be done. I don't think copying is an\nacceptable solution in this scenario. Anything else seems way too complicated\nto me, given that this type is currently only used for iterating a `MetadataList`,\nor for certain function arguments (but the latter use case is safe).\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> \n>> +\t{\n>> +\t\tassert(isArray || numElements == 1);\n>> +\t}\n>> +#endif\n>> +\n>> +\t[[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }\n>> +\t[[nodiscard]] ControlType type() const { return type_; }\n>> +\t[[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }\n>> +\t[[nodiscard]] bool isArray() const { return isArray_; }\n>> +\t[[nodiscard]] std::size_t numElements() const { return numElements_; }\n>> +\t[[nodiscard]] Span<const std::byte> data() const;\n>> +\n>> +\t[[nodiscard]] bool operator==(const ControlValueView &other) const;\n>> +\n>> +\t[[nodiscard]] bool operator!=(const ControlValueView &other) const\n>> +\t{\n>> +\t\treturn !(*this == other);\n>> +\t}\n>> +\n>> +\ttemplate<typename T>\n>> +\t[[nodiscard]] auto get() const\n>> +\t{\n>> +\t\tusing TypeInfo = details::control_type<std::remove_cv_t<T>>;\n>> +\n>> +\t\tassert(type_ == TypeInfo::value);\n>> +\t\tassert(isArray_ == (TypeInfo::size > 0));\n>> +\n>> +\t\tif constexpr (TypeInfo::size > 0) {\n>> +\t\t\treturn T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);\n>> +\t\t} else {\n>> +\t\t\tassert(numElements_ == 1);\n>> +\t\t\treturn *reinterpret_cast<const T *>(data().data());\n>> +\t\t}\n>> +\t}\n>> +\n>> +private:\n>> +\tControlType type_ : 8;\n>> +\tbool isArray_ = false;\n>> +\tuint32_t numElements_ = 0;\n>> +\tconst std::byte *data_ = nullptr;\n>> +};\n>> +\n>> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v);\n>> +\n>>   class ControlId\n>>   {\n>>   public:\n>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>> index 98fa7583d..a238141a5 100644\n>> --- a/src/libcamera/controls.cpp\n>> +++ b/src/libcamera/controls.cpp\n>> @@ -106,6 +106,16 @@ ControlValue::ControlValue()\n>>   {\n>>   }\n>>\n>> +/**\n>> + * \\brief Construct a ControlValue from a ControlValueView\n>> + */\n>> +ControlValue::ControlValue(const ControlValueView &cvv)\n>> +\t: ControlValue()\n>> +{\n>> +\tset(cvv.type(), cvv.isArray(), cvv.data().data(),\n>> +\t    cvv.numElements(), ControlValueSize[cvv.type()]);\n>> +}\n>> +\n>>   /**\n>>    * \\fn template<typename T> T ControlValue::ControlValue(const T &value)\n>>    * \\brief Construct a ControlValue of type T\n>> @@ -213,84 +223,7 @@ Span<uint8_t> ControlValue::data()\n>>    */\n>>   std::string ControlValue::toString() const\n>>   {\n>> -\tif (type_ == ControlTypeNone)\n>> -\t\treturn \"<ValueType Error>\";\n>> -\n>> -\tconst uint8_t *data = ControlValue::data().data();\n>> -\n>> -\tif (type_ == ControlTypeString)\n>> -\t\treturn std::string(reinterpret_cast<const char *>(data),\n>> -\t\t\t\t   numElements_);\n>> -\n>> -\tstd::string str(isArray_ ? \"[ \" : \"\");\n>> -\n>> -\tfor (unsigned int i = 0; i < numElements_; ++i) {\n>> -\t\tswitch (type_) {\n>> -\t\tcase ControlTypeBool: {\n>> -\t\t\tconst bool *value = reinterpret_cast<const bool *>(data);\n>> -\t\t\tstr += *value ? \"true\" : \"false\";\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeByte: {\n>> -\t\t\tconst uint8_t *value = reinterpret_cast<const uint8_t *>(data);\n>> -\t\t\tstr += std::to_string(*value);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeUnsigned16: {\n>> -\t\t\tconst uint16_t *value = reinterpret_cast<const uint16_t *>(data);\n>> -\t\t\tstr += std::to_string(*value);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeUnsigned32: {\n>> -\t\t\tconst uint32_t *value = reinterpret_cast<const uint32_t *>(data);\n>> -\t\t\tstr += std::to_string(*value);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeInteger32: {\n>> -\t\t\tconst int32_t *value = reinterpret_cast<const int32_t *>(data);\n>> -\t\t\tstr += std::to_string(*value);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeInteger64: {\n>> -\t\t\tconst int64_t *value = reinterpret_cast<const int64_t *>(data);\n>> -\t\t\tstr += std::to_string(*value);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeFloat: {\n>> -\t\t\tconst float *value = reinterpret_cast<const float *>(data);\n>> -\t\t\tstr += std::to_string(*value);\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeRectangle: {\n>> -\t\t\tconst Rectangle *value = reinterpret_cast<const Rectangle *>(data);\n>> -\t\t\tstr += value->toString();\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeSize: {\n>> -\t\t\tconst Size *value = reinterpret_cast<const Size *>(data);\n>> -\t\t\tstr += value->toString();\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypePoint: {\n>> -\t\t\tconst Point *value = reinterpret_cast<const Point *>(data);\n>> -\t\t\tstr += value->toString();\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\t\tcase ControlTypeNone:\n>> -\t\tcase ControlTypeString:\n>> -\t\t\tbreak;\n>> -\t\t}\n>> -\n>> -\t\tif (i + 1 != numElements_)\n>> -\t\t\tstr += \", \";\n>> -\n>> -\t\tdata += ControlValueSize[type_];\n>> -\t}\n>> -\n>> -\tif (isArray_)\n>> -\t\tstr += \" ]\";\n>> -\n>> -\treturn str;\n>> +\treturn static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();\n>>   }\n>>\n>>   /**\n>> @@ -395,6 +328,190 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n>>   \t\tstorage_ = reinterpret_cast<void *>(new uint8_t[newSize]);\n>>   }\n>>\n>> +/**\n>> + * \\class ControlValueView\n>> + * \\brief A non-owning view-like type to the value of a control\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValueView::ControlValueView()\n>> + * \\brief Construct an empty view\n>> + * \\sa ControlValue::ControlValue()\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValueView::ControlValueView(const ControlValue &v)\n>> + * \\brief Construct a view referring to \\a v\n>> + *\n>> + * The constructed view will refer to the value stored by \\a v, and\n>> + * thus \\a v must not be modified or destroyed before the view.\n>> + *\n>> + * \\sa ControlValue::ControlValue()\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValueView::operator bool() const\n>> + * \\brief Determine if the referred value is valid\n>> + * \\sa ControlValueView::isNone()\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlType ControlValueView::type() const\n>> + * \\copydoc ControlValue::type()\n>> + * \\sa ControlValue::type()\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValueView::isNone() const\n>> + * \\copydoc ControlValue::isNone()\n>> + * \\sa ControlValue::isNone()\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValueView::isArray() const\n>> + * \\copydoc ControlValue::isArray()\n>> + * \\sa ControlValue::isArray()\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValueView::numElements() const\n>> + * \\copydoc ControlValue::numElements()\n>> + * \\sa ControlValue::numElements()\n>> + */\n>> +\n>> +/**\n>> + * \\copydoc ControlValue::data()\n>> + * \\sa ControlValue::data()\n>> + */\n>> +Span<const std::byte> ControlValueView::data() const\n>> +{\n>> +\treturn { data_, numElements_ * ControlValueSize[type_] };\n>> +}\n>> +\n>> +/**\n>> + * \\copydoc ControlValue::operator==()\n>> + * \\sa ControlValue::operator==()\n>> + * \\sa ControlValueView::operator!=()\n>> + */\n>> +bool ControlValueView::operator==(const ControlValueView &other) const\n>> +{\n>> +\tif (type_ != other.type_)\n>> +\t\treturn false;\n>> +\n>> +\tif (numElements_ != other.numElements_)\n>> +\t\treturn false;\n>> +\n>> +\tif (isArray_ != other.isArray_)\n>> +\t\treturn false;\n>> +\n>> +\tconst auto d = data();\n>> +\n>> +\treturn memcmp(d.data(), other.data_, d.size_bytes()) == 0;\n>> +}\n>> +\n>> +/**\n>> + * \\fn ControlValueView::operator!=() const\n>> + * \\copydoc ControlValue::operator!=()\n>> + * \\sa ControlValue::operator!=()\n>> + * \\sa ControlValueView::operator==()\n>> + */\n>> +\n>> +/**\n>> + * \\fn template<typename T> T ControlValueView::get() const\n>> + * \\copydoc ControlValue::get()\n>> + * \\sa ControlValue::get()\n>> + */\n>> +\n>> +/**\n>> + * \\brief Insert a text representation of a value into an output stream\n>> + * \\sa ControlValue::toString()\n>> + */\n>> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v)\n>> +{\n>> +\tconst auto type = v.type();\n>> +\tif (type == ControlTypeNone)\n>> +\t\treturn s << \"None\";\n>> +\n>> +\tconst auto *data = v.data().data();\n>> +\tconst auto numElements = v.numElements();\n>> +\n>> +\tif (type == ControlTypeString)\n>> +\t\treturn s << std::string_view(reinterpret_cast<const char *>(data),\n>> +\t\t\t\t\t     numElements);\n>> +\n>> +\tconst bool isArray = v.isArray();\n>> +\tif (isArray)\n>> +\t\ts << \"[ \";\n>> +\n>> +\tfor (std::size_t i = 0; i < numElements; ++i) {\n>> +\t\tif (i > 0)\n>> +\t\t\ts << \", \";\n>> +\n>> +\t\tswitch (type) {\n>> +\t\tcase ControlTypeBool: {\n>> +\t\t\tconst bool *value = reinterpret_cast<const bool *>(data);\n>> +\t\t\ts << (*value ? \"true\" : \"false\");\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeByte: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const uint8_t *>(data);\n>> +\t\t\ts << static_cast<unsigned int>(*value);\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeUnsigned16: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const uint16_t *>(data);\n>> +\t\t\ts << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeUnsigned32: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const uint32_t *>(data);\n>> +\t\t\ts << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeInteger32: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const int32_t *>(data);\n>> +\t\t\ts << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeInteger64: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const int64_t *>(data);\n>> +\t\t\ts << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeFloat: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const float *>(data);\n>> +\t\t\ts << std::fixed << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeRectangle: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const Rectangle *>(data);\n>> +\t\t\ts << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeSize: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const Size *>(data);\n>> +\t\t\ts << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypePoint: {\n>> +\t\t\tconst auto *value = reinterpret_cast<const Point *>(data);\n>> +\t\t\ts << *value;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcase ControlTypeNone:\n>> +\t\tcase ControlTypeString:\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\n>> +\t\tdata += ControlValueSize[type];\n>> +\t}\n>> +\n>> +\tif (isArray)\n>> +\t\ts << \" ]\";\n>> +\n>> +\treturn s;\n>> +}\n>> +\n>>   /**\n>>    * \\class ControlId\n>>    * \\brief Control static metadata\n>> --\n>> 2.50.1\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 46FD4BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 30 Jul 2025 14:48:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04D9069205;\n\tWed, 30 Jul 2025 16:48:39 +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 A206B691F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 30 Jul 2025 16:48:37 +0200 (CEST)","from [192.168.33.11] (185.182.214.105.nat.pool.zt.hu\n\t[185.182.214.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1D964C7A;\n\tWed, 30 Jul 2025 16:47:54 +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=\"PdMj7yhx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753886874;\n\tbh=zSawTuO/GGwwEps8ScSxLuD6CyAQ9F2narAjMKKeqTs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=PdMj7yhxlB5YFGRj3pozIu1uWClnv4hcmAssPX2zXLEzsNsDEBpq9rCd5B7SRS7eT\n\tpHAQ7O+PBW7BVJyMoPivDq2o8OtXp0J3SDJ0WNExeiRZj8CJ44eatuZ+45MMgdgaJm\n\thMBtRCfS9It0hwNQ0TPWyGwSTNFkY/N+UOwBocoU=","Message-ID":"<1a831e9f-1951-4af1-ab07-5d988f814eab@ideasonboard.com>","Date":"Wed, 30 Jul 2025 16:48:33 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-3-barnabas.pocze@ideasonboard.com>\n\t<6z6linckfhzco5ntwmulglgtrktiemtyozzirmjrivd472at5q@kymf6c3moxgj>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<6z6linckfhzco5ntwmulglgtrktiemtyozzirmjrivd472at5q@kymf6c3moxgj>","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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35266,"web_url":"https://patchwork.libcamera.org/comment/35266/","msgid":"<hzetp42lnavatmrkry55gekif4uzga3p7hx65fz5w5iw2xb3yp@tc2dohirta43>","date":"2025-08-03T18:35:35","subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","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 Wed, Jul 30, 2025 at 04:48:33PM +0200, Barnabás Pőcze wrote:\n> Hi\n>\n> 2025. 07. 25. 14:55 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Mon, Jul 21, 2025 at 12:46:02PM +0200, Barnabás Pőcze wrote:\n> > > Add `ControlValueView`, which is a non-owning read-only handle to a control\n> > > value with a very similar interface.\n> > >\n> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> > > ---\n> > > changes in v2:\n> > >    * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)\n> > >      to avoid duplication\n> > > ---\n> > >   include/libcamera/controls.h |  68 +++++++++\n> > >   src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------\n> > >   2 files changed, 263 insertions(+), 78 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > index b170e30cb..259fc913b 100644\n> > > --- a/include/libcamera/controls.h\n> > > +++ b/include/libcamera/controls.h\n> > > @@ -8,6 +8,7 @@\n> > >   #pragma once\n> > >\n> > >   #include <assert.h>\n> > > +#include <cstddef>\n> > >   #include <map>\n> > >   #include <optional>\n> > >   #include <set>\n> > > @@ -25,6 +26,7 @@\n> > >   namespace libcamera {\n> > >\n> > >   class ControlValidator;\n> > > +class ControlValueView;\n> > >\n> > >   enum ControlType {\n> > >   \tControlTypeNone,\n> > > @@ -160,6 +162,8 @@ public:\n> > >   \t\t    value.data(), value.size(), sizeof(typename T::value_type));\n> > >   \t}\n> > >\n> > > +\texplicit ControlValue(const ControlValueView &cvv);\n> > > +\n> > >   \t~ControlValue();\n> > >\n> > >   \tControlValue(const ControlValue &other);\n> > > @@ -247,6 +251,70 @@ private:\n> > >   \t\t std::size_t numElements, std::size_t elementSize);\n> > >   };\n> > >\n> > > +class ControlValueView\n> > > +{\n> > > +public:\n> > > +\tconstexpr ControlValueView() noexcept\n> > > +\t\t: type_(ControlTypeNone)\n> > > +\t{\n> > > +\t}\n> > > +\n> > > +\tControlValueView(const ControlValue &cv) noexcept\n> > > +\t\t: ControlValueView(cv.type(), cv.isArray(), cv.numElements(),\n> > > +\t\t\t\t   reinterpret_cast<const std::byte *>(cv.data().data()))\n> > > +\t{\n> > > +\t}\n> > > +\n> > > +#ifndef __DOXYGEN__\n> > > +\t// TODO: should have restricted access?\n> > > +\tControlValueView(ControlType type, bool isArray, std::size_t numElements,\n> > > +\t\t\t const std::byte *data) noexcept\n> > > +\t\t: type_(type), isArray_(isArray), numElements_(numElements),\n> > > +\t\t  data_(data)\n> >\n> > I'm still a little concerned about simply copying the data address\n> > here. If the ControlValue this view has been constructed with goes\n> > away before its view, we would access invalid memory.\n> >\n> > Am I too concerned ?\n>\n> That is true. I am not sure what could be done. I don't think copying is an\n> acceptable solution in this scenario. Anything else seems way too complicated\n> to me, given that this type is currently only used for iterating a `MetadataList`,\n> or for certain function arguments (but the latter use case is safe).\n\nI agree, actually I'm wondering if not all of our ControlValue should\nactually be views over a storage space allocated somewhere else\n(likely in a ControlList or a MetadataList).\n\nI went for a little experiment, renaming the existing ControlValue\nclass to ControlStorage and make your new ControlValueView the\nControlValue type we have so far used.\n\nMy main goal was to make ControlList and MetadataList\nuse the same type in their public interface, however I think there is\nvalue in having a \"storage\" control type and \"view\" control type to\nclearly define the data ownership model.\n\nI think I would also like to test if we can even make the newly introduced\nControlStorage (was ControlValue) a move-only interface: this won't\nhave any performance improvement as the \"move\" would anyway be a\nmemcpy but it would make it clear there is only a single storage for a\ncontrol value at the time.\n\nI think it will also make easier to serialize ControlList as your\nseries does with MetadataList, as ControlValue (was ControlValueView)\nis already instumented for being re-constructed from a serialized\nbuffer in your series.\n\nFor reference:\nhttps://gitlab.freedesktop.org/camera/libcamera/-/commits/b4/control_storage\n\nStill WIP as the CI loops on the pi IPA, I'll work on this but in the\nmeantime comment are appreciated. I might have missed some trivial use\ncase for which always operating a ControlList/MetadataList::get() with\nviews is not desirable.\n\nThanks\n  j\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n>\n> >\n> >\n> > > +\t{\n> > > +\t\tassert(isArray || numElements == 1);\n> > > +\t}\n> > > +#endif\n> > > +\n> > > +\t[[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }\n> > > +\t[[nodiscard]] ControlType type() const { return type_; }\n> > > +\t[[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }\n> > > +\t[[nodiscard]] bool isArray() const { return isArray_; }\n> > > +\t[[nodiscard]] std::size_t numElements() const { return numElements_; }\n> > > +\t[[nodiscard]] Span<const std::byte> data() const;\n> > > +\n> > > +\t[[nodiscard]] bool operator==(const ControlValueView &other) const;\n> > > +\n> > > +\t[[nodiscard]] bool operator!=(const ControlValueView &other) const\n> > > +\t{\n> > > +\t\treturn !(*this == other);\n> > > +\t}\n> > > +\n> > > +\ttemplate<typename T>\n> > > +\t[[nodiscard]] auto get() const\n> > > +\t{\n> > > +\t\tusing TypeInfo = details::control_type<std::remove_cv_t<T>>;\n> > > +\n> > > +\t\tassert(type_ == TypeInfo::value);\n> > > +\t\tassert(isArray_ == (TypeInfo::size > 0));\n> > > +\n> > > +\t\tif constexpr (TypeInfo::size > 0) {\n> > > +\t\t\treturn T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);\n> > > +\t\t} else {\n> > > +\t\t\tassert(numElements_ == 1);\n> > > +\t\t\treturn *reinterpret_cast<const T *>(data().data());\n> > > +\t\t}\n> > > +\t}\n> > > +\n> > > +private:\n> > > +\tControlType type_ : 8;\n> > > +\tbool isArray_ = false;\n> > > +\tuint32_t numElements_ = 0;\n> > > +\tconst std::byte *data_ = nullptr;\n> > > +};\n> > > +\n> > > +std::ostream &operator<<(std::ostream &s, const ControlValueView &v);\n> > > +\n> > >   class ControlId\n> > >   {\n> > >   public:\n> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > index 98fa7583d..a238141a5 100644\n> > > --- a/src/libcamera/controls.cpp\n> > > +++ b/src/libcamera/controls.cpp\n> > > @@ -106,6 +106,16 @@ ControlValue::ControlValue()\n> > >   {\n> > >   }\n> > >\n> > > +/**\n> > > + * \\brief Construct a ControlValue from a ControlValueView\n> > > + */\n> > > +ControlValue::ControlValue(const ControlValueView &cvv)\n> > > +\t: ControlValue()\n> > > +{\n> > > +\tset(cvv.type(), cvv.isArray(), cvv.data().data(),\n> > > +\t    cvv.numElements(), ControlValueSize[cvv.type()]);\n> > > +}\n> > > +\n> > >   /**\n> > >    * \\fn template<typename T> T ControlValue::ControlValue(const T &value)\n> > >    * \\brief Construct a ControlValue of type T\n> > > @@ -213,84 +223,7 @@ Span<uint8_t> ControlValue::data()\n> > >    */\n> > >   std::string ControlValue::toString() const\n> > >   {\n> > > -\tif (type_ == ControlTypeNone)\n> > > -\t\treturn \"<ValueType Error>\";\n> > > -\n> > > -\tconst uint8_t *data = ControlValue::data().data();\n> > > -\n> > > -\tif (type_ == ControlTypeString)\n> > > -\t\treturn std::string(reinterpret_cast<const char *>(data),\n> > > -\t\t\t\t   numElements_);\n> > > -\n> > > -\tstd::string str(isArray_ ? \"[ \" : \"\");\n> > > -\n> > > -\tfor (unsigned int i = 0; i < numElements_; ++i) {\n> > > -\t\tswitch (type_) {\n> > > -\t\tcase ControlTypeBool: {\n> > > -\t\t\tconst bool *value = reinterpret_cast<const bool *>(data);\n> > > -\t\t\tstr += *value ? \"true\" : \"false\";\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeByte: {\n> > > -\t\t\tconst uint8_t *value = reinterpret_cast<const uint8_t *>(data);\n> > > -\t\t\tstr += std::to_string(*value);\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeUnsigned16: {\n> > > -\t\t\tconst uint16_t *value = reinterpret_cast<const uint16_t *>(data);\n> > > -\t\t\tstr += std::to_string(*value);\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeUnsigned32: {\n> > > -\t\t\tconst uint32_t *value = reinterpret_cast<const uint32_t *>(data);\n> > > -\t\t\tstr += std::to_string(*value);\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeInteger32: {\n> > > -\t\t\tconst int32_t *value = reinterpret_cast<const int32_t *>(data);\n> > > -\t\t\tstr += std::to_string(*value);\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeInteger64: {\n> > > -\t\t\tconst int64_t *value = reinterpret_cast<const int64_t *>(data);\n> > > -\t\t\tstr += std::to_string(*value);\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeFloat: {\n> > > -\t\t\tconst float *value = reinterpret_cast<const float *>(data);\n> > > -\t\t\tstr += std::to_string(*value);\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeRectangle: {\n> > > -\t\t\tconst Rectangle *value = reinterpret_cast<const Rectangle *>(data);\n> > > -\t\t\tstr += value->toString();\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeSize: {\n> > > -\t\t\tconst Size *value = reinterpret_cast<const Size *>(data);\n> > > -\t\t\tstr += value->toString();\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypePoint: {\n> > > -\t\t\tconst Point *value = reinterpret_cast<const Point *>(data);\n> > > -\t\t\tstr += value->toString();\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\t\tcase ControlTypeNone:\n> > > -\t\tcase ControlTypeString:\n> > > -\t\t\tbreak;\n> > > -\t\t}\n> > > -\n> > > -\t\tif (i + 1 != numElements_)\n> > > -\t\t\tstr += \", \";\n> > > -\n> > > -\t\tdata += ControlValueSize[type_];\n> > > -\t}\n> > > -\n> > > -\tif (isArray_)\n> > > -\t\tstr += \" ]\";\n> > > -\n> > > -\treturn str;\n> > > +\treturn static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();\n> > >   }\n> > >\n> > >   /**\n> > > @@ -395,6 +328,190 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > >   \t\tstorage_ = reinterpret_cast<void *>(new uint8_t[newSize]);\n> > >   }\n> > >\n> > > +/**\n> > > + * \\class ControlValueView\n> > > + * \\brief A non-owning view-like type to the value of a control\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn ControlValueView::ControlValueView()\n> > > + * \\brief Construct an empty view\n> > > + * \\sa ControlValue::ControlValue()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn ControlValueView::ControlValueView(const ControlValue &v)\n> > > + * \\brief Construct a view referring to \\a v\n> > > + *\n> > > + * The constructed view will refer to the value stored by \\a v, and\n> > > + * thus \\a v must not be modified or destroyed before the view.\n> > > + *\n> > > + * \\sa ControlValue::ControlValue()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn ControlValueView::operator bool() const\n> > > + * \\brief Determine if the referred value is valid\n> > > + * \\sa ControlValueView::isNone()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn ControlType ControlValueView::type() const\n> > > + * \\copydoc ControlValue::type()\n> > > + * \\sa ControlValue::type()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn ControlValueView::isNone() const\n> > > + * \\copydoc ControlValue::isNone()\n> > > + * \\sa ControlValue::isNone()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn ControlValueView::isArray() const\n> > > + * \\copydoc ControlValue::isArray()\n> > > + * \\sa ControlValue::isArray()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn ControlValueView::numElements() const\n> > > + * \\copydoc ControlValue::numElements()\n> > > + * \\sa ControlValue::numElements()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\copydoc ControlValue::data()\n> > > + * \\sa ControlValue::data()\n> > > + */\n> > > +Span<const std::byte> ControlValueView::data() const\n> > > +{\n> > > +\treturn { data_, numElements_ * ControlValueSize[type_] };\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc ControlValue::operator==()\n> > > + * \\sa ControlValue::operator==()\n> > > + * \\sa ControlValueView::operator!=()\n> > > + */\n> > > +bool ControlValueView::operator==(const ControlValueView &other) const\n> > > +{\n> > > +\tif (type_ != other.type_)\n> > > +\t\treturn false;\n> > > +\n> > > +\tif (numElements_ != other.numElements_)\n> > > +\t\treturn false;\n> > > +\n> > > +\tif (isArray_ != other.isArray_)\n> > > +\t\treturn false;\n> > > +\n> > > +\tconst auto d = data();\n> > > +\n> > > +\treturn memcmp(d.data(), other.data_, d.size_bytes()) == 0;\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\fn ControlValueView::operator!=() const\n> > > + * \\copydoc ControlValue::operator!=()\n> > > + * \\sa ControlValue::operator!=()\n> > > + * \\sa ControlValueView::operator==()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn template<typename T> T ControlValueView::get() const\n> > > + * \\copydoc ControlValue::get()\n> > > + * \\sa ControlValue::get()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Insert a text representation of a value into an output stream\n> > > + * \\sa ControlValue::toString()\n> > > + */\n> > > +std::ostream &operator<<(std::ostream &s, const ControlValueView &v)\n> > > +{\n> > > +\tconst auto type = v.type();\n> > > +\tif (type == ControlTypeNone)\n> > > +\t\treturn s << \"None\";\n> > > +\n> > > +\tconst auto *data = v.data().data();\n> > > +\tconst auto numElements = v.numElements();\n> > > +\n> > > +\tif (type == ControlTypeString)\n> > > +\t\treturn s << std::string_view(reinterpret_cast<const char *>(data),\n> > > +\t\t\t\t\t     numElements);\n> > > +\n> > > +\tconst bool isArray = v.isArray();\n> > > +\tif (isArray)\n> > > +\t\ts << \"[ \";\n> > > +\n> > > +\tfor (std::size_t i = 0; i < numElements; ++i) {\n> > > +\t\tif (i > 0)\n> > > +\t\t\ts << \", \";\n> > > +\n> > > +\t\tswitch (type) {\n> > > +\t\tcase ControlTypeBool: {\n> > > +\t\t\tconst bool *value = reinterpret_cast<const bool *>(data);\n> > > +\t\t\ts << (*value ? \"true\" : \"false\");\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeByte: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const uint8_t *>(data);\n> > > +\t\t\ts << static_cast<unsigned int>(*value);\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeUnsigned16: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const uint16_t *>(data);\n> > > +\t\t\ts << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeUnsigned32: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const uint32_t *>(data);\n> > > +\t\t\ts << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeInteger32: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const int32_t *>(data);\n> > > +\t\t\ts << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeInteger64: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const int64_t *>(data);\n> > > +\t\t\ts << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeFloat: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const float *>(data);\n> > > +\t\t\ts << std::fixed << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeRectangle: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const Rectangle *>(data);\n> > > +\t\t\ts << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeSize: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const Size *>(data);\n> > > +\t\t\ts << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypePoint: {\n> > > +\t\t\tconst auto *value = reinterpret_cast<const Point *>(data);\n> > > +\t\t\ts << *value;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\t\tcase ControlTypeNone:\n> > > +\t\tcase ControlTypeString:\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\n> > > +\t\tdata += ControlValueSize[type];\n> > > +\t}\n> > > +\n> > > +\tif (isArray)\n> > > +\t\ts << \" ]\";\n> > > +\n> > > +\treturn s;\n> > > +}\n> > > +\n> > >   /**\n> > >    * \\class ControlId\n> > >    * \\brief Control static metadata\n> > > --\n> > > 2.50.1\n> > >\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1CE27BDCC1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  3 Aug 2025 18:35:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D4B36926E;\n\tSun,  3 Aug 2025 20:35:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BF72E691D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  3 Aug 2025 20:35:38 +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 0EA6C6AF;\n\tSun,  3 Aug 2025 20:34:52 +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=\"Aihcg8m9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1754246092;\n\tbh=Kwcx5XiJGFsnEDw2bUS0Ha5k0/7+Rml59+Si6H7CjBE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Aihcg8m9Ae4+m+HkCtiHyOXUt8blyjmA3lMbncyUXFjhsBqthm9QTwj/ke9UGk7mh\n\tyuIN5QJb37X/UWvEx1G9shfa/Xg1pP+DcmsHtMrbzaoIqNL5cRhJpM0GmPlrgKLXac\n\tXunEaH/abIl76vDcRzIlwzoRJ5tUjVQNhxTJBNzk=","Date":"Sun, 3 Aug 2025 20:35:35 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","Message-ID":"<hzetp42lnavatmrkry55gekif4uzga3p7hx65fz5w5iw2xb3yp@tc2dohirta43>","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-3-barnabas.pocze@ideasonboard.com>\n\t<6z6linckfhzco5ntwmulglgtrktiemtyozzirmjrivd472at5q@kymf6c3moxgj>\n\t<1a831e9f-1951-4af1-ab07-5d988f814eab@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<1a831e9f-1951-4af1-ab07-5d988f814eab@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35921,"web_url":"https://patchwork.libcamera.org/comment/35921/","msgid":"<175827898733.2127323.15908921583716523852@neptunite.rasen.tech>","date":"2025-09-19T10:49:47","subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-07-21 19:46:02)\n> Add `ControlValueView`, which is a non-owning read-only handle to a control\n> value with a very similar interface.\n> \n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> ---\n> changes in v2:\n>   * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)\n>     to avoid duplication\n> ---\n>  include/libcamera/controls.h |  68 +++++++++\n>  src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------\n>  2 files changed, 263 insertions(+), 78 deletions(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index b170e30cb..259fc913b 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -8,6 +8,7 @@\n>  #pragma once\n>  \n>  #include <assert.h>\n> +#include <cstddef>\n>  #include <map>\n>  #include <optional>\n>  #include <set>\n> @@ -25,6 +26,7 @@\n>  namespace libcamera {\n>  \n>  class ControlValidator;\n> +class ControlValueView;\n>  \n>  enum ControlType {\n>         ControlTypeNone,\n> @@ -160,6 +162,8 @@ public:\n>                     value.data(), value.size(), sizeof(typename T::value_type));\n>         }\n>  \n> +       explicit ControlValue(const ControlValueView &cvv);\n> +\n>         ~ControlValue();\n>  \n>         ControlValue(const ControlValue &other);\n> @@ -247,6 +251,70 @@ private:\n>                  std::size_t numElements, std::size_t elementSize);\n>  };\n>  \n> +class ControlValueView\n> +{\n> +public:\n> +       constexpr ControlValueView() noexcept\n> +               : type_(ControlTypeNone)\n> +       {\n> +       }\n> +\n> +       ControlValueView(const ControlValue &cv) noexcept\n> +               : ControlValueView(cv.type(), cv.isArray(), cv.numElements(),\n> +                                  reinterpret_cast<const std::byte *>(cv.data().data()))\n> +       {\n> +       }\n> +\n> +#ifndef __DOXYGEN__\n> +       // TODO: should have restricted access?\n> +       ControlValueView(ControlType type, bool isArray, std::size_t numElements,\n> +                        const std::byte *data) noexcept\n> +               : type_(type), isArray_(isArray), numElements_(numElements),\n> +                 data_(data)\n> +       {\n> +               assert(isArray || numElements == 1);\n> +       }\n> +#endif\n> +\n> +       [[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }\n> +       [[nodiscard]] ControlType type() const { return type_; }\n> +       [[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }\n> +       [[nodiscard]] bool isArray() const { return isArray_; }\n> +       [[nodiscard]] std::size_t numElements() const { return numElements_; }\n> +       [[nodiscard]] Span<const std::byte> data() const;\n> +\n> +       [[nodiscard]] bool operator==(const ControlValueView &other) const;\n> +\n> +       [[nodiscard]] bool operator!=(const ControlValueView &other) const\n> +       {\n> +               return !(*this == other);\n> +       }\n> +\n> +       template<typename T>\n> +       [[nodiscard]] auto get() const\n> +       {\n> +               using TypeInfo = details::control_type<std::remove_cv_t<T>>;\n> +\n> +               assert(type_ == TypeInfo::value);\n> +               assert(isArray_ == (TypeInfo::size > 0));\n> +\n> +               if constexpr (TypeInfo::size > 0) {\n> +                       return T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);\n> +               } else {\n> +                       assert(numElements_ == 1);\n> +                       return *reinterpret_cast<const T *>(data().data());\n> +               }\n> +       }\n> +\n> +private:\n> +       ControlType type_ : 8;\n> +       bool isArray_ = false;\n> +       uint32_t numElements_ = 0;\n> +       const std::byte *data_ = nullptr;\n> +};\n> +\n> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v);\n> +\n>  class ControlId\n>  {\n>  public:\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 98fa7583d..a238141a5 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -106,6 +106,16 @@ ControlValue::ControlValue()\n>  {\n>  }\n>  \n> +/**\n> + * \\brief Construct a ControlValue from a ControlValueView\n> + */\n> +ControlValue::ControlValue(const ControlValueView &cvv)\n> +       : ControlValue()\n> +{\n> +       set(cvv.type(), cvv.isArray(), cvv.data().data(),\n> +           cvv.numElements(), ControlValueSize[cvv.type()]);\n> +}\n> +\n>  /**\n>   * \\fn template<typename T> T ControlValue::ControlValue(const T &value)\n>   * \\brief Construct a ControlValue of type T\n> @@ -213,84 +223,7 @@ Span<uint8_t> ControlValue::data()\n>   */\n>  std::string ControlValue::toString() const\n>  {\n> -       if (type_ == ControlTypeNone)\n> -               return \"<ValueType Error>\";\n> -\n> -       const uint8_t *data = ControlValue::data().data();\n> -\n> -       if (type_ == ControlTypeString)\n> -               return std::string(reinterpret_cast<const char *>(data),\n> -                                  numElements_);\n> -\n> -       std::string str(isArray_ ? \"[ \" : \"\");\n> -\n> -       for (unsigned int i = 0; i < numElements_; ++i) {\n> -               switch (type_) {\n> -               case ControlTypeBool: {\n> -                       const bool *value = reinterpret_cast<const bool *>(data);\n> -                       str += *value ? \"true\" : \"false\";\n> -                       break;\n> -               }\n> -               case ControlTypeByte: {\n> -                       const uint8_t *value = reinterpret_cast<const uint8_t *>(data);\n> -                       str += std::to_string(*value);\n> -                       break;\n> -               }\n> -               case ControlTypeUnsigned16: {\n> -                       const uint16_t *value = reinterpret_cast<const uint16_t *>(data);\n> -                       str += std::to_string(*value);\n> -                       break;\n> -               }\n> -               case ControlTypeUnsigned32: {\n> -                       const uint32_t *value = reinterpret_cast<const uint32_t *>(data);\n> -                       str += std::to_string(*value);\n> -                       break;\n> -               }\n> -               case ControlTypeInteger32: {\n> -                       const int32_t *value = reinterpret_cast<const int32_t *>(data);\n> -                       str += std::to_string(*value);\n> -                       break;\n> -               }\n> -               case ControlTypeInteger64: {\n> -                       const int64_t *value = reinterpret_cast<const int64_t *>(data);\n> -                       str += std::to_string(*value);\n> -                       break;\n> -               }\n> -               case ControlTypeFloat: {\n> -                       const float *value = reinterpret_cast<const float *>(data);\n> -                       str += std::to_string(*value);\n> -                       break;\n> -               }\n> -               case ControlTypeRectangle: {\n> -                       const Rectangle *value = reinterpret_cast<const Rectangle *>(data);\n> -                       str += value->toString();\n> -                       break;\n> -               }\n> -               case ControlTypeSize: {\n> -                       const Size *value = reinterpret_cast<const Size *>(data);\n> -                       str += value->toString();\n> -                       break;\n> -               }\n> -               case ControlTypePoint: {\n> -                       const Point *value = reinterpret_cast<const Point *>(data);\n> -                       str += value->toString();\n> -                       break;\n> -               }\n> -               case ControlTypeNone:\n> -               case ControlTypeString:\n> -                       break;\n> -               }\n> -\n> -               if (i + 1 != numElements_)\n> -                       str += \", \";\n> -\n> -               data += ControlValueSize[type_];\n> -       }\n> -\n> -       if (isArray_)\n> -               str += \" ]\";\n> -\n> -       return str;\n> +       return static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();\n>  }\n>  \n>  /**\n> @@ -395,6 +328,190 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n>                 storage_ = reinterpret_cast<void *>(new uint8_t[newSize]);\n>  }\n>  \n> +/**\n> + * \\class ControlValueView\n> + * \\brief A non-owning view-like type to the value of a control\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::ControlValueView()\n> + * \\brief Construct an empty view\n> + * \\sa ControlValue::ControlValue()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::ControlValueView(const ControlValue &v)\n> + * \\brief Construct a view referring to \\a v\n> + *\n> + * The constructed view will refer to the value stored by \\a v, and\n> + * thus \\a v must not be modified or destroyed before the view.\n\ns/view/view is destroyed/ might be slightly better (since the inference is \"the\nview is modified or destroyed\" but it can't be modified, so specifying the\nrestriction is be a bit better)\n\n> + *\n> + * \\sa ControlValue::ControlValue()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::operator bool() const\n> + * \\brief Determine if the referred value is valid\n\nimo s/value/ControlValue/ is better, since \"value\" sounds like a simple scalar\nnon-wrapped numerical-like value.\n\nWith those changes,\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> + * \\sa ControlValueView::isNone()\n> + */\n> +\n> +/**\n> + * \\fn ControlType ControlValueView::type() const\n> + * \\copydoc ControlValue::type()\n> + * \\sa ControlValue::type()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::isNone() const\n> + * \\copydoc ControlValue::isNone()\n> + * \\sa ControlValue::isNone()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::isArray() const\n> + * \\copydoc ControlValue::isArray()\n> + * \\sa ControlValue::isArray()\n> + */\n> +\n> +/**\n> + * \\fn ControlValueView::numElements() const\n> + * \\copydoc ControlValue::numElements()\n> + * \\sa ControlValue::numElements()\n> + */\n> +\n> +/**\n> + * \\copydoc ControlValue::data()\n> + * \\sa ControlValue::data()\n> + */\n> +Span<const std::byte> ControlValueView::data() const\n> +{\n> +       return { data_, numElements_ * ControlValueSize[type_] };\n> +}\n> +\n> +/**\n> + * \\copydoc ControlValue::operator==()\n> + * \\sa ControlValue::operator==()\n> + * \\sa ControlValueView::operator!=()\n> + */\n> +bool ControlValueView::operator==(const ControlValueView &other) const\n> +{\n> +       if (type_ != other.type_)\n> +               return false;\n> +\n> +       if (numElements_ != other.numElements_)\n> +               return false;\n> +\n> +       if (isArray_ != other.isArray_)\n> +               return false;\n> +\n> +       const auto d = data();\n> +\n> +       return memcmp(d.data(), other.data_, d.size_bytes()) == 0;\n> +}\n> +\n> +/**\n> + * \\fn ControlValueView::operator!=() const\n> + * \\copydoc ControlValue::operator!=()\n> + * \\sa ControlValue::operator!=()\n> + * \\sa ControlValueView::operator==()\n> + */\n> +\n> +/**\n> + * \\fn template<typename T> T ControlValueView::get() const\n> + * \\copydoc ControlValue::get()\n> + * \\sa ControlValue::get()\n> + */\n> +\n> +/**\n> + * \\brief Insert a text representation of a value into an output stream\n> + * \\sa ControlValue::toString()\n> + */\n> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v)\n> +{\n> +       const auto type = v.type();\n> +       if (type == ControlTypeNone)\n> +               return s << \"None\";\n> +\n> +       const auto *data = v.data().data();\n> +       const auto numElements = v.numElements();\n> +\n> +       if (type == ControlTypeString)\n> +               return s << std::string_view(reinterpret_cast<const char *>(data),\n> +                                            numElements);\n> +\n> +       const bool isArray = v.isArray();\n> +       if (isArray)\n> +               s << \"[ \";\n> +\n> +       for (std::size_t i = 0; i < numElements; ++i) {\n> +               if (i > 0)\n> +                       s << \", \";\n> +\n> +               switch (type) {\n> +               case ControlTypeBool: {\n> +                       const bool *value = reinterpret_cast<const bool *>(data);\n> +                       s << (*value ? \"true\" : \"false\");\n> +                       break;\n> +               }\n> +               case ControlTypeByte: {\n> +                       const auto *value = reinterpret_cast<const uint8_t *>(data);\n> +                       s << static_cast<unsigned int>(*value);\n> +                       break;\n> +               }\n> +               case ControlTypeUnsigned16: {\n> +                       const auto *value = reinterpret_cast<const uint16_t *>(data);\n> +                       s << *value;\n> +                       break;\n> +               }\n> +               case ControlTypeUnsigned32: {\n> +                       const auto *value = reinterpret_cast<const uint32_t *>(data);\n> +                       s << *value;\n> +                       break;\n> +               }\n> +               case ControlTypeInteger32: {\n> +                       const auto *value = reinterpret_cast<const int32_t *>(data);\n> +                       s << *value;\n> +                       break;\n> +               }\n> +               case ControlTypeInteger64: {\n> +                       const auto *value = reinterpret_cast<const int64_t *>(data);\n> +                       s << *value;\n> +                       break;\n> +               }\n> +               case ControlTypeFloat: {\n> +                       const auto *value = reinterpret_cast<const float *>(data);\n> +                       s << std::fixed << *value;\n> +                       break;\n> +               }\n> +               case ControlTypeRectangle: {\n> +                       const auto *value = reinterpret_cast<const Rectangle *>(data);\n> +                       s << *value;\n> +                       break;\n> +               }\n> +               case ControlTypeSize: {\n> +                       const auto *value = reinterpret_cast<const Size *>(data);\n> +                       s << *value;\n> +                       break;\n> +               }\n> +               case ControlTypePoint: {\n> +                       const auto *value = reinterpret_cast<const Point *>(data);\n> +                       s << *value;\n> +                       break;\n> +               }\n> +               case ControlTypeNone:\n> +               case ControlTypeString:\n> +                       break;\n> +               }\n> +\n> +               data += ControlValueSize[type];\n> +       }\n> +\n> +       if (isArray)\n> +               s << \" ]\";\n> +\n> +       return s;\n> +}\n> +\n>  /**\n>   * \\class ControlId\n>   * \\brief Control static metadata\n> -- \n> 2.50.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 89AA1BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 19 Sep 2025 10:49:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 652736B597;\n\tFri, 19 Sep 2025 12:49:58 +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 2055962C35\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 19 Sep 2025 12:49:56 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:ae7e:60b0:e249:fbe5])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 18DA23A4;\n\tFri, 19 Sep 2025 12:48:34 +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=\"AVFO+a5i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758278915;\n\tbh=14CqNQXfMIYfY7BLxp+mBYb0DYkHcZi4uLKVVvqh9fk=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=AVFO+a5iJAVxcKo2PSfrK/iKuq1SPcsBtFEnCfNxAxtHbg/p8esGVq/SyCNlw1n4J\n\tJ15SdmsijSdHIXjtmFv++YHSIIKDpSEcj1qrD4EljE8fnYG3g2WTK5p6rb7aQVqtJr\n\t9mqzgkUfs2cbnuqxsIA4HPQx5I0cauy/UNYjAhr0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250721104622.1550908-3-barnabas.pocze@ideasonboard.com>","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-3-barnabas.pocze@ideasonboard.com>","Subject":"Re: [RFC PATCH v2 02/22] libcamera: controls: Add `ControlValueView`","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 19 Sep 2025 19:49:47 +0900","Message-ID":"<175827898733.2127323.15908921583716523852@neptunite.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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]