Re: [PATCHv2 11/12] media: docs-rst: Document m2m stateless video decoder interface

[Hans Verkuil <hverkuil-cisco@xs4all.nl>]

On 8/16/19 7:49 AM, Alexandre Courbot wrote:
> On Mon, Aug 12, 2019 at 8:07 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> From: Alexandre Courbot <acourbot@chromium.org>
>>
>> Documents the protocol that user-space should follow when
>> communicating with stateless video decoders.
>>
>> The stateless video decoding API makes use of the new request and tags
>> APIs. While it has been implemented with the Cedrus driver so far, it
>> should probably still be considered staging for a short while.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
>> ---
>>  Documentation/media/uapi/v4l/dev-mem2mem.rst  |   1 +
>>  .../media/uapi/v4l/dev-stateless-decoder.rst  | 424 ++++++++++++++++++
>>  2 files changed, 425 insertions(+)
>>  create mode 100644 Documentation/media/uapi/v4l/dev-stateless-decoder.rst
>>

<snip>

>> +Dynamic resolution change
>> +=========================
>> +
>> +If the client detects a resolution change in the stream, it will need to perform
>> +the initialization sequence again with the new resolution:
>> +
>> +1. If the last submitted request resulted in a ``CAPTURE`` buffer being
>> +   held by the use of the ``V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF`` flag, then the
>> +   last frame is not available on the ``CAPTURE`` queue. In this case, a
>> +   ``V4L2_DEC_CMD_FLUSH`` command shall be sent. This will make the driver
>> +   dequeue the held ``CAPTURE`` buffer.
>> +
>> +2. Wait until all submitted requests have completed and dequeue the
>> +   corresponding output buffers.
>> +
>> +3. Call :c:func:`VIDIOC_STREAMOFF` on both the ``OUTPUT`` and ``CAPTURE``
>> +   queues.
>> +
>> +4. Free all ``CAPTURE`` buffers by calling :c:func:`VIDIOC_REQBUFS` on the
>> +   ``CAPTURE`` queue with a buffer count of zero.
>> +
>> +5. Perform the initialization sequence again (minus the allocation of
>> +   ``OUTPUT`` buffers),
> 
> We have just hit an issue on the Hantro driver related to this. At the
> moment, Hantro will reject calls to VIDIOC_S_FMT on the OUTPUT queue
> if buffers are allocated. And indeed, the documentation for
> VIDIOC_S_FMT mentions this behavior:
> 
>     EBUSY
>       The device is busy and cannot change the format. This could be
> because or the device is streaming or buffers are allocated or queued
> to the driver.
> 
> However in our case it does not make much sense to force reallocating
> the OUTPUT buffers if user-space knows that the current ones are still
> large enough for the new resolution. Should Hantro be adapted to allow
> this, or shall we reword the specification?
> 
> Note that if we allow this, we may also allow OUTPUT buffers to be
> allocated before the CAPTURE format is set during the initialization
> sequence (i.e. move step 6. somewhere after step 2.).
> 
> Thoughts?

Drivers can allow S_FMT while buffers are allocated. But it needs to be
done carefully: for MMAP streaming mode the driver will have to check
that the allocated buffers are large enough for the new format (you
probably want to make a helper function for this check), for USERPTR and
DMABUF this needs to be checked in the buf_prepare vb2 callback. This
probably happens already.

Calling S_FMT while streaming is probably not a good idea and should
still result in a EBUSY. Mostly because it is not clear whether a S_FMT
should take immediate effect (thus affecting all already queued buffers)
or only with newly queued buffers. Let's just avoid this situation for
now.

It was always the intention to relax the rules of when you can call S_FMT,
but in most cases it is easier to just prohibit calling S_FMT when buffers
are allocated.

Regards,

	Hans