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

[Laurent Pinchart]

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." ?

> > + */
> > +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.

> > +}
> > +
> >  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart