[RFC,v2,02/22] libcamera: controls: Add `ControlValueView`
diff mbox series

Message ID 20250721104622.1550908-3-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: Add `MetadataList`
Related show

Commit Message

Barnabás Pőcze July 21, 2025, 10:46 a.m. UTC
Add `ControlValueView`, which is a non-owning read-only handle to a control
value with a very similar interface.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
changes in v2:
  * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)
    to avoid duplication
---
 include/libcamera/controls.h |  68 +++++++++
 src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------
 2 files changed, 263 insertions(+), 78 deletions(-)

Comments

Jacopo Mondi July 25, 2025, 12:55 p.m. UTC | #1
Hi Barnabás

On Mon, Jul 21, 2025 at 12:46:02PM +0200, Barnabás Pőcze wrote:
> Add `ControlValueView`, which is a non-owning read-only handle to a control
> value with a very similar interface.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
> changes in v2:
>   * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)
>     to avoid duplication
> ---
>  include/libcamera/controls.h |  68 +++++++++
>  src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------
>  2 files changed, 263 insertions(+), 78 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index b170e30cb..259fc913b 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -8,6 +8,7 @@
>  #pragma once
>
>  #include <assert.h>
> +#include <cstddef>
>  #include <map>
>  #include <optional>
>  #include <set>
> @@ -25,6 +26,7 @@
>  namespace libcamera {
>
>  class ControlValidator;
> +class ControlValueView;
>
>  enum ControlType {
>  	ControlTypeNone,
> @@ -160,6 +162,8 @@ public:
>  		    value.data(), value.size(), sizeof(typename T::value_type));
>  	}
>
> +	explicit ControlValue(const ControlValueView &cvv);
> +
>  	~ControlValue();
>
>  	ControlValue(const ControlValue &other);
> @@ -247,6 +251,70 @@ private:
>  		 std::size_t numElements, std::size_t elementSize);
>  };
>
> +class ControlValueView
> +{
> +public:
> +	constexpr ControlValueView() noexcept
> +		: type_(ControlTypeNone)
> +	{
> +	}
> +
> +	ControlValueView(const ControlValue &cv) noexcept
> +		: ControlValueView(cv.type(), cv.isArray(), cv.numElements(),
> +				   reinterpret_cast<const std::byte *>(cv.data().data()))
> +	{
> +	}
> +
> +#ifndef __DOXYGEN__
> +	// TODO: should have restricted access?
> +	ControlValueView(ControlType type, bool isArray, std::size_t numElements,
> +			 const std::byte *data) noexcept
> +		: type_(type), isArray_(isArray), numElements_(numElements),
> +		  data_(data)

I'm still a little concerned about simply copying the data address
here. If the ControlValue this view has been constructed with goes
away before its view, we would access invalid memory.

Am I too concerned ?


> +	{
> +		assert(isArray || numElements == 1);
> +	}
> +#endif
> +
> +	[[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }
> +	[[nodiscard]] ControlType type() const { return type_; }
> +	[[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }
> +	[[nodiscard]] bool isArray() const { return isArray_; }
> +	[[nodiscard]] std::size_t numElements() const { return numElements_; }
> +	[[nodiscard]] Span<const std::byte> data() const;
> +
> +	[[nodiscard]] bool operator==(const ControlValueView &other) const;
> +
> +	[[nodiscard]] bool operator!=(const ControlValueView &other) const
> +	{
> +		return !(*this == other);
> +	}
> +
> +	template<typename T>
> +	[[nodiscard]] auto get() const
> +	{
> +		using TypeInfo = details::control_type<std::remove_cv_t<T>>;
> +
> +		assert(type_ == TypeInfo::value);
> +		assert(isArray_ == (TypeInfo::size > 0));
> +
> +		if constexpr (TypeInfo::size > 0) {
> +			return T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);
> +		} else {
> +			assert(numElements_ == 1);
> +			return *reinterpret_cast<const T *>(data().data());
> +		}
> +	}
> +
> +private:
> +	ControlType type_ : 8;
> +	bool isArray_ = false;
> +	uint32_t numElements_ = 0;
> +	const std::byte *data_ = nullptr;
> +};
> +
> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v);
> +
>  class ControlId
>  {
>  public:
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 98fa7583d..a238141a5 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -106,6 +106,16 @@ ControlValue::ControlValue()
>  {
>  }
>
> +/**
> + * \brief Construct a ControlValue from a ControlValueView
> + */
> +ControlValue::ControlValue(const ControlValueView &cvv)
> +	: ControlValue()
> +{
> +	set(cvv.type(), cvv.isArray(), cvv.data().data(),
> +	    cvv.numElements(), ControlValueSize[cvv.type()]);
> +}
> +
>  /**
>   * \fn template<typename T> T ControlValue::ControlValue(const T &value)
>   * \brief Construct a ControlValue of type T
> @@ -213,84 +223,7 @@ Span<uint8_t> ControlValue::data()
>   */
>  std::string ControlValue::toString() const
>  {
> -	if (type_ == ControlTypeNone)
> -		return "<ValueType Error>";
> -
> -	const uint8_t *data = ControlValue::data().data();
> -
> -	if (type_ == ControlTypeString)
> -		return std::string(reinterpret_cast<const char *>(data),
> -				   numElements_);
> -
> -	std::string str(isArray_ ? "[ " : "");
> -
> -	for (unsigned int i = 0; i < numElements_; ++i) {
> -		switch (type_) {
> -		case ControlTypeBool: {
> -			const bool *value = reinterpret_cast<const bool *>(data);
> -			str += *value ? "true" : "false";
> -			break;
> -		}
> -		case ControlTypeByte: {
> -			const uint8_t *value = reinterpret_cast<const uint8_t *>(data);
> -			str += std::to_string(*value);
> -			break;
> -		}
> -		case ControlTypeUnsigned16: {
> -			const uint16_t *value = reinterpret_cast<const uint16_t *>(data);
> -			str += std::to_string(*value);
> -			break;
> -		}
> -		case ControlTypeUnsigned32: {
> -			const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
> -			str += std::to_string(*value);
> -			break;
> -		}
> -		case ControlTypeInteger32: {
> -			const int32_t *value = reinterpret_cast<const int32_t *>(data);
> -			str += std::to_string(*value);
> -			break;
> -		}
> -		case ControlTypeInteger64: {
> -			const int64_t *value = reinterpret_cast<const int64_t *>(data);
> -			str += std::to_string(*value);
> -			break;
> -		}
> -		case ControlTypeFloat: {
> -			const float *value = reinterpret_cast<const float *>(data);
> -			str += std::to_string(*value);
> -			break;
> -		}
> -		case ControlTypeRectangle: {
> -			const Rectangle *value = reinterpret_cast<const Rectangle *>(data);
> -			str += value->toString();
> -			break;
> -		}
> -		case ControlTypeSize: {
> -			const Size *value = reinterpret_cast<const Size *>(data);
> -			str += value->toString();
> -			break;
> -		}
> -		case ControlTypePoint: {
> -			const Point *value = reinterpret_cast<const Point *>(data);
> -			str += value->toString();
> -			break;
> -		}
> -		case ControlTypeNone:
> -		case ControlTypeString:
> -			break;
> -		}
> -
> -		if (i + 1 != numElements_)
> -			str += ", ";
> -
> -		data += ControlValueSize[type_];
> -	}
> -
> -	if (isArray_)
> -		str += " ]";
> -
> -	return str;
> +	return static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();
>  }
>
>  /**
> @@ -395,6 +328,190 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>  		storage_ = reinterpret_cast<void *>(new uint8_t[newSize]);
>  }
>
> +/**
> + * \class ControlValueView
> + * \brief A non-owning view-like type to the value of a control
> + */
> +
> +/**
> + * \fn ControlValueView::ControlValueView()
> + * \brief Construct an empty view
> + * \sa ControlValue::ControlValue()
> + */
> +
> +/**
> + * \fn ControlValueView::ControlValueView(const ControlValue &v)
> + * \brief Construct a view referring to \a v
> + *
> + * The constructed view will refer to the value stored by \a v, and
> + * thus \a v must not be modified or destroyed before the view.
> + *
> + * \sa ControlValue::ControlValue()
> + */
> +
> +/**
> + * \fn ControlValueView::operator bool() const
> + * \brief Determine if the referred value is valid
> + * \sa ControlValueView::isNone()
> + */
> +
> +/**
> + * \fn ControlType ControlValueView::type() const
> + * \copydoc ControlValue::type()
> + * \sa ControlValue::type()
> + */
> +
> +/**
> + * \fn ControlValueView::isNone() const
> + * \copydoc ControlValue::isNone()
> + * \sa ControlValue::isNone()
> + */
> +
> +/**
> + * \fn ControlValueView::isArray() const
> + * \copydoc ControlValue::isArray()
> + * \sa ControlValue::isArray()
> + */
> +
> +/**
> + * \fn ControlValueView::numElements() const
> + * \copydoc ControlValue::numElements()
> + * \sa ControlValue::numElements()
> + */
> +
> +/**
> + * \copydoc ControlValue::data()
> + * \sa ControlValue::data()
> + */
> +Span<const std::byte> ControlValueView::data() const
> +{
> +	return { data_, numElements_ * ControlValueSize[type_] };
> +}
> +
> +/**
> + * \copydoc ControlValue::operator==()
> + * \sa ControlValue::operator==()
> + * \sa ControlValueView::operator!=()
> + */
> +bool ControlValueView::operator==(const ControlValueView &other) const
> +{
> +	if (type_ != other.type_)
> +		return false;
> +
> +	if (numElements_ != other.numElements_)
> +		return false;
> +
> +	if (isArray_ != other.isArray_)
> +		return false;
> +
> +	const auto d = data();
> +
> +	return memcmp(d.data(), other.data_, d.size_bytes()) == 0;
> +}
> +
> +/**
> + * \fn ControlValueView::operator!=() const
> + * \copydoc ControlValue::operator!=()
> + * \sa ControlValue::operator!=()
> + * \sa ControlValueView::operator==()
> + */
> +
> +/**
> + * \fn template<typename T> T ControlValueView::get() const
> + * \copydoc ControlValue::get()
> + * \sa ControlValue::get()
> + */
> +
> +/**
> + * \brief Insert a text representation of a value into an output stream
> + * \sa ControlValue::toString()
> + */
> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v)
> +{
> +	const auto type = v.type();
> +	if (type == ControlTypeNone)
> +		return s << "None";
> +
> +	const auto *data = v.data().data();
> +	const auto numElements = v.numElements();
> +
> +	if (type == ControlTypeString)
> +		return s << std::string_view(reinterpret_cast<const char *>(data),
> +					     numElements);
> +
> +	const bool isArray = v.isArray();
> +	if (isArray)
> +		s << "[ ";
> +
> +	for (std::size_t i = 0; i < numElements; ++i) {
> +		if (i > 0)
> +			s << ", ";
> +
> +		switch (type) {
> +		case ControlTypeBool: {
> +			const bool *value = reinterpret_cast<const bool *>(data);
> +			s << (*value ? "true" : "false");
> +			break;
> +		}
> +		case ControlTypeByte: {
> +			const auto *value = reinterpret_cast<const uint8_t *>(data);
> +			s << static_cast<unsigned int>(*value);
> +			break;
> +		}
> +		case ControlTypeUnsigned16: {
> +			const auto *value = reinterpret_cast<const uint16_t *>(data);
> +			s << *value;
> +			break;
> +		}
> +		case ControlTypeUnsigned32: {
> +			const auto *value = reinterpret_cast<const uint32_t *>(data);
> +			s << *value;
> +			break;
> +		}
> +		case ControlTypeInteger32: {
> +			const auto *value = reinterpret_cast<const int32_t *>(data);
> +			s << *value;
> +			break;
> +		}
> +		case ControlTypeInteger64: {
> +			const auto *value = reinterpret_cast<const int64_t *>(data);
> +			s << *value;
> +			break;
> +		}
> +		case ControlTypeFloat: {
> +			const auto *value = reinterpret_cast<const float *>(data);
> +			s << std::fixed << *value;
> +			break;
> +		}
> +		case ControlTypeRectangle: {
> +			const auto *value = reinterpret_cast<const Rectangle *>(data);
> +			s << *value;
> +			break;
> +		}
> +		case ControlTypeSize: {
> +			const auto *value = reinterpret_cast<const Size *>(data);
> +			s << *value;
> +			break;
> +		}
> +		case ControlTypePoint: {
> +			const auto *value = reinterpret_cast<const Point *>(data);
> +			s << *value;
> +			break;
> +		}
> +		case ControlTypeNone:
> +		case ControlTypeString:
> +			break;
> +		}
> +
> +		data += ControlValueSize[type];
> +	}
> +
> +	if (isArray)
> +		s << " ]";
> +
> +	return s;
> +}
> +
>  /**
>   * \class ControlId
>   * \brief Control static metadata
> --
> 2.50.1
>
Barnabás Pőcze July 30, 2025, 2:48 p.m. UTC | #2
Hi

2025. 07. 25. 14:55 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Mon, Jul 21, 2025 at 12:46:02PM +0200, Barnabás Pőcze wrote:
>> Add `ControlValueView`, which is a non-owning read-only handle to a control
>> value with a very similar interface.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>> changes in v2:
>>    * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)
>>      to avoid duplication
>> ---
>>   include/libcamera/controls.h |  68 +++++++++
>>   src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------
>>   2 files changed, 263 insertions(+), 78 deletions(-)
>>
>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
>> index b170e30cb..259fc913b 100644
>> --- a/include/libcamera/controls.h
>> +++ b/include/libcamera/controls.h
>> @@ -8,6 +8,7 @@
>>   #pragma once
>>
>>   #include <assert.h>
>> +#include <cstddef>
>>   #include <map>
>>   #include <optional>
>>   #include <set>
>> @@ -25,6 +26,7 @@
>>   namespace libcamera {
>>
>>   class ControlValidator;
>> +class ControlValueView;
>>
>>   enum ControlType {
>>   	ControlTypeNone,
>> @@ -160,6 +162,8 @@ public:
>>   		    value.data(), value.size(), sizeof(typename T::value_type));
>>   	}
>>
>> +	explicit ControlValue(const ControlValueView &cvv);
>> +
>>   	~ControlValue();
>>
>>   	ControlValue(const ControlValue &other);
>> @@ -247,6 +251,70 @@ private:
>>   		 std::size_t numElements, std::size_t elementSize);
>>   };
>>
>> +class ControlValueView
>> +{
>> +public:
>> +	constexpr ControlValueView() noexcept
>> +		: type_(ControlTypeNone)
>> +	{
>> +	}
>> +
>> +	ControlValueView(const ControlValue &cv) noexcept
>> +		: ControlValueView(cv.type(), cv.isArray(), cv.numElements(),
>> +				   reinterpret_cast<const std::byte *>(cv.data().data()))
>> +	{
>> +	}
>> +
>> +#ifndef __DOXYGEN__
>> +	// TODO: should have restricted access?
>> +	ControlValueView(ControlType type, bool isArray, std::size_t numElements,
>> +			 const std::byte *data) noexcept
>> +		: type_(type), isArray_(isArray), numElements_(numElements),
>> +		  data_(data)
> 
> I'm still a little concerned about simply copying the data address
> here. If the ControlValue this view has been constructed with goes
> away before its view, we would access invalid memory.
> 
> Am I too concerned ?

That is true. I am not sure what could be done. I don't think copying is an
acceptable solution in this scenario. Anything else seems way too complicated
to me, given that this type is currently only used for iterating a `MetadataList`,
or for certain function arguments (but the latter use case is safe).


Regards,
Barnabás Pőcze


> 
> 
>> +	{
>> +		assert(isArray || numElements == 1);
>> +	}
>> +#endif
>> +
>> +	[[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }
>> +	[[nodiscard]] ControlType type() const { return type_; }
>> +	[[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }
>> +	[[nodiscard]] bool isArray() const { return isArray_; }
>> +	[[nodiscard]] std::size_t numElements() const { return numElements_; }
>> +	[[nodiscard]] Span<const std::byte> data() const;
>> +
>> +	[[nodiscard]] bool operator==(const ControlValueView &other) const;
>> +
>> +	[[nodiscard]] bool operator!=(const ControlValueView &other) const
>> +	{
>> +		return !(*this == other);
>> +	}
>> +
>> +	template<typename T>
>> +	[[nodiscard]] auto get() const
>> +	{
>> +		using TypeInfo = details::control_type<std::remove_cv_t<T>>;
>> +
>> +		assert(type_ == TypeInfo::value);
>> +		assert(isArray_ == (TypeInfo::size > 0));
>> +
>> +		if constexpr (TypeInfo::size > 0) {
>> +			return T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);
>> +		} else {
>> +			assert(numElements_ == 1);
>> +			return *reinterpret_cast<const T *>(data().data());
>> +		}
>> +	}
>> +
>> +private:
>> +	ControlType type_ : 8;
>> +	bool isArray_ = false;
>> +	uint32_t numElements_ = 0;
>> +	const std::byte *data_ = nullptr;
>> +};
>> +
>> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v);
>> +
>>   class ControlId
>>   {
>>   public:
>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> index 98fa7583d..a238141a5 100644
>> --- a/src/libcamera/controls.cpp
>> +++ b/src/libcamera/controls.cpp
>> @@ -106,6 +106,16 @@ ControlValue::ControlValue()
>>   {
>>   }
>>
>> +/**
>> + * \brief Construct a ControlValue from a ControlValueView
>> + */
>> +ControlValue::ControlValue(const ControlValueView &cvv)
>> +	: ControlValue()
>> +{
>> +	set(cvv.type(), cvv.isArray(), cvv.data().data(),
>> +	    cvv.numElements(), ControlValueSize[cvv.type()]);
>> +}
>> +
>>   /**
>>    * \fn template<typename T> T ControlValue::ControlValue(const T &value)
>>    * \brief Construct a ControlValue of type T
>> @@ -213,84 +223,7 @@ Span<uint8_t> ControlValue::data()
>>    */
>>   std::string ControlValue::toString() const
>>   {
>> -	if (type_ == ControlTypeNone)
>> -		return "<ValueType Error>";
>> -
>> -	const uint8_t *data = ControlValue::data().data();
>> -
>> -	if (type_ == ControlTypeString)
>> -		return std::string(reinterpret_cast<const char *>(data),
>> -				   numElements_);
>> -
>> -	std::string str(isArray_ ? "[ " : "");
>> -
>> -	for (unsigned int i = 0; i < numElements_; ++i) {
>> -		switch (type_) {
>> -		case ControlTypeBool: {
>> -			const bool *value = reinterpret_cast<const bool *>(data);
>> -			str += *value ? "true" : "false";
>> -			break;
>> -		}
>> -		case ControlTypeByte: {
>> -			const uint8_t *value = reinterpret_cast<const uint8_t *>(data);
>> -			str += std::to_string(*value);
>> -			break;
>> -		}
>> -		case ControlTypeUnsigned16: {
>> -			const uint16_t *value = reinterpret_cast<const uint16_t *>(data);
>> -			str += std::to_string(*value);
>> -			break;
>> -		}
>> -		case ControlTypeUnsigned32: {
>> -			const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
>> -			str += std::to_string(*value);
>> -			break;
>> -		}
>> -		case ControlTypeInteger32: {
>> -			const int32_t *value = reinterpret_cast<const int32_t *>(data);
>> -			str += std::to_string(*value);
>> -			break;
>> -		}
>> -		case ControlTypeInteger64: {
>> -			const int64_t *value = reinterpret_cast<const int64_t *>(data);
>> -			str += std::to_string(*value);
>> -			break;
>> -		}
>> -		case ControlTypeFloat: {
>> -			const float *value = reinterpret_cast<const float *>(data);
>> -			str += std::to_string(*value);
>> -			break;
>> -		}
>> -		case ControlTypeRectangle: {
>> -			const Rectangle *value = reinterpret_cast<const Rectangle *>(data);
>> -			str += value->toString();
>> -			break;
>> -		}
>> -		case ControlTypeSize: {
>> -			const Size *value = reinterpret_cast<const Size *>(data);
>> -			str += value->toString();
>> -			break;
>> -		}
>> -		case ControlTypePoint: {
>> -			const Point *value = reinterpret_cast<const Point *>(data);
>> -			str += value->toString();
>> -			break;
>> -		}
>> -		case ControlTypeNone:
>> -		case ControlTypeString:
>> -			break;
>> -		}
>> -
>> -		if (i + 1 != numElements_)
>> -			str += ", ";
>> -
>> -		data += ControlValueSize[type_];
>> -	}
>> -
>> -	if (isArray_)
>> -		str += " ]";
>> -
>> -	return str;
>> +	return static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();
>>   }
>>
>>   /**
>> @@ -395,6 +328,190 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>>   		storage_ = reinterpret_cast<void *>(new uint8_t[newSize]);
>>   }
>>
>> +/**
>> + * \class ControlValueView
>> + * \brief A non-owning view-like type to the value of a control
>> + */
>> +
>> +/**
>> + * \fn ControlValueView::ControlValueView()
>> + * \brief Construct an empty view
>> + * \sa ControlValue::ControlValue()
>> + */
>> +
>> +/**
>> + * \fn ControlValueView::ControlValueView(const ControlValue &v)
>> + * \brief Construct a view referring to \a v
>> + *
>> + * The constructed view will refer to the value stored by \a v, and
>> + * thus \a v must not be modified or destroyed before the view.
>> + *
>> + * \sa ControlValue::ControlValue()
>> + */
>> +
>> +/**
>> + * \fn ControlValueView::operator bool() const
>> + * \brief Determine if the referred value is valid
>> + * \sa ControlValueView::isNone()
>> + */
>> +
>> +/**
>> + * \fn ControlType ControlValueView::type() const
>> + * \copydoc ControlValue::type()
>> + * \sa ControlValue::type()
>> + */
>> +
>> +/**
>> + * \fn ControlValueView::isNone() const
>> + * \copydoc ControlValue::isNone()
>> + * \sa ControlValue::isNone()
>> + */
>> +
>> +/**
>> + * \fn ControlValueView::isArray() const
>> + * \copydoc ControlValue::isArray()
>> + * \sa ControlValue::isArray()
>> + */
>> +
>> +/**
>> + * \fn ControlValueView::numElements() const
>> + * \copydoc ControlValue::numElements()
>> + * \sa ControlValue::numElements()
>> + */
>> +
>> +/**
>> + * \copydoc ControlValue::data()
>> + * \sa ControlValue::data()
>> + */
>> +Span<const std::byte> ControlValueView::data() const
>> +{
>> +	return { data_, numElements_ * ControlValueSize[type_] };
>> +}
>> +
>> +/**
>> + * \copydoc ControlValue::operator==()
>> + * \sa ControlValue::operator==()
>> + * \sa ControlValueView::operator!=()
>> + */
>> +bool ControlValueView::operator==(const ControlValueView &other) const
>> +{
>> +	if (type_ != other.type_)
>> +		return false;
>> +
>> +	if (numElements_ != other.numElements_)
>> +		return false;
>> +
>> +	if (isArray_ != other.isArray_)
>> +		return false;
>> +
>> +	const auto d = data();
>> +
>> +	return memcmp(d.data(), other.data_, d.size_bytes()) == 0;
>> +}
>> +
>> +/**
>> + * \fn ControlValueView::operator!=() const
>> + * \copydoc ControlValue::operator!=()
>> + * \sa ControlValue::operator!=()
>> + * \sa ControlValueView::operator==()
>> + */
>> +
>> +/**
>> + * \fn template<typename T> T ControlValueView::get() const
>> + * \copydoc ControlValue::get()
>> + * \sa ControlValue::get()
>> + */
>> +
>> +/**
>> + * \brief Insert a text representation of a value into an output stream
>> + * \sa ControlValue::toString()
>> + */
>> +std::ostream &operator<<(std::ostream &s, const ControlValueView &v)
>> +{
>> +	const auto type = v.type();
>> +	if (type == ControlTypeNone)
>> +		return s << "None";
>> +
>> +	const auto *data = v.data().data();
>> +	const auto numElements = v.numElements();
>> +
>> +	if (type == ControlTypeString)
>> +		return s << std::string_view(reinterpret_cast<const char *>(data),
>> +					     numElements);
>> +
>> +	const bool isArray = v.isArray();
>> +	if (isArray)
>> +		s << "[ ";
>> +
>> +	for (std::size_t i = 0; i < numElements; ++i) {
>> +		if (i > 0)
>> +			s << ", ";
>> +
>> +		switch (type) {
>> +		case ControlTypeBool: {
>> +			const bool *value = reinterpret_cast<const bool *>(data);
>> +			s << (*value ? "true" : "false");
>> +			break;
>> +		}
>> +		case ControlTypeByte: {
>> +			const auto *value = reinterpret_cast<const uint8_t *>(data);
>> +			s << static_cast<unsigned int>(*value);
>> +			break;
>> +		}
>> +		case ControlTypeUnsigned16: {
>> +			const auto *value = reinterpret_cast<const uint16_t *>(data);
>> +			s << *value;
>> +			break;
>> +		}
>> +		case ControlTypeUnsigned32: {
>> +			const auto *value = reinterpret_cast<const uint32_t *>(data);
>> +			s << *value;
>> +			break;
>> +		}
>> +		case ControlTypeInteger32: {
>> +			const auto *value = reinterpret_cast<const int32_t *>(data);
>> +			s << *value;
>> +			break;
>> +		}
>> +		case ControlTypeInteger64: {
>> +			const auto *value = reinterpret_cast<const int64_t *>(data);
>> +			s << *value;
>> +			break;
>> +		}
>> +		case ControlTypeFloat: {
>> +			const auto *value = reinterpret_cast<const float *>(data);
>> +			s << std::fixed << *value;
>> +			break;
>> +		}
>> +		case ControlTypeRectangle: {
>> +			const auto *value = reinterpret_cast<const Rectangle *>(data);
>> +			s << *value;
>> +			break;
>> +		}
>> +		case ControlTypeSize: {
>> +			const auto *value = reinterpret_cast<const Size *>(data);
>> +			s << *value;
>> +			break;
>> +		}
>> +		case ControlTypePoint: {
>> +			const auto *value = reinterpret_cast<const Point *>(data);
>> +			s << *value;
>> +			break;
>> +		}
>> +		case ControlTypeNone:
>> +		case ControlTypeString:
>> +			break;
>> +		}
>> +
>> +		data += ControlValueSize[type];
>> +	}
>> +
>> +	if (isArray)
>> +		s << " ]";
>> +
>> +	return s;
>> +}
>> +
>>   /**
>>    * \class ControlId
>>    * \brief Control static metadata
>> --
>> 2.50.1
>>
Jacopo Mondi Aug. 3, 2025, 6:35 p.m. UTC | #3
Hi Barnabás

On Wed, Jul 30, 2025 at 04:48:33PM +0200, Barnabás Pőcze wrote:
> Hi
>
> 2025. 07. 25. 14:55 keltezéssel, Jacopo Mondi írta:
> > Hi Barnabás
> >
> > On Mon, Jul 21, 2025 at 12:46:02PM +0200, Barnabás Pőcze wrote:
> > > Add `ControlValueView`, which is a non-owning read-only handle to a control
> > > value with a very similar interface.
> > >
> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > > ---
> > > changes in v2:
> > >    * rewrite `ControlValue::toString()` in terms of `operator<<(std::ostream&, const ControlValueView&)
> > >      to avoid duplication
> > > ---
> > >   include/libcamera/controls.h |  68 +++++++++
> > >   src/libcamera/controls.cpp   | 273 +++++++++++++++++++++++++----------
> > >   2 files changed, 263 insertions(+), 78 deletions(-)
> > >
> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > index b170e30cb..259fc913b 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -8,6 +8,7 @@
> > >   #pragma once
> > >
> > >   #include <assert.h>
> > > +#include <cstddef>
> > >   #include <map>
> > >   #include <optional>
> > >   #include <set>
> > > @@ -25,6 +26,7 @@
> > >   namespace libcamera {
> > >
> > >   class ControlValidator;
> > > +class ControlValueView;
> > >
> > >   enum ControlType {
> > >   	ControlTypeNone,
> > > @@ -160,6 +162,8 @@ public:
> > >   		    value.data(), value.size(), sizeof(typename T::value_type));
> > >   	}
> > >
> > > +	explicit ControlValue(const ControlValueView &cvv);
> > > +
> > >   	~ControlValue();
> > >
> > >   	ControlValue(const ControlValue &other);
> > > @@ -247,6 +251,70 @@ private:
> > >   		 std::size_t numElements, std::size_t elementSize);
> > >   };
> > >
> > > +class ControlValueView
> > > +{
> > > +public:
> > > +	constexpr ControlValueView() noexcept
> > > +		: type_(ControlTypeNone)
> > > +	{
> > > +	}
> > > +
> > > +	ControlValueView(const ControlValue &cv) noexcept
> > > +		: ControlValueView(cv.type(), cv.isArray(), cv.numElements(),
> > > +				   reinterpret_cast<const std::byte *>(cv.data().data()))
> > > +	{
> > > +	}
> > > +
> > > +#ifndef __DOXYGEN__
> > > +	// TODO: should have restricted access?
> > > +	ControlValueView(ControlType type, bool isArray, std::size_t numElements,
> > > +			 const std::byte *data) noexcept
> > > +		: type_(type), isArray_(isArray), numElements_(numElements),
> > > +		  data_(data)
> >
> > I'm still a little concerned about simply copying the data address
> > here. If the ControlValue this view has been constructed with goes
> > away before its view, we would access invalid memory.
> >
> > Am I too concerned ?
>
> That is true. I am not sure what could be done. I don't think copying is an
> acceptable solution in this scenario. Anything else seems way too complicated
> to me, given that this type is currently only used for iterating a `MetadataList`,
> or for certain function arguments (but the latter use case is safe).

I agree, actually I'm wondering if not all of our ControlValue should
actually be views over a storage space allocated somewhere else
(likely in a ControlList or a MetadataList).

I went for a little experiment, renaming the existing ControlValue
class to ControlStorage and make your new ControlValueView the
ControlValue type we have so far used.

My main goal was to make ControlList and MetadataList
use the same type in their public interface, however I think there is
value in having a "storage" control type and "view" control type to
clearly define the data ownership model.

I think I would also like to test if we can even make the newly introduced
ControlStorage (was ControlValue) a move-only interface: this won't
have any performance improvement as the "move" would anyway be a
memcpy but it would make it clear there is only a single storage for a
control value at the time.

I think it will also make easier to serialize ControlList as your
series does with MetadataList, as ControlValue (was ControlValueView)
is already instumented for being re-constructed from a serialized
buffer in your series.

For reference:
https://gitlab.freedesktop.org/camera/libcamera/-/commits/b4/control_storage

Still WIP as the CI loops on the pi IPA, I'll work on this but in the
meantime comment are appreciated. I might have missed some trivial use
case for which always operating a ControlList/MetadataList::get() with
views is not desirable.

Thanks
  j

>
>
> Regards,
> Barnabás Pőcze
>
>
> >
> >
> > > +	{
> > > +		assert(isArray || numElements == 1);
> > > +	}
> > > +#endif
> > > +
> > > +	[[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }
> > > +	[[nodiscard]] ControlType type() const { return type_; }
> > > +	[[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }
> > > +	[[nodiscard]] bool isArray() const { return isArray_; }
> > > +	[[nodiscard]] std::size_t numElements() const { return numElements_; }
> > > +	[[nodiscard]] Span<const std::byte> data() const;
> > > +
> > > +	[[nodiscard]] bool operator==(const ControlValueView &other) const;
> > > +
> > > +	[[nodiscard]] bool operator!=(const ControlValueView &other) const
> > > +	{
> > > +		return !(*this == other);
> > > +	}
> > > +
> > > +	template<typename T>
> > > +	[[nodiscard]] auto get() const
> > > +	{
> > > +		using TypeInfo = details::control_type<std::remove_cv_t<T>>;
> > > +
> > > +		assert(type_ == TypeInfo::value);
> > > +		assert(isArray_ == (TypeInfo::size > 0));
> > > +
> > > +		if constexpr (TypeInfo::size > 0) {
> > > +			return T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);
> > > +		} else {
> > > +			assert(numElements_ == 1);
> > > +			return *reinterpret_cast<const T *>(data().data());
> > > +		}
> > > +	}
> > > +
> > > +private:
> > > +	ControlType type_ : 8;
> > > +	bool isArray_ = false;
> > > +	uint32_t numElements_ = 0;
> > > +	const std::byte *data_ = nullptr;
> > > +};
> > > +
> > > +std::ostream &operator<<(std::ostream &s, const ControlValueView &v);
> > > +
> > >   class ControlId
> > >   {
> > >   public:
> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > > index 98fa7583d..a238141a5 100644
> > > --- a/src/libcamera/controls.cpp
> > > +++ b/src/libcamera/controls.cpp
> > > @@ -106,6 +106,16 @@ ControlValue::ControlValue()
> > >   {
> > >   }
> > >
> > > +/**
> > > + * \brief Construct a ControlValue from a ControlValueView
> > > + */
> > > +ControlValue::ControlValue(const ControlValueView &cvv)
> > > +	: ControlValue()
> > > +{
> > > +	set(cvv.type(), cvv.isArray(), cvv.data().data(),
> > > +	    cvv.numElements(), ControlValueSize[cvv.type()]);
> > > +}
> > > +
> > >   /**
> > >    * \fn template<typename T> T ControlValue::ControlValue(const T &value)
> > >    * \brief Construct a ControlValue of type T
> > > @@ -213,84 +223,7 @@ Span<uint8_t> ControlValue::data()
> > >    */
> > >   std::string ControlValue::toString() const
> > >   {
> > > -	if (type_ == ControlTypeNone)
> > > -		return "<ValueType Error>";
> > > -
> > > -	const uint8_t *data = ControlValue::data().data();
> > > -
> > > -	if (type_ == ControlTypeString)
> > > -		return std::string(reinterpret_cast<const char *>(data),
> > > -				   numElements_);
> > > -
> > > -	std::string str(isArray_ ? "[ " : "");
> > > -
> > > -	for (unsigned int i = 0; i < numElements_; ++i) {
> > > -		switch (type_) {
> > > -		case ControlTypeBool: {
> > > -			const bool *value = reinterpret_cast<const bool *>(data);
> > > -			str += *value ? "true" : "false";
> > > -			break;
> > > -		}
> > > -		case ControlTypeByte: {
> > > -			const uint8_t *value = reinterpret_cast<const uint8_t *>(data);
> > > -			str += std::to_string(*value);
> > > -			break;
> > > -		}
> > > -		case ControlTypeUnsigned16: {
> > > -			const uint16_t *value = reinterpret_cast<const uint16_t *>(data);
> > > -			str += std::to_string(*value);
> > > -			break;
> > > -		}
> > > -		case ControlTypeUnsigned32: {
> > > -			const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
> > > -			str += std::to_string(*value);
> > > -			break;
> > > -		}
> > > -		case ControlTypeInteger32: {
> > > -			const int32_t *value = reinterpret_cast<const int32_t *>(data);
> > > -			str += std::to_string(*value);
> > > -			break;
> > > -		}
> > > -		case ControlTypeInteger64: {
> > > -			const int64_t *value = reinterpret_cast<const int64_t *>(data);
> > > -			str += std::to_string(*value);
> > > -			break;
> > > -		}
> > > -		case ControlTypeFloat: {
> > > -			const float *value = reinterpret_cast<const float *>(data);
> > > -			str += std::to_string(*value);
> > > -			break;
> > > -		}
> > > -		case ControlTypeRectangle: {
> > > -			const Rectangle *value = reinterpret_cast<const Rectangle *>(data);
> > > -			str += value->toString();
> > > -			break;
> > > -		}
> > > -		case ControlTypeSize: {
> > > -			const Size *value = reinterpret_cast<const Size *>(data);
> > > -			str += value->toString();
> > > -			break;
> > > -		}
> > > -		case ControlTypePoint: {
> > > -			const Point *value = reinterpret_cast<const Point *>(data);
> > > -			str += value->toString();
> > > -			break;
> > > -		}
> > > -		case ControlTypeNone:
> > > -		case ControlTypeString:
> > > -			break;
> > > -		}
> > > -
> > > -		if (i + 1 != numElements_)
> > > -			str += ", ";
> > > -
> > > -		data += ControlValueSize[type_];
> > > -	}
> > > -
> > > -	if (isArray_)
> > > -		str += " ]";
> > > -
> > > -	return str;
> > > +	return static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();
> > >   }
> > >
> > >   /**
> > > @@ -395,6 +328,190 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
> > >   		storage_ = reinterpret_cast<void *>(new uint8_t[newSize]);
> > >   }
> > >
> > > +/**
> > > + * \class ControlValueView
> > > + * \brief A non-owning view-like type to the value of a control
> > > + */
> > > +
> > > +/**
> > > + * \fn ControlValueView::ControlValueView()
> > > + * \brief Construct an empty view
> > > + * \sa ControlValue::ControlValue()
> > > + */
> > > +
> > > +/**
> > > + * \fn ControlValueView::ControlValueView(const ControlValue &v)
> > > + * \brief Construct a view referring to \a v
> > > + *
> > > + * The constructed view will refer to the value stored by \a v, and
> > > + * thus \a v must not be modified or destroyed before the view.
> > > + *
> > > + * \sa ControlValue::ControlValue()
> > > + */
> > > +
> > > +/**
> > > + * \fn ControlValueView::operator bool() const
> > > + * \brief Determine if the referred value is valid
> > > + * \sa ControlValueView::isNone()
> > > + */
> > > +
> > > +/**
> > > + * \fn ControlType ControlValueView::type() const
> > > + * \copydoc ControlValue::type()
> > > + * \sa ControlValue::type()
> > > + */
> > > +
> > > +/**
> > > + * \fn ControlValueView::isNone() const
> > > + * \copydoc ControlValue::isNone()
> > > + * \sa ControlValue::isNone()
> > > + */
> > > +
> > > +/**
> > > + * \fn ControlValueView::isArray() const
> > > + * \copydoc ControlValue::isArray()
> > > + * \sa ControlValue::isArray()
> > > + */
> > > +
> > > +/**
> > > + * \fn ControlValueView::numElements() const
> > > + * \copydoc ControlValue::numElements()
> > > + * \sa ControlValue::numElements()
> > > + */
> > > +
> > > +/**
> > > + * \copydoc ControlValue::data()
> > > + * \sa ControlValue::data()
> > > + */
> > > +Span<const std::byte> ControlValueView::data() const
> > > +{
> > > +	return { data_, numElements_ * ControlValueSize[type_] };
> > > +}
> > > +
> > > +/**
> > > + * \copydoc ControlValue::operator==()
> > > + * \sa ControlValue::operator==()
> > > + * \sa ControlValueView::operator!=()
> > > + */
> > > +bool ControlValueView::operator==(const ControlValueView &other) const
> > > +{
> > > +	if (type_ != other.type_)
> > > +		return false;
> > > +
> > > +	if (numElements_ != other.numElements_)
> > > +		return false;
> > > +
> > > +	if (isArray_ != other.isArray_)
> > > +		return false;
> > > +
> > > +	const auto d = data();
> > > +
> > > +	return memcmp(d.data(), other.data_, d.size_bytes()) == 0;
> > > +}
> > > +
> > > +/**
> > > + * \fn ControlValueView::operator!=() const
> > > + * \copydoc ControlValue::operator!=()
> > > + * \sa ControlValue::operator!=()
> > > + * \sa ControlValueView::operator==()
> > > + */
> > > +
> > > +/**
> > > + * \fn template<typename T> T ControlValueView::get() const
> > > + * \copydoc ControlValue::get()
> > > + * \sa ControlValue::get()
> > > + */
> > > +
> > > +/**
> > > + * \brief Insert a text representation of a value into an output stream
> > > + * \sa ControlValue::toString()
> > > + */
> > > +std::ostream &operator<<(std::ostream &s, const ControlValueView &v)
> > > +{
> > > +	const auto type = v.type();
> > > +	if (type == ControlTypeNone)
> > > +		return s << "None";
> > > +
> > > +	const auto *data = v.data().data();
> > > +	const auto numElements = v.numElements();
> > > +
> > > +	if (type == ControlTypeString)
> > > +		return s << std::string_view(reinterpret_cast<const char *>(data),
> > > +					     numElements);
> > > +
> > > +	const bool isArray = v.isArray();
> > > +	if (isArray)
> > > +		s << "[ ";
> > > +
> > > +	for (std::size_t i = 0; i < numElements; ++i) {
> > > +		if (i > 0)
> > > +			s << ", ";
> > > +
> > > +		switch (type) {
> > > +		case ControlTypeBool: {
> > > +			const bool *value = reinterpret_cast<const bool *>(data);
> > > +			s << (*value ? "true" : "false");
> > > +			break;
> > > +		}
> > > +		case ControlTypeByte: {
> > > +			const auto *value = reinterpret_cast<const uint8_t *>(data);
> > > +			s << static_cast<unsigned int>(*value);
> > > +			break;
> > > +		}
> > > +		case ControlTypeUnsigned16: {
> > > +			const auto *value = reinterpret_cast<const uint16_t *>(data);
> > > +			s << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypeUnsigned32: {
> > > +			const auto *value = reinterpret_cast<const uint32_t *>(data);
> > > +			s << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypeInteger32: {
> > > +			const auto *value = reinterpret_cast<const int32_t *>(data);
> > > +			s << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypeInteger64: {
> > > +			const auto *value = reinterpret_cast<const int64_t *>(data);
> > > +			s << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypeFloat: {
> > > +			const auto *value = reinterpret_cast<const float *>(data);
> > > +			s << std::fixed << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypeRectangle: {
> > > +			const auto *value = reinterpret_cast<const Rectangle *>(data);
> > > +			s << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypeSize: {
> > > +			const auto *value = reinterpret_cast<const Size *>(data);
> > > +			s << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypePoint: {
> > > +			const auto *value = reinterpret_cast<const Point *>(data);
> > > +			s << *value;
> > > +			break;
> > > +		}
> > > +		case ControlTypeNone:
> > > +		case ControlTypeString:
> > > +			break;
> > > +		}
> > > +
> > > +		data += ControlValueSize[type];
> > > +	}
> > > +
> > > +	if (isArray)
> > > +		s << " ]";
> > > +
> > > +	return s;
> > > +}
> > > +
> > >   /**
> > >    * \class ControlId
> > >    * \brief Control static metadata
> > > --
> > > 2.50.1
> > >
>

Patch
diff mbox series

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index b170e30cb..259fc913b 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -8,6 +8,7 @@ 
 #pragma once
 
 #include <assert.h>
+#include <cstddef>
 #include <map>
 #include <optional>
 #include <set>
@@ -25,6 +26,7 @@ 
 namespace libcamera {
 
 class ControlValidator;
+class ControlValueView;
 
 enum ControlType {
 	ControlTypeNone,
@@ -160,6 +162,8 @@  public:
 		    value.data(), value.size(), sizeof(typename T::value_type));
 	}
 
+	explicit ControlValue(const ControlValueView &cvv);
+
 	~ControlValue();
 
 	ControlValue(const ControlValue &other);
@@ -247,6 +251,70 @@  private:
 		 std::size_t numElements, std::size_t elementSize);
 };
 
+class ControlValueView
+{
+public:
+	constexpr ControlValueView() noexcept
+		: type_(ControlTypeNone)
+	{
+	}
+
+	ControlValueView(const ControlValue &cv) noexcept
+		: ControlValueView(cv.type(), cv.isArray(), cv.numElements(),
+				   reinterpret_cast<const std::byte *>(cv.data().data()))
+	{
+	}
+
+#ifndef __DOXYGEN__
+	// TODO: should have restricted access?
+	ControlValueView(ControlType type, bool isArray, std::size_t numElements,
+			 const std::byte *data) noexcept
+		: type_(type), isArray_(isArray), numElements_(numElements),
+		  data_(data)
+	{
+		assert(isArray || numElements == 1);
+	}
+#endif
+
+	[[nodiscard]] explicit operator bool() const { return type_ != ControlTypeNone; }
+	[[nodiscard]] ControlType type() const { return type_; }
+	[[nodiscard]] bool isNone() const { return type_ == ControlTypeNone; }
+	[[nodiscard]] bool isArray() const { return isArray_; }
+	[[nodiscard]] std::size_t numElements() const { return numElements_; }
+	[[nodiscard]] Span<const std::byte> data() const;
+
+	[[nodiscard]] bool operator==(const ControlValueView &other) const;
+
+	[[nodiscard]] bool operator!=(const ControlValueView &other) const
+	{
+		return !(*this == other);
+	}
+
+	template<typename T>
+	[[nodiscard]] auto get() const
+	{
+		using TypeInfo = details::control_type<std::remove_cv_t<T>>;
+
+		assert(type_ == TypeInfo::value);
+		assert(isArray_ == (TypeInfo::size > 0));
+
+		if constexpr (TypeInfo::size > 0) {
+			return T(reinterpret_cast<const typename T::value_type *>(data().data()), numElements_);
+		} else {
+			assert(numElements_ == 1);
+			return *reinterpret_cast<const T *>(data().data());
+		}
+	}
+
+private:
+	ControlType type_ : 8;
+	bool isArray_ = false;
+	uint32_t numElements_ = 0;
+	const std::byte *data_ = nullptr;
+};
+
+std::ostream &operator<<(std::ostream &s, const ControlValueView &v);
+
 class ControlId
 {
 public:
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 98fa7583d..a238141a5 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -106,6 +106,16 @@  ControlValue::ControlValue()
 {
 }
 
+/**
+ * \brief Construct a ControlValue from a ControlValueView
+ */
+ControlValue::ControlValue(const ControlValueView &cvv)
+	: ControlValue()
+{
+	set(cvv.type(), cvv.isArray(), cvv.data().data(),
+	    cvv.numElements(), ControlValueSize[cvv.type()]);
+}
+
 /**
  * \fn template<typename T> T ControlValue::ControlValue(const T &value)
  * \brief Construct a ControlValue of type T
@@ -213,84 +223,7 @@  Span<uint8_t> ControlValue::data()
  */
 std::string ControlValue::toString() const
 {
-	if (type_ == ControlTypeNone)
-		return "<ValueType Error>";
-
-	const uint8_t *data = ControlValue::data().data();
-
-	if (type_ == ControlTypeString)
-		return std::string(reinterpret_cast<const char *>(data),
-				   numElements_);
-
-	std::string str(isArray_ ? "[ " : "");
-
-	for (unsigned int i = 0; i < numElements_; ++i) {
-		switch (type_) {
-		case ControlTypeBool: {
-			const bool *value = reinterpret_cast<const bool *>(data);
-			str += *value ? "true" : "false";
-			break;
-		}
-		case ControlTypeByte: {
-			const uint8_t *value = reinterpret_cast<const uint8_t *>(data);
-			str += std::to_string(*value);
-			break;
-		}
-		case ControlTypeUnsigned16: {
-			const uint16_t *value = reinterpret_cast<const uint16_t *>(data);
-			str += std::to_string(*value);
-			break;
-		}
-		case ControlTypeUnsigned32: {
-			const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
-			str += std::to_string(*value);
-			break;
-		}
-		case ControlTypeInteger32: {
-			const int32_t *value = reinterpret_cast<const int32_t *>(data);
-			str += std::to_string(*value);
-			break;
-		}
-		case ControlTypeInteger64: {
-			const int64_t *value = reinterpret_cast<const int64_t *>(data);
-			str += std::to_string(*value);
-			break;
-		}
-		case ControlTypeFloat: {
-			const float *value = reinterpret_cast<const float *>(data);
-			str += std::to_string(*value);
-			break;
-		}
-		case ControlTypeRectangle: {
-			const Rectangle *value = reinterpret_cast<const Rectangle *>(data);
-			str += value->toString();
-			break;
-		}
-		case ControlTypeSize: {
-			const Size *value = reinterpret_cast<const Size *>(data);
-			str += value->toString();
-			break;
-		}
-		case ControlTypePoint: {
-			const Point *value = reinterpret_cast<const Point *>(data);
-			str += value->toString();
-			break;
-		}
-		case ControlTypeNone:
-		case ControlTypeString:
-			break;
-		}
-
-		if (i + 1 != numElements_)
-			str += ", ";
-
-		data += ControlValueSize[type_];
-	}
-
-	if (isArray_)
-		str += " ]";
-
-	return str;
+	return static_cast<std::ostringstream&&>(std::ostringstream{} << ControlValueView(*this)).str();
 }
 
 /**
@@ -395,6 +328,190 @@  void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
 		storage_ = reinterpret_cast<void *>(new uint8_t[newSize]);
 }
 
+/**
+ * \class ControlValueView
+ * \brief A non-owning view-like type to the value of a control
+ */
+
+/**
+ * \fn ControlValueView::ControlValueView()
+ * \brief Construct an empty view
+ * \sa ControlValue::ControlValue()
+ */
+
+/**
+ * \fn ControlValueView::ControlValueView(const ControlValue &v)
+ * \brief Construct a view referring to \a v
+ *
+ * The constructed view will refer to the value stored by \a v, and
+ * thus \a v must not be modified or destroyed before the view.
+ *
+ * \sa ControlValue::ControlValue()
+ */
+
+/**
+ * \fn ControlValueView::operator bool() const
+ * \brief Determine if the referred value is valid
+ * \sa ControlValueView::isNone()
+ */
+
+/**
+ * \fn ControlType ControlValueView::type() const
+ * \copydoc ControlValue::type()
+ * \sa ControlValue::type()
+ */
+
+/**
+ * \fn ControlValueView::isNone() const
+ * \copydoc ControlValue::isNone()
+ * \sa ControlValue::isNone()
+ */
+
+/**
+ * \fn ControlValueView::isArray() const
+ * \copydoc ControlValue::isArray()
+ * \sa ControlValue::isArray()
+ */
+
+/**
+ * \fn ControlValueView::numElements() const
+ * \copydoc ControlValue::numElements()
+ * \sa ControlValue::numElements()
+ */
+
+/**
+ * \copydoc ControlValue::data()
+ * \sa ControlValue::data()
+ */
+Span<const std::byte> ControlValueView::data() const
+{
+	return { data_, numElements_ * ControlValueSize[type_] };
+}
+
+/**
+ * \copydoc ControlValue::operator==()
+ * \sa ControlValue::operator==()
+ * \sa ControlValueView::operator!=()
+ */
+bool ControlValueView::operator==(const ControlValueView &other) const
+{
+	if (type_ != other.type_)
+		return false;
+
+	if (numElements_ != other.numElements_)
+		return false;
+
+	if (isArray_ != other.isArray_)
+		return false;
+
+	const auto d = data();
+
+	return memcmp(d.data(), other.data_, d.size_bytes()) == 0;
+}
+
+/**
+ * \fn ControlValueView::operator!=() const
+ * \copydoc ControlValue::operator!=()
+ * \sa ControlValue::operator!=()
+ * \sa ControlValueView::operator==()
+ */
+
+/**
+ * \fn template<typename T> T ControlValueView::get() const
+ * \copydoc ControlValue::get()
+ * \sa ControlValue::get()
+ */
+
+/**
+ * \brief Insert a text representation of a value into an output stream
+ * \sa ControlValue::toString()
+ */
+std::ostream &operator<<(std::ostream &s, const ControlValueView &v)
+{
+	const auto type = v.type();
+	if (type == ControlTypeNone)
+		return s << "None";
+
+	const auto *data = v.data().data();
+	const auto numElements = v.numElements();
+
+	if (type == ControlTypeString)
+		return s << std::string_view(reinterpret_cast<const char *>(data),
+					     numElements);
+
+	const bool isArray = v.isArray();
+	if (isArray)
+		s << "[ ";
+
+	for (std::size_t i = 0; i < numElements; ++i) {
+		if (i > 0)
+			s << ", ";
+
+		switch (type) {
+		case ControlTypeBool: {
+			const bool *value = reinterpret_cast<const bool *>(data);
+			s << (*value ? "true" : "false");
+			break;
+		}
+		case ControlTypeByte: {
+			const auto *value = reinterpret_cast<const uint8_t *>(data);
+			s << static_cast<unsigned int>(*value);
+			break;
+		}
+		case ControlTypeUnsigned16: {
+			const auto *value = reinterpret_cast<const uint16_t *>(data);
+			s << *value;
+			break;
+		}
+		case ControlTypeUnsigned32: {
+			const auto *value = reinterpret_cast<const uint32_t *>(data);
+			s << *value;
+			break;
+		}
+		case ControlTypeInteger32: {
+			const auto *value = reinterpret_cast<const int32_t *>(data);
+			s << *value;
+			break;
+		}
+		case ControlTypeInteger64: {
+			const auto *value = reinterpret_cast<const int64_t *>(data);
+			s << *value;
+			break;
+		}
+		case ControlTypeFloat: {
+			const auto *value = reinterpret_cast<const float *>(data);
+			s << std::fixed << *value;
+			break;
+		}
+		case ControlTypeRectangle: {
+			const auto *value = reinterpret_cast<const Rectangle *>(data);
+			s << *value;
+			break;
+		}
+		case ControlTypeSize: {
+			const auto *value = reinterpret_cast<const Size *>(data);
+			s << *value;
+			break;
+		}
+		case ControlTypePoint: {
+			const auto *value = reinterpret_cast<const Point *>(data);
+			s << *value;
+			break;
+		}
+		case ControlTypeNone:
+		case ControlTypeString:
+			break;
+		}
+
+		data += ControlValueSize[type];
+	}
+
+	if (isArray)
+		s << " ]";
+
+	return s;
+}
+
 /**
  * \class ControlId
  * \brief Control static metadata