[libcamera-devel] [PATCH 11/12] libcamera: Add ControlValidator implementation for Camera

[Laurent Pinchart]

Hi Jacopo,

On Sun, Sep 29, 2019 at 12:19:39PM +0200, Jacopo Mondi wrote:
> On Sat, Sep 28, 2019 at 06:22:37PM +0300, Laurent Pinchart wrote:
> > Add a new CameraControlValidator class that implements the
> > ControlValidator interface for a Camera object.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/camera_controls.cpp       | 53 +++++++++++++++++++++++++
> >  src/libcamera/include/camera_controls.h | 30 ++++++++++++++
> >  src/libcamera/include/meson.build       |  1 +
> >  src/libcamera/meson.build               |  1 +
> >  4 files changed, 85 insertions(+)
> >  create mode 100644 src/libcamera/camera_controls.cpp
> >  create mode 100644 src/libcamera/include/camera_controls.h
> >
> > diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp
> > new file mode 100644
> > index 000000000000..341da56019f7
> > --- /dev/null
> > +++ b/src/libcamera/camera_controls.cpp
> > @@ -0,0 +1,53 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * camera_controls.cpp - Camera controls
> > + */
> > +
> > +#include "camera_controls.h"
> > +
> > +#include <libcamera/camera.h>
> > +#include <libcamera/controls.h>
> > +
> > +/**
> > + * \file camera_controls.h
> > + * \brief Controls for Camera instances
> > + */
> > +
> > +namespace libcamera {
> > +
> > +/**
> > + * \class CameraControlValidator
> > + * \brief A control validator for Camera instances
> > + *
> > + * This ControlValidator specialisation validates that controls exist in the
> > + * Camera associated with the validator.
> > + */
> > +
> > +/**
> > + * \brief Construst a CameraControlValidator for the \a camera
> > + * \param[in] camera The camera
> > + */
> > +CameraControlValidator::CameraControlValidator(Camera *camera)
> > +	: camera_(camera)
> > +{
> > +}
> 
> I'm not sure how we're supposed to serialize this, considering
> ControlList depends on ControlValidator, and ControlValidator depends
> on Camera.

I think the validator can easily be modified to take a ControlInfoMap
instead of a Camera. This series tries not to introduce regressions when
it comes to serialisation, but it also doesn't provide everything you'll
need for serialisation :-) My goal here was to remove the ControlList
dependency on Camera, more can be built on top.

> Also, I'm not sure what is best in hainvg a separate validation object
> that basically checks if the control is part of the ControlInfoMap of
> the Camera.
> 
> I would keep using the ControlInfoMap in the ControlList, as they're
> easily serializable.
> 
> For V4L2Controls I assume you want to do the same and go directly to
> the video device when adding controls to a list. I would rather
> retrieve the V4L2ControlnfoMap from the video device and use that to
> construct the V4L2ControlList.

Yes, it should be based on V4L2ControlnfoMap for V4L2 controls.

> > +
> > +const std::string &CameraControlValidator::name() const
> > +{
> > +	return camera_->name();
> > +}
> > +
> > +/**
> > + * \brief Validate a control
> > + * \param[in] id The control ID
> > + * \return True if the control is valid, false otherwise
> > + */
> > +bool CameraControlValidator::validate(const ControlId &id) const
> > +{
> > +	const ControlInfoMap &controls = camera_->controls();
> > +	return controls.find(&id) != controls.end();
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h
> > new file mode 100644
> > index 000000000000..998a2d155a44
> > --- /dev/null
> > +++ b/src/libcamera/include/camera_controls.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * camera_controls.h - Camera controls
> > + */
> > +#ifndef __LIBCAMERA_CAMERA_CONTROLS_H__
> > +#define __LIBCAMERA_CAMERA_CONTROLS_H__
> > +
> > +#include "control_validator.h"
> > +
> > +namespace libcamera {
> > +
> > +class Camera;
> > +
> > +class CameraControlValidator final : public ControlValidator
> > +{
> > +public:
> > +	CameraControlValidator(Camera *camera);
> > +
> > +	const std::string &name() const override;
> > +	bool validate(const ControlId &id) const override;
> > +
> > +private:
> > +	Camera *camera_;
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_CAMERA_CONTROLS_H__ */
> > diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build
> > index 1cf47204f2b5..2c74d29bd925 100644
> > --- a/src/libcamera/include/meson.build
> > +++ b/src/libcamera/include/meson.build
> > @@ -1,4 +1,5 @@
> >  libcamera_headers = files([
> > +    'camera_controls.h',
> >      'camera_sensor.h',
> >      'control_validator.h',
> >      'device_enumerator.h',
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 984cb9985f49..fbfa11d469e4 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -2,6 +2,7 @@ libcamera_sources = files([
> >      'bound_method.cpp',
> >      'buffer.cpp',
> >      'camera.cpp',
> > +    'camera_controls.cpp',
> >      'camera_manager.cpp',
> >      'camera_sensor.cpp',
> >      'controls.cpp',

-- 
Regards,

Laurent Pinchart