[libcamera-devel] [PATCH 9/9] libcamera: v4l2_controls: Add control validator

[Jacopo Mondi]

Hi Laurent,

On Wed, Oct 09, 2019 at 12:48:36PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wed, Oct 09, 2019 at 11:27:39AM +0200, Jacopo Mondi wrote:
> > On Tue, Oct 08, 2019 at 01:46:42AM +0300, Laurent Pinchart wrote:
> > > Add a ControlValidator specialisation that validates controls based on a
> > > V4L2 device.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  src/libcamera/include/v4l2_controls.h | 14 ++++++++++++
> > >  src/libcamera/v4l2_controls.cpp       | 33 +++++++++++++++++++++++++++
> > >  2 files changed, 47 insertions(+)
> > >
> > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> > > index 133ab9ec208b..5fd92e3ac4e5 100644
> > > --- a/src/libcamera/include/v4l2_controls.h
> > > +++ b/src/libcamera/include/v4l2_controls.h
> > > @@ -17,6 +17,8 @@
> > >
> > >  #include <libcamera/controls.h>
> > >
> > > +#include "control_validator.h"
> > > +
> > >  namespace libcamera {
> > >
> > >  class V4L2ControlId : public ControlId
> > > @@ -57,6 +59,18 @@ private:
> > >  	const V4L2ControlInfoMap &controls_;
> > >  };
> > >
> > > +class V4L2ControlValidator final : public ControlValidator
> > > +{
> > > +public:
> > > +	V4L2ControlValidator(V4L2Device *device);
> > > +
> > > +	const std::string &name() const override;
> > > +	bool validate(const ControlId &id) const override;
> > > +
> > > +private:
> > > +	V4L2Device *dev_;
> > > +};
> > > +
> > >  } /* namespace libcamera */
> > >
> > >  #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */
> > > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> > > index d3f94709e821..498567ee0bf0 100644
> > > --- a/src/libcamera/v4l2_controls.cpp
> > > +++ b/src/libcamera/v4l2_controls.cpp
> > > @@ -239,4 +239,37 @@ void V4L2ControlList::set(unsigned int id, const ControlValue &value)
> > >  	ControlList::set(ctrl->second.id(), value);
> > >  }
> > >
> > > +/**
> > > + * \class V4L2ControlValidator
> > > + * \brief A control validator for V4L2Devoe instances
> >
> > V4L2Device
>
> Probably better than introducing a V4L2Devoe class, yes.
>
> > > + *
> > > + * This ControlValidator specialisation validates that controls exist in the
> > > + * V4L2Device associated with the validator.
> > > + */
> > > +
> > > +/**
> > > + * \brief Construst a V4L2ControlValidator for the V4L2 \a device
> > > + * \param[in] device The V4L2 device
> > > + */
> > > +V4L2ControlValidator::V4L2ControlValidator(V4L2Device *device)
> > > +	: dev_(device)
> > > +{
> > > +}
> > > +
> > > +const std::string &V4L2ControlValidator::name() const
> > > +{
> > > +	return dev_->deviceNode();
> > > +}
> > > +
> > > +/**
> > > + * \brief Validate a control
> > > + * \param[in] id The control ID
> > > + * \return True if the control is valid, false otherwise
> >
> > I would expand what "valid" means in this context.
> > All validators 'validates' controls, this one does it against the list
> > of controls supported by the video device.
>
> "A control is considered valid if it is supported by the V4L2Device
> associated with the validator." ?
>

Better, thanks!

> > > + */
> > > +bool V4L2ControlValidator::validate(const ControlId &id) const
> > > +{
> > > +	const V4L2ControlInfoMap &controls = dev_->controls();
> > > +	return controls.find(id.id()) != controls.end();
> >
> > The fact std::map::contains(T key) has only been introduced in C++ is
> > really a shame, we have this find() == end() pattern everywhere :(
>
> I know :-( At some point we could bump our minimal C++ version, but
> C++20 is definitely too recent.
>

Not just too recent, it's in the future!1!1

Thanks
   j

> > > +}
> > > +
> > >  } /* namespace libcamera */
>
> --
> 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/20191009/019dc484/attachment.sig>