[libcamera-devel] [PATCH 04/10] libcamera: controls: Add default to ControlRange

[Jacopo Mondi]

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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191205/0002e8de/attachment.sig>