[v4,3/4] libcamera: controls: Add support for querying direction information

22297 diff mbox series

Add support to ControlId for querying direction information. This allows
applications to query whether a ControlId is meant for being set in
controls or to be returned in metadata or both. This also has a side
effect of properly encoding this information, as previously it was only
mentioned losely and inconsistently in the control id definition.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
Changes in v4:
- add ControlId::direction()

Changes in v3:
- make direction parameter to ControlId and Control consutrctors
  mandatory
  - this entails expanding the control serializer/deserializer for
    V4L2Device and ControlSerializer to properly handle the direction
  - also the constructor parameters have to be reordered, compared to
    the last vesrion at least

Changes in v2:
- simplify code
---
 include/libcamera/controls.h         | 20 +++++++++-
 include/libcamera/ipa/ipa_controls.h |  3 +-
 src/libcamera/control_ids.cpp.in     |  4 +-
 src/libcamera/control_serializer.cpp |  8 +++-
 src/libcamera/controls.cpp           | 56 ++++++++++++++++++++++++++--
 src/libcamera/v4l2_device.cpp        | 10 ++++-
 6 files changed, 91 insertions(+), 10 deletions(-)

Quoting Paul Elder (2024-12-12 05:24:37)
> Add support to ControlId for querying direction information. This allows
> applications to query whether a ControlId is meant for being set in
> controls or to be returned in metadata or both. This also has a side
> effect of properly encoding this information, as previously it was only
> mentioned losely and inconsistently in the control id definition.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ---
> Changes in v4:
> - add ControlId::direction()
> 
> Changes in v3:
> - make direction parameter to ControlId and Control consutrctors
>   mandatory
>   - this entails expanding the control serializer/deserializer for
>     V4L2Device and ControlSerializer to properly handle the direction
>   - also the constructor parameters have to be reordered, compared to
>     the last vesrion at least
> 
> Changes in v2:
> - simplify code
> ---
>  include/libcamera/controls.h         | 20 +++++++++-
>  include/libcamera/ipa/ipa_controls.h |  3 +-
>  src/libcamera/control_ids.cpp.in     |  4 +-
>  src/libcamera/control_serializer.cpp |  8 +++-
>  src/libcamera/controls.cpp           | 56 ++++++++++++++++++++++++++--
>  src/libcamera/v4l2_device.cpp        | 10 ++++-
>  6 files changed, 91 insertions(+), 10 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index b24336cc280f..7c7828ae5db0 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -17,6 +17,7 @@
>  #include <vector>
>  
>  #include <libcamera/base/class.h>
> +#include <libcamera/base/flags.h>
>  #include <libcamera/base/span.h>
>  
>  #include <libcamera/geometry.h>
> @@ -249,14 +250,25 @@ private:
>  class ControlId
>  {
>  public:
> +       enum class Direction {
> +               In = (1 << 0),
> +               Out = (1 << 1),
> +       };
> +
> +       using DirectionFlags = Flags<Direction>;
> +
>         ControlId(unsigned int id, const std::string &name, const std::string &vendor,
> -                 ControlType type, std::size_t size = 0,
> +                 ControlType type, DirectionFlags direction,
> +                 std::size_t size = 0,
>                   const std::map<std::string, int32_t> &enumStrMap = {});
>  
>         unsigned int id() const { return id_; }
>         const std::string &name() const { return name_; }
>         const std::string &vendor() const { return vendor_; }
>         ControlType type() const { return type_; }
> +       DirectionFlags direction() const { return direction_; }
> +       bool isInput() const { return !!(direction_ & Direction::In); }
> +       bool isOutput() const { return !!(direction_ & Direction::Out); }
>         bool isArray() const { return size_ > 0; }
>         std::size_t size() const { return size_; }
>         const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; }
> @@ -268,11 +280,14 @@ private:
>         std::string name_;
>         std::string vendor_;
>         ControlType type_;
> +       DirectionFlags direction_;
>         std::size_t size_;
>         std::map<std::string, int32_t> enumStrMap_;
>         std::map<int32_t, std::string> reverseMap_;
>  };
>  
> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction)
> +
>  static inline bool operator==(unsigned int lhs, const ControlId &rhs)
>  {
>         return lhs == rhs.id();
> @@ -300,9 +315,10 @@ public:
>         using type = T;
>  
>         Control(unsigned int id, const char *name, const char *vendor,
> +               ControlId::DirectionFlags direction,
>                 const std::map<std::string, int32_t> &enumStrMap = {})
>                 : ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value,
> -                           details::control_type<std::remove_cv_t<T>>::size, enumStrMap)
> +                           direction, details::control_type<std::remove_cv_t<T>>::size, enumStrMap)
>         {
>         }
>  
> diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h
> index 5fd13394fcef..980668c86bcc 100644
> --- a/include/libcamera/ipa/ipa_controls.h
> +++ b/include/libcamera/ipa/ipa_controls.h
> @@ -46,7 +46,8 @@ struct ipa_control_info_entry {
>         uint32_t id;
>         uint32_t type;
>         uint32_t offset;
> -       uint32_t padding[1];
> +       uint8_t direction;
> +       uint8_t padding[3];
>  };
>  
>  #ifdef __cplusplus
> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> index afe9e2c96610..65668d486dbc 100644
> --- a/src/libcamera/control_ids.cpp.in
> +++ b/src/libcamera/control_ids.cpp.in
> @@ -89,9 +89,9 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {
>         { "{{enum.name}}", {{enum.name}} },
>  {%- endfor %}
>  };
> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.name}}NameValueMap);
> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap);
>  {% else -%}
> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}");
> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}});
>  {% endif -%}
>  {%- endfor %}
>  
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 0a5e8220a0ff..17834648c3f0 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -281,6 +281,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
>                 entry.id = id->id();
>                 entry.type = id->type();
>                 entry.offset = values.offset();
> +               entry.direction = static_cast<ControlId::DirectionFlags::Type>(id->direction());
>                 entries.write(&entry);
>  
>                 store(info, values);
> @@ -493,12 +494,17 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
>  
>                 /* If we're using a local id map, populate it. */
>                 if (localIdMap) {
> +                       ControlId::DirectionFlags flags{
> +                               static_cast<ControlId::Direction>(entry->direction)
> +                       };
> +
>                         /**
>                          * \todo Find a way to preserve the control name for
>                          * debugging purpose.
>                          */
>                         controlIds_.emplace_back(std::make_unique<ControlId>(entry->id,
> -                                                                            "", "local", type));
> +                                                                            "", "local", type,
> +                                                                            flags));
>                         (*localIdMap)[entry->id] = controlIds_.back().get();
>                 }
>  
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 65eeef2db9c2..6296aac6fff2 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -412,15 +412,16 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * \param[in] name The control name
>   * \param[in] vendor The vendor name
>   * \param[in] type The control data type
> + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata
>   * \param[in] size The size of the array control, or 0 if scalar control
>   * \param[in] enumStrMap The map from enum names to values (optional)
>   */
>  ControlId::ControlId(unsigned int id, const std::string &name,
>                      const std::string &vendor, ControlType type,
> -                    std::size_t size,
> +                    DirectionFlags direction, std::size_t size,
>                      const std::map<std::string, int32_t> &enumStrMap)
> -       : id_(id), name_(name), vendor_(vendor), type_(type), size_(size),
> -         enumStrMap_(enumStrMap)
> +       : id_(id), name_(name), vendor_(vendor), type_(type),
> +         direction_(direction), size_(size), enumStrMap_(enumStrMap)
>  {
>         for (const auto &pair : enumStrMap_)
>                 reverseMap_[pair.second] = pair.first;
> @@ -450,6 +451,37 @@ ControlId::ControlId(unsigned int id, const std::string &name,
>   * \return The control data type
>   */
>  
> +/**
> + * \fn DirectionFlags direction() const
> + * \brief Return the direction that the control can be used in
> + *
> + * This is similar to \sa isInput() and \sa isOutput(), but returns the flags
> + * direction instead of booleans for each direction.
> + *
> + * \return The direction flags corresponding to if the control can be used as
> + * an input control or as output metadata
> + */
> +
> +/**
> + * \fn bool ControlId::isInput() const
> + * \brief Determine if the control is available to be used as an input control
> + *
> + * Controls can be used either as input in controls, or as output in metadata.
> + * This function checks if the control is allowed to be used as the former.
> + *
> + * \return True if the control can be used as an input control, false otherwise
> + */
> +
> +/**
> + * \fn bool ControlId::isOutput() const
> + * \brief Determine if the control is available to be used in output metadata
> + *
> + * Controls can be used either as input in controls, or as output in metadata.
> + * This function checks if the control is allowed to be used as the latter.
> + *
> + * \return True if the control can be returned in output metadata, false otherwise
> + */
> +
>  /**
>   * \fn bool ControlId::isArray() const
>   * \brief Determine if the control is an array control
> @@ -487,6 +519,22 @@ ControlId::ControlId(unsigned int id, const std::string &name,
>   * \return True if \a lhs.id() is equal to \a rhs, false otherwise
>   */
>  
> +/**
> + * \enum ControlId::Direction
> + * \brief The direction the control is capable of being passed from/to
> + *
> + * \var ControlId::Direction::In
> + * \brief The control can be passed as input in controls
> + *
> + * \var ControlId::Direction::Out
> + * \brief The control can be returned as output in metadata
> + */
> +
> +/**
> + * \typedef ControlId::DirectionFlags
> + * \brief A wrapper for ControlId::Direction so that it can be used as flags
> + */
> +
>  /**
>   * \class Control
>   * \brief Describe a control and its intrinsic properties
> @@ -520,6 +568,8 @@ ControlId::ControlId(unsigned int id, const std::string &name,
>   * \param[in] id The control numerical ID
>   * \param[in] name The control name
>   * \param[in] vendor The vendor name
> + * \param[in] direction The direction of the control, if it can be used in
> + * Controls or Metadata
>   * \param[in] enumStrMap The map from enum names to values (optional)
>   *
>   * The control data type is automatically deduced from the template type T.
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 664b74afe6a1..2f65a43a0547 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -565,7 +565,15 @@ std::unique_ptr<ControlId> V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &
>         const std::string name(static_cast<const char *>(ctrl.name), len);
>         const ControlType type = v4l2CtrlType(ctrl.type);
>  
> -       return std::make_unique<ControlId>(ctrl.id, name, "v4l2", type);
> +       ControlId::DirectionFlags flags;
> +       if (ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)
> +               flags = ControlId::Direction::Out;
> +       else if (ctrl.flags & V4L2_CTRL_FLAG_WRITE_ONLY)
> +               flags = ControlId::Direction::In;
> +       else
> +               flags = ControlId::Direction::In | ControlId::Direction::Out;
> +

All my comments addressed, so 


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +       return std::make_unique<ControlId>(ctrl.id, name, "v4l2", type, flags);
>  }
>  
>  /**
> -- 
> 2.39.2
>

Hi Paul,

Thank you for the patch.

On Thu, Dec 12, 2024 at 02:24:37PM +0900, Paul Elder wrote:
> Add support to ControlId for querying direction information. This allows
> applications to query whether a ControlId is meant for being set in
> controls or to be returned in metadata or both. This also has a side
> effect of properly encoding this information, as previously it was only
> mentioned losely and inconsistently in the control id definition.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ---
> Changes in v4:
> - add ControlId::direction()
> 
> Changes in v3:
> - make direction parameter to ControlId and Control consutrctors
>   mandatory
>   - this entails expanding the control serializer/deserializer for
>     V4L2Device and ControlSerializer to properly handle the direction
>   - also the constructor parameters have to be reordered, compared to
>     the last vesrion at least
> 
> Changes in v2:
> - simplify code
> ---
>  include/libcamera/controls.h         | 20 +++++++++-
>  include/libcamera/ipa/ipa_controls.h |  3 +-
>  src/libcamera/control_ids.cpp.in     |  4 +-
>  src/libcamera/control_serializer.cpp |  8 +++-
>  src/libcamera/controls.cpp           | 56 ++++++++++++++++++++++++++--
>  src/libcamera/v4l2_device.cpp        | 10 ++++-
>  6 files changed, 91 insertions(+), 10 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index b24336cc280f..7c7828ae5db0 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -17,6 +17,7 @@
>  #include <vector>
>  
>  #include <libcamera/base/class.h>
> +#include <libcamera/base/flags.h>
>  #include <libcamera/base/span.h>
>  
>  #include <libcamera/geometry.h>
> @@ -249,14 +250,25 @@ private:
>  class ControlId
>  {
>  public:
> +	enum class Direction {
> +		In = (1 << 0),
> +		Out = (1 << 1),
> +	};
> +
> +	using DirectionFlags = Flags<Direction>;
> +
>  	ControlId(unsigned int id, const std::string &name, const std::string &vendor,
> -		  ControlType type, std::size_t size = 0,
> +		  ControlType type, DirectionFlags direction,
> +		  std::size_t size = 0,
>  		  const std::map<std::string, int32_t> &enumStrMap = {});
>  
>  	unsigned int id() const { return id_; }
>  	const std::string &name() const { return name_; }
>  	const std::string &vendor() const { return vendor_; }
>  	ControlType type() const { return type_; }
> +	DirectionFlags direction() const { return direction_; }
> +	bool isInput() const { return !!(direction_ & Direction::In); }
> +	bool isOutput() const { return !!(direction_ & Direction::Out); }
>  	bool isArray() const { return size_ > 0; }
>  	std::size_t size() const { return size_; }
>  	const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; }
> @@ -268,11 +280,14 @@ private:
>  	std::string name_;
>  	std::string vendor_;
>  	ControlType type_;
> +	DirectionFlags direction_;
>  	std::size_t size_;
>  	std::map<std::string, int32_t> enumStrMap_;
>  	std::map<int32_t, std::string> reverseMap_;
>  };
>  
> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction)
> +
>  static inline bool operator==(unsigned int lhs, const ControlId &rhs)
>  {
>  	return lhs == rhs.id();
> @@ -300,9 +315,10 @@ public:
>  	using type = T;
>  
>  	Control(unsigned int id, const char *name, const char *vendor,
> +		ControlId::DirectionFlags direction,
>  		const std::map<std::string, int32_t> &enumStrMap = {})
>  		: ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value,
> -			    details::control_type<std::remove_cv_t<T>>::size, enumStrMap)
> +			    direction, details::control_type<std::remove_cv_t<T>>::size, enumStrMap)
>  	{
>  	}
>  
> diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h
> index 5fd13394fcef..980668c86bcc 100644
> --- a/include/libcamera/ipa/ipa_controls.h
> +++ b/include/libcamera/ipa/ipa_controls.h
> @@ -46,7 +46,8 @@ struct ipa_control_info_entry {
>  	uint32_t id;
>  	uint32_t type;
>  	uint32_t offset;
> -	uint32_t padding[1];
> +	uint8_t direction;
> +	uint8_t padding[3];
>  };
>  
>  #ifdef __cplusplus
> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> index afe9e2c96610..65668d486dbc 100644
> --- a/src/libcamera/control_ids.cpp.in
> +++ b/src/libcamera/control_ids.cpp.in
> @@ -89,9 +89,9 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {
>  	{ "{{enum.name}}", {{enum.name}} },
>  {%- endfor %}
>  };
> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.name}}NameValueMap);
> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap);
>  {% else -%}
> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}");
> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}});
>  {% endif -%}
>  {%- endfor %}
>  
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 0a5e8220a0ff..17834648c3f0 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -281,6 +281,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
>  		entry.id = id->id();
>  		entry.type = id->type();
>  		entry.offset = values.offset();
> +		entry.direction = static_cast<ControlId::DirectionFlags::Type>(id->direction());
>  		entries.write(&entry);
>  
>  		store(info, values);
> @@ -493,12 +494,17 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
>  
>  		/* If we're using a local id map, populate it. */
>  		if (localIdMap) {

Should there be a check here that entry->direction doesn't contain any
unsupported flag ? If so, and if it does, I haven't thought about which
error handling strategy would be best, between making it a hard error,
or logging a warning and clearing unsupported flags.

With that addressed (if needed),

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +			ControlId::DirectionFlags flags{
> +				static_cast<ControlId::Direction>(entry->direction)
> +			};
> +
>  			/**
>  			 * \todo Find a way to preserve the control name for
>  			 * debugging purpose.
>  			 */
>  			controlIds_.emplace_back(std::make_unique<ControlId>(entry->id,
> -									     "", "local", type));
> +									     "", "local", type,
> +									     flags));
>  			(*localIdMap)[entry->id] = controlIds_.back().get();
>  		}
>  
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 65eeef2db9c2..6296aac6fff2 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -412,15 +412,16 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * \param[in] name The control name
>   * \param[in] vendor The vendor name
>   * \param[in] type The control data type
> + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata
>   * \param[in] size The size of the array control, or 0 if scalar control
>   * \param[in] enumStrMap The map from enum names to values (optional)
>   */
>  ControlId::ControlId(unsigned int id, const std::string &name,
>  		     const std::string &vendor, ControlType type,
> -		     std::size_t size,
> +		     DirectionFlags direction, std::size_t size,
>  		     const std::map<std::string, int32_t> &enumStrMap)
> -	: id_(id), name_(name), vendor_(vendor), type_(type), size_(size),
> -	  enumStrMap_(enumStrMap)
> +	: id_(id), name_(name), vendor_(vendor), type_(type),
> +	  direction_(direction), size_(size), enumStrMap_(enumStrMap)
>  {
>  	for (const auto &pair : enumStrMap_)
>  		reverseMap_[pair.second] = pair.first;
> @@ -450,6 +451,37 @@ ControlId::ControlId(unsigned int id, const std::string &name,
>   * \return The control data type
>   */
>  
> +/**
> + * \fn DirectionFlags direction() const
> + * \brief Return the direction that the control can be used in
> + *
> + * This is similar to \sa isInput() and \sa isOutput(), but returns the flags
> + * direction instead of booleans for each direction.
> + *
> + * \return The direction flags corresponding to if the control can be used as
> + * an input control or as output metadata
> + */
> +
> +/**
> + * \fn bool ControlId::isInput() const
> + * \brief Determine if the control is available to be used as an input control
> + *
> + * Controls can be used either as input in controls, or as output in metadata.
> + * This function checks if the control is allowed to be used as the former.
> + *
> + * \return True if the control can be used as an input control, false otherwise
> + */
> +
> +/**
> + * \fn bool ControlId::isOutput() const
> + * \brief Determine if the control is available to be used in output metadata
> + *
> + * Controls can be used either as input in controls, or as output in metadata.
> + * This function checks if the control is allowed to be used as the latter.
> + *
> + * \return True if the control can be returned in output metadata, false otherwise
> + */
> +
>  /**
>   * \fn bool ControlId::isArray() const
>   * \brief Determine if the control is an array control
> @@ -487,6 +519,22 @@ ControlId::ControlId(unsigned int id, const std::string &name,
>   * \return True if \a lhs.id() is equal to \a rhs, false otherwise
>   */
>  
> +/**
> + * \enum ControlId::Direction
> + * \brief The direction the control is capable of being passed from/to
> + *
> + * \var ControlId::Direction::In
> + * \brief The control can be passed as input in controls
> + *
> + * \var ControlId::Direction::Out
> + * \brief The control can be returned as output in metadata
> + */
> +
> +/**
> + * \typedef ControlId::DirectionFlags
> + * \brief A wrapper for ControlId::Direction so that it can be used as flags
> + */
> +
>  /**
>   * \class Control
>   * \brief Describe a control and its intrinsic properties
> @@ -520,6 +568,8 @@ ControlId::ControlId(unsigned int id, const std::string &name,
>   * \param[in] id The control numerical ID
>   * \param[in] name The control name
>   * \param[in] vendor The vendor name
> + * \param[in] direction The direction of the control, if it can be used in
> + * Controls or Metadata
>   * \param[in] enumStrMap The map from enum names to values (optional)
>   *
>   * The control data type is automatically deduced from the template type T.
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 664b74afe6a1..2f65a43a0547 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -565,7 +565,15 @@ std::unique_ptr<ControlId> V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &
>  	const std::string name(static_cast<const char *>(ctrl.name), len);
>  	const ControlType type = v4l2CtrlType(ctrl.type);
>  
> -	return std::make_unique<ControlId>(ctrl.id, name, "v4l2", type);
> +	ControlId::DirectionFlags flags;
> +	if (ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)
> +		flags = ControlId::Direction::Out;
> +	else if (ctrl.flags & V4L2_CTRL_FLAG_WRITE_ONLY)
> +		flags = ControlId::Direction::In;
> +	else
> +		flags = ControlId::Direction::In | ControlId::Direction::Out;
> +
> +	return std::make_unique<ControlId>(ctrl.id, name, "v4l2", type, flags);
>  }
>  
>  /**