Hi all, While working on the support for deleting buffers we came across messy driver code that needs some work to clean up. It concerns drivers that want (or sometimes require) a minimum number of buffers, and will check that in the vb2 queue_setup() callback. Analyzing drivers shows that there are a number of variations, each with different problems. First of all there are drivers that abuse the min_buffers_needed field: this field is meant to specify the minimum number of buffers that have to be queued before the DMA engine can start. Typically 0, 1 or 2. To actually be able to dequeue a buffer you need to allocate one more buffer, otherwise the DMA engine would never be able to return a buffer. Unfortunately, some drivers set this field to the minimum number of buffers that they want to allocate, entirely unrelated to the DMA engine requirements. It is really a bug since start_streaming won't be called until that many buffers are queued, even though the DMA engine didn't need that many buffers at all. Other drivers check the number of requested buffers in queue_setup(): they increase the number of buffers requested if it is less than what they require. This code is often buggy in that they do not handle the VIDIOC_CREATE_BUFS case correctly. Regardless, drivers that want a minimum of N allocated buffers pose a problem when we add support for deleting buffers. Since what to do if you delete buffers so you are below N? Stop streaming? Ignore? Note that deleting buffers is not a problem with min_buffers_needed: there are always that many buffers in use by the DMA engine, and you can't delete buffers that are in use. The only way to delete those buffers is to stop streaming altogether. In my view changing the number of buffers requested should only be done when called from VIDIOC_REQBUFS: applications can request 1 buffer, but the driver has to be able to increase that to the minimum viable number of buffers (1 plus the minimum needed for DMA). Applications today rely on that feature as this was there from the very beginnings of the V4L2 API. However, for VIDIOC_CREATE_BUFS I believe it should not increase the number of requested buffers (it may of course decrease if there is not enough memory for all requested buffers). The whole point of CREATE_BUFS is to provide better control of the buffers you allocate, and it shouldn't unexpectedly allocate more. Especially when DELETE_BUFS is added you don't want surprises like that. So a number of cleanups need to be made: 1) Rename min_buffers_needed to min_dma_buffers_needed, and improve the corresponding documentation. 2) Add a new field: min_reqbufs_buffers (name TBD). Drivers can set this if they want VIDIOC_REQBUFS to allocate a minimum number of buffers. If 0, then the core uses min_dma_buffers_needed + 1. The core will handle this, so drivers no longer need to care about it. Note: some drivers seem to do this for no clear reason, we might want to consider just removing this minimum. Usually it is for codecs that really need more than the typical 3 buffers to operate. I think 1+2 should be done before adding DELETE_BUFS support: it simplifies drivers and especially allows CREATE_BUFS to be used to get more fine grained control over the number of buffers allocated. Note that 2 does change the uAPI slightly: CREATE_BUFS will no longer allocate more than requested if it is called for the first time (i.e. instead of REQBUFS). I am not aware of any application doing that, though. An alternative is to make the CREATE_BUFS behavior driver dependent (i.e. should it obey min_reqbufs_buffers when called for the first time?) to keep existing behavior. It will probably depend a bit on which drivers are affected. Regards, Hans
