[libcamera-devel,v3,1/2] libcamera: camera: Create a CameraControlValidator

14205 diff mbox series

Create a Camera specific CameraControlValidator for the Camera instance.
This will allow requests to use a single validor instance without having
to construct their own.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
v3:
 - Use a unique_ptr instead of deriving from the _o/O_PTRs

v2:
 - Use LIBCAMERA_O_PTR() instead of _o<Public>()
---
 include/libcamera/internal/camera.h | 6 ++++++
 src/libcamera/camera.cpp            | 7 +++++++
 2 files changed, 13 insertions(+)

Hi Kieran,

Thank you for the patch.

On Wed, Oct 20, 2021 at 02:33:52PM +0100, Kieran Bingham wrote:
> Create a Camera specific CameraControlValidator for the Camera instance.

s/Camera specific/Camera-specific/

> This will allow requests to use a single validor instance without having

s/validor/validator/

> to construct their own.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> ---
> v3:
>  - Use a unique_ptr instead of deriving from the _o/O_PTRs
> 
> v2:
>  - Use LIBCAMERA_O_PTR() instead of _o<Public>()
> ---
>  include/libcamera/internal/camera.h | 6 ++++++
>  src/libcamera/camera.cpp            | 7 +++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> index 1a08da0cedc4..f0baaea07bdc 100644
> --- a/include/libcamera/internal/camera.h
> +++ b/include/libcamera/internal/camera.h
> @@ -17,6 +17,8 @@
>  
>  #include <libcamera/camera.h>
>  
> +#include "libcamera/internal/camera_controls.h"

You could forward-declare the CameraControlValidator class here.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
>  namespace libcamera {
>  
>  class PipelineHandler;
> @@ -38,6 +40,8 @@ public:
>  
>  	uint32_t requestSequence_;
>  
> +	const CameraControlValidator *validator() const { return validator_.get(); }
> +
>  private:
>  	enum State {
>  		CameraAvailable,
> @@ -64,6 +68,8 @@ private:
>  
>  	bool disconnected_;
>  	std::atomic<State> state_;
> +
> +	std::unique_ptr<CameraControlValidator> validator_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 71809bcd36a3..f84cdc522d0d 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -366,6 +366,12 @@ Camera::Private::~Private()
>   * \return The pipeline handler that created this camera
>   */
>  
> +/**
> + * \fn Camera::Private::validator()
> + * \brief Retrieve the control validator related to this camera
> + * \return The control validator associated with this camera
> + */
> +
>  /**
>   * \var Camera::Private::queuedRequests_
>   * \brief The list of queued and not yet completed requests
> @@ -665,6 +671,7 @@ Camera::Camera(std::unique_ptr<Private> d, const std::string &id,
>  {
>  	_d()->id_ = id;
>  	_d()->streams_ = streams;
> +	_d()->validator_ = std::make_unique<CameraControlValidator>(this);
>  }
>  
>  Camera::~Camera()

Quoting Laurent Pinchart (2021-10-22 15:11:48)
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Wed, Oct 20, 2021 at 02:33:52PM +0100, Kieran Bingham wrote:
> > Create a Camera specific CameraControlValidator for the Camera instance.
> 
> s/Camera specific/Camera-specific/
> 
> > This will allow requests to use a single validor instance without having
> 
> s/validor/validator/
> 
> > to construct their own.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > ---
> > v3:
> >  - Use a unique_ptr instead of deriving from the _o/O_PTRs
> > 
> > v2:
> >  - Use LIBCAMERA_O_PTR() instead of _o<Public>()
> > ---
> >  include/libcamera/internal/camera.h | 6 ++++++
> >  src/libcamera/camera.cpp            | 7 +++++++
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> > index 1a08da0cedc4..f0baaea07bdc 100644
> > --- a/include/libcamera/internal/camera.h
> > +++ b/include/libcamera/internal/camera.h
> > @@ -17,6 +17,8 @@
> >  
> >  #include <libcamera/camera.h>
> >  
> > +#include "libcamera/internal/camera_controls.h"
> 
> You could forward-declare the CameraControlValidator class here.

Good point. I'll update that, it saves the compiler from loading a whole
file to compile this one.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > +
> >  namespace libcamera {
> >  
> >  class PipelineHandler;
> > @@ -38,6 +40,8 @@ public:
> >  
> >       uint32_t requestSequence_;
> >  
> > +     const CameraControlValidator *validator() const { return validator_.get(); }
> > +
> >  private:
> >       enum State {
> >               CameraAvailable,
> > @@ -64,6 +68,8 @@ private:
> >  
> >       bool disconnected_;
> >       std::atomic<State> state_;
> > +
> > +     std::unique_ptr<CameraControlValidator> validator_;
> >  };
> >  
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 71809bcd36a3..f84cdc522d0d 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -366,6 +366,12 @@ Camera::Private::~Private()
> >   * \return The pipeline handler that created this camera
> >   */
> >  
> > +/**
> > + * \fn Camera::Private::validator()
> > + * \brief Retrieve the control validator related to this camera
> > + * \return The control validator associated with this camera
> > + */
> > +
> >  /**
> >   * \var Camera::Private::queuedRequests_
> >   * \brief The list of queued and not yet completed requests
> > @@ -665,6 +671,7 @@ Camera::Camera(std::unique_ptr<Private> d, const std::string &id,
> >  {
> >       _d()->id_ = id;
> >       _d()->streams_ = streams;
> > +     _d()->validator_ = std::make_unique<CameraControlValidator>(this);
> >  }
> >  
> >  Camera::~Camera()
> 
> -- 
> Regards,
> 
> Laurent Pinchart