Re: [PATCH 1/2] media: docs-rst: Document memory-to-memory video decoder interface

On Wed, Aug 8, 2018 at 11:55 AM Tomasz Figa [off-list ref] wrote:

On Tue, Aug 7, 2018 at 4:37 PM Hans Verkuil [off-list ref] wrote:

quoted

On 08/07/2018 09:05 AM, Tomasz Figa wrote:

quoted

On Thu, Jul 26, 2018 at 7:57 PM Hans Verkuil [off-list ref] wrote:

quoted

quoted

quoted

I wonder if we should make these min buffer controls required. It might be easier
that way.

Agreed. Although userspace is still free to ignore it, because REQBUFS
would do the right thing anyway.

It's never been entirely clear to me what the purpose of those min buffers controls
is. REQBUFS ensures that the number of buffers is at least the minimum needed to
make the HW work. So why would you need these controls? It only makes sense if they
return something different from REQBUFS.

The purpose of those controls is to let the client allocate a number
of buffers bigger than minimum, without the need to allocate the
minimum number of buffers first (to just learn the number), free them
and then allocate a bigger number again.

I don't feel this is particularly useful. One problem with the minimum number
of buffers as used in the kernel is that it is often the minimum number of
buffers required to make the hardware work, but it may not be optimal. E.g.
quite a few capture drivers set the minimum to 2, which is enough for the
hardware, but it will likely lead to dropped frames. You really need 3
(one is being DMAed, one is queued and linked into the DMA engine and one is
being processed by userspace).

I would actually prefer this to be the recommended minimum number of buffers,
which is >= the minimum REQBUFS uses.

I.e., if you use this number and you have no special requirements, then you'll
get good performance.

I guess we could make it so. It would make existing user space request
more buffers than it used to with the original meaning, but I guess it
shouldn't be a big problem.

I gave it a bit more thought and I feel like kernel is not the right
place to put any assumptions on what the userspace expects "good
performance" to be. Actually, having these controls return the minimum
number of buffers as REQBUFS would allocate makes it very well
specified - with this number you can only process frame by frame and
the number of buffers added by userspace defines exactly the queue
depth. It leaves no space for driver-specific quirks, because the
driver doesn't decide what's "good performance" anymore.

Best regards,
Tomasz