[libcamera-devel] [PATCH v5 1/3] qcam: Queue requests only through MainWindow::queueRequest()

[Laurent Pinchart]

Hi Utkarsh,

Thank you for the patch.

On Wed, Jul 27, 2022 at 01:11:21AM +0530, Utkarsh Tiwari via libcamera-devel wrote:
> Currently to request a frame, we operate the camera directly.
> This approach is also scattered in two places,
> MainWindow::startCapture() and MainWindow::queueRequest().
> This makes it difficult to account for requests.
> 
> Centralize all the queuing to a single function queueRequest()
> 
> Rename the current queueRequest() to renderComplete().
> This makes more sense as this slot is triggered when
> the render is complete and we want to queue another
> request.
> 
> Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

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

> ---
>  Changes from v4
> 	- Grammer fixes in the commit message
>  src/qcam/main_window.cpp | 14 +++++++++-----
>  src/qcam/main_window.h   |  3 ++-
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 7433d647..4e773c31 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -120,14 +120,14 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  	if (renderType == "qt") {
>  		ViewFinderQt *viewfinder = new ViewFinderQt(this);
>  		connect(viewfinder, &ViewFinderQt::renderComplete,
> -			this, &MainWindow::queueRequest);
> +			this, &MainWindow::renderComplete);
>  		viewfinder_ = viewfinder;
>  		setCentralWidget(viewfinder);
>  #ifndef QT_NO_OPENGL
>  	} else if (renderType == "gles") {
>  		ViewFinderGL *viewfinder = new ViewFinderGL(this);
>  		connect(viewfinder, &ViewFinderGL::renderComplete,
> -			this, &MainWindow::queueRequest);
> +			this, &MainWindow::renderComplete);
>  		viewfinder_ = viewfinder;
>  		setCentralWidget(viewfinder);
>  #endif
> @@ -522,7 +522,7 @@ int MainWindow::startCapture()
>  
>  	/* Queue all requests. */
>  	for (std::unique_ptr<Request> &request : requests_) {
> -		ret = camera_->queueRequest(request.get());
> +		ret = queueRequest(request.get());
>  		if (ret < 0) {
>  			qWarning() << "Can't queue request";
>  			goto error_disconnect;
> @@ -756,7 +756,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>  	viewfinder_->render(buffer, mappedBuffers_[buffer].get());
>  }
>  
> -void MainWindow::queueRequest(FrameBuffer *buffer)
> +void MainWindow::renderComplete(FrameBuffer *buffer)
>  {
>  	Request *request;
>  	{
> @@ -785,6 +785,10 @@ void MainWindow::queueRequest(FrameBuffer *buffer)
>  			qWarning() << "No free buffer available for RAW capture";
>  		}
>  	}
> +	queueRequest(request);
> +}
>  
> -	camera_->queueRequest(request);
> +int MainWindow::queueRequest(Request *request)
> +{
> +	return camera_->queueRequest(request);
>  }
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index fc70920f..bc844711 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -67,7 +67,7 @@ private Q_SLOTS:
>  	void processRaw(libcamera::FrameBuffer *buffer,
>  			const libcamera::ControlList &metadata);
>  
> -	void queueRequest(libcamera::FrameBuffer *buffer);
> +	void renderComplete(libcamera::FrameBuffer *buffer);
>  
>  private:
>  	int createToolbars();
> @@ -81,6 +81,7 @@ private:
>  	void addCamera(std::shared_ptr<libcamera::Camera> camera);
>  	void removeCamera(std::shared_ptr<libcamera::Camera> camera);
>  
> +	int queueRequest(libcamera::Request *request);
>  	void requestComplete(libcamera::Request *request);
>  	void processCapture();
>  	void processHotplug(HotplugEvent *e);

-- 
Regards,

Laurent Pinchart