[RFC,v1,02/23] libcamera: controls: Add `ControlValueView`
diff mbox series

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

Commit Message

Barnabás Pőcze June 6, 2025, 4:41 p.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>
---
 include/libcamera/controls.h |  69 +++++++++++++
 src/libcamera/controls.cpp   | 194 +++++++++++++++++++++++++++++++++++
 2 files changed, 263 insertions(+)

Comments

Jacopo Mondi June 10, 2025, 4:01 p.m. UTC | #1
Hi Barnabás

On Fri, Jun 06, 2025 at 06:41:35PM +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>
> ---
>  include/libcamera/controls.h |  69 +++++++++++++
>  src/libcamera/controls.cpp   | 194 +++++++++++++++++++++++++++++++++++
>  2 files changed, 263 insertions(+)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index b170e30cb..8734479cf 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,71 @@ private:
>  		 std::size_t numElements, std::size_t elementSize);
>  };
>
> +class ControlValueView
> +{
> +public:
> +	constexpr ControlValueView() noexcept

We don't disable exception at compile time as far as I remember but in
any case we don't thrown any. Do you think for public API it might be
useful to mark a function noexcept anyway ?

> +		: 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?

Who would you like to be able to call this function ? Would making it
private be enough

> +	ControlValueView(ControlType type, bool isArray, std::size_t numElements, const std::byte *data) noexcept

better fit in 2 lines

> +		: type_(type),
> +		  isArray_(isArray),
> +		  numElements_(numElements),
> +		  data_(data)

Fits on 2 lines

	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;

I wonder if we should really make these nodiscard. I agree it doesn't
make sense to ignore the return value, but at the same time it has no
side effects..

> +
> +	[[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) {

I guess you could also check numElements_ but static compile-time
branching is probably more efficient ?

> +			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;

The base type of the ControlType enumeration is a int32, right ?
Does this have any implications ?

> +	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..e7e2781e8 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()

Does calling the default constructor help ?

> +{
> +	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
> @@ -395,6 +405,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

I was about to ask for more documentation, but seeing how little we
have for ControlValue I'm not sure it's fair :)

> + */
> +
> +/**
> + * \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.

Right, we have a view now, which refers to the data buffer owned by a
ControlValue. If the control value goes away the storage gets released
and the view will point to invalid memory ?

(also, I'm not sure why it can't be modified)


> + *
> + * \sa ControlValue::ControlValue()
> + */
> +
> +/**
> + * \fn ControlValueView::operator bool() const
> + * \brief Determine if the referred value is valid
> + * \sa ControlValueView::isNone()

Makes me wonder if we shouldn't instead check for data_ to be !=
nullptr to protect against the above "release the Value before the
View situation". However, thinking a bit more, the
ControlValueView::data_ pointer won't be set to nullptr when a
ControlValue is released. It will just point to invalid memory if I'm
not mistaken.

Ok, we just need to wrap ControlValue with shared_ptr<> everywhere,
what could it take.... (kidding)

> + */
> +
> +/**
> + * \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;

As these are views, isn't enough to compare the address pointed to by
data_ ?

> +}
> +
> +/**
> + * \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)

Unsurprisingly similar to ControlValue::toString().

Have you considered factoring out the common code parts ?

> +{
> +	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.49.0
>
Barnabás Pőcze June 11, 2025, 3:16 p.m. UTC | #2
2025. 06. 10. 18:01 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Fri, Jun 06, 2025 at 06:41:35PM +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>
>> ---
>>   include/libcamera/controls.h |  69 +++++++++++++
>>   src/libcamera/controls.cpp   | 194 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 263 insertions(+)
>>
>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
>> index b170e30cb..8734479cf 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,71 @@ private:
>>   		 std::size_t numElements, std::size_t elementSize);
>>   };
>>
>> +class ControlValueView
>> +{
>> +public:
>> +	constexpr ControlValueView() noexcept
> 
> We don't disable exception at compile time as far as I remember but in
> any case we don't thrown any. Do you think for public API it might be
> useful to mark a function noexcept anyway ?

Having noexcept on constructors, assignment operators, and certain functions
(e.g. swap()) can result in better code in STL or third-party libraries. Most
significantly e.g. resizing std::vector<T> is more efficient if `T` has a noexcept
move constructor.


> 
>> +		: 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?
> 
> Who would you like to be able to call this function ? Would making it
> private be enough

`MetadataList` is the main user of this constructor, so making it
private is not enough.


> 
>> +	ControlValueView(ControlType type, bool isArray, std::size_t numElements, const std::byte *data) noexcept
> 
> better fit in 2 lines
> 
>> +		: type_(type),
>> +		  isArray_(isArray),
>> +		  numElements_(numElements),
>> +		  data_(data)
> 
> Fits on 2 lines
> 
> 	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;
> 
> I wonder if we should really make these nodiscard. I agree it doesn't
> make sense to ignore the return value, but at the same time it has no
> side effects..

I think the fact that they make no side effects is one reason why you want
to make them nodiscard.


> 
>> +
>> +	[[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) {
> 
> I guess you could also check numElements_ but static compile-time
> branching is probably more efficient ?
> 

With a runtime branch, the code wouldn't compile in all cases. E.g. `T == bool`
would try to access `bool::value_type` and such a thing does not exist.


>> +			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;
> 
> The base type of the ControlType enumeration is a int32, right ?
> Does this have any implications ?

The `ControlValue` class has the same 8-bit wide field, so I just copied it from there.
Currently it is wide enough for every enumerator.


> 
>> +	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..e7e2781e8 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()
> 
> Does calling the default constructor help ?

Yes because otherwise `type_`, etc. would not be set, which would cause
issues when `set()` tries to free the current content.


> 
>> +{
>> +	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
>> @@ -395,6 +405,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
> 
> I was about to ask for more documentation, but seeing how little we
> have for ControlValue I'm not sure it's fair :)
> 
>> + */
>> +
>> +/**
>> + * \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.
> 
> Right, we have a view now, which refers to the data buffer owned by a
> ControlValue. If the control value goes away the storage gets released
> and the view will point to invalid memory ?
> 
> (also, I'm not sure why it can't be modified)

The type/size cannot be modified, but the value(s) themselves could indeed
be modified. But given that the main use case of this is `MetadataList`
where concurrent modifications are undesirable I want to disallow modifications.


> 
> 
>> + *
>> + * \sa ControlValue::ControlValue()
>> + */
>> +
>> +/**
>> + * \fn ControlValueView::operator bool() const
>> + * \brief Determine if the referred value is valid
>> + * \sa ControlValueView::isNone()
> 
> Makes me wonder if we shouldn't instead check for data_ to be !=
> nullptr to protect against the above "release the Value before the
> View situation". However, thinking a bit more, the
> ControlValueView::data_ pointer won't be set to nullptr when a
> ControlValue is released. It will just point to invalid memory if I'm
> not mistaken.
> 
> Ok, we just need to wrap ControlValue with shared_ptr<> everywhere,
> what could it take.... (kidding)
> 
>> + */
>> +
>> +/**
>> + * \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;
> 
> As these are views, isn't enough to compare the address pointed to by
> data_ ?

I think that largely depends on how one defines equality. I think considering the values
themselves makes sense since this is what `ControlValue` does, and the following might be
surprising:

   ControlValue a(1), b(1);
   assert(a == b);
   assert(ControlValueView(a) != ControlValueView(b));


> 
>> +}
>> +
>> +/**
>> + * \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)
> 
> Unsurprisingly similar to ControlValue::toString().
> 
> Have you considered factoring out the common code parts ?

Yes, it's a copy of that. I'll modify `ControlValue::toString()` to avoid
the repetition.


> [...]


Regards,
Barnabás Pőcze

Patch
diff mbox series

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index b170e30cb..8734479cf 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,71 @@  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..e7e2781e8 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
@@ -395,6 +405,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