[v2,1/5] libcamera: pipeline_handler: Move waitingRequests_ into camera class

23744 diff mbox series

The waiting requests of one camera should not be able to influence
queuing to another camera. Currently a request that is in the
waitingQUeue and waiting for preparation blocks all requests of other
cameras even if they are already prepared. To fix that replace the
single waitingRequests_ queue with one queue per camera.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Umang Jain <uajain@igalia.com>

---

Changes in v2:
- Fixed capturing on lambda expression
- Added documentation for waitingRequests_
- Improved commit message

Changes in v1:
- Added camera param to doQueueRequests() which allows to remove a loop
---
 include/libcamera/internal/camera.h           |  2 ++
 include/libcamera/internal/pipeline_handler.h |  5 +---
 src/libcamera/camera.cpp                      |  9 +++++++
 src/libcamera/pipeline_handler.cpp            | 26 ++++++++++++-------
 4 files changed, 28 insertions(+), 14 deletions(-)

Quoting Stefan Klug (2025-07-07 08:53:35)
> The waiting requests of one camera should not be able to influence
> queuing to another camera. Currently a request that is in the
> waitingQUeue and waiting for preparation blocks all requests of other

s/waitingQUeue/waitingQueue/

> cameras even if they are already prepared. To fix that replace the
> single waitingRequests_ queue with one queue per camera.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Umang Jain <uajain@igalia.com>
> 

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
> 
> Changes in v2:
> - Fixed capturing on lambda expression
> - Added documentation for waitingRequests_
> - Improved commit message
> 
> Changes in v1:
> - Added camera param to doQueueRequests() which allows to remove a loop
> ---
>  include/libcamera/internal/camera.h           |  2 ++
>  include/libcamera/internal/pipeline_handler.h |  5 +---
>  src/libcamera/camera.cpp                      |  9 +++++++
>  src/libcamera/pipeline_handler.cpp            | 26 ++++++++++++-------
>  4 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> index 18f5c32a18e4..8a2e9ed5894d 100644
> --- a/include/libcamera/internal/camera.h
> +++ b/include/libcamera/internal/camera.h
> @@ -10,6 +10,7 @@
>  #include <atomic>
>  #include <list>
>  #include <memory>
> +#include <queue>
>  #include <set>
>  #include <stdint.h>
>  #include <string>
> @@ -36,6 +37,7 @@ public:
>         const PipelineHandler *pipe() const { return pipe_.get(); }
>  
>         std::list<Request *> queuedRequests_;
> +       std::queue<Request *> waitingRequests_;
>         ControlInfoMap controlInfo_;
>         ControlList properties_;
>  
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 972a2fa65310..be017ad47219 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -8,7 +8,6 @@
>  #pragma once
>  
>  #include <memory>
> -#include <queue>
>  #include <string>
>  #include <sys/types.h>
>  #include <vector>
> @@ -89,13 +88,11 @@ private:
>         virtual void disconnect();
>  
>         void doQueueRequest(Request *request);
> -       void doQueueRequests();
> +       void doQueueRequests(Camera *camera);
>  
>         std::vector<std::shared_ptr<MediaDevice>> mediaDevices_;
>         std::vector<std::weak_ptr<Camera>> cameras_;
>  
> -       std::queue<Request *> waitingRequests_;
> -
>         const char *name_;
>         unsigned int useCount_;
>  
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index c180a5fdde93..2ae50eaeaa5a 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -628,6 +628,15 @@ Camera::Private::~Private()
>   * PipelineHandler::completeRequest()
>   */
>  
> +/**
> + * \var Camera::Private::waitingRequests_
> + * \brief The queue of waiting requests
> + *
> + * This queue tracks all requests that can not yet be queued to the device.
> + * Either because they are not yet prepared or because the maximum number of
> + * queued requests was reached.
> + */
> +
>  /**
>   * \var Camera::Private::controlInfo_
>   * \brief The set of controls supported by the camera
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index d84dff3c9f19..b50eda5e0f86 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -363,15 +363,16 @@ void PipelineHandler::stop(Camera *camera)
>         /* Stop the pipeline handler and let the queued requests complete. */
>         stopDevice(camera);
>  
> +       Camera::Private *data = camera->_d();
> +
>         /* Cancel and signal as complete all waiting requests. */
> -       while (!waitingRequests_.empty()) {
> -               Request *request = waitingRequests_.front();
> -               waitingRequests_.pop();
> +       while (!data->waitingRequests_.empty()) {
> +               Request *request = data->waitingRequests_.front();
> +               data->waitingRequests_.pop();
>                 cancelRequest(request);
>         }
>  
>         /* Make sure no requests are pending. */
> -       Camera::Private *data = camera->_d();
>         ASSERT(data->queuedRequests_.empty());
>  
>         data->requestSequence_ = 0;
> @@ -414,7 +415,9 @@ void PipelineHandler::registerRequest(Request *request)
>          * Connect the request prepared signal to notify the pipeline handler
>          * when a request is ready to be processed.
>          */
> -       request->_d()->prepared.connect(this, &PipelineHandler::doQueueRequests);
> +       request->_d()->prepared.connect(this, [this, request]() {
> +               doQueueRequests(request->_d()->camera());
> +       });
>  }
>  
>  /**
> @@ -444,7 +447,9 @@ void PipelineHandler::queueRequest(Request *request)
>  {
>         LIBCAMERA_TRACEPOINT(request_queue, request);
>  
> -       waitingRequests_.push(request);
> +       Camera *camera = request->_d()->camera();
> +       Camera::Private *data = camera->_d();
> +       data->waitingRequests_.push(request);
>  
>         request->_d()->prepare(300ms);
>  }
> @@ -478,15 +483,16 @@ void PipelineHandler::doQueueRequest(Request *request)
>   * Iterate the list of waiting requests and queue them to the device one
>   * by one if they have been prepared.
>   */
> -void PipelineHandler::doQueueRequests()
> +void PipelineHandler::doQueueRequests(Camera *camera)
>  {
> -       while (!waitingRequests_.empty()) {
> -               Request *request = waitingRequests_.front();
> +       Camera::Private *data = camera->_d();
> +       while (!data->waitingRequests_.empty()) {
> +               Request *request = data->waitingRequests_.front();
>                 if (!request->_d()->prepared_)
>                         break;
>  
>                 doQueueRequest(request);
> -               waitingRequests_.pop();
> +               data->waitingRequests_.pop();
>         }
>  }
>  
> -- 
> 2.48.1
>