[libcamera-devel] [PATCH 3/4] libcamera: controls: Store array control size in Control class

Jacopo Mondi jacopo at jmondi.org
Wed Aug 10 10:50:32 CEST 2022


Hi Laurent

On Wed, Aug 10, 2022 at 03:29:05AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Baside just identifying a control, the Control class enables usage of
> the control type for metaprogramming. This is mainly used by the
> ControlList get() and set() functions to type-check and cast the control
> value automatically at compilation time.
>
> Extend this with a new Size template argument for the Control class that
> specifies the size of array controls. Use it already in the
> ControlList::set() overload that takes an std::initializer list to
> construct the Span with an explicit size, enabling usage of the function
> for fixed-size array controls.
>
> A possible future extension would be to pass the size to the ControlId
> constructor and store it internally, enabling access to the size at
> runtime, for instance to perform validity checks.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/controls.h | 20 +++++++++++--------
>  src/libcamera/controls.cpp   | 37 +++++++++++++++++++++++++++---------
>  utils/gen-controls.py        | 24 +++++++++++++++++++++--
>  3 files changed, 62 insertions(+), 19 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index ebc168fc28b7..dd474a807d68 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -8,6 +8,7 @@
>  #pragma once
>
>  #include <assert.h>
> +#include <limits>
>  #include <optional>
>  #include <set>
>  #include <stdint.h>
> @@ -213,6 +214,8 @@ private:
>  class ControlId
>  {
>  public:
> +	static constexpr size_t dynamic_size = std::numeric_limits<std::size_t>::max();

nit: should this be kDynamicSize (the notion we use for constants) ?

> +
>  	ControlId(unsigned int id, const std::string &name, ControlType type)
>  		: id_(id), name_(name), type_(type)
>  	{
> @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)
>  	return !(lhs == rhs);
>  }
>
> -template<typename T>
> +template<typename T, size_t Size = 0>

this bothers me a bit. Non-array controls are of size 0 for real ? or
should they be of size 1 ?

I don't see implications in this patch in having fixed size controls
with size set to 1, have I missed them ?


>  class Control : public ControlId
>  {
>  public:
>  	using type = T;
> +	static constexpr size_t size = Size;
>
>  	Control(unsigned int id, const char *name)
>  		: ControlId(id, name, details::control_type<std::remove_cv_t<T>>::value)
> @@ -372,8 +376,8 @@ public:
>
>  	bool contains(unsigned int id) const;
>
> -	template<typename T>
> -	std::optional<T> get(const Control<T> &ctrl) const
> +	template<typename T, size_t Size>
> +	std::optional<T> get(const Control<T, Size> &ctrl) const
>  	{
>  		const auto entry = controls_.find(ctrl.id());
>  		if (entry == controls_.end())
> @@ -383,8 +387,8 @@ public:
>  		return val.get<T>();
>  	}
>
> -	template<typename T, typename V>
> -	void set(const Control<T> &ctrl, const V &value)
> +	template<typename T, typename V, size_t Size>
> +	void set(const Control<T, Size> &ctrl, const V &value)
>  	{
>  		ControlValue *val = find(ctrl.id());
>  		if (!val)
> @@ -393,14 +397,14 @@ public:
>  		val->set<T>(value);
>  	}
>
> -	template<typename T, typename V>
> -	void set(const Control<T> &ctrl, const std::initializer_list<V> &value)
> +	template<typename T, typename V, size_t Size>
> +	void set(const Control<T, Size> &ctrl, const std::initializer_list<V> &value)
>  	{
>  		ControlValue *val = find(ctrl.id());
>  		if (!val)
>  			return;
>
> -		val->set<T>(Span<const typename std::remove_cv_t<V>>{ value.begin(), value.size() });
> +		val->set<T>(Span<const typename std::remove_cv_t<V>, Size>{ value.begin(), value.size() });
>  	}
>
>  	const ControlValue &get(unsigned int id) const;
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index bc3db4f69388..24c836382f10 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * Control class for more information.
>   */
>
> +/**
> + * \var ControlId::dynamic_size
> + * \brief Size value for dynamic array controls
> + */
> +
>  /**
>   * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)
>   * \brief Construct a ControlId instance
> @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>  /**
>   * \class Control
>   * \brief Describe a control and its intrinsic properties
> + * \tparam T The control data type
> + * \tparam Size The number of elements (for array controls only)
>   *
> - * The Control class models a control exposed by an object. Its template type
> - * name T refers to the control data type, and allows functions that operate on
> - * control values to be defined as template functions using the same type T for
> - * the control value. See for instance how the ControlList::get() function
> - * returns a value corresponding to the type of the requested control.
> + * The Control class models a control exposed by an object. Its template
> + * paramter \a T refers to the control data type, and allows functions that
> + * operate on control values to be defined as template functions using the same
> + * type T for the control value. See for instance how the ControlList::get()
> + * function returns a value corresponding to the type of the requested control.
> + *
> + * Similarly, for array controls the template parameter \a Size indicates the
> + * number of elements in the array.
>   *
>   * While this class is the main means to refer to a control, the control
>   * identifying information is stored in the non-template base ControlId class.
> @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * \brief The Control template type T
>   */
>
> +/**
> + * \var Control::size
> + * \brief The number of elements for array controls
> + *
> + * The \a size reports the number of elements stored by an array Control. It is
> + * equal to 0 for non-array controls, and to ControlId::dynamic_size for
> + * variable-size controls.
> + */
> +
>  /**
>   * \class ControlInfo
>   * \brief Describe the limits of valid values for a Control
> @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const
>  }
>
>  /**
> - * \fn ControlList::get(const Control<T> &ctrl) const
> + * \fn ControlList::get(const Control<T, Size> &ctrl) const
>   * \brief Get the value of control \a ctrl
>   * \param[in] ctrl The control
>   *
> @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const
>   */
>
>  /**
> - * \fn ControlList::set(const Control<T> &ctrl, const V &value)
> + * \fn ControlList::set(const Control<T, Size> &ctrl, const V &value)
>   * \brief Set the control \a ctrl value to \a value
>   * \param[in] ctrl The control
>   * \param[in] value The control value
> @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const
>   */
>
>  /**
> - * \fn ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value)
> - * \copydoc ControlList::set(const Control<T> &ctrl, const V &value)
> + * \fn ControlList::set(const Control<T, Size> &ctrl, const std::initializer_list<V> &value)
> + * \copydoc ControlList::set(const Control<T, Size> &ctrl, const V &value)
>   */
>
>  /**
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index bcfbeb7f9f17..a3b9c58beec6 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -100,6 +100,10 @@ class Control(object):
>          ns = 'draft::' if self.is_draft else ''
>          return ns + self.__name
>
> +    @property
> +    def size(self):
> +        return self.__size
> +
>      @property
>      def type(self):
>          typ = self.__data.get('type')
> @@ -137,7 +141,7 @@ ${description}''')
>   * \\var ${name}
>  ${description}
>   */''')
> -    def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, "${name}");')
> +    def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, "${name}");')
>      enum_values_doc = string.Template('''/**
>   * \\var ${name}Values
>   * \\brief List of all $name supported values
> @@ -154,9 +158,17 @@ ${description}
>      for ctrl in controls:
>          id_name = snake_case(ctrl.name).upper()
>
> +        if ctrl.size is None:
> +            size = ''
> +        elif ctrl.size == 0:
> +            size = ', ControlId::dynamic_size'
> +        else:
> +            size = f', {ctrl.size}'
> +
>          info = {
>              'name': ctrl.name,
>              'type': ctrl.type,
> +            'size': size,
>              'description': format_description(ctrl.description),
>              'id_name': id_name,
>          }
> @@ -216,7 +228,7 @@ def generate_h(controls):
>      enum_template_start = string.Template('''enum ${name}Enum {''')
>      enum_value_template = string.Template('''\t${name} = ${value},''')
>      enum_values_template = string.Template('''extern const std::array<const ControlValue, ${size}> ${name}Values;''')
> -    template = string.Template('''extern const Control<${type}> ${name};''')
> +    template = string.Template('''extern const Control<${type}${size}> ${name};''')
>
>      ctrls = []
>      draft_ctrls = []
> @@ -228,9 +240,17 @@ def generate_h(controls):
>
>          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
>
> +        if ctrl.size is None:
> +            size = ''
> +        elif ctrl.size == 0:
> +            size = ', ControlId::dynamic_size'
> +        else:
> +            size = f', {ctrl.size}'
> +
>          info = {
>              'name': ctrl.name,
>              'type': ctrl.type,
> +            'size': size,
>          }
>
>          target_ctrls = ctrls
> --
> Regards,
>
> Laurent Pinchart
>


More information about the libcamera-devel mailing list