[libcamera-devel] [PATCH 3/8] libcamera: pipeline_handler: Notify Request queueing failure

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2021-05-10 12:52:30 +0200, Jacopo Mondi wrote:
> Capture requests are queued by the PipelineHandler base class to each
> pipeline handler implementation using the virtual queueRequestDevice()
> function.
> 
> However, if the pipeline handler fails to queue the request to the
> hardware, the request gets silently deleted from the list of queued
> ones, without notifying application of the error.
> 
> Allowing applications to know if a Request has failed to queue
> by emitting the Camera::bufferCompleted and Camera::requestCompleted
> signals allows them to maintain a state consistent with the one
> internal to the library.
> 
> To do so, if queuing a request to the device fails, cancel the request
> and emit the completion signal to report the failure to applications.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline_handler.cpp | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index f41b7a7b3308..260a52018a4d 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -391,6 +391,8 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const
>   * This method queues a capture request to the pipeline handler for processing.
>   * The request is first added to the internal list of queued requests, and
>   * then passed to the pipeline handler with a call to queueRequestDevice().
> + * If the pipeline handler fails in queuing the request to the hardware the
> + * request is immediately completed with error.
>   *
>   * Keeping track of queued requests ensures automatic completion of all requests
>   * when the pipeline handler is stopped with stop(). Request completion shall be
> @@ -409,8 +411,19 @@ void PipelineHandler::queueRequest(Request *request)
>  	request->sequence_ = data->requestSequence_++;
>  
>  	int ret = queueRequestDevice(camera, request);
> -	if (ret)
> +	if (ret) {
> +		/*
> +		 * Cancel the request and notify completion of its buffers in
> +		 * error state. Then complete the cancelled request and remove
> +		 * it from the queued requests list.
> +		 */
> +		request->cancel();
> +		for (auto bufferMap : request->buffers())
> +			camera->bufferCompleted.emit(request, bufferMap.second);

I wonder if the emitting of bufferCompleted should be moved to 
Camera::requestComplete()? This would be a common thing for all requests 
that completes in the cancel state right?

> +
> +		camera->requestComplete(request);
>  		data->queuedRequests_.remove(request);
> +	}
>  }
>  
>  /**
> -- 
> 2.31.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund