[PATCH v11 01/19] libcamera: property: Add MinimumRequests property

[Jacopo Mondi]

Hi Sevn,
  thanks for resurecting this!

Can I ask if this solves any actual problem and on which platform ?

On Mon, Apr 28, 2025 at 11:02:26AM +0200, Sven Püschel wrote:
> From: Nícolas F. R. A. Prado <nfraprado at collabora.com>
>
> The MinimumRequests property reports the minimum number of capture
> requests that the camera pipeline requires to have queued in order to
> sustain capture operations without frame drops. At this quantity,
> there's no guarantee that manual per-frame controls will apply
> correctly. The quantity needed for that may be added as a separate
> property in the future.
>
> The mali-c55 driver defines min_queued_buffers = 1 [1]. Therefore set
> set the minimum requests to 2 to account one request in the userspace.
>
> The dcmipp and j721e drivers both defines min_queued_buffers = 1
> in the kernel under
> drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-bytecap.c
> and drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c .
> Therefore use these values, as they are added 1 more.
>
> For the intel-ipu6, mtk-seninf simple devices and the virtual pipeline
> use a conservative value of 3 minimumBuffers, as no further requirements
> are known about them.
>
> [1] https://lore.kernel.org/linux-media/20240131174355.GB20792@pendragon.ideasonboard.com/T/#t
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Acked-by: Umang Jain <umang.jain at ideasonboard.com>
> Signed-off-by: Sven Püschel <s.pueschel at pengutronix.de>
>
> ---
> Changes in v11
> - Add mali-c55, dcmipp, virtual, j721e, intel-ipu6 and mtk-seninf
> - Hold only the minimum buffers instead of the whole deviceInfo pointer in SimpleCameraData
> - Adjusted min_buffers_needed property to min_reqbufs_allocation in docs
> - Relax property description from no frame drops -> only minimal frame
>   drops, based on the comment from Naush [10]
> - Changed property type from int32_t -> uint32_t to match all of the
>   indirect casts to an unsigned value throughout the existing patchset.
> - Removed Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> - Removed Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> [10] https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/035872.html
>
> Changes in v10:
> - ipu3: add a constant to populate MinimumRequests, as we'll also use it
>   elsewhere
> - update pipeline handler guide to set vivid' MinimumRequests to 2
> - expand the MinimumRequests property description to include a line
>   about startup
> - add isi
>
> Changes in v9:
> - rebased
>
> Changes in v8:
> - Changed the MinimumRequests property meaning to require that frames aren't
>   dropped
> - Set MinimumRequests on SimplePipeline depending on device and usage of
>   converter
> - Undid definition of default MinimumRequests on CameraSensor constructor
> - Updated pipeline-handler guides with new MinimumRequests property
>
> Changes in v7:
> - Renamed property from MinNumRequests to MinimumRequests
> - Changed MinimumRequests property's description
>
> Changes in v6:
> - Removed comment from Raspberrypi MinNumRequests setting
> - Removed an extra blank line
> ---
>  Documentation/guides/pipeline-handler.rst     | 20 ++++++---
>  src/libcamera/pipeline/imx8-isi/imx8-isi.cpp  |  2 +
>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 ++
>  src/libcamera/pipeline/mali-c55/mali-c55.cpp  |  1 +
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  1 +
>  .../pipeline/rpi/common/pipeline_base.cpp     |  2 +
>  src/libcamera/pipeline/simple/simple.cpp      | 41 +++++++++++++++----
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  2 +
>  src/libcamera/pipeline/vimc/vimc.cpp          |  2 +
>  src/libcamera/pipeline/virtual/virtual.cpp    |  1 +
>  src/libcamera/property_ids_core.yaml          | 22 ++++++++++
>  11 files changed, 85 insertions(+), 13 deletions(-)
>

[snip]

> index 834454a4..cc28d677 100644
> --- a/src/libcamera/property_ids_core.yaml
> +++ b/src/libcamera/property_ids_core.yaml
> @@ -701,4 +701,26 @@ controls:
>
>          Different cameras may report identical devices.
>
> +  - MinimumRequests:

I'm still not at ease with the definition of this property, I'm
afraid.

It seems to be mixing two different things (maybe 3 :)

> +      type: uint32_t
> +      description: |
> +        The minimum number of capture requests that the camera pipeline requires
> +        to have queued in order to sustain capture operations with only minimal


The reference to "minimal frame drops" is a bit too loosely
specified ?

> +        frame drops. At this quantity, there's no guarantee that manual per
> +        frame controls will apply correctly. This is also the minimum number of

The per-frame control pipeline depth does depend on other factors
such as the sensor delays, specifically for manual AE/AWB control


> +        requests that must be queued before capture starts.

The number of buffers required to start streaming comes directly from
the min_queued_buffers variable defined by drivers (unfortunatetly not
available to users through any uAPI). One thing I would like to see
happening is drivers stop setting min_queued_buffers at all, or if not
possible maybe expose it through an api (a RO v4l2 control ?).

Drivers should allocate internal buffers and use them if the
application doesn't keep, and handle both the runtime underruns and
start streaming conditions using scratch buffers.

Even if this won't happen in drivers, the requirement should not be
exposed to libcamera users. In the "great scheme of things" the idea
is to decouple application's Requests from the frames pipeline, and
providing enough buffers to sustain the frame rate should be a
pipeline/IPA job.

True, for platforms where drivers already require min_queued_buffers
applications right now have to queue enough requests before
start receiving frames back, and we goofly report it through
StreamConfiguration::bufferCount.

Unless there's an actual use case or any angle I'm grossly
missing I don't think this property is a good idea..

> +
> +        This property is based on the minimum number of memory buffers
> +        needed to fill the capture pipeline composed of hardware processing
> +        blocks plus as many buffers as needed to take into account propagation
> +        delays and avoid dropping frames.
> +
> +        \todo Should this be a per-stream property?
> +
> +        \todo Extend this property to expose the different minimum buffer and
> +        request counts (the minimum number of buffers to be able to capture at
> +        all, the minimum number of buffers to sustain capture without frame
> +        drop, and the minimum number of requests to ensure proper operation of
> +        per-frame controls).
> +
>  ...
> --
> 2.49.0
>