On Fri, Aug 16, 2019 at 3:59 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > > On 8/16/19 7:49 AM, Alexandre Courbot wrote: > > On Mon, Aug 12, 2019 at 8:07 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > >> > >> From: Alexandre Courbot <acourbot@xxxxxxxxxxxx> > >> > >> 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@xxxxxxxxxxxx> > >> --- > >> 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. Yes, to be clear the scenario I have in mind is allowing S_FMT while streaming is off, but OUTPUT buffers still allocated. Doing S_FMT when streaming is on should remain prohibited. > > 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