Re: [PATCHv2 11/12] media: docs-rst: Document m2m stateless video decoder interface
On Fri, Aug 16, 2019 at 3:59 PM Hans Verkuil wrote: > > On 8/16/19 7:49 AM, Alexandre Courbot wrote: > > On Mon, Aug 12, 2019 at 8:07 PM Hans Verkuil > > wrote: > >> > >> From: Alexandre Courbot > >> > >> 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 > >> --- > >> 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 > >> > > > > >> +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
Re: [PATCHv2 11/12] media: docs-rst: Document m2m stateless video decoder interface
On 8/16/19 7:49 AM, Alexandre Courbot wrote: > On Mon, Aug 12, 2019 at 8:07 PM Hans Verkuil wrote: >> >> From: Alexandre Courbot >> >> 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 >> --- >> 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 >> >> +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
Re: [PATCHv2 11/12] media: docs-rst: Document m2m stateless video decoder interface
On Mon, Aug 12, 2019 at 8:07 PM Hans Verkuil wrote:
>
> From: Alexandre Courbot
>
> 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
> ---
> 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
>
> diff --git a/Documentation/media/uapi/v4l/dev-mem2mem.rst
> b/Documentation/media/uapi/v4l/dev-mem2mem.rst
> index caa05f5f6380..70953958cee6 100644
> --- a/Documentation/media/uapi/v4l/dev-mem2mem.rst
> +++ b/Documentation/media/uapi/v4l/dev-mem2mem.rst
> @@ -46,3 +46,4 @@ devices are given in the following sections.
> :maxdepth: 1
>
> dev-decoder
> +dev-stateless-decoder
> diff --git a/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> b/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> new file mode 100644
> index ..4a26646eeec5
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> @@ -0,0 +1,424 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _stateless_decoder:
> +
> +**
> +Memory-to-memory Stateless Video Decoder Interface
> +**
> +
> +A stateless decoder is a decoder that works without retaining any kind of
> state
> +between processed frames. This means that each frame is decoded independently
> +of any previous and future frames, and that the client is responsible for
> +maintaining the decoding state and providing it to the decoder with each
> +decoding request. This is in contrast to the stateful video decoder
> interface,
> +where the hardware and driver maintain the decoding state and all the client
> +has to do is to provide the raw encoded stream and dequeue decoded frames in
> +display order.
> +
> +This section describes how user-space ("the client") is expected to
> communicate
> +with stateless decoders in order to successfully decode an encoded stream.
> +Compared to stateful codecs, the decoder/client sequence is simpler, but the
> +cost of this simplicity is extra complexity in the client which is
> responsible
> +for maintaining a consistent decoding state.
> +
> +Stateless decoders make use of the :ref:`media-request-api`. A stateless
> +decoder must expose the ``V4L2_BUF_CAP_SUPPORTS_REQUESTS`` capability on its
> +``OUTPUT`` queue when :c:func:`VIDIOC_REQBUFS` or
> :c:func:`VIDIOC_CREATE_BUFS`
> +are invoked.
> +
> +Depending on the encoded formats supported by the decoder, a single decoded
> +frame may be the result of several decode requests (for instance, H.264
> streams
> +with multiple slices per frame). Decoders that support such formats must also
> +expose the ``V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF`` capability on their
> +``OUTPUT`` queue.
> +
> +Querying capabilities
> +=
> +
> +1. To enumerate the set of coded formats supported by the decoder, the client
> + calls :c:func:`VIDIOC_ENUM_FMT` on the ``OUTPUT`` queue.
> +
> + * The driver must always return the full set of supported ``OUTPUT``
> formats,
> + irrespective of the format currently set on the ``CAPTURE`` queue.
> +
> + * Simultaneously, the driver must restrain the set of values returned by
> + codec-specific capability controls (such as H.264 profiles) to the set
> + actually supported by the hardware.
> +
> +2. To enumerate the set of supported raw formats, the client calls
> + :c:func:`VIDIOC_ENUM_FMT` on the ``CAPTURE`` queue.
> +
> + * The driver must return only the formats supported for the format
> currently
> + active on the ``OUTPUT`` queue.
> +
> + * Depending on the currently set ``OUTPUT`` format, the set of supported
> raw
> + formats may depend on the value of some codec-dependent controls.
> + The client is responsible for making sure that these controls are set
> + before querying the ``CAPTURE`` queue. Failure to do so will result in
> the
> + default values for these controls being used, and a returned set of
> formats
> + that may not be usable for the media the client is trying to decode.
> +
> +3. The client may use :c:func:`VIDIOC_ENUM_FRAMESIZES` to detect supported
> + resolutions for a given format, passing desired pixel format in
> + :c:type:`v4l2_frmsizeenum`'s ``pixel_format``.
> +
> +4. Supported profiles and levels for the current ``OUTPUT`` format, if
> + applicable, may be queried using their respective controls via
> + :c:func:`VIDIOC_QUERYCTRL`.
> +
> +Initialization
> +==
> +
> +1. Set the coded format on the ``OUTPUT`` queue
Re: [PATCHv2 11/12] media: docs-rst: Document m2m stateless video decoder interface
On Mon, Aug 12, 2019 at 8:07 PM Hans Verkuil wrote:
>
> From: Alexandre Courbot
>
> 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
> ---
> 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
>
> diff --git a/Documentation/media/uapi/v4l/dev-mem2mem.rst
> b/Documentation/media/uapi/v4l/dev-mem2mem.rst
> index caa05f5f6380..70953958cee6 100644
> --- a/Documentation/media/uapi/v4l/dev-mem2mem.rst
> +++ b/Documentation/media/uapi/v4l/dev-mem2mem.rst
> @@ -46,3 +46,4 @@ devices are given in the following sections.
> :maxdepth: 1
>
> dev-decoder
> +dev-stateless-decoder
> diff --git a/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> b/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> new file mode 100644
> index ..4a26646eeec5
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> @@ -0,0 +1,424 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _stateless_decoder:
> +
> +**
> +Memory-to-memory Stateless Video Decoder Interface
> +**
> +
> +A stateless decoder is a decoder that works without retaining any kind of
> state
> +between processed frames. This means that each frame is decoded independently
> +of any previous and future frames, and that the client is responsible for
> +maintaining the decoding state and providing it to the decoder with each
> +decoding request. This is in contrast to the stateful video decoder
> interface,
> +where the hardware and driver maintain the decoding state and all the client
> +has to do is to provide the raw encoded stream and dequeue decoded frames in
> +display order.
> +
> +This section describes how user-space ("the client") is expected to
> communicate
> +with stateless decoders in order to successfully decode an encoded stream.
> +Compared to stateful codecs, the decoder/client sequence is simpler, but the
> +cost of this simplicity is extra complexity in the client which is
> responsible
> +for maintaining a consistent decoding state.
> +
> +Stateless decoders make use of the :ref:`media-request-api`. A stateless
> +decoder must expose the ``V4L2_BUF_CAP_SUPPORTS_REQUESTS`` capability on its
> +``OUTPUT`` queue when :c:func:`VIDIOC_REQBUFS` or
> :c:func:`VIDIOC_CREATE_BUFS`
> +are invoked.
> +
> +Depending on the encoded formats supported by the decoder, a single decoded
> +frame may be the result of several decode requests (for instance, H.264
> streams
> +with multiple slices per frame). Decoders that support such formats must also
> +expose the ``V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF`` capability on their
> +``OUTPUT`` queue.
> +
> +Querying capabilities
> +=
> +
> +1. To enumerate the set of coded formats supported by the decoder, the client
> + calls :c:func:`VIDIOC_ENUM_FMT` on the ``OUTPUT`` queue.
> +
> + * The driver must always return the full set of supported ``OUTPUT``
> formats,
> + irrespective of the format currently set on the ``CAPTURE`` queue.
> +
> + * Simultaneously, the driver must restrain the set of values returned by
> + codec-specific capability controls (such as H.264 profiles) to the set
> + actually supported by the hardware.
> +
> +2. To enumerate the set of supported raw formats, the client calls
> + :c:func:`VIDIOC_ENUM_FMT` on the ``CAPTURE`` queue.
> +
> + * The driver must return only the formats supported for the format
> currently
> + active on the ``OUTPUT`` queue.
> +
> + * Depending on the currently set ``OUTPUT`` format, the set of supported
> raw
> + formats may depend on the value of some codec-dependent controls.
> + The client is responsible for making sure that these controls are set
> + before querying the ``CAPTURE`` queue. Failure to do so will result in
> the
> + default values for these controls being used, and a returned set of
> formats
> + that may not be usable for the media the client is trying to decode.
> +
> +3. The client may use :c:func:`VIDIOC_ENUM_FRAMESIZES` to detect supported
> + resolutions for a given format, passing desired pixel format in
> + :c:type:`v4l2_frmsizeenum`'s ``pixel_format``.
> +
> +4. Supported profiles and levels for the current ``OUTPUT`` format, if
> + applicable, may be queried using their respective controls via
> + :c:func:`VIDIOC_QUERYCTRL`.
Re-reading this I think we should add a step describing how to check
the capabilities on the
