[libcamera-devel] [PATCH 3/4] libcamera: controls: Store array control size in Control class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Aug 11 00:33:36 CEST 2022
Hi Jacopo,
On Wed, Aug 10, 2022 at 10:50:32AM +0200, Jacopo Mondi wrote:
> 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) ?
Yes, I modelled it after utils::dynamic_extent, which comes from the
std::dynamic_extent of C++20, but here there are less justifications
for departing from the libcamera coding style.
> > +
> > 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 ?
That would probably work, I can give it a try. I'm wondering, though, if
there's a need to distinguish arrays that have a single element from
non-array controls. I don't really see a reason why we would define an
array control with a fixed size set to 1 though.
> > 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