[libcamera-devel,11/12] libcamera: Add ControlValidator implementation for Camera
diff mbox series

Message ID 20190928152238.23752-12-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Improve the application-facing controls API
Related show

Commit Message

Laurent Pinchart Sept. 28, 2019, 3:22 p.m. UTC 
Add a new CameraControlValidator class that implements the
ControlValidator interface for a Camera object.

Signed-off-by: Laurent Pinchart <laurent.pinchart@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

Comments

Jacopo Mondi Sept. 29, 2019, 10:19 a.m. UTC | #1 
Hi Laurent,

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@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.

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.

> +
> +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
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Sept. 29, 2019, 1:37 p.m. UTC | #2 
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@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',

Patch
diff mbox series 

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)
+{
+}
+
+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',