| Message ID | 20200229164254.23604-10-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On 29/02/2020 16:42, Laurent Pinchart wrote: > The ControlList::set() method takes a reference to a Control<T>, and > requires the value to be a reference to T. This prevents the set() > method to be used with value types that are convertible to T, s/method to be used/method from being used/ :gqip ? > and in particular with std::array or std::vector values types when the /values/value/ /in particular/particularly/ > Control type will be a Span<> to support compound controls. /will be a/is a/ > > Fix this by decoupling the control type and value type in the template > parameters. The compiler will still catch invalid conversions, including > cases where constructor a T from the value type is explicit. "where constructor a T from" ? That's confusing, not sure what you actually meant... "where a constructor T form the value .." ? "where the constructor of type T from the value..." ? > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/controls.h | 4 ++-- > src/libcamera/controls.cpp | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 9f8a9031bd74..9d93064c11b2 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -223,8 +223,8 @@ public: > return val->get<T>(); > } > > - template<typename T> > - void set(const Control<T> &ctrl, const T &value) > + template<typename T, typename V> > + void set(const Control<T> &ctrl, const V &value) > { > ControlValue *val = find(ctrl.id()); > if (!val) > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index f632d60adc8b..a136ebd2653b 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -735,7 +735,7 @@ bool ControlList::contains(unsigned int id) const > */ > > /** > - * \fn template<typename T> void ControlList::set(const Control<T> &ctrl, const T &value) > + * \fn template<typename T, typename V> void ControlList::set(const Control<T> &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 >
Hi Kieran, On Mon, Mar 02, 2020 at 11:06:15PM +0000, Kieran Bingham wrote: > On 29/02/2020 16:42, Laurent Pinchart wrote: > > The ControlList::set() method takes a reference to a Control<T>, and > > requires the value to be a reference to T. This prevents the set() > > method to be used with value types that are convertible to T, > > s/method to be used/method from being used/ > > :gqip ? Not an editor command :-) (s/://) > > and in particular with std::array or std::vector values types when the > > /values/value/ > > /in particular/particularly/ > > > Control type will be a Span<> to support compound controls. > > /will be a/is a/ > > > > > Fix this by decoupling the control type and value type in the template > > parameters. The compiler will still catch invalid conversions, including > > cases where constructor a T from the value type is explicit. > > "where constructor a T from" ? > > That's confusing, not sure what you actually meant... > > "where a constructor T form the value .." ? > "where the constructor of type T from the value..." ? Oops. I meant "where constructing". > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > include/libcamera/controls.h | 4 ++-- > > src/libcamera/controls.cpp | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > index 9f8a9031bd74..9d93064c11b2 100644 > > --- a/include/libcamera/controls.h > > +++ b/include/libcamera/controls.h > > @@ -223,8 +223,8 @@ public: > > return val->get<T>(); > > } > > > > - template<typename T> > > - void set(const Control<T> &ctrl, const T &value) > > + template<typename T, typename V> > > + void set(const Control<T> &ctrl, const V &value) > > { > > ControlValue *val = find(ctrl.id()); > > if (!val) > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > > index f632d60adc8b..a136ebd2653b 100644 > > --- a/src/libcamera/controls.cpp > > +++ b/src/libcamera/controls.cpp > > @@ -735,7 +735,7 @@ bool ControlList::contains(unsigned int id) const > > */ > > > > /** > > - * \fn template<typename T> void ControlList::set(const Control<T> &ctrl, const T &value) > > + * \fn template<typename T, typename V> void ControlList::set(const Control<T> &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
On Tue, Mar 03, 2020 at 07:56:56PM +0200, Laurent Pinchart wrote: > On Mon, Mar 02, 2020 at 11:06:15PM +0000, Kieran Bingham wrote: > > On 29/02/2020 16:42, Laurent Pinchart wrote: > > > The ControlList::set() method takes a reference to a Control<T>, and > > > requires the value to be a reference to T. This prevents the set() > > > method to be used with value types that are convertible to T, > > > > s/method to be used/method from being used/ > > > > :gqip ? > > Not an editor command :-) (s/://) > > > > and in particular with std::array or std::vector values types when the > > > > /values/value/ > > > > /in particular/particularly/ > > > > > Control type will be a Span<> to support compound controls. > > > > /will be a/is a/ > > > > > > > > Fix this by decoupling the control type and value type in the template > > > parameters. The compiler will still catch invalid conversions, including > > > cases where constructor a T from the value type is explicit. > > > > "where constructor a T from" ? > > > > That's confusing, not sure what you actually meant... > > > > "where a constructor T form the value .." ? > > "where the constructor of type T from the value..." ? > > Oops. I meant "where constructing". But I like your last suggestion better, I'll use it. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > --- > > > include/libcamera/controls.h | 4 ++-- > > > src/libcamera/controls.cpp | 2 +- > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > > index 9f8a9031bd74..9d93064c11b2 100644 > > > --- a/include/libcamera/controls.h > > > +++ b/include/libcamera/controls.h > > > @@ -223,8 +223,8 @@ public: > > > return val->get<T>(); > > > } > > > > > > - template<typename T> > > > - void set(const Control<T> &ctrl, const T &value) > > > + template<typename T, typename V> > > > + void set(const Control<T> &ctrl, const V &value) > > > { > > > ControlValue *val = find(ctrl.id()); > > > if (!val) > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > > > index f632d60adc8b..a136ebd2653b 100644 > > > --- a/src/libcamera/controls.cpp > > > +++ b/src/libcamera/controls.cpp > > > @@ -735,7 +735,7 @@ bool ControlList::contains(unsigned int id) const > > > */ > > > > > > /** > > > - * \fn template<typename T> void ControlList::set(const Control<T> &ctrl, const T &value) > > > + * \fn template<typename T, typename V> void ControlList::set(const Control<T> &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
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 9f8a9031bd74..9d93064c11b2 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -223,8 +223,8 @@ public: return val->get<T>(); } - template<typename T> - void set(const Control<T> &ctrl, const T &value) + template<typename T, typename V> + void set(const Control<T> &ctrl, const V &value) { ControlValue *val = find(ctrl.id()); if (!val) diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index f632d60adc8b..a136ebd2653b 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -735,7 +735,7 @@ bool ControlList::contains(unsigned int id) const */ /** - * \fn template<typename T> void ControlList::set(const Control<T> &ctrl, const T &value) + * \fn template<typename T, typename V> void ControlList::set(const Control<T> &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
The ControlList::set() method takes a reference to a Control<T>, and requires the value to be a reference to T. This prevents the set() method to be used with value types that are convertible to T, and in particular with std::array or std::vector values types when the Control type will be a Span<> to support compound controls. Fix this by decoupling the control type and value type in the template parameters. The compiler will still catch invalid conversions, including cases where constructor a T from the value type is explicit. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/controls.h | 4 ++-- src/libcamera/controls.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)