| Message ID | 20191204132106.21582-5-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Wed, Dec 04, 2019 at 02:21:00PM +0100, Jacopo Mondi wrote: > Augment the the ControlRange class to store the control default value. > > This is particularly relevant for v4l2 controls used to create > Camera properties, which are constructed using immutable video device > properties, whose value won't change at runtime. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/controls.h | 7 ++++++- > src/libcamera/controls.cpp | 17 +++++++++++++++-- > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index b1b73367e874..1e2f284eafeb 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -114,10 +114,12 @@ class ControlRange > { > public: > explicit ControlRange(const ControlValue &min = 0, > - const ControlValue &max = 0); > + const ControlValue &max = 0, > + const ControlValue &defaultValue = 0); I wish we could call this default, but that's a reserved keyword :-/ Maybe just 'def' to match min and max ? I'll leave it up to you. > > const ControlValue &min() const { return min_; } > const ControlValue &max() const { return max_; } > + const ControlValue &defaultValue() const { return defaultValue_; } > > std::string toString() const; > > @@ -131,6 +133,9 @@ public: > return !(*this == other); > } > > +protected: > + ControlValue defaultValue_; > + Why is this protected while min_ and max_ are private ? I don't think the default value should be special. > private: > ControlValue min_; > ControlValue max_; > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 7d8a0e97ee3a..bacba0fbf68a 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -353,14 +353,21 @@ Control<int64_t>::Control(unsigned int id, const char *name) > * pipeline handlers to describe the controls they support. > */ > > +/** > + * \var ControlRange::defaultValue_ > + * \brief The control default value > + */ > + > /** > * \brief Construct a ControlRange with minimum and maximum range parameters > * \param[in] min The control minimum value > * \param[in] max The control maximum value > + * \param[in] defaultValue The control default value > */ > ControlRange::ControlRange(const ControlValue &min, > - const ControlValue &max) > - : min_(min), max_(max) > + const ControlValue &max, > + const ControlValue &defaultValue) > + : defaultValue_(defaultValue), min_(min), max_(max) > { > } > > @@ -376,6 +383,12 @@ ControlRange::ControlRange(const ControlValue &min, > * \return A ControlValue with the maximum value for the control > */ > > +/** > + * \fn ControlRange::defaultValue() > + * \brief Retrieve the default value of the control > + * \return A ControlValue with the default value for the control > + */ > + > /** > * \brief Provide a string representation of the ControlRange > */
Hi Laurent, On Wed, Dec 04, 2019 at 05:48:32PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Wed, Dec 04, 2019 at 02:21:00PM +0100, Jacopo Mondi wrote: > > Augment the the ControlRange class to store the control default value. > > > > This is particularly relevant for v4l2 controls used to create > > Camera properties, which are constructed using immutable video device > > properties, whose value won't change at runtime. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > include/libcamera/controls.h | 7 ++++++- > > src/libcamera/controls.cpp | 17 +++++++++++++++-- > > 2 files changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > index b1b73367e874..1e2f284eafeb 100644 > > --- a/include/libcamera/controls.h > > +++ b/include/libcamera/controls.h > > @@ -114,10 +114,12 @@ class ControlRange > > { > > public: > > explicit ControlRange(const ControlValue &min = 0, > > - const ControlValue &max = 0); > > + const ControlValue &max = 0, > > + const ControlValue &defaultValue = 0); > > I wish we could call this default, but that's a reserved keyword :-/ > Maybe just 'def' to match min and max ? I'll leave it up to you. > > > > > > > const ControlValue &min() const { return min_; } > > const ControlValue &max() const { return max_; } > > + const ControlValue &defaultValue() const { return defaultValue_; } > > > > std::string toString() const; > > > > @@ -131,6 +133,9 @@ public: > > return !(*this == other); > > } > > > > +protected: > > + ControlValue defaultValue_; > > + > > Why is this protected while min_ and max_ are private ? I don't think > the default value should be special. > Oh, that's a leftover from when I set the default value outside of the constructor in V4L2ControlRange subclass. I'll fix Thanks j > > private: > > ControlValue min_; > > ControlValue max_; > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > > index 7d8a0e97ee3a..bacba0fbf68a 100644 > > --- a/src/libcamera/controls.cpp > > +++ b/src/libcamera/controls.cpp > > @@ -353,14 +353,21 @@ Control<int64_t>::Control(unsigned int id, const char *name) > > * pipeline handlers to describe the controls they support. > > */ > > > > +/** > > + * \var ControlRange::defaultValue_ > > + * \brief The control default value > > + */ > > + > > /** > > * \brief Construct a ControlRange with minimum and maximum range parameters > > * \param[in] min The control minimum value > > * \param[in] max The control maximum value > > + * \param[in] defaultValue The control default value > > */ > > ControlRange::ControlRange(const ControlValue &min, > > - const ControlValue &max) > > - : min_(min), max_(max) > > + const ControlValue &max, > > + const ControlValue &defaultValue) > > + : defaultValue_(defaultValue), min_(min), max_(max) > > { > > } > > > > @@ -376,6 +383,12 @@ ControlRange::ControlRange(const ControlValue &min, > > * \return A ControlValue with the maximum value for the control > > */ > > > > +/** > > + * \fn ControlRange::defaultValue() > > + * \brief Retrieve the default value of the control > > + * \return A ControlValue with the default value for the control > > + */ > > + > > /** > > * \brief Provide a string representation of the ControlRange > > */ > > -- > Regards, > > Laurent Pinchart
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index b1b73367e874..1e2f284eafeb 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -114,10 +114,12 @@ class ControlRange { public: explicit ControlRange(const ControlValue &min = 0, - const ControlValue &max = 0); + const ControlValue &max = 0, + const ControlValue &defaultValue = 0); const ControlValue &min() const { return min_; } const ControlValue &max() const { return max_; } + const ControlValue &defaultValue() const { return defaultValue_; } std::string toString() const; @@ -131,6 +133,9 @@ public: return !(*this == other); } +protected: + ControlValue defaultValue_; + private: ControlValue min_; ControlValue max_; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 7d8a0e97ee3a..bacba0fbf68a 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -353,14 +353,21 @@ Control<int64_t>::Control(unsigned int id, const char *name) * pipeline handlers to describe the controls they support. */ +/** + * \var ControlRange::defaultValue_ + * \brief The control default value + */ + /** * \brief Construct a ControlRange with minimum and maximum range parameters * \param[in] min The control minimum value * \param[in] max The control maximum value + * \param[in] defaultValue The control default value */ ControlRange::ControlRange(const ControlValue &min, - const ControlValue &max) - : min_(min), max_(max) + const ControlValue &max, + const ControlValue &defaultValue) + : defaultValue_(defaultValue), min_(min), max_(max) { } @@ -376,6 +383,12 @@ ControlRange::ControlRange(const ControlValue &min, * \return A ControlValue with the maximum value for the control */ +/** + * \fn ControlRange::defaultValue() + * \brief Retrieve the default value of the control + * \return A ControlValue with the default value for the control + */ + /** * \brief Provide a string representation of the ControlRange */
Augment the the ControlRange class to store the control default value. This is particularly relevant for v4l2 controls used to create Camera properties, which are constructed using immutable video device properties, whose value won't change at runtime. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/controls.h | 7 ++++++- src/libcamera/controls.cpp | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-)