[v2] libcamera: pipeline_handler: Fix requestComplete on waiting requests on stop

24978 diff mbox series

The requestComplete signal is not emitted when the camera is stopped and
the request is still in the waitingRequests_ queue. Fix that by calling
doQueueRequest() on the waiting requests after marking them as
cancelled. This ensures that the requests gets a proper sequence number
and are added to the queuedRequests_ list. This list is then iterated in
completeRequest() and leads to the requestComplete signal.

Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/281
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---

Hey all,

In the discussion on v1 https://patchwork.libcamera.org/patch/24719/ it
was rightly noted that the solution does feel a bit hacky. However there
was no clear better solution and it was also acknowledged that the
change is quite small and solves the problem. So I'd propose to merge it
in as long as there is no better proposal.

Changes in v2:
- Collected rby tag
- Added closes tag to automatically close the issue
- Slightly modified the comment and commit message to mention that this
  logic also ensures that the requests get a sequence number.

Best regards,
Stefan

---
 src/libcamera/pipeline_handler.cpp | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

2025. 11. 05. 14:58 keltezéssel, Stefan Klug írta:
> The requestComplete signal is not emitted when the camera is stopped and
> the request is still in the waitingRequests_ queue. Fix that by calling
> doQueueRequest() on the waiting requests after marking them as
> cancelled. This ensures that the requests gets a proper sequence number
> and are added to the queuedRequests_ list. This list is then iterated in
> completeRequest() and leads to the requestComplete signal.
> 
> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/281
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> 
> Hey all,
> 
> In the discussion on v1 https://patchwork.libcamera.org/patch/24719/ it
> was rightly noted that the solution does feel a bit hacky. However there
> was no clear better solution and it was also acknowledged that the
> change is quite small and solves the problem. So I'd propose to merge it
> in as long as there is no better proposal.
> 
> Changes in v2:
> - Collected rby tag
> - Added closes tag to automatically close the issue
> - Slightly modified the comment and commit message to mention that this
>    logic also ensures that the requests get a sequence number.
> 
> Best regards,
> Stefan
> 
> ---

Looks OK.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/libcamera/pipeline_handler.cpp | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index e5f9e55c9783..f398f62e0085 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -380,7 +380,16 @@ void PipelineHandler::stop(Camera *camera)
>   	while (!waitingRequests.empty()) {
>   		Request *request = waitingRequests.front();
>   		waitingRequests.pop();
> -		cancelRequest(request);
> +
> +		/*
> +		 * Cancel all requests by marking them as cancelled and calling
> +		 * doQueueRequest() instead of cancelRequest(). This ensures
> +		 * that the requests get a sequence number and are temporarily
> +		 * added to queuedRequests_ so they can be properly completed in
> +		 * completeRequest().
> +		 */
> +		request->_d()->cancel();
> +		doQueueRequest(request);
>   	}
>   
>   	/* Make sure no requests are pending. */