[PATCH v11 01/19] libcamera: property: Add MinimumRequests property
Sven Püschel
s.pueschel at pengutronix.de
Mon May 26 16:08:21 CEST 2025
Hi Stefan,
On 5/5/25 16:40, Stefan Klug wrote:
> Hi Jacopo,
>
> On Mon, May 05, 2025 at 03:59:33PM +0200, Jacopo Mondi wrote:
>> Hi Sven
>>
>> On Mon, May 05, 2025 at 02:23:13PM +0200, Sven Püschel wrote:
>>> Hi Jacopo,
>>>
>>> On 5/2/25 18:28, Jacopo Mondi wrote:
>>>> Hi Sevn,
>>>> thanks for resurecting this!
>>>>
>>>> Can I ask if this solves any actual problem and on which platform ?
>>> The problem I'm facing is that in a more complex setup with the rkisp1 we
>>> need to keep multiple dmabufs in the userspace. But the hardcoded value of 4
>>> buffers in the rkisp1 driver seems to expect that the application only hold
>>> around one buffer and have the other 3 buffers queued for a smooth frame
>>> capture. But due to holding more dmabufs, I'm running into massive frame
>>> drops due to usually only having one request queued and waiting for it to
>>> finish.
>> Thanks for the overview.
>>
>> Can't more buffers be allocated by setting a larger value in
>> StreamConfiguration::bufferCount (which is indeed now hardcoded to 4) ?
>>
>>> Therefore I want to allocate more dmabufs for this problematic use case and
>>> avoid patching libcamera to increase the hardcoded buffer count of rkisp1
>>> (which would also apply globally).
>>>
>>> Kieran then pointed me towards this patchset to allow the application to
>>> control the amount of dmabufs allocated.
>>>
>>>
>>>> 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 ?
>>> I've changed it due to Naushir's claim that there is no practical number to
>>> gurantee no frame drops [1]
>>>
>>> [1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/035872.html
>>>
>>>
>>>>> + 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
>>> and that is the reason it isn't or cannot be part of this property? Or
>>> what's your point?
>>>
>> My point is that the pipeline depth (which would need a more formal
>> definition as we use it quite freely without having ever set in stone
>> its definition) is a property unrelated to the minium number of
>> buffers or requests that has to be queued.
>>
>> I would leave pipeline depth and per-frame control out of the picture.
>>
>>>>> + 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..
>>> I understand your problem with this property. But I don't know if we can get
>>> away without providing the application a hint how many dmabufs it should
>>> allocate at least.
>>>
>> Well, the default value set by generateConfiguration() should be an
>> indication of what value the pipeline expects.
>>
>>> To recall my problem, the rkisp1 driver seems to already use scratch buffers
>> Yes, I would love all drivers to remove min_queued_buffers as it was
>> done for rkisp1
>> https://www.spinics.net/lists/linux-media/msg264330.html
>>
>>> when it doesn't have enough buffers queued. So while queuing one buffer at a
>>> time worked, it wasn't desired due to the frame drops from the scratch
>>> buffers.
>> Keeping up with the frame rate shouldn't be a driver concern but an
>> application's one. But of course if you can't allocate enough buffers
>> to queue them back fast enough, you can't do that.
>>
>>> But the application developer cannot really know how many buffers are really
>>> required. From my perspective the current operation mode is that libcamera
>> The fact is that "really required" is an ill-formed concepts. As said
>> there are many different aspects at play: keeping up with the sensor's
>> frame rate, the pipeline depth required to guarantee per-frame control
>> and the number of buffers required to start streaming.
>>
>> We currently report some min number of buffers (most of the times, but
>> not always coming from min_queued_buffers in the driver) in
>> StreamConfiguration::bufferCount, but this is certainly is a
>> sub-optimal API.
>>
>>> allocates a sensible fixed amount of dmabufs and application developers just
>>> queue all unused buffers to libcamera [2]. So when the allocation is now put
>>> into the hands of the application developer, it may just always allocate 2
>>> buffers (to have still one queued when handling a completed buffer) and
>>> therefore only queue a maximum of 2 buffers. The driver fills the lack of
>>> buffers with scratch buffers, which cause unintended frame drops.
>> That's what should happen in the driver. The alternative would be not
>> being able to dequeue buffers at all. Simply, if an application
>> allocates two buffers only, it's calling for troubles, isn't it ?
>>
>>> One potential solution may be to pass the desired amount of buffers held in
>>> the application to the allocator. Then the pipeline could allocate a
>>> sensible amount of buffers (e.g. 1 requested for the application + 3 queued
>>> => 4 buffers) and the application would queue all unused buffers as usual.
>>>
>> I wonder why RkISP1Path::validate() reset bufferCount to 4.
>> We should allow apps to allocate more buffers if they want to.
>>
>> Kieran, Stefan, ideas ?
> That code dates back 5 years, so I can only guess that it was done to
> ensure stable operation without becoming too laggy. I completely agree
> that an application should be able to freely decide how many buffers to
> allocate. The issue with RkISP1 is that the algorithm control loop has
> the same length as the buffer count. So when you increase that hardcoded
> value to a bigger value, the regulation get's awfully slow. If I
> remember correctly this get's partially solved/worked around in this
> series. It is also addressed in the PFC topics we'll discuss this week.
Just noticed: Why only "partially solved/worked around"? What did I miss?
> So I think we need a bit of internal discussions to come up with a well
> defined path forward.
Given that now 3 weeks gone by, are there any updates on the internal
discussions?
As noted in [1] I wouldn't mind to drop the property and instead allow
applications to set the bufferCount variable to specify the desired
amount of buffers to allocate. But I would like to know if this is the
desired path or if some other path is better suited before investing the
time to create a new patch series.
[1]
https://lists.libcamera.org/pipermail/libcamera-devel/2025-May/050167.html
Sincerely
Sven
>
> Best regards,
> Stefan
>
>>> Other than that I don't see how libcamera could abstract this property away,
>>> as applications control which buffers are used for a given request.
>>>
>> We certainly need a way to allow apps to allocate more than 4 buffers.
>> My main problem is with this property, which mixes in too many things
>> and is ill-defined.
>>
>>> [2]
>>> https://www.libcamera.org/guides/application-developer.html#request-queueing
>>>
>>>
>>>
>>>>> + 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
>>>>>
More information about the libcamera-devel
mailing list