| Message ID | 20220810002906.5406-4-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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 >
Dear Laurent, I don't see yet, how the "ControlId::dynamic_size" would be used additionally to the "Span::extent". Since the size of the Span is already encoded in the type, what is the benefit of duplicating this information in the ControlId? Will this information be identical? And what would be the meaning of "Control::size" when its T is a scalar and not a Span? Best, Christian Am 10.08.22 um 02:29 schrieb Laurent Pinchart: > 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@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(); > + > 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> > 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
Hi Christian, On Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote: > Dear Laurent, > > I don't see yet, how the "ControlId::dynamic_size" would be used > additionally to the "Span::extent". The two have the same nuumerical value, but they differ in their intent. ControlId::dynamic_size is meant to indicate that the array control has a dynamic size, while utils::dynamic_extent is meant to indicate that a span has a dynamic number of elements. > Since the size of the Span is > already encoded in the type, what is the benefit of duplicating this > information in the ControlId? Will this information be identical? It allows the ControlList::set() function to access the size, and construct the Span with the right size, removing the need for explicit Span construction in the caller as shown by patch 4/4. Now that I'm writing this, I wonder if the following could work template<typename T, size_t Size> void set(const Control<Span<T, Size>> &ctrl, const std::initializer_list<T> &value) { ControlValue *val = find(ctrl.id()); if (!val) return; val->set(Span<const typename std::remove_cv_t<T>, Size>{ value.begin(), value.size() }); } without adding the Size template argument to the Control class. I'll give it a try. I'm also tempted to figure out if there would be a way to define a control as Control<T, Size> with T being the type of each element, not a Span. Size would be 1 for non-array controls, >1 for array controls with a fixed size, and std::numeric_limits<std::size_t>::max() for array controls with a dynamic size. I'm not sure of the implications yet, but having the size available as a template argument of the Control class would allow passing it easily to the ControlId constructor, to make it available at runtime. I think there are use cases for validation for instance. > And > what would be the meaning of "Control::size" when its T is a scalar and > not a Span? It has no meaning in that case, which is indicated by setting it to 0. Jacopo mentioned that 0 may be a confusing value, I agree with that. > Am 10.08.22 um 02:29 schrieb Laurent Pinchart: > > 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@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(); > > + > > 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> > > 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
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@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
Hi Christian, On Thu, Aug 11, 2022 at 01:23:18AM +0300, Laurent Pinchart via libcamera-devel wrote: > On Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote: > > Dear Laurent, > > > > I don't see yet, how the "ControlId::dynamic_size" would be used > > additionally to the "Span::extent". > > The two have the same nuumerical value, but they differ in their intent. > ControlId::dynamic_size is meant to indicate that the array control has > a dynamic size, while utils::dynamic_extent is meant to indicate that a > span has a dynamic number of elements. > > > Since the size of the Span is > > already encoded in the type, what is the benefit of duplicating this > > information in the ControlId? Will this information be identical? > > It allows the ControlList::set() function to access the size, and > construct the Span with the right size, removing the need for explicit > Span construction in the caller as shown by patch 4/4. > > Now that I'm writing this, I wonder if the following could work > > template<typename T, size_t Size> > void set(const Control<Span<T, Size>> &ctrl, const std::initializer_list<T> &value) > { > ControlValue *val = find(ctrl.id()); > if (!val) > return; > > val->set(Span<const typename std::remove_cv_t<T>, Size>{ value.begin(), value.size() }); > } > > without adding the Size template argument to the Control class. I'll > give it a try. I can replace this patch with diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index ebc168fc28b7..2305202c4570 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -393,14 +393,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<Span<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(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..6dbf9b348709 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -970,7 +970,7 @@ bool ControlList::contains(unsigned int id) const */ /** - * \fn ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value) + * \fn ControlList::set(const Control<Span<T, Size>> &ctrl, const std::initializer_list<V> &value) * \copydoc ControlList::set(const Control<T> &ctrl, const V &value) */ and 4/4 still compiles. I think I'll go with this for the time being. I may revive the idea of adding a size directly to the Control class in the future when a use case will arise though, but that's for later. > I'm also tempted to figure out if there would be a way to define a > control as Control<T, Size> with T being the type of each element, not a > Span. Size would be 1 for non-array controls, >1 for array controls with > a fixed size, and std::numeric_limits<std::size_t>::max() for array > controls with a dynamic size. I'm not sure of the implications yet, but > having the size available as a template argument of the Control class > would allow passing it easily to the ControlId constructor, to make it > available at runtime. I think there are use cases for validation for > instance. > > > And > > what would be the meaning of "Control::size" when its T is a scalar and > > not a Span? > > It has no meaning in that case, which is indicated by setting it to 0. > Jacopo mentioned that 0 may be a confusing value, I agree with that. > > > Am 10.08.22 um 02:29 schrieb Laurent Pinchart: > > > 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@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(); > > > + > > > 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> > > > 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
Hi Laurent, Am 11.08.22 um 00:52 schrieb Laurent Pinchart: > Hi Christian, > > On Thu, Aug 11, 2022 at 01:23:18AM +0300, Laurent Pinchart via libcamera-devel wrote: >> On Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote: >>> Dear Laurent, >>> >>> I don't see yet, how the "ControlId::dynamic_size" would be used >>> additionally to the "Span::extent". >> >> The two have the same nuumerical value, but they differ in their intent. >> ControlId::dynamic_size is meant to indicate that the array control has >> a dynamic size, while utils::dynamic_extent is meant to indicate that a >> span has a dynamic number of elements. >> >>> Since the size of the Span is >>> already encoded in the type, what is the benefit of duplicating this >>> information in the ControlId? Will this information be identical? >> >> It allows the ControlList::set() function to access the size, and >> construct the Span with the right size, removing the need for explicit >> Span construction in the caller as shown by patch 4/4. >> >> Now that I'm writing this, I wonder if the following could work >> >> template<typename T, size_t Size> >> void set(const Control<Span<T, Size>> &ctrl, const std::initializer_list<T> &value) >> { >> ControlValue *val = find(ctrl.id()); >> if (!val) >> return; >> >> val->set(Span<const typename std::remove_cv_t<T>, Size>{ value.begin(), value.size() }); >> } >> >> without adding the Size template argument to the Control class. I'll >> give it a try. > > I can replace this patch with > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index ebc168fc28b7..2305202c4570 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -393,14 +393,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<Span<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(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..6dbf9b348709 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -970,7 +970,7 @@ bool ControlList::contains(unsigned int id) const > */ > > /** > - * \fn ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value) > + * \fn ControlList::set(const Control<Span<T, Size>> &ctrl, const std::initializer_list<V> &value) > * \copydoc ControlList::set(const Control<T> &ctrl, const V &value) > */ > > and 4/4 still compiles. I think I'll go with this for the time being. I > may revive the idea of adding a size directly to the Control class in > the future when a use case will arise though, but that's for later. If all you want to know is the size of the stored Span (or scalar), wouldn't something like template<typename T> class Control : public ControlId { std::size_t extent() const { return T::extent; } } be sufficient? You could use "is_span<T>::value" to implement an "array" (returning 'extent') and "scalar" (returning 0) version of this. > >> I'm also tempted to figure out if there would be a way to define a >> control as Control<T, Size> with T being the type of each element, not a >> Span. Size would be 1 for non-array controls, >1 for array controls with >> a fixed size, and std::numeric_limits<std::size_t>::max() for array >> controls with a dynamic size. I'm not sure of the implications yet, but >> having the size available as a template argument of the Control class >> would allow passing it easily to the ControlId constructor, to make it >> available at runtime. I think there are use cases for validation for >> instance. >> >>> And >>> what would be the meaning of "Control::size" when its T is a scalar and >>> not a Span? >> >> It has no meaning in that case, which is indicated by setting it to 0. >> Jacopo mentioned that 0 may be a confusing value, I agree with that. >> >>> Am 10.08.22 um 02:29 schrieb Laurent Pinchart: >>>> 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@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(); >>>> + >>>> 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> >>>> 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 >
Hi Christian, On Fri, Aug 12, 2022 at 12:11:21AM +0200, Christian Rauch via libcamera-devel wrote: > Am 11.08.22 um 00:52 schrieb Laurent Pinchart: > > On Thu, Aug 11, 2022 at 01:23:18AM +0300, Laurent Pinchart via libcamera-devel wrote: > >> On Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote: > >>> Dear Laurent, > >>> > >>> I don't see yet, how the "ControlId::dynamic_size" would be used > >>> additionally to the "Span::extent". > >> > >> The two have the same nuumerical value, but they differ in their intent. > >> ControlId::dynamic_size is meant to indicate that the array control has > >> a dynamic size, while utils::dynamic_extent is meant to indicate that a > >> span has a dynamic number of elements. > >> > >>> Since the size of the Span is > >>> already encoded in the type, what is the benefit of duplicating this > >>> information in the ControlId? Will this information be identical? > >> > >> It allows the ControlList::set() function to access the size, and > >> construct the Span with the right size, removing the need for explicit > >> Span construction in the caller as shown by patch 4/4. > >> > >> Now that I'm writing this, I wonder if the following could work > >> > >> template<typename T, size_t Size> > >> void set(const Control<Span<T, Size>> &ctrl, const std::initializer_list<T> &value) > >> { > >> ControlValue *val = find(ctrl.id()); > >> if (!val) > >> return; > >> > >> val->set(Span<const typename std::remove_cv_t<T>, Size>{ value.begin(), value.size() }); > >> } > >> > >> without adding the Size template argument to the Control class. I'll > >> give it a try. > > > > I can replace this patch with > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > index ebc168fc28b7..2305202c4570 100644 > > --- a/include/libcamera/controls.h > > +++ b/include/libcamera/controls.h > > @@ -393,14 +393,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<Span<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(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..6dbf9b348709 100644 > > --- a/src/libcamera/controls.cpp > > +++ b/src/libcamera/controls.cpp > > @@ -970,7 +970,7 @@ bool ControlList::contains(unsigned int id) const > > */ > > > > /** > > - * \fn ControlList::set(const Control<T> &ctrl, const std::initializer_list<V> &value) > > + * \fn ControlList::set(const Control<Span<T, Size>> &ctrl, const std::initializer_list<V> &value) > > * \copydoc ControlList::set(const Control<T> &ctrl, const V &value) > > */ > > > > and 4/4 still compiles. I think I'll go with this for the time being. I > > may revive the idea of adding a size directly to the Control class in > > the future when a use case will arise though, but that's for later. > > If all you want to know is the size of the stored Span (or scalar), > wouldn't something like > > template<typename T> > class Control : public ControlId > { > std::size_t extent() const > { > return T::extent; > } > } > > be sufficient? You could use "is_span<T>::value" to implement an "array" > (returning 'extent') and "scalar" (returning 0) version of this. We'd have to guard that with a Span check on T indeed. It would otherwise work, and should be a constexpr. The issue is that it won't give access to the size from ControlId, which is something that could be fixed by passing it to the ControlId constructor. Regardless, adding the size as a template parameter to the Control class is indeed redundant, so I would likely do so only if coupled with turning T to the item type for array controls: Control<int32_t> Control<Span<int32_t>> Control<Span<int32_t, 4>> would become respectively Control<int32_t> Control<int32_t, dynamic_size> Control<int32_t, 4> I think it looks cleaner, but that's not something I'll spend time on for now. I have in the back of my head a plan to rework ControlList to make it possible to avoid most of the dynamic memory allocations, turning it into a convenient accessor API on top of a large buffer of serialized data. I have no idea yet how that would look like exactly, and I don't want to think about it now, there are more urgent topics to handle. > >> I'm also tempted to figure out if there would be a way to define a > >> control as Control<T, Size> with T being the type of each element, not a > >> Span. Size would be 1 for non-array controls, >1 for array controls with > >> a fixed size, and std::numeric_limits<std::size_t>::max() for array > >> controls with a dynamic size. I'm not sure of the implications yet, but > >> having the size available as a template argument of the Control class > >> would allow passing it easily to the ControlId constructor, to make it > >> available at runtime. I think there are use cases for validation for > >> instance. > >> > >>> And > >>> what would be the meaning of "Control::size" when its T is a scalar and > >>> not a Span? > >> > >> It has no meaning in that case, which is indicated by setting it to 0. > >> Jacopo mentioned that 0 may be a confusing value, I agree with that. > >> > >>> Am 10.08.22 um 02:29 schrieb Laurent Pinchart: > >>>> 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@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(); > >>>> + > >>>> 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> > >>>> 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
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(); + 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> 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
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@ideasonboard.com> --- include/libcamera/controls.h | 20 +++++++++++-------- src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++--------- utils/gen-controls.py | 24 +++++++++++++++++++++-- 3 files changed, 62 insertions(+), 19 deletions(-)