Re: [RFC PATCH] media: docs-rst: Document m2m stateless video decoder interface
On Tue, Sep 11, 2018 at 5:48 PM Hans Verkuil wrote: > > On 09/11/18 10:40, Alexandre Courbot wrote: > >> I am not sure whether this should be documented, but there are some > >> additional > >> restrictions w.r.t. reference frames: > >> > >> Since decoders need access to the decoded reference frames there are some > >> corner > >> cases that need to be checked: > >> > >> 1) V4L2_MEMORY_USERPTR cannot be used for the capture queue: the driver > >> does not > >>know when a malloced but dequeued buffer is freed, so the reference > >> frame > >>could suddenly be gone. > > > > It it is confirmed that we cannot use USERPTR buffers as CAPTURE then > > this probably needs to be documented. I wonder if there isn't a way to > > avoid this by having vb2 keep a reference to the pages in such a way > > that they would not be recycled after after userspace calls free() on > > the buffer. Is that possible with user-allocated memory? > > vb2 keeps a reference while the buffer is queued, but that reference is > released once the buffer is dequeued. Correctly, IMHO. If you provide > USERPTR, than userspace is responsible for the memory. Changing this > would require changing the API, since USERPTR has always worked like > this. That would be a userspace bug wouldn't it? The next try to get user pages would fail in that case and we could just fail such decode request, couldn't we? (Personally I'm not a big fan of USERPTR, though.) > > > > > Not that I think that forbidding USERPTR buffers in this scenario > > would be a big problem. > > I think it is perfectly OK to forbid this. Ideally I would like to have > some test in v4l2-compliance or (even better) v4l2-mem2mem.c for this, > but it is actually not that easy to identify drivers like this. > > Suggestions for this on a postcard... > > > > >> > >> 2) V4L2_MEMORY_DMABUF can be used, but drivers should check that the dma > >> buffer is > >>still available AND increase the dmabuf refcount while it is used by > >> the HW. > > > > Yeah, with DMABUF the above problem can easily be avoided at least. > > > >> > >> 3) What to do if userspace has requeued a buffer containing a reference > >> frame, > >>and you want to decode a B/P-frame that refers to that buffer? We need > >> to > >>check against that: I think that when you queue a capture buffer whose > >> index > >>is used in a pending request as a reference frame, than that should > >> fail with > >>an error. And trying to queue a request referring to a buffer that has > >> been > >>requeued should also fail. > >> > >> We might need to add some support for this in v4l2-mem2mem.c or vb2. > > > > Sounds good, and we should document this as well. > > > > Right. And test it! I'm not convinced that we should be enforcing this. Moreover, requeuing a buffer containing a reference frame for a pending request is not bound to be an error. It might be a legit case when the same entry in the reference list is replaced with a different key frame decoded into the same buffer as the reference list entry pointed until now. Best regards, Tomasz
Re: [RFC PATCH] media: docs-rst: Document m2m stateless video decoder interface
On Tue, Sep 11, 2018 at 5:48 PM Hans Verkuil wrote: > > On 09/11/18 10:40, Alexandre Courbot wrote: > >> I am not sure whether this should be documented, but there are some > >> additional > >> restrictions w.r.t. reference frames: > >> > >> Since decoders need access to the decoded reference frames there are some > >> corner > >> cases that need to be checked: > >> > >> 1) V4L2_MEMORY_USERPTR cannot be used for the capture queue: the driver > >> does not > >>know when a malloced but dequeued buffer is freed, so the reference > >> frame > >>could suddenly be gone. > > > > It it is confirmed that we cannot use USERPTR buffers as CAPTURE then > > this probably needs to be documented. I wonder if there isn't a way to > > avoid this by having vb2 keep a reference to the pages in such a way > > that they would not be recycled after after userspace calls free() on > > the buffer. Is that possible with user-allocated memory? > > vb2 keeps a reference while the buffer is queued, but that reference is > released once the buffer is dequeued. Correctly, IMHO. If you provide > USERPTR, than userspace is responsible for the memory. Changing this > would require changing the API, since USERPTR has always worked like > this. That would be a userspace bug wouldn't it? The next try to get user pages would fail in that case and we could just fail such decode request, couldn't we? (Personally I'm not a big fan of USERPTR, though.) > > > > > Not that I think that forbidding USERPTR buffers in this scenario > > would be a big problem. > > I think it is perfectly OK to forbid this. Ideally I would like to have > some test in v4l2-compliance or (even better) v4l2-mem2mem.c for this, > but it is actually not that easy to identify drivers like this. > > Suggestions for this on a postcard... > > > > >> > >> 2) V4L2_MEMORY_DMABUF can be used, but drivers should check that the dma > >> buffer is > >>still available AND increase the dmabuf refcount while it is used by > >> the HW. > > > > Yeah, with DMABUF the above problem can easily be avoided at least. > > > >> > >> 3) What to do if userspace has requeued a buffer containing a reference > >> frame, > >>and you want to decode a B/P-frame that refers to that buffer? We need > >> to > >>check against that: I think that when you queue a capture buffer whose > >> index > >>is used in a pending request as a reference frame, than that should > >> fail with > >>an error. And trying to queue a request referring to a buffer that has > >> been > >>requeued should also fail. > >> > >> We might need to add some support for this in v4l2-mem2mem.c or vb2. > > > > Sounds good, and we should document this as well. > > > > Right. And test it! I'm not convinced that we should be enforcing this. Moreover, requeuing a buffer containing a reference frame for a pending request is not bound to be an error. It might be a legit case when the same entry in the reference list is replaced with a different key frame decoded into the same buffer as the reference list entry pointed until now. Best regards, Tomasz
Re: [RFC PATCH] media: docs-rst: Document m2m stateless video decoder interface
Hi Alexandre,
Thank you very much for working on this, much appreciated!
On 08/31/2018 09:47 AM, Alexandre Courbot wrote:
> This patch documents the protocol that user-space should follow when
> communicating with stateless video decoders. It is based on the
> following references:
>
> * The current protocol used by Chromium (converted from config store to
> request API)
>
> * The submitted Cedrus VPU driver
>
> As such, some things may not be entirely consistent with the current
> state of drivers, so it would be great if all stakeholders could point
> out these inconsistencies. :)
>
> This patch is supposed to be applied on top of the Request API V18 as
> well as the memory-to-memory video decoder interface series by Tomasz
> Figa.
>
> It should be considered an early RFC.
>
> Signed-off-by: Alexandre Courbot
> ---
> .../media/uapi/v4l/dev-stateless-decoder.rst | 413 ++
> Documentation/media/uapi/v4l/devices.rst | 1 +
> .../media/uapi/v4l/extended-controls.rst | 23 +
> 3 files changed, 437 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/dev-stateless-decoder.rst
>
> 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 ..bf7b13a8ee16
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> @@ -0,0 +1,413 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _stateless_decoder:
> +
> +**
> +Memory-to-memory Stateless Video Decoder Interface
> +**
> +
> +A stateless decoder is a decoder that works without retaining any kind of
> state
> +between processing 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 driver. This is in
> +contrast to the stateful video decoder interface, where the hardware
> maintains
> +the decoding state and all the client has to do is to provide the raw encoded
> +stream.
> +
> +This section describes how user-space ("the client") is expected to
> communicate
> +with such decoders in order to successfully decode an encoded stream.
> Compared
> +to stateful codecs, the driver/client protocol is simpler, but cost of this
> +simplicity is extra complexity in the client which must maintain the decoding
> +state.
> +
> +Querying capabilities
> +=
> +
> +1. To enumerate the set of coded formats supported by the driver, the client
> + calls :c:func:`VIDIOC_ENUM_FMT` on the ``OUTPUT`` queue.
> +
> + * The driver must always return the full set of supported formats for the
> + currently set ``OUTPUT`` format, irrespective of the format currently
> set
> + on the ``CAPTURE`` queue.
> +
> +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.
> +
> + * In order to enumerate raw formats supported by a given coded format, the
> + client must thus set that coded format on the ``OUTPUT`` queue first,
> and
> + then enumerate the ``CAPTURE`` queue.
> +
> +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` ``pixel_format``.
> +
> + * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` on ``OUTPUT`` queue
> + must include all possible coded resolutions supported by the decoder
> + for given coded pixel format.
> +
> + * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` on ``CAPTURE`` queue
> + must include all possible frame buffer resolutions supported by the
> + decoder for given raw pixel format and coded format currently set on
> + ``OUTPUT`` queue.
> +
> +.. note::
> +
> + The client may derive the supported resolution range for a
> + combination of coded and raw format by setting width and height of
> + ``OUTPUT`` format to 0 and calculating the intersection of
> + resolutions returned from calls to :c:func:`VIDIOC_ENUM_FRAMESIZES`
> + for the given coded and raw formats.
> +
> +4. Supported profiles and levels for given format, if applicable, may be
> + queried using their respective controls via :c:func:`VIDIOC_QUERYCTRL`.
> +
> +Initialization
> +==
> +
> +1. *[optional]* Enumerate supported ``OUTPUT`` formats and resolutions. See
> + capability enumeration.
> +
> +2. Set the coded format on the ``OUTPUT`` queue via :c:func:`VIDIOC_S_FMT`
> +
> + * **Required fields:**
> +
> + ``type``
> + a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``
> +
> + ``pixelformat``
> + a coded pixel
Re: [RFC PATCH] media: docs-rst: Document m2m stateless video decoder interface
Hi Alexandre,
Thank you very much for working on this, much appreciated!
On 08/31/2018 09:47 AM, Alexandre Courbot wrote:
> This patch documents the protocol that user-space should follow when
> communicating with stateless video decoders. It is based on the
> following references:
>
> * The current protocol used by Chromium (converted from config store to
> request API)
>
> * The submitted Cedrus VPU driver
>
> As such, some things may not be entirely consistent with the current
> state of drivers, so it would be great if all stakeholders could point
> out these inconsistencies. :)
>
> This patch is supposed to be applied on top of the Request API V18 as
> well as the memory-to-memory video decoder interface series by Tomasz
> Figa.
>
> It should be considered an early RFC.
>
> Signed-off-by: Alexandre Courbot
> ---
> .../media/uapi/v4l/dev-stateless-decoder.rst | 413 ++
> Documentation/media/uapi/v4l/devices.rst | 1 +
> .../media/uapi/v4l/extended-controls.rst | 23 +
> 3 files changed, 437 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/dev-stateless-decoder.rst
>
> 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 ..bf7b13a8ee16
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> @@ -0,0 +1,413 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _stateless_decoder:
> +
> +**
> +Memory-to-memory Stateless Video Decoder Interface
> +**
> +
> +A stateless decoder is a decoder that works without retaining any kind of
> state
> +between processing 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 driver. This is in
> +contrast to the stateful video decoder interface, where the hardware
> maintains
> +the decoding state and all the client has to do is to provide the raw encoded
> +stream.
> +
> +This section describes how user-space ("the client") is expected to
> communicate
> +with such decoders in order to successfully decode an encoded stream.
> Compared
> +to stateful codecs, the driver/client protocol is simpler, but cost of this
> +simplicity is extra complexity in the client which must maintain the decoding
> +state.
> +
> +Querying capabilities
> +=
> +
> +1. To enumerate the set of coded formats supported by the driver, the client
> + calls :c:func:`VIDIOC_ENUM_FMT` on the ``OUTPUT`` queue.
> +
> + * The driver must always return the full set of supported formats for the
> + currently set ``OUTPUT`` format, irrespective of the format currently
> set
> + on the ``CAPTURE`` queue.
> +
> +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.
> +
> + * In order to enumerate raw formats supported by a given coded format, the
> + client must thus set that coded format on the ``OUTPUT`` queue first,
> and
> + then enumerate the ``CAPTURE`` queue.
> +
> +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` ``pixel_format``.
> +
> + * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` on ``OUTPUT`` queue
> + must include all possible coded resolutions supported by the decoder
> + for given coded pixel format.
> +
> + * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` on ``CAPTURE`` queue
> + must include all possible frame buffer resolutions supported by the
> + decoder for given raw pixel format and coded format currently set on
> + ``OUTPUT`` queue.
> +
> +.. note::
> +
> + The client may derive the supported resolution range for a
> + combination of coded and raw format by setting width and height of
> + ``OUTPUT`` format to 0 and calculating the intersection of
> + resolutions returned from calls to :c:func:`VIDIOC_ENUM_FRAMESIZES`
> + for the given coded and raw formats.
> +
> +4. Supported profiles and levels for given format, if applicable, may be
> + queried using their respective controls via :c:func:`VIDIOC_QUERYCTRL`.
> +
> +Initialization
> +==
> +
> +1. *[optional]* Enumerate supported ``OUTPUT`` formats and resolutions. See
> + capability enumeration.
> +
> +2. Set the coded format on the ``OUTPUT`` queue via :c:func:`VIDIOC_S_FMT`
> +
> + * **Required fields:**
> +
> + ``type``
> + a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``
> +
> + ``pixelformat``
> + a coded pixel
