[libcamera-devel] [PATCH v2 2/7] qcam: Move requestCompleted signal mapping

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Feb 14, 2020 at 12:18:05AM +0000, Kieran Bingham wrote:
> The MainWindow connects a handler to the Camera requestCompleted signal
> when the camera is opened, but never disconnects it.
> 
> Move the connection to the startCapture() function, and ensure that it
> is disconnected again when the stream is stopped.
> 
> This ensures that we can successfully tear down the stream, and restart
> with a new camera.

The commit also adds a camera_->stop() call in an error path, it should
be mentioned in the commit message (or split to a separate patch).

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

I think we could avoid the nasty error_disconnect by splitting out code
to openCamera/closeCamera methods. This patch however does the job for
now, so

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

> ---
>  src/qcam/main_window.cpp | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index db14245d7f51..38d7063363f0 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -113,8 +113,6 @@ int MainWindow::openCamera(CameraManager *cm)
>  
>  	std::cout << "Using camera " << camera_->name() << std::endl;
>  
> -	camera_->requestCompleted.connect(this, &MainWindow::requestComplete);
> -
>  	return 0;
>  }
>  
> @@ -212,17 +210,23 @@ int MainWindow::startCapture()
>  		goto error;
>  	}
>  
> +	camera_->requestCompleted.connect(this, &MainWindow::requestComplete);
> +
>  	for (Request *request : requests) {
>  		ret = camera_->queueRequest(request);
>  		if (ret < 0) {
>  			std::cerr << "Can't queue request" << std::endl;
> -			goto error;
> +			goto error_disconnect;
>  		}
>  	}
>  
>  	isCapturing_ = true;
>  	return 0;
>  
> +error_disconnect:
> +	camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);
> +	camera_->stop();
> +
>  error:
>  	for (Request *request : requests)
>  		delete request;
> @@ -249,6 +253,8 @@ void MainWindow::stopCapture()
>  	if (ret)
>  		std::cout << "Failed to stop capture" << std::endl;
>  
> +	camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);
> +
>  	for (auto &iter : mappedBuffers_) {
>  		void *memory = iter.second.first;
>  		unsigned int length = iter.second.second;

-- 
Regards,

Laurent Pinchart