Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: Alexandre Courbot <acourbot@chromium.org>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Tomasz Figa <tfiga@chromium.org>,
	Francois Buergisser <fbuergisser@google.com>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	Boris Brezillon <boris.brezillon@collabora.com>
Subject: Re: [PATCHv2 11/12] media: docs-rst: Document m2m stateless video decoder interface
Date: Fri, 16 Aug 2019 08:59:13 +0200
Message-ID: <0b947ac2-6d2e-cdc9-4c0b-6eb3d5a45fbe@xs4all.nl> (raw)
In-Reply-To: <CAPBb6MWMeepMw=f=4DL5Qgx-H+cpsyCehyNuV5PVimEAN6nJZg@mail.gmail.com>

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

  reply index

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-12 11:05 [PATCHv2 00/12] Stateful/stateless codec core support Hans Verkuil
2019-08-12 11:05 ` [PATCHv2 01/12] videodev2.h: add V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM Hans Verkuil
2019-08-14 12:52   ` Paul Kocialkowski
2019-08-15  8:11   ` Alexandre Courbot
2019-08-15  8:13     ` Alexandre Courbot
2019-08-12 11:05 ` [PATCHv2 02/12] videodev2.h: add V4L2_FMT_FLAG_DYN_RESOLUTION Hans Verkuil
2019-08-14 12:53   ` Paul Kocialkowski
2019-08-15  8:11     ` Alexandre Courbot
2019-08-15 10:09       ` Tomasz Figa
2019-08-15 14:22     ` Hans Verkuil
2019-08-12 11:05 ` [PATCHv2 03/12] media: venus: vdec: flag OUTPUT formats with V4L2_FMT_FLAG_DYN_RESOLUTION Hans Verkuil
2019-08-12 13:49   ` Stanimir Varbanov
2019-08-14 12:54   ` Paul Kocialkowski
2019-08-12 11:05 ` [PATCHv2 04/12] media: s5p_mfc_dec: set flags for OUTPUT coded formats Hans Verkuil
2019-08-14 12:55   ` Paul Kocialkowski
2019-08-15 10:14   ` Tomasz Figa
2019-08-16 11:37     ` Marek Szyprowski
2019-08-16 11:47       ` Tomasz Figa
2019-08-12 11:05 ` [PATCHv2 05/12] media: mtk-vcodec: flag OUTPUT formats with V4L2_FMT_FLAG_DYN_RESOLUTION Hans Verkuil
2019-08-14 12:59   ` Paul Kocialkowski
2019-08-15  8:12   ` Alexandre Courbot
2019-08-12 11:05 ` [PATCHv2 06/12] media: vicodec: set flags for vdec/stateful OUTPUT coded formats Hans Verkuil
2019-08-14 13:04   ` Paul Kocialkowski
2019-08-12 11:05 ` [PATCHv2 07/12] media: docs-rst: Document memory-to-memory video decoder interface Hans Verkuil
2019-08-12 11:05 ` [PATCHv2 08/12] pixfmt-compressed.rst: improve H264/HEVC/MPEG1+2/VP8+9 documentation Hans Verkuil
2019-08-14 13:09   ` Paul Kocialkowski
2019-08-14 13:15     ` Hans Verkuil
2019-08-12 11:05 ` [PATCHv2 09/12] vb2: add V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF Hans Verkuil
2019-08-15  8:12   ` Alexandre Courbot
2019-08-15 11:53     ` Hans Verkuil
2019-08-15 12:27       ` Tomasz Figa
2019-08-15 12:31         ` Hans Verkuil
2019-08-15 14:27       ` Alexandre Courbot
2019-08-12 11:05 ` [PATCHv2 10/12] videodev2.h: add V4L2_DEC_CMD_FLUSH Hans Verkuil
2019-08-15  8:12   ` Alexandre Courbot
2019-08-15 11:53     ` Hans Verkuil
2019-08-12 11:05 ` [PATCHv2 11/12] media: docs-rst: Document m2m stateless video decoder interface Hans Verkuil
2019-08-15  9:58   ` Alexandre Courbot
2019-08-16  5:49   ` Alexandre Courbot
2019-08-16  6:59     ` Hans Verkuil [this message]
2019-08-16  7:17       ` Alexandre Courbot
2019-08-12 11:05 ` [PATCHv2 12/12] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0b947ac2-6d2e-cdc9-4c0b-6eb3d5a45fbe@xs4all.nl \
    --to=hverkuil-cisco@xs4all.nl \
    --cc=acourbot@chromium.org \
    --cc=boris.brezillon@collabora.com \
    --cc=ezequiel@collabora.com \
    --cc=fbuergisser@google.com \
    --cc=linux-media@vger.kernel.org \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox