[libcamera-devel] [PATCH v3 1/3] libcamera: pipeline: Pass libcamera controls into pipeline_handler::start()

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sat Dec 5 22:07:29 CET 2020


Hi Naush,

Thank you for the patch.

On Fri, Dec 04, 2020 at 03:31:19PM +0000, Naushir Patuck wrote:
> Applications now have the ability to pass in controls that need to be
> applied on startup, rather than doing it through Request where there might
> be some frames of delay in getting the controls applied.
> 
> This commit adds the ability to pass in a set of libcamera controls into
> the pipeline handlers through the pipeline_handler::start() method. These
> controls are provided by the application through the camera::start()
> public API.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> Tested-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  Documentation/guides/pipeline-handler.rst          |  4 ++--
>  include/libcamera/camera.h                         |  2 +-
>  include/libcamera/internal/pipeline_handler.h      |  2 +-
>  src/libcamera/camera.cpp                           | 11 ++++++-----
>  src/libcamera/pipeline/ipu3/ipu3.cpp               |  4 ++--
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  4 ++--
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp           |  4 ++--
>  src/libcamera/pipeline/simple/simple.cpp           |  4 ++--
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       |  4 ++--
>  src/libcamera/pipeline/vimc/vimc.cpp               |  4 ++--
>  src/libcamera/pipeline_handler.cpp                 |  1 +
>  11 files changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 57aee455..63275a12 100644
> --- a/Documentation/guides/pipeline-handler.rst
> +++ b/Documentation/guides/pipeline-handler.rst
> @@ -209,7 +209,7 @@ methods for the overridden class members.
>            int exportFrameBuffers(Camera *camera, Stream *stream,
>            std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>  
> -          int start(Camera *camera) override;
> +          int start(Camera *camera, ControlList *controls) override;
>            void stop(Camera *camera) override;
>  
>            int queueRequestDevice(Camera *camera, Request *request) override;
> @@ -239,7 +239,7 @@ methods for the overridden class members.
>            return -1;
>     }
>  
> -   int PipelineHandlerVivid::start(Camera *camera)
> +   int PipelineHandlerVivid::start(Camera *camera, ControlList *controls)
>     {
>            return -1;
>     }
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 5c5f1a05..f94f8599 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -103,7 +103,7 @@ public:
>  	std::unique_ptr<Request> createRequest(uint64_t cookie = 0);
>  	int queueRequest(Request *request);
>  
> -	int start();
> +	int start(ControlList *controls = nullptr);
>  	int stop();
>  
>  private:
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c12c8904..bd3c4a81 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -78,7 +78,7 @@ public:
>  	virtual int exportFrameBuffers(Camera *camera, Stream *stream,
>  				       std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
>  
> -	virtual int start(Camera *camera) = 0;
> +	virtual int start(Camera *camera, ControlList *controls) = 0;
>  	virtual void stop(Camera *camera) = 0;
>  
>  	int queueRequest(Camera *camera, Request *request);
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index bcb7b046..ffb63fb2 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -1003,9 +1003,10 @@ int Camera::queueRequest(Request *request)
>  /**
>   * \brief Start capture from camera

Missing

 * \param[in] controls Controls to be applied before starting the Camera

>   *
> - * Start the camera capture session. Once the camera is started the application
> - * can queue requests to the camera to process and return to the application
> - * until the capture session is terminated with \a stop().
> + * Start the camera capture session, optionally providing a list of controls to
> + * action before starting. Once the camera is started the application can queue

s/action/apply/ ?

> + * requests to the camera to process and return to the application until the
> + * capture session is terminated with \a stop().
>   *
>   * \context This function may only be called when the camera is in the
>   * Configured state as defined in \ref camera_operation, and shall be
> @@ -1016,7 +1017,7 @@ int Camera::queueRequest(Request *request)
>   * \retval -ENODEV The camera has been disconnected from the system
>   * \retval -EACCES The camera is not in a state where it can be started
>   */
> -int Camera::start()
> +int Camera::start(ControlList *controls)
>  {
>  	Private *const d = LIBCAMERA_D_PTR();
>  
> @@ -1027,7 +1028,7 @@ int Camera::start()
>  	LOG(Camera, Debug) << "Starting capture";
>  
>  	ret = d->pipe_->invokeMethod(&PipelineHandler::start,
> -				     ConnectionTypeBlocking, this);
> +				     ConnectionTypeBlocking, this, controls);
>  	if (ret)
>  		return ret;
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 4cedb32b..8a1918d5 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -105,7 +105,7 @@ public:
>  	int exportFrameBuffers(Camera *camera, Stream *stream,
>  			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>  
> -	int start(Camera *camera) override;
> +	int start(Camera *camera, ControlList *controls) override;
>  	void stop(Camera *camera) override;
>  
>  	int queueRequestDevice(Camera *camera, Request *request) override;
> @@ -596,7 +596,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)
>  	return 0;
>  }
>  
> -int PipelineHandlerIPU3::start(Camera *camera)
> +int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *controls)

Could you wrap the line ?

int PipelineHandlerIPU3::start(Camera *camera,
			       [[maybe_unused]] ControlList *controls)

Same in a few places below.

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

>  {
>  	IPU3CameraData *data = cameraData(camera);
>  	CIO2Device *cio2 = &data->cio2_;
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 6fcdf557..9937db73 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -242,7 +242,7 @@ public:
>  	int exportFrameBuffers(Camera *camera, Stream *stream,
>  			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>  
> -	int start(Camera *camera) override;
> +	int start(Camera *camera, ControlList *controls) override;
>  	void stop(Camera *camera) override;
>  
>  	int queueRequestDevice(Camera *camera, Request *request) override;
> @@ -731,7 +731,7 @@ int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stre
>  	return ret;
>  }
>  
> -int PipelineHandlerRPi::start(Camera *camera)
> +int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *controls)
>  {
>  	RPiCameraData *data = cameraData(camera);
>  	int ret;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 6e74a49a..4e959fde 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -187,7 +187,7 @@ public:
>  	int exportFrameBuffers(Camera *camera, Stream *stream,
>  			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>  
> -	int start(Camera *camera) override;
> +	int start(Camera *camera, ControlList *controls) override;
>  	void stop(Camera *camera) override;
>  
>  	int queueRequestDevice(Camera *camera, Request *request) override;
> @@ -832,7 +832,7 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
>  	return 0;
>  }
>  
> -int PipelineHandlerRkISP1::start(Camera *camera)
> +int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *controls)
>  {
>  	RkISP1CameraData *data = cameraData(camera);
>  	int ret;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 0d3078f7..b047aeb9 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -126,7 +126,7 @@ public:
>  	int exportFrameBuffers(Camera *camera, Stream *stream,
>  			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>  
> -	int start(Camera *camera) override;
> +	int start(Camera *camera, ControlList *controls) override;
>  	void stop(Camera *camera) override;
>  
>  	bool match(DeviceEnumerator *enumerator) override;
> @@ -646,7 +646,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
>  		return data->video_->exportBuffers(count, buffers);
>  }
>  
> -int SimplePipelineHandler::start(Camera *camera)
> +int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] ControlList *controls)
>  {
>  	SimpleCameraData *data = cameraData(camera);
>  	V4L2VideoDevice *video = data->video_;
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 0f3241cc..87b0f03d 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -76,7 +76,7 @@ public:
>  	int exportFrameBuffers(Camera *camera, Stream *stream,
>  			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>  
> -	int start(Camera *camera) override;
> +	int start(Camera *camera, ControlList *controls) override;
>  	void stop(Camera *camera) override;
>  
>  	int queueRequestDevice(Camera *camera, Request *request) override;
> @@ -236,7 +236,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
>  	return data->video_->exportBuffers(count, buffers);
>  }
>  
> -int PipelineHandlerUVC::start(Camera *camera)
> +int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] ControlList *controls)
>  {
>  	UVCCameraData *data = cameraData(camera);
>  	unsigned int count = data->stream_.configuration().bufferCount;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 914b6b54..d81b8598 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -92,7 +92,7 @@ public:
>  	int exportFrameBuffers(Camera *camera, Stream *stream,
>  			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>  
> -	int start(Camera *camera) override;
> +	int start(Camera *camera, ControlList *controls) override;
>  	void stop(Camera *camera) override;
>  
>  	int queueRequestDevice(Camera *camera, Request *request) override;
> @@ -313,7 +313,7 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
>  	return data->video_->exportBuffers(count, buffers);
>  }
>  
> -int PipelineHandlerVimc::start(Camera *camera)
> +int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *controls)
>  {
>  	VimcCameraData *data = cameraData(camera);
>  	unsigned int count = data->stream_.configuration().bufferCount;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 894200ee..c75ebbd5 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -351,6 +351,7 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
>   * \fn PipelineHandler::start()
>   * \brief Start capturing from a group of streams
>   * \param[in] camera The camera to start
> + * \param[in] controls Controls to be applied before starting the Camera
>   *
>   * Start the group of streams that have been configured for capture by
>   * \a configure(). The intended caller of this method is the Camera class which

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list