<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>