<div dir="ltr"><div dir="ltr">Hi Jacopo and Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, May 17, 2021 at 11:45 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Jacopo,<br>
<br>
Thank you for the patch.<br>
<br>
On Thu, May 13, 2021 at 11:22:41AM +0200, Jacopo Mondi wrote:<br>
> Capture requests are queued by the PipelineHandler base class to each<br>
> pipeline handler implementation using the virtual queueRequestDevice()<br>
> function.<br>
> <br>
> However, if the pipeline handler fails to queue the request to the<br>
> hardware, the request gets silently deleted from the list of queued<br>
> ones, without notifying application of the error.<br>
> <br>
> Allowing applications to know if a Request has failed to queue<br>
> by emitting the Camera::bufferCompleted and Camera::requestCompleted<br>
> signals allows them to maintain a state consistent with the one<br>
> internal to the library.<br>
> <br>
> To do so, if queuing a request to the device fails, cancel the request<br>
> and emit the completion signal to report the failure to applications.<br>
> <br>
> Signed-off-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
> ---<br>
> src/libcamera/pipeline_handler.cpp | 15 ++++++++++++++-<br>
> 1 file changed, 14 insertions(+), 1 deletion(-)<br>
> <br>
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp<br>
> index f41b7a7b3308..260a52018a4d 100644<br>
> --- a/src/libcamera/pipeline_handler.cpp<br>
> +++ b/src/libcamera/pipeline_handler.cpp<br>
> @@ -391,6 +391,8 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const<br>
> * This method queues a capture request to the pipeline handler for processing.<br>
> * The request is first added to the internal list of queued requests, and<br>
> * then passed to the pipeline handler with a call to queueRequestDevice().<br>
> + * If the pipeline handler fails in queuing the request to the hardware the<br>
> + * request is immediately completed with error.<br>
> *<br>
> * Keeping track of queued requests ensures automatic completion of all requests<br>
> * when the pipeline handler is stopped with stop(). Request completion shall be<br>
> @@ -409,8 +411,19 @@ void PipelineHandler::queueRequest(Request *request)<br>
> request->sequence_ = data->requestSequence_++;<br>
> <br>
> int ret = queueRequestDevice(camera, request);<br>
> - if (ret)<br>
> + if (ret) {<br>
> + /*<br>
> + * Cancel the request and notify completion of its buffers in<br>
> + * error state. Then complete the cancelled request and remove<br>
> + * it from the queued requests list.<br>
> + */<br>
> + request->cancel();<br>
> + for (auto bufferMap : request->buffers())<br>
> + camera->bufferCompleted.emit(request, bufferMap.second);<br>
> +<br>
> + camera->requestComplete(request);<br>
<br>
This breaks the request completion ordering. I think you could just call<br>
PipelineHandler::completeRequest() instead.<br></blockquote><div><br></div><div><div>I had the same comment in the previous version. <a href="https://patchwork.libcamera.org/patch/12243/">https://patchwork.libcamera.org/patch/12243/</a><br></div><div>We were waiting for your reply. Alright we got the reply now. </div></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> data->queuedRequests_.remove(request);<br>
> + }<br>
> }<br>
> <br>
> /**<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>