[libcamera-devel] [PATCH v3 2/2] libcamera: request: Use external CameraControlValidator

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wed, Oct 20, 2021 at 02:33:53PM +0100, Kieran Bingham wrote:
> Each Request is currently creating its own CameraControlValidator
> using the Camera instance at construction.
> 
> Now that the Camera exposes its own CameraControlValidator on its
> private interface, use that one on all Requests.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

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

> ---
>  include/libcamera/request.h |  1 -
>  src/libcamera/request.cpp   | 10 +++-------
>  2 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 2d361c9d97dc..d16904e6b679 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -71,7 +71,6 @@ private:
>  	bool completeBuffer(FrameBuffer *buffer);
>  
>  	Camera *camera_;
> -	CameraControlValidator *validator_;
>  	ControlList *controls_;
>  	ControlList *metadata_;
>  	BufferMap bufferMap_;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index f95ce4db5eaa..17fefab7ad0e 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -17,6 +17,7 @@
>  #include <libcamera/framebuffer.h>
>  #include <libcamera/stream.h>
>  
> +#include "libcamera/internal/camera.h"
>  #include "libcamera/internal/camera_controls.h"
>  #include "libcamera/internal/framebuffer.h"
>  #include "libcamera/internal/tracepoints.h"
> @@ -77,12 +78,8 @@ Request::Request(Camera *camera, uint64_t cookie)
>  	: camera_(camera), sequence_(0), cookie_(cookie),
>  	  status_(RequestPending), cancelled_(false)
>  {
> -	/**
> -	 * \todo Should the Camera expose a validator instance, to avoid
> -	 * creating a new instance for each request?
> -	 */
> -	validator_ = new CameraControlValidator(camera);
> -	controls_ = new ControlList(controls::controls, validator_);
> +	controls_ = new ControlList(controls::controls,
> +				    camera->_d()->validator());
>  
>  	/**
>  	 * \todo: Add a validator for metadata controls.
> @@ -100,7 +97,6 @@ Request::~Request()
>  
>  	delete metadata_;
>  	delete controls_;
> -	delete validator_;
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart