[libcamera-devel] [PATCH v4 2/5] lc-compliance: Make SimpleCapture::stop() idempotent

[Laurent Pinchart]

Hi Nícolas,

Thank you for the patch.

On Fri, May 21, 2021 at 10:30:51AM -0300, Nícolas F. R. A. Prado wrote:
> Make SimpleCapture::stop() be able to be called multiple times and at
> any point so that it can be called from the destructor and an assert
> failure can return immediately.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>

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

> ---
> No changes in v4
> 
> No changes in v3
> 
> Changes in v2:
> - Suggested by Laurent:
>   - Fixed code style
> 
>  src/lc-compliance/simple_capture.cpp | 33 +++++++++++-----------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> index 64e862a08e3a..f90fe6d0f9aa 100644
> --- a/src/lc-compliance/simple_capture.cpp
> +++ b/src/lc-compliance/simple_capture.cpp
> @@ -17,6 +17,7 @@ SimpleCapture::SimpleCapture(std::shared_ptr<Camera> camera)
>  
>  SimpleCapture::~SimpleCapture()
>  {
> +	stop();
>  }
>  
>  Results::Result SimpleCapture::configure(StreamRole role)
> @@ -55,12 +56,17 @@ Results::Result SimpleCapture::start()
>  
>  void SimpleCapture::stop()
>  {
> -	Stream *stream = config_->at(0).stream();
> +	if (!config_)
> +		return;
>  
>  	camera_->stop();
>  
>  	camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete);
>  
> +	if (!allocator_->allocated())
> +		return;
> +
> +	Stream *stream = config_->at(0).stream();
>  	allocator_->free(stream);
>  }
>  
> @@ -84,7 +90,6 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
>  	if (buffers.size() > numRequests) {
>  		/* Cache buffers.size() before we destroy it in stop() */
>  		int buffers_size = buffers.size();
> -		stop();
>  
>  		return { Results::Skip, "Camera needs " + std::to_string(buffers_size)
>  			+ " requests, can't test only " + std::to_string(numRequests) };
> @@ -98,20 +103,14 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
>  	std::vector<std::unique_ptr<libcamera::Request>> requests;
>  	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
>  		std::unique_ptr<Request> request = camera_->createRequest();
> -		if (!request) {
> -			stop();
> +		if (!request)
>  			return { Results::Fail, "Can't create request" };
> -		}
>  
> -		if (request->addBuffer(stream, buffer.get())) {
> -			stop();
> +		if (request->addBuffer(stream, buffer.get()))
>  			return { Results::Fail, "Can't set buffer for request" };
> -		}
>  
> -		if (queueRequest(request.get()) < 0) {
> -			stop();
> +		if (queueRequest(request.get()) < 0)
>  			return { Results::Fail, "Failed to queue request" };
> -		}
>  
>  		requests.push_back(std::move(request));
>  	}
> @@ -175,20 +174,14 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
>  	std::vector<std::unique_ptr<libcamera::Request>> requests;
>  	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
>  		std::unique_ptr<Request> request = camera_->createRequest();
> -		if (!request) {
> -			stop();
> +		if (!request)
>  			return { Results::Fail, "Can't create request" };
> -		}
>  
> -		if (request->addBuffer(stream, buffer.get())) {
> -			stop();
> +		if (request->addBuffer(stream, buffer.get()))
>  			return { Results::Fail, "Can't set buffer for request" };
> -		}
>  
> -		if (camera_->queueRequest(request.get()) < 0) {
> -			stop();
> +		if (camera_->queueRequest(request.get()) < 0)
>  			return { Results::Fail, "Failed to queue request" };
> -		}
>  
>  		requests.push_back(std::move(request));
>  	}

-- 
Regards,

Laurent Pinchart