[v2,3/5] pipeline: rkisp1: Limit the maximum number of buffers queued in

23746 diff mbox series

To keep the regulation of the algorithms as short as possible and to
allow more buffers to be created than the v4l2 device allows to be queued,
limit the amount of buffers that get queued into the device to the pipeline depth.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

Changes in v1:
- Replaced function overload with constructor param
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Quoting Stefan Klug (2025-07-07 08:53:37)
> To keep the regulation of the algorithms as short as possible and to
> allow more buffers to be created than the v4l2 device allows to be queued,
> limit the amount of buffers that get queued into the device to the pipeline depth.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v1:
> - Replaced function overload with constructor param

This looks easier/cleaner in fact!

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

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 675f0a7490a6..fff42359cbff 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -155,6 +155,12 @@ private:
>         Transform combinedTransform_;
>  };
>  
> +namespace {
> +
> +const unsigned int kPipelineDepth = 4;
> +
> +} // namespace
> +
>  class PipelineHandlerRkISP1 : public PipelineHandler
>  {
>  public:
> @@ -684,7 +690,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>   */
>  
>  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> -       : PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false)
> +       : PipelineHandler(manager, kPipelineDepth), hasSelfPath_(true), useDewarper_(false)
>  {
>  }
>  
> -- 
> 2.48.1
>

On Mon, Jul 07, 2025 at 09:53:37AM +0200, Stefan Klug wrote:
> To keep the regulation of the algorithms as short as possible and to
> allow more buffers to be created than the v4l2 device allows to be queued,
> limit the amount of buffers that get queued into the device to the pipeline depth.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v1:
> - Replaced function overload with constructor param
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 675f0a7490a6..fff42359cbff 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -155,6 +155,12 @@ private:
>  	Transform combinedTransform_;
>  };
>  
> +namespace {
> +
> +const unsigned int kPipelineDepth = 4;

possibly `static constexpr unsigned int`

I would not name this as kPipelineDepth and simply stick to
kRkISPMaxQueuedRequests_ or something on those lines. The term "Pipeline
Depth" in defined in draft controls as no. of stages a frame went
through.

> +
> +} // namespace
> +
>  class PipelineHandlerRkISP1 : public PipelineHandler
>  {
>  public:
> @@ -684,7 +690,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>   */
>  
>  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> -	: PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false)
> +	: PipelineHandler(manager, kPipelineDepth), hasSelfPath_(true), useDewarper_(false)
>  {
>  }
>  
> -- 
> 2.48.1
>