All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 0/5] Stateful Encoding: final bits
@ 2020-05-26 10:09 Hans Verkuil
  2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Hans Verkuil @ 2020-05-26 10:09 UTC (permalink / raw)
  To: linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter

This series adds the encoder spec and updates the VIDIOC_G/S_PARM
documentation.

This is a follow-up of the original "Stateful Encoding: final bits"
series (1). 

The patches in that series that add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
and V4L2_BUF_FLAG_TOO_SMALL have been dropped (the first is not necessary
and the second can be skipped for now, see the irc discussion with
Nicolas [3]).

The encoder spec has been updated since [2] with the following
changes:

- Document the optional VIDIOC_ENUM_FRAMEINTERVALS ioctl.

- Document how to use VIDIOC_S_PARM:

  1) calling S_PARM for the OUTPUT queue sets both the raw frame interval
     (this is a hint only) and the coded frame interval.

  2) calling S_PARM for the CAPTURE queue sets the coded frame interval
     only. This is optional and can be used for off-line encoding. In
     that case the OUTPUT frame interval can be used by the driver to
     schedule multiple encoders.
 
  Ideally S_PARM for the OUTPUT queue would just provide a hint, but
  existing encoder drivers all use S_PARM for the OUTPUT queue to
  define the coded frame interval, and that can't be changed.

- Added a note that if a capture buffer is too small it will be
  returned with V4L2_BUF_FLAG_ERROR and that more work has to be
  done to properly support this corner case.

- Clarify in the 'Encoding' section that there are more reasons
  why 'a buffer queued to OUTPUT may result in more than one buffer
  produced on CAPTURE'.

Added in v3:

- Fix some minor typos.

- Make it more explicit that setting S_PARM(OUTPUT) also sets the
  CAPTURE frame interval.

- Added a new V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag to indicate
  that S_PARM(CAPTURE) can be set separately.

I think that with these changes this stateful encoder spec is ready
to be merged.

Regards,

	Hans

[1] https://lore.kernel.org/linux-media/20191119113457.57833-6-hverkuil-cisco@xs4all.nl/T/
[2] https://www.mail-archive.com/linux-media@vger.kernel.org/msg149211.html
[3] https://linuxtv.org/irc/irclogger_log/v4l?date=2020-05-19,Tue

Hans Verkuil (4):
  vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation
  dev-decoder.rst: small fixes
  videodev2.h: add V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag
  dev-encoder.rst: add reference to V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL

Tomasz Figa (1):
  media: docs-rst: Document memory-to-memory video encoder interface

 .../userspace-api/media/v4l/dev-decoder.rst   |   6 +-
 .../userspace-api/media/v4l/dev-encoder.rst   | 729 ++++++++++++++++++
 .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
 .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
 .../userspace-api/media/v4l/v4l2.rst          |   2 +
 .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
 .../media/v4l/vidioc-enum-fmt.rst             |  30 +-
 .../userspace-api/media/v4l/vidioc-g-parm.rst |  51 +-
 .../media/videodev2.h.rst.exceptions          |   1 +
 include/uapi/linux/videodev2.h                |   1 +
 10 files changed, 830 insertions(+), 47 deletions(-)
 create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst

-- 
2.25.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
@ 2020-05-26 10:09 ` Hans Verkuil
  2020-05-29  9:57   ` Stanimir Varbanov
                     ` (3 more replies)
  2020-05-26 10:09 ` [PATCHv3 2/5] vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation Hans Verkuil
                   ` (5 subsequent siblings)
  6 siblings, 4 replies; 23+ messages in thread
From: Hans Verkuil @ 2020-05-26 10:09 UTC (permalink / raw)
  To: linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter, Hans Verkuil

From: Tomasz Figa <tfiga@chromium.org>

Due to complexity of the video encoding process, the V4L2 drivers of
stateful encoder hardware require specific sequences of V4L2 API calls
to be followed. These include capability enumeration, initialization,
encoding, encode parameters change, drain and reset.

Specifics of the above have been discussed during Media Workshops at
LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
Conference Europe 2014 in Düsseldorf. The de facto Codec API that
originated at those events was later implemented by the drivers we already
have merged in mainline, such as s5p-mfc or coda.

The only thing missing was the real specification included as a part of
Linux Media documentation. Fix it now and document the encoder part of
the Codec API.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
 .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
 .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
 .../userspace-api/media/v4l/v4l2.rst          |   2 +
 .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
 5 files changed, 767 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst

diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
new file mode 100644
index 000000000000..aace7b812a9c
--- /dev/null
+++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
@@ -0,0 +1,728 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _encoder:
+
+*************************************************
+Memory-to-Memory Stateful Video Encoder Interface
+*************************************************
+
+A stateful video encoder takes raw video frames in display order and encodes
+them into a bytestream. It generates complete chunks of the bytestream, including
+all metadata, headers, etc. The resulting bytestream does not require any
+further post-processing by the client.
+
+Performing software stream processing, header generation etc. in the driver
+in order to support this interface is strongly discouraged. In case such
+operations are needed, use of the Stateless Video Encoder Interface (in
+development) is strongly advised.
+
+Conventions and Notations Used in This Document
+===============================================
+
+1. The general V4L2 API rules apply if not specified in this document
+   otherwise.
+
+2. The meaning of words "must", "may", "should", etc. is as per `RFC
+   2119 <https://tools.ietf.org/html/rfc2119>`_.
+
+3. All steps not marked "optional" are required.
+
+4. :c:func:`VIDIOC_G_EXT_CTRLS` and :c:func:`VIDIOC_S_EXT_CTRLS` may be used
+   interchangeably with :c:func:`VIDIOC_G_CTRL` and :c:func:`VIDIOC_S_CTRL`,
+   unless specified otherwise.
+
+5. Single-planar API (see :ref:`planar-apis`) and applicable structures may be
+   used interchangeably with multi-planar API, unless specified otherwise,
+   depending on encoder capabilities and following the general V4L2 guidelines.
+
+6. i = [a..b]: sequence of integers from a to b, inclusive, i.e. i =
+   [0..2]: i = 0, 1, 2.
+
+7. Given an ``OUTPUT`` buffer A, then A' represents a buffer on the ``CAPTURE``
+   queue containing data that resulted from processing buffer A.
+
+Glossary
+========
+
+Refer to :ref:`decoder-glossary`.
+
+State Machine
+=============
+
+.. kernel-render:: DOT
+   :alt: DOT digraph of encoder state machine
+   :caption: Encoder State Machine
+
+   digraph encoder_state_machine {
+       node [shape = doublecircle, label="Encoding"] Encoding;
+
+       node [shape = circle, label="Initialization"] Initialization;
+       node [shape = circle, label="Stopped"] Stopped;
+       node [shape = circle, label="Drain"] Drain;
+       node [shape = circle, label="Reset"] Reset;
+
+       node [shape = point]; qi
+       qi -> Initialization [ label = "open()" ];
+
+       Initialization -> Encoding [ label = "Both queues streaming" ];
+
+       Encoding -> Drain [ label = "V4L2_ENC_CMD_STOP" ];
+       Encoding -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+       Encoding -> Stopped [ label = "VIDIOC_STREAMOFF(OUTPUT)" ];
+       Encoding -> Encoding;
+
+       Drain -> Stopped [ label = "All CAPTURE\nbuffers dequeued\nor\nVIDIOC_STREAMOFF(OUTPUT)" ];
+       Drain -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+
+       Reset -> Encoding [ label = "VIDIOC_STREAMON(CAPTURE)" ];
+       Reset -> Initialization [ label = "VIDIOC_REQBUFS(OUTPUT, 0)" ];
+
+       Stopped -> Encoding [ label = "V4L2_ENC_CMD_START\nor\nVIDIOC_STREAMON(OUTPUT)" ];
+       Stopped -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+   }
+
+Querying Capabilities
+=====================
+
+1. To enumerate the set of coded formats supported by the encoder, the
+   client may call :c:func:`VIDIOC_ENUM_FMT` on ``CAPTURE``.
+
+   * The full set of supported formats will be returned, regardless of the
+     format set on ``OUTPUT``.
+
+2. To enumerate the set of supported raw formats, the client may call
+   :c:func:`VIDIOC_ENUM_FMT` on ``OUTPUT``.
+
+   * Only the formats supported for the format currently active on ``CAPTURE``
+     will be returned.
+
+   * In order to enumerate raw formats supported by a given coded format,
+     the client must first set that coded format on ``CAPTURE`` and then
+     enumerate the formats on ``OUTPUT``.
+
+3. The client may use :c:func:`VIDIOC_ENUM_FRAMESIZES` to detect supported
+   resolutions for a given format, passing the desired pixel format in
+   :c:type:`v4l2_frmsizeenum` ``pixel_format``.
+
+   * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` for a coded pixel
+     format will include all possible coded resolutions supported by the
+     encoder for the given coded pixel format.
+
+   * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` for a raw pixel format
+     will include all possible frame buffer resolutions supported by the
+     encoder for the given raw pixel format and coded format currently set on
+     ``CAPTURE``.
+
+4. The client may use :c:func:`VIDIOC_ENUM_FRAMEINTERVALS` to detect supported
+   frame intervals for a given format and resolution, passing the desired pixel
+   format in :c:type:`v4l2_frmsizeenum` ``pixel_format`` and the resolution
+   in :c:type:`v4l2_frmsizeenum` ``width`` and :c:type:`v4l2_frmsizeenum`
+   ``height``.
+
+   * Values returned by :c:func:`VIDIOC_ENUM_FRAMEINTERVALS` for a coded pixel
+     format and coded resolution will include all possible frame intervals
+     supported by the encoder for the given coded pixel format and resolution.
+
+   * Values returned by :c:func:`VIDIOC_ENUM_FRAMEINTERVALS` for a raw pixel
+     format and resolution will include all possible frame intervals supported
+     by the encoder for the given raw pixel format and resolution and for the
+     coded format, coded resolution and coded frame interval currently set on
+     ``CAPTURE``.
+
+   * Support for :c:func:`VIDIOC_ENUM_FRAMEINTERVALS` is optional. If it is
+     not implemented, then there are no special restrictions other than the
+     limits of the codec itself.
+
+5. Supported profiles and levels for the coded format currently set on
+   ``CAPTURE``, if applicable, may be queried using their respective controls
+   via :c:func:`VIDIOC_QUERYCTRL`.
+
+6. Any additional encoder capabilities may be discovered by querying
+   their respective controls.
+
+Initialization
+==============
+
+1. Set the coded format on the ``CAPTURE`` queue via :c:func:`VIDIOC_S_FMT`.
+
+   * **Required fields:**
+
+     ``type``
+         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``CAPTURE``.
+
+     ``pixelformat``
+         the coded format to be produced.
+
+     ``sizeimage``
+         desired size of ``CAPTURE`` buffers; the encoder may adjust it to
+         match hardware requirements.
+
+     ``width``, ``height``
+         ignored (read-only).
+
+     other fields
+         follow standard semantics.
+
+   * **Return fields:**
+
+     ``sizeimage``
+         adjusted size of ``CAPTURE`` buffers.
+
+     ``width``, ``height``
+         the coded size selected by the encoder based on current state, e.g.
+         ``OUTPUT`` format, selection rectangles, etc. (read-only).
+
+   .. important::
+
+      Changing the ``CAPTURE`` format may change the currently set ``OUTPUT``
+      format. How the new ``OUTPUT`` format is determined is up to the encoder
+      and the client must ensure it matches its needs afterwards.
+
+2. **Optional.** Enumerate supported ``OUTPUT`` formats (raw formats for
+   source) for the selected coded format via :c:func:`VIDIOC_ENUM_FMT`.
+
+   * **Required fields:**
+
+     ``type``
+         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+     other fields
+         follow standard semantics.
+
+   * **Return fields:**
+
+     ``pixelformat``
+         raw format supported for the coded format currently selected on
+         the ``CAPTURE`` queue.
+
+     other fields
+         follow standard semantics.
+
+3. Set the raw source format on the ``OUTPUT`` queue via
+   :c:func:`VIDIOC_S_FMT`.
+
+   * **Required fields:**
+
+     ``type``
+         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+     ``pixelformat``
+         raw format of the source.
+
+     ``width``, ``height``
+         source resolution.
+
+     other fields
+         follow standard semantics.
+
+   * **Return fields:**
+
+     ``width``, ``height``
+         may be adjusted to match encoder minimums, maximums and alignment
+         requirements, as required by the currently selected formats, as
+         reported by :c:func:`VIDIOC_ENUM_FRAMESIZES`.
+
+     other fields
+         follow standard semantics.
+
+   * Setting the ``OUTPUT`` format will reset the selection rectangles to their
+     default values, based on the new resolution, as described in the next
+     step.
+
+4. Set the raw frame interval on the ``OUTPUT`` queue via
+   :c:func:`VIDIOC_S_PARM`. This also sets the coded frame interval on the
+   ``CAPTURE`` queue to the same value.
+
+   * ** Required fields:**
+
+     ``type``
+	 a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+     ``parm.output``
+	 set all fields except ``parm.output.timeperframe`` to 0.
+
+     ``parm.output.timeperframe``
+	 the desired frame interval; the encoder may adjust it to
+	 match hardware requirements.
+
+   * **Return fields:**
+
+     ``parm.output.timeperframe``
+	 the adjusted frame interval.
+
+   .. important::
+
+      Changing the ``OUTPUT`` frame interval *also* sets the framerate that
+      the encoder uses to encode the video. So setting the frame interval
+      to 1/24 (or 24 frames per second) will produce a coded video stream
+      that can be played back at that speed. The frame interval for the
+      ``OUTPUT`` queue is just a hint, the application may provide raw
+      frames at a different rate. It can be used by the driver to help
+      schedule multiple encoders running in parallel.
+
+      In the next step the ``CAPTURE`` frame interval can optionally be
+      changed to a different value. This is useful for off-line encoding
+      were the coded frame interval can be different from the rate at
+      which raw frames are supplied.
+
+   .. important::
+
+      ``timeperframe`` deals with *frames*, not fields. So for interlaced
+      formats this is the time per two fields, since a frame consists of
+      a top and a bottom field.
+
+   .. note::
+
+      It is due to historical reasons that changing the ``OUTPUT`` frame
+      interval also changes the coded frame interval on the ``CAPTURE``
+      queue. Ideally these would be independent settings, but that would
+      break the existing API.
+
+5. **Optional** Set the coded frame interval on the ``CAPTURE`` queue via
+   :c:func:`VIDIOC_S_PARM`. This is only necessary if the coded frame
+   interval is different from the raw frame interval, which is typically
+   the case for off-line encoding.
+
+   * ** Required fields:**
+
+     ``type``
+	 a ``V4L2_BUF_TYPE_*`` enum appropriate for ``CAPTURE``.
+
+     ``parm.capture``
+	 set all fields except ``parm.capture.timeperframe`` to 0.
+
+     ``parm.capture.timeperframe``
+	 the desired coded frame interval; the encoder may adjust it to
+	 match hardware requirements.
+
+   * **Return fields:**
+
+     ``parm.capture.timeperframe``
+	 the adjusted frame interval.
+
+   .. important::
+
+      Changing the ``CAPTURE`` frame interval sets the framerate for the
+      coded video. It does *not* set the rate at which buffers arrive on the
+      ``CAPTURE`` queue, that depends on how fast the encoder is and how
+      fast raw frames are queued on the ``OUTPUT`` queue.
+
+   .. important::
+
+      ``timeperframe`` deals with *frames*, not fields. So for interlaced
+      formats this is the time per two fields, since a frame consists of
+      a top and a bottom field.
+
+   .. note::
+
+      Not all drivers support this functionality, in that case just set
+      the desired coded frame interval for the ``OUTPUT`` queue.
+
+      However, drivers that can schedule multiple encoders based on the
+      ``OUTPUT`` frame interval must support this optional feature.
+
+6. **Optional.** Set the visible resolution for the stream metadata via
+   :c:func:`VIDIOC_S_SELECTION` on the ``OUTPUT`` queue if it is desired
+   to be different than the full OUTPUT resolution.
+
+   * **Required fields:**
+
+     ``type``
+         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+     ``target``
+         set to ``V4L2_SEL_TGT_CROP``.
+
+     ``r.left``, ``r.top``, ``r.width``, ``r.height``
+         visible rectangle; this must fit within the `V4L2_SEL_TGT_CROP_BOUNDS`
+         rectangle and may be subject to adjustment to match codec and
+         hardware constraints.
+
+   * **Return fields:**
+
+     ``r.left``, ``r.top``, ``r.width``, ``r.height``
+         visible rectangle adjusted by the encoder.
+
+   * The following selection targets are supported on ``OUTPUT``:
+
+     ``V4L2_SEL_TGT_CROP_BOUNDS``
+         equal to the full source frame, matching the active ``OUTPUT``
+         format.
+
+     ``V4L2_SEL_TGT_CROP_DEFAULT``
+         equal to ``V4L2_SEL_TGT_CROP_BOUNDS``.
+
+     ``V4L2_SEL_TGT_CROP``
+         rectangle within the source buffer to be encoded into the
+         ``CAPTURE`` stream; defaults to ``V4L2_SEL_TGT_CROP_DEFAULT``.
+
+         .. note::
+
+            A common use case for this selection target is encoding a source
+            video with a resolution that is not a multiple of a macroblock,
+            e.g.  the common 1920x1080 resolution may require the source
+            buffers to be aligned to 1920x1088 for codecs with 16x16 macroblock
+            size. To avoid encoding the padding, the client needs to explicitly
+            configure this selection target to 1920x1080.
+
+   .. warning::
+
+      The encoder may adjust the crop/compose rectangles to the nearest
+      supported ones to meet codec and hardware requirements. The client needs
+      to check the adjusted rectangle returned by :c:func:`VIDIOC_S_SELECTION`.
+
+7. Allocate buffers for both ``OUTPUT`` and ``CAPTURE`` via
+   :c:func:`VIDIOC_REQBUFS`. This may be performed in any order.
+
+   * **Required fields:**
+
+     ``count``
+         requested number of buffers to allocate; greater than zero.
+
+     ``type``
+         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT`` or
+         ``CAPTURE``.
+
+     other fields
+         follow standard semantics.
+
+   * **Return fields:**
+
+     ``count``
+          actual number of buffers allocated.
+
+   .. warning::
+
+      The actual number of allocated buffers may differ from the ``count``
+      given. The client must check the updated value of ``count`` after the
+      call returns.
+
+   .. note::
+
+      To allocate more than the minimum number of OUTPUT buffers (for pipeline
+      depth), the client may query the ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT``
+      control to get the minimum number of buffers required, and pass the
+      obtained value plus the number of additional buffers needed in the
+      ``count`` field to :c:func:`VIDIOC_REQBUFS`.
+
+   Alternatively, :c:func:`VIDIOC_CREATE_BUFS` can be used to have more
+   control over buffer allocation.
+
+   * **Required fields:**
+
+     ``count``
+         requested number of buffers to allocate; greater than zero.
+
+     ``type``
+         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``.
+
+     other fields
+         follow standard semantics.
+
+   * **Return fields:**
+
+     ``count``
+         adjusted to the number of allocated buffers.
+
+8. Begin streaming on both ``OUTPUT`` and ``CAPTURE`` queues via
+   :c:func:`VIDIOC_STREAMON`. This may be performed in any order. The actual
+   encoding process starts when both queues start streaming.
+
+.. note::
+
+   If the client stops the ``CAPTURE`` queue during the encode process and then
+   restarts it again, the encoder will begin generating a stream independent
+   from the stream generated before the stop. The exact constraints depend
+   on the coded format, but may include the following implications:
+
+   * encoded frames produced after the restart must not reference any
+     frames produced before the stop, e.g. no long term references for
+     H.264/HEVC,
+
+   * any headers that must be included in a standalone stream must be
+     produced again, e.g. SPS and PPS for H.264/HEVC.
+
+Encoding
+========
+
+This state is reached after the `Initialization` sequence finishes
+successfully.  In this state, the client queues and dequeues buffers to both
+queues via :c:func:`VIDIOC_QBUF` and :c:func:`VIDIOC_DQBUF`, following the
+standard semantics.
+
+The content of encoded ``CAPTURE`` buffers depends on the active coded pixel
+format and may be affected by codec-specific extended controls, as stated
+in the documentation of each format.
+
+Both queues operate independently, following standard behavior of V4L2 buffer
+queues and memory-to-memory devices. In addition, the order of encoded frames
+dequeued from the ``CAPTURE`` queue may differ from the order of queuing raw
+frames to the ``OUTPUT`` queue, due to properties of the selected coded format,
+e.g. frame reordering.
+
+The client must not assume any direct relationship between ``CAPTURE`` and
+``OUTPUT`` buffers and any specific timing of buffers becoming
+available to dequeue. Specifically:
+
+* a buffer queued to ``OUTPUT`` may result in more than one buffer produced on
+  ``CAPTURE`` (for example, if returning an encoded frame allowed the encoder
+  to return a frame that preceded it in display, but succeeded it in the decode
+  order; however, there may be other reasons for this as well),
+
+* a buffer queued to ``OUTPUT`` may result in a buffer being produced on
+  ``CAPTURE`` later into encode process, and/or after processing further
+  ``OUTPUT`` buffers, or be returned out of order, e.g. if display
+  reordering is used,
+
+* buffers may become available on the ``CAPTURE`` queue without additional
+  buffers queued to ``OUTPUT`` (e.g. during drain or ``EOS``), because of the
+  ``OUTPUT`` buffers queued in the past whose encoding results are only
+  available at later time, due to specifics of the encoding process,
+
+* buffers queued to ``OUTPUT`` may not become available to dequeue instantly
+  after being encoded into a corresponding ``CAPTURE`` buffer, e.g. if the
+  encoder needs to use the frame as a reference for encoding further frames.
+
+.. note::
+
+   To allow matching encoded ``CAPTURE`` buffers with ``OUTPUT`` buffers they
+   originated from, the client can set the ``timestamp`` field of the
+   :c:type:`v4l2_buffer` struct when queuing an ``OUTPUT`` buffer. The
+   ``CAPTURE`` buffer(s), which resulted from encoding that ``OUTPUT`` buffer
+   will have their ``timestamp`` field set to the same value when dequeued.
+
+   In addition to the straightforward case of one ``OUTPUT`` buffer producing
+   one ``CAPTURE`` buffer, the following cases are defined:
+
+   * one ``OUTPUT`` buffer generates multiple ``CAPTURE`` buffers: the same
+     ``OUTPUT`` timestamp will be copied to multiple ``CAPTURE`` buffers,
+
+   * the encoding order differs from the presentation order (i.e. the
+     ``CAPTURE`` buffers are out-of-order compared to the ``OUTPUT`` buffers):
+     ``CAPTURE`` timestamps will not retain the order of ``OUTPUT`` timestamps.
+
+.. note::
+
+   To let the client distinguish between frame types (keyframes, intermediate
+   frames; the exact list of types depends on the coded format), the
+   ``CAPTURE`` buffers will have corresponding flag bits set in their
+   :c:type:`v4l2_buffer` struct when dequeued. See the documentation of
+   :c:type:`v4l2_buffer` and each coded pixel format for exact list of flags
+   and their meanings.
+
+Should an encoding error occur, it will be reported to the client with the level
+of details depending on the encoder capabilities. Specifically:
+
+* the ``CAPTURE`` buffer (if any) that contains the results of the failed encode
+  operation will be returned with the ``V4L2_BUF_FLAG_ERROR`` flag set,
+
+* if the encoder is able to precisely report the ``OUTPUT`` buffer(s) that triggered
+  the error, such buffer(s) will be returned with the ``V4L2_BUF_FLAG_ERROR`` flag
+  set.
+
+.. note::
+
+   If a ``CAPTURE`` buffer is too small then it is just returned with the
+   ``V4L2_BUF_FLAG_ERROR`` flag set. More work is needed to detect that this
+   error occurred because the buffer was too small, and to provide support to
+   free existing buffers that were too small.
+
+In case of a fatal failure that does not allow the encoding to continue, any
+further operations on corresponding encoder file handle will return the -EIO
+error code. The client may close the file handle and open a new one, or
+alternatively reinitialize the instance by stopping streaming on both queues,
+releasing all buffers and performing the Initialization sequence again.
+
+Encoding Parameter Changes
+==========================
+
+The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
+parameters at any time. The availability of parameters is encoder-specific
+and the client must query the encoder to find the set of available controls.
+
+The ability to change each parameter during encoding is encoder-specific, as
+per the standard semantics of the V4L2 control interface. The client may
+attempt to set a control during encoding and if the operation fails with the
+-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
+configuration change to be allowed. To do this, it may follow the `Drain`
+sequence to avoid losing the already queued/encoded frames.
+
+The timing of parameter updates is encoder-specific, as per the standard
+semantics of the V4L2 control interface. If the client needs to apply the
+parameters exactly at specific frame, using the Request API
+(:ref:`media-request-api`) should be considered, if supported by the encoder.
+
+Drain
+=====
+
+To ensure that all the queued ``OUTPUT`` buffers have been processed and the
+related ``CAPTURE`` buffers are given to the client, the client must follow the
+drain sequence described below. After the drain sequence ends, the client has
+received all encoded frames for all ``OUTPUT`` buffers queued before the
+sequence was started.
+
+1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
+
+   * **Required fields:**
+
+     ``cmd``
+         set to ``V4L2_ENC_CMD_STOP``.
+
+     ``flags``
+         set to 0.
+
+     ``pts``
+         set to 0.
+
+   .. warning::
+
+      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
+      queues are streaming. For compatibility reasons, the call to
+      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
+      not streaming, but at the same time it will not initiate the `Drain`
+      sequence and so the steps described below would not be applicable.
+
+2. Any ``OUTPUT`` buffers queued by the client before the
+   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
+   normal. The client must continue to handle both queues independently,
+   similarly to normal encode operation. This includes:
+
+   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
+     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
+
+     .. warning::
+
+        The last buffer may be empty (with :c:type:`v4l2_buffer`
+        ``bytesused`` = 0) and in that case it must be ignored by the client,
+        as it does not contain an encoded frame.
+
+     .. note::
+
+        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
+        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
+        :c:func:`VIDIOC_DQBUF`.
+
+   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
+     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
+
+   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
+
+   .. note::
+
+      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
+      event when the last frame has been encoded and all frames are ready to be
+      dequeued. It is deprecated behavior and the client must not rely on it.
+      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
+
+3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
+   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
+   and it will accept, but not process any newly queued ``OUTPUT`` buffers
+   until the client issues any of the following operations:
+
+   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
+     operation normally, with all the state from before the drain,
+
+   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
+     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
+     and then resume encoding,
+
+   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
+     ``OUTPUT`` queue - the encoder will resume operation normally, however any
+     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
+     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
+
+.. note::
+
+   Once the drain sequence is initiated, the client needs to drive it to
+   completion, as described by the steps above, unless it aborts the process by
+   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
+   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
+   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
+   will fail with -EBUSY error code if attempted.
+
+   For reference, handling of various corner cases is described below:
+
+   * In case of no buffer in the ``OUTPUT`` queue at the time the
+     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
+     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
+     ``V4L2_BUF_FLAG_LAST`` flag set.
+
+   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
+     sequence completes, the next time the client queues a ``CAPTURE`` buffer
+     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
+     flag set.
+
+   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
+     middle of the drain sequence, the drain sequence is canceled and all
+     ``CAPTURE`` buffers are implicitly returned to the client.
+
+   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
+     middle of the drain sequence, the drain sequence completes immediately and
+     next ``CAPTURE`` buffer will be returned empty with the
+     ``V4L2_BUF_FLAG_LAST`` flag set.
+
+   Although not mandatory, the availability of encoder commands may be queried
+   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
+
+Reset
+=====
+
+The client may want to request the encoder to reinitialize the encoding, so
+that the following stream data becomes independent from the stream data
+generated before. Depending on the coded format, that may imply that:
+
+* encoded frames produced after the restart must not reference any frames
+  produced before the stop, e.g. no long term references for H.264/HEVC,
+
+* any headers that must be included in a standalone stream must be produced
+  again, e.g. SPS and PPS for H.264/HEVC.
+
+This can be achieved by performing the reset sequence.
+
+1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
+   and respective buffers are dequeued.
+
+2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
+   will return all currently queued ``CAPTURE`` buffers to the client, without
+   valid frame data.
+
+3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
+   continue with regular encoding sequence. The encoded frames produced into
+   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
+   decoded without the need for frames encoded before the reset sequence,
+   starting at the first ``OUTPUT`` buffer queued after issuing the
+   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
+
+This sequence may be also used to change encoding parameters for encoders
+without the ability to change the parameters on the fly.
+
+Commit Points
+=============
+
+Setting formats and allocating buffers triggers changes in the behavior of the
+encoder.
+
+1. Setting the format on the ``CAPTURE`` queue may change the set of formats
+   supported/advertised on the ``OUTPUT`` queue. In particular, it also means
+   that the ``OUTPUT`` format may be reset and the client must not rely on the
+   previously set format being preserved.
+
+2. Enumerating formats on the ``OUTPUT`` queue always returns only formats
+   supported for the current ``CAPTURE`` format.
+
+3. Setting the format on the ``OUTPUT`` queue does not change the list of
+   formats available on the ``CAPTURE`` queue. An attempt to set the ``OUTPUT``
+   format that is not supported for the currently selected ``CAPTURE`` format
+   will result in the encoder adjusting the requested ``OUTPUT`` format to a
+   supported one.
+
+4. Enumerating formats on the ``CAPTURE`` queue always returns the full set of
+   supported coded formats, irrespective of the current ``OUTPUT`` format.
+
+5. While buffers are allocated on any of the ``OUTPUT`` or ``CAPTURE`` queues,
+   the client must not change the format on the ``CAPTURE`` queue. Drivers will
+   return the -EBUSY error code for any such format change attempt.
+
+To summarize, setting formats and allocation must always start with the
+``CAPTURE`` queue and the ``CAPTURE`` queue is the master that governs the
+set of supported formats for the ``OUTPUT`` queue.
diff --git a/Documentation/userspace-api/media/v4l/dev-mem2mem.rst b/Documentation/userspace-api/media/v4l/dev-mem2mem.rst
index 9279d87c08a1..40aff9c95267 100644
--- a/Documentation/userspace-api/media/v4l/dev-mem2mem.rst
+++ b/Documentation/userspace-api/media/v4l/dev-mem2mem.rst
@@ -46,4 +46,5 @@ devices are given in the following sections.
     :maxdepth: 1
 
     dev-decoder
+    dev-encoder
     dev-stateless-decoder
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
index 759420a872d6..e0ee2823ab1f 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
@@ -44,6 +44,11 @@ Single-planar format structure
 	inside the stream, when fed to a stateful mem2mem decoder, the fields
 	may be zero to rely on the decoder to detect the right values. For more
 	details see :ref:`decoder` and format descriptions.
+
+	For compressed formats on the CAPTURE side of a stateful mem2mem
+	encoder, the fields must be zero, since the coded size is expected to
+	be calculated internally by the encoder itself, based on the OUTPUT
+	side. For more details see :ref:`encoder` and format descriptions.
     * - __u32
       - ``pixelformat``
       - The pixel format or type of compression, set by the application.
diff --git a/Documentation/userspace-api/media/v4l/v4l2.rst b/Documentation/userspace-api/media/v4l/v4l2.rst
index ab7c97c39b97..35796c4fbe52 100644
--- a/Documentation/userspace-api/media/v4l/v4l2.rst
+++ b/Documentation/userspace-api/media/v4l/v4l2.rst
@@ -63,6 +63,7 @@ Authors, in alphabetical order:
 - Figa, Tomasz <tfiga@chromium.org>
 
   - Documented the memory-to-memory decoder interface.
+  - Documented the memory-to-memory encoder interface.
 
 - H Schimek, Michael <mschimek@gmx.at>
 
@@ -75,6 +76,7 @@ Authors, in alphabetical order:
 - Osciak, Pawel <posciak@chromium.org>
 
   - Documented the memory-to-memory decoder interface.
+  - Documented the memory-to-memory encoder interface.
 
 - Osciak, Pawel <pawel@osciak.com>
 
diff --git a/Documentation/userspace-api/media/v4l/vidioc-encoder-cmd.rst b/Documentation/userspace-api/media/v4l/vidioc-encoder-cmd.rst
index 16269b3b1715..d0eacce5485e 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-encoder-cmd.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-encoder-cmd.rst
@@ -51,25 +51,26 @@ To send a command applications must initialize all fields of a struct
 ``VIDIOC_ENCODER_CMD`` or ``VIDIOC_TRY_ENCODER_CMD`` with a pointer to
 this structure.
 
-The ``cmd`` field must contain the command code. The ``flags`` field is
-currently only used by the STOP command and contains one bit: If the
-``V4L2_ENC_CMD_STOP_AT_GOP_END`` flag is set, encoding will continue
-until the end of the current *Group Of Pictures*, otherwise it will stop
-immediately.
+The ``cmd`` field must contain the command code. Some commands use the
+``flags`` field for additional information.
 
-A :ref:`read() <func-read>` or :ref:`VIDIOC_STREAMON <VIDIOC_STREAMON>`
-call sends an implicit START command to the encoder if it has not been
-started yet. After a STOP command, :ref:`read() <func-read>` calls will read
+After a STOP command, :ref:`read() <func-read>` calls will read
 the remaining data buffered by the driver. When the buffer is empty,
 :ref:`read() <func-read>` will return zero and the next :ref:`read() <func-read>`
 call will restart the encoder.
 
+A :ref:`read() <func-read>` or :ref:`VIDIOC_STREAMON <VIDIOC_STREAMON>`
+call sends an implicit START command to the encoder if it has not been
+started yet. Applies to both queues of mem2mem encoders.
+
 A :ref:`close() <func-close>` or :ref:`VIDIOC_STREAMOFF <VIDIOC_STREAMON>`
 call of a streaming file descriptor sends an implicit immediate STOP to
-the encoder, and all buffered data is discarded.
+the encoder, and all buffered data is discarded. Applies to both queues of
+mem2mem encoders.
 
 These ioctls are optional, not all drivers may support them. They were
-introduced in Linux 2.6.21.
+introduced in Linux 2.6.21. They are, however, mandatory for stateful mem2mem
+encoders (as further documented in :ref:`encoder`).
 
 
 .. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
@@ -109,21 +110,24 @@ introduced in Linux 2.6.21.
       - 0
       - Start the encoder. When the encoder is already running or paused,
 	this command does nothing. No flags are defined for this command.
+
+	For a device implementing the :ref:`encoder`, once the drain sequence
+	is initiated with the ``V4L2_ENC_CMD_STOP`` command, it must be driven
+	to completion before this command can be invoked.  Any attempt to
+	invoke the command while the drain sequence is in progress will trigger
+	an ``EBUSY`` error code. See :ref:`encoder` for more details.
     * - ``V4L2_ENC_CMD_STOP``
       - 1
       - Stop the encoder. When the ``V4L2_ENC_CMD_STOP_AT_GOP_END`` flag
 	is set, encoding will continue until the end of the current *Group
 	Of Pictures*, otherwise encoding will stop immediately. When the
-	encoder is already stopped, this command does nothing. mem2mem
-	encoders will send a ``V4L2_EVENT_EOS`` event when the last frame
-	has been encoded and all frames are ready to be dequeued and will
-	set the ``V4L2_BUF_FLAG_LAST`` buffer flag on the last buffer of
-	the capture queue to indicate there will be no new buffers
-	produced to dequeue. This buffer may be empty, indicated by the
-	driver setting the ``bytesused`` field to 0. Once the
-	``V4L2_BUF_FLAG_LAST`` flag was set, the
-	:ref:`VIDIOC_DQBUF <VIDIOC_QBUF>` ioctl will not block anymore,
-	but return an ``EPIPE`` error code.
+	encoder is already stopped, this command does nothing.
+
+	For a device implementing the :ref:`encoder`, the command will initiate
+	the drain sequence as documented in :ref:`encoder`. No flags or other
+	arguments are accepted in this case. Any attempt to invoke the command
+	again before the sequence completes will trigger an ``EBUSY`` error
+	code.
     * - ``V4L2_ENC_CMD_PAUSE``
       - 2
       - Pause the encoder. When the encoder has not been started yet, the
@@ -152,6 +156,8 @@ introduced in Linux 2.6.21.
       - Stop encoding at the end of the current *Group Of Pictures*,
 	rather than immediately.
 
+        Does not apply to :ref:`encoder`.
+
 
 Return Value
 ============
@@ -160,6 +166,11 @@ On success 0 is returned, on error -1 and the ``errno`` variable is set
 appropriately. The generic error codes are described at the
 :ref:`Generic Error Codes <gen-errors>` chapter.
 
+EBUSY
+    A drain sequence of a device implementing the :ref:`encoder` is still in
+    progress. It is not allowed to issue another encoder command until it
+    completes.
+
 EINVAL
     The ``cmd`` field is invalid.
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCHv3 2/5] vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation
  2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
  2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
@ 2020-05-26 10:09 ` Hans Verkuil
  2020-05-28  7:54   ` Michael Tretter
  2020-05-26 10:09 ` [PATCHv3 3/5] dev-decoder.rst: small fixes Hans Verkuil
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Hans Verkuil @ 2020-05-26 10:09 UTC (permalink / raw)
  To: linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter, Hans Verkuil

This documentation is very outdated. In particular, it is
not obvious at all that this is used to change the framerate of
sensors.

Fix it, and include references to the stateful encoder API where
this works slightly different.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Acked-by: Tomasz Figa <tfiga@chromium.org>
---
 .../userspace-api/media/v4l/vidioc-g-parm.rst | 51 ++++++++++++-------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst b/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst
index 42e9f6ee7a59..af3c5863bb87 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst
@@ -42,12 +42,13 @@ Arguments
 Description
 ===========
 
-The current video standard determines a nominal number of frames per
-second. If less than this number of frames is to be captured or output,
-applications can request frame skipping or duplicating on the driver
-side. This is especially useful when using the :ref:`read() <func-read>` or
-:ref:`write() <func-write>`, which are not augmented by timestamps or sequence
-counters, and to avoid unnecessary data copying.
+Applications can request a different frame interval. The capture or
+output device will be reconfigured to support the requested frame
+interval if possible. Optionally drivers may choose to skip or
+repeat frames to achieve the requested frame interval.
+
+For stateful encoders (see :ref:`encoder`) this represents the
+frame interval that is typically embedded in the encoded video stream.
 
 Changing the frame interval shall never change the format. Changing the
 format, on the other hand, may change the frame interval.
@@ -57,7 +58,8 @@ internally by a driver in read/write mode. For implications see the
 section discussing the :ref:`read() <func-read>` function.
 
 To get and set the streaming parameters applications call the
-:ref:`VIDIOC_G_PARM <VIDIOC_G_PARM>` and :ref:`VIDIOC_S_PARM <VIDIOC_G_PARM>` ioctl, respectively. They take a
+:ref:`VIDIOC_G_PARM <VIDIOC_G_PARM>` and
+:ref:`VIDIOC_S_PARM <VIDIOC_G_PARM>` ioctl, respectively. They take a
 pointer to a struct :c:type:`v4l2_streamparm` which contains a
 union holding separate parameters for input and output devices.
 
@@ -113,14 +115,21 @@ union holding separate parameters for input and output devices.
     * - struct :c:type:`v4l2_fract`
       - ``timeperframe``
       - This is the desired period between successive frames captured by
-	the driver, in seconds. The field is intended to skip frames on
-	the driver side, saving I/O bandwidth.
+	the driver, in seconds.
+    * - :cspan:`2`
+
+	This will configure the speed at which the video source (e.g. a sensor)
+	generates video frames. If the speed is fixed, then the driver may
+	choose to skip or repeat frames in order to achieve the requested
+	frame rate.
+
+        For stateful encoders (see :ref:`encoder`) this represents the
+	frame interval that is typically embedded in the encoded video stream.
 
 	Applications store here the desired frame period, drivers return
-	the actual frame period, which must be greater or equal to the
-	nominal frame period determined by the current video standard
-	(struct :c:type:`v4l2_standard` ``frameperiod``
-	field). Changing the video standard (also implicitly by switching
+	the actual frame period.
+
+	Changing the video standard (also implicitly by switching
 	the video input) may reset this parameter to the nominal frame
 	period. To reset manually applications can just set this field to
 	zero.
@@ -173,11 +182,15 @@ union holding separate parameters for input and output devices.
 	:ref:`write() <func-write>` mode (in streaming mode timestamps
 	can be used to throttle the output), saving I/O bandwidth.
 
+        For stateful encoders (see :ref:`encoder`) this represents the
+	frame interval that is typically embedded in the encoded video stream
+	and it provides a hint to the encoder of the speed at which raw
+	frames are queued up to the encoder.
+
 	Applications store here the desired frame period, drivers return
-	the actual frame period, which must be greater or equal to the
-	nominal frame period determined by the current video standard
-	(struct :c:type:`v4l2_standard` ``frameperiod``
-	field). Changing the video standard (also implicitly by switching
+	the actual frame period.
+
+	Changing the video standard (also implicitly by switching
 	the video output) may reset this parameter to the nominal frame
 	period. To reset manually applications can just set this field to
 	zero.
@@ -216,8 +229,8 @@ union holding separate parameters for input and output devices.
 
     * - ``V4L2_CAP_TIMEPERFRAME``
       - 0x1000
-      - The frame skipping/repeating controlled by the ``timeperframe``
-	field is supported.
+      - The frame period can be modified by setting the ``timeperframe``
+	field.
 
 
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCHv3 3/5] dev-decoder.rst: small fixes
  2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
  2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
  2020-05-26 10:09 ` [PATCHv3 2/5] vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation Hans Verkuil
@ 2020-05-26 10:09 ` Hans Verkuil
  2020-05-26 10:09 ` [PATCHv3 4/5] videodev2.h: add V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag Hans Verkuil
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2020-05-26 10:09 UTC (permalink / raw)
  To: linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter, Hans Verkuil

Add missing periods at the end of two sentences.

Although mandatory -> Although not mandatory

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/userspace-api/media/v4l/dev-decoder.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/dev-decoder.rst b/Documentation/userspace-api/media/v4l/dev-decoder.rst
index 606b54947e10..04034dbc0b3e 100644
--- a/Documentation/userspace-api/media/v4l/dev-decoder.rst
+++ b/Documentation/userspace-api/media/v4l/dev-decoder.rst
@@ -247,7 +247,7 @@ Querying Capabilities
 Initialization
 ==============
 
-1. Set the coded format on ``OUTPUT`` via :c:func:`VIDIOC_S_FMT`
+1. Set the coded format on ``OUTPUT`` via :c:func:`VIDIOC_S_FMT`.
 
    * **Required fields:**
 
@@ -803,7 +803,7 @@ it may be affected as per normal decoder operation.
    * The decoder will drop all the pending ``OUTPUT`` buffers and they must be
      treated as returned to the client (following standard semantics).
 
-2. Restart the ``OUTPUT`` queue via :c:func:`VIDIOC_STREAMON`
+2. Restart the ``OUTPUT`` queue via :c:func:`VIDIOC_STREAMON`.
 
    * **Required fields:**
 
@@ -1059,7 +1059,7 @@ sequence was started.
    ``V4L2_DEC_CMD_STOP`` again while the drain sequence is in progress and they
    will fail with -EBUSY error code if attempted.
 
-   Although mandatory, the availability of decoder commands may be queried
+   Although not mandatory, the availability of decoder commands may be queried
    using :c:func:`VIDIOC_TRY_DECODER_CMD`.
 
 End of Stream
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCHv3 4/5] videodev2.h: add V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag
  2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
                   ` (2 preceding siblings ...)
  2020-05-26 10:09 ` [PATCHv3 3/5] dev-decoder.rst: small fixes Hans Verkuil
@ 2020-05-26 10:09 ` Hans Verkuil
  2020-05-26 10:09 ` [PATCHv3 5/5] dev-encoder.rst: add reference to V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL Hans Verkuil
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2020-05-26 10:09 UTC (permalink / raw)
  To: linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter, Hans Verkuil

Add the V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag to signal that
the coded frame interval can be set separately from the raw frame
interval for stateful encoders.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../media/v4l/vidioc-enum-fmt.rst             | 30 +++++++++++++++----
 .../media/videodev2.h.rst.exceptions          |  1 +
 include/uapi/linux/videodev2.h                |  1 +
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
index a53dd3d7f7e2..05835e04c20b 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
@@ -167,17 +167,37 @@ the ``mbus_code`` field is handled differently:
       - The hardware decoder for this compressed bytestream format (aka coded
 	format) is capable of parsing a continuous bytestream. Applications do
 	not need to parse the bytestream themselves to find the boundaries
-	between frames/fields. This flag can only be used in combination with
-	the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
+	between frames/fields.
+
+	This flag can only be used in combination with the
+	``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
 	formats only. This flag is valid for stateful decoders only.
     * - ``V4L2_FMT_FLAG_DYN_RESOLUTION``
       - 0x0008
       - Dynamic resolution switching is supported by the device for this
 	compressed bytestream format (aka coded format). It will notify the user
 	via the event ``V4L2_EVENT_SOURCE_CHANGE`` when changes in the video
-	parameters are detected. This flag can only be used in combination
-	with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
-	compressed formats only. It is also only applies to stateful codecs.
+	parameters are detected.
+
+	This flag can only be used in combination with the
+	``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
+	compressed formats only. This flag is valid for stateful codecs only.
+    * - ``V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL``
+      - 0x0010
+      - The hardware encoder supports setting the ``CAPTURE`` coded frame
+	interval separately from the ``OUTPUT`` raw frame interval.
+	Setting the ``OUTPUT`` raw frame interval with :ref:`VIDIOC_S_PARM <VIDIOC_G_PARM>`
+	also sets the ``CAPTURE`` coded frame interval to the same value.
+	If this flag is set, then the ``CAPTURE`` coded frame interval can be
+	set to a different value afterwards. This is typically used for
+	offline encoding where the ``OUTPUT`` raw frame interval is used as
+	a hint for reserving hardware encoder resources and the ``CAPTURE`` coded
+	frame interval is the actual frame rate embedded in the encoded video
+	stream.
+
+	This flag can only be used in combination with the
+	``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
+        compressed formats only. This flag is valid for stateful encoders only.
 
 
 Return Value
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index a625fb90e3a9..ca05e4e126b2 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -187,6 +187,7 @@ replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
 replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
 replace define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM fmtdesc-flags
 replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
+replace define V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL fmtdesc-flags
 
 # V4L2 timecode types
 replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c3a1cf1c507f..18781f556ad8 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -792,6 +792,7 @@ struct v4l2_fmtdesc {
 #define V4L2_FMT_FLAG_EMULATED			0x0002
 #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM	0x0004
 #define V4L2_FMT_FLAG_DYN_RESOLUTION		0x0008
+#define V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL	0x0010
 
 	/* Frame Size and frame rate enumeration */
 /*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCHv3 5/5] dev-encoder.rst: add reference to V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL
  2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
                   ` (3 preceding siblings ...)
  2020-05-26 10:09 ` [PATCHv3 4/5] videodev2.h: add V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag Hans Verkuil
@ 2020-05-26 10:09 ` Hans Verkuil
  2020-05-28  7:58 ` [PATCHv3 0/5] Stateful Encoding: final bits Michael Tretter
  2020-06-02  9:01 ` Tomasz Figa
  6 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2020-05-26 10:09 UTC (permalink / raw)
  To: linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter, Hans Verkuil

Setting the stateful encoder capture frame interval is only supported
if this flag is set. Document this.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/userspace-api/media/v4l/dev-encoder.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
index aace7b812a9c..c3a21bfe0052 100644
--- a/Documentation/userspace-api/media/v4l/dev-encoder.rst
+++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
@@ -281,7 +281,8 @@ Initialization
 5. **Optional** Set the coded frame interval on the ``CAPTURE`` queue via
    :c:func:`VIDIOC_S_PARM`. This is only necessary if the coded frame
    interval is different from the raw frame interval, which is typically
-   the case for off-line encoding.
+   the case for off-line encoding. Support for this feature is signalled
+   by the :ref:`V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL <fmtdesc-flags>` format flag.
 
    * ** Required fields:**
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 2/5] vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation
  2020-05-26 10:09 ` [PATCHv3 2/5] vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation Hans Verkuil
@ 2020-05-28  7:54   ` Michael Tretter
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Tretter @ 2020-05-28  7:54 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Nicolas Dufresne, Tomasz Figa

On Tue, May 26, 2020 at 12:09:29PM +0200, Hans Verkuil wrote:
> This documentation is very outdated. In particular, it is
> not obvious at all that this is used to change the framerate of
> sensors.
> 
> Fix it, and include references to the stateful encoder API where
> this works slightly different.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Acked-by: Tomasz Figa <tfiga@chromium.org>
> ---
>  .../userspace-api/media/v4l/vidioc-g-parm.rst | 51 ++++++++++++-------
>  1 file changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst b/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst
> index 42e9f6ee7a59..af3c5863bb87 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-parm.rst
[snip]
> @@ -113,14 +115,21 @@ union holding separate parameters for input and output devices.
>      * - struct :c:type:`v4l2_fract`
>        - ``timeperframe``
>        - This is the desired period between successive frames captured by
> -	the driver, in seconds. The field is intended to skip frames on
> -	the driver side, saving I/O bandwidth.
> +	the driver, in seconds.
> +    * - :cspan:`2`
> +
> +	This will configure the speed at which the video source (e.g. a sensor)
> +	generates video frames. If the speed is fixed, then the driver may
> +	choose to skip or repeat frames in order to achieve the requested
> +	frame rate.
> +
> +        For stateful encoders (see :ref:`encoder`) this represents the

Spaces instead of tab.

> +	frame interval that is typically embedded in the encoded video stream.
>  
>  	Applications store here the desired frame period, drivers return
> -	the actual frame period, which must be greater or equal to the
> -	nominal frame period determined by the current video standard
> -	(struct :c:type:`v4l2_standard` ``frameperiod``
> -	field). Changing the video standard (also implicitly by switching
> +	the actual frame period.
> +
> +	Changing the video standard (also implicitly by switching
>  	the video input) may reset this parameter to the nominal frame
>  	period. To reset manually applications can just set this field to
>  	zero.
> @@ -173,11 +182,15 @@ union holding separate parameters for input and output devices.
>  	:ref:`write() <func-write>` mode (in streaming mode timestamps
>  	can be used to throttle the output), saving I/O bandwidth.
>  
> +        For stateful encoders (see :ref:`encoder`) this represents the

Spaces instead of tab.

Michael

> +	frame interval that is typically embedded in the encoded video stream
> +	and it provides a hint to the encoder of the speed at which raw
> +	frames are queued up to the encoder.
> +
>  	Applications store here the desired frame period, drivers return
> -	the actual frame period, which must be greater or equal to the
> -	nominal frame period determined by the current video standard
> -	(struct :c:type:`v4l2_standard` ``frameperiod``
> -	field). Changing the video standard (also implicitly by switching
> +	the actual frame period.
> +
> +	Changing the video standard (also implicitly by switching
>  	the video output) may reset this parameter to the nominal frame
>  	period. To reset manually applications can just set this field to
>  	zero.
> @@ -216,8 +229,8 @@ union holding separate parameters for input and output devices.
>  
>      * - ``V4L2_CAP_TIMEPERFRAME``
>        - 0x1000
> -      - The frame skipping/repeating controlled by the ``timeperframe``
> -	field is supported.
> +      - The frame period can be modified by setting the ``timeperframe``
> +	field.
>  
>  
>  
> -- 
> 2.25.1
> 
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 0/5] Stateful Encoding: final bits
  2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
                   ` (4 preceding siblings ...)
  2020-05-26 10:09 ` [PATCHv3 5/5] dev-encoder.rst: add reference to V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL Hans Verkuil
@ 2020-05-28  7:58 ` Michael Tretter
  2020-06-02  9:01 ` Tomasz Figa
  6 siblings, 0 replies; 23+ messages in thread
From: Michael Tretter @ 2020-05-28  7:58 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Nicolas Dufresne, Tomasz Figa

On Tue, May 26, 2020 at 12:09:27PM +0200, Hans Verkuil wrote:
> This series adds the encoder spec and updates the VIDIOC_G/S_PARM
> documentation.
> 
> This is a follow-up of the original "Stateful Encoding: final bits"
> series (1). 
> 
> The patches in that series that add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
> and V4L2_BUF_FLAG_TOO_SMALL have been dropped (the first is not necessary
> and the second can be skipped for now, see the irc discussion with
> Nicolas [3]).

Thanks for all the work.

Apart from two really small formatting issues the entire series is

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

Michael

> 
> The encoder spec has been updated since [2] with the following
> changes:
> 
> - Document the optional VIDIOC_ENUM_FRAMEINTERVALS ioctl.
> 
> - Document how to use VIDIOC_S_PARM:
> 
>   1) calling S_PARM for the OUTPUT queue sets both the raw frame interval
>      (this is a hint only) and the coded frame interval.
> 
>   2) calling S_PARM for the CAPTURE queue sets the coded frame interval
>      only. This is optional and can be used for off-line encoding. In
>      that case the OUTPUT frame interval can be used by the driver to
>      schedule multiple encoders.
>  
>   Ideally S_PARM for the OUTPUT queue would just provide a hint, but
>   existing encoder drivers all use S_PARM for the OUTPUT queue to
>   define the coded frame interval, and that can't be changed.
> 
> - Added a note that if a capture buffer is too small it will be
>   returned with V4L2_BUF_FLAG_ERROR and that more work has to be
>   done to properly support this corner case.
> 
> - Clarify in the 'Encoding' section that there are more reasons
>   why 'a buffer queued to OUTPUT may result in more than one buffer
>   produced on CAPTURE'.
> 
> Added in v3:
> 
> - Fix some minor typos.
> 
> - Make it more explicit that setting S_PARM(OUTPUT) also sets the
>   CAPTURE frame interval.
> 
> - Added a new V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag to indicate
>   that S_PARM(CAPTURE) can be set separately.
> 
> I think that with these changes this stateful encoder spec is ready
> to be merged.
> 
> Regards,
> 
> 	Hans
> 
> [1] https://lore.kernel.org/linux-media/20191119113457.57833-6-hverkuil-cisco@xs4all.nl/T/
> [2] https://www.mail-archive.com/linux-media@vger.kernel.org/msg149211.html
> [3] https://linuxtv.org/irc/irclogger_log/v4l?date=2020-05-19,Tue
> 
> Hans Verkuil (4):
>   vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation
>   dev-decoder.rst: small fixes
>   videodev2.h: add V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag
>   dev-encoder.rst: add reference to V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL
> 
> Tomasz Figa (1):
>   media: docs-rst: Document memory-to-memory video encoder interface
> 
>  .../userspace-api/media/v4l/dev-decoder.rst   |   6 +-
>  .../userspace-api/media/v4l/dev-encoder.rst   | 729 ++++++++++++++++++
>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
>  .../userspace-api/media/v4l/v4l2.rst          |   2 +
>  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
>  .../media/v4l/vidioc-enum-fmt.rst             |  30 +-
>  .../userspace-api/media/v4l/vidioc-g-parm.rst |  51 +-
>  .../media/videodev2.h.rst.exceptions          |   1 +
>  include/uapi/linux/videodev2.h                |   1 +
>  10 files changed, 830 insertions(+), 47 deletions(-)
>  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> 
> -- 
> 2.25.1
> 
> 

-- 
Pengutronix e.K.                           | Michael Tretter             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
@ 2020-05-29  9:57   ` Stanimir Varbanov
  2020-05-29 10:17     ` Stanimir Varbanov
  2020-05-29 12:21     ` Tomasz Figa
  2020-06-05  7:19   ` Stanimir Varbanov
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 23+ messages in thread
From: Stanimir Varbanov @ 2020-05-29  9:57 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter



On 5/26/20 1:09 PM, Hans Verkuil wrote:
> From: Tomasz Figa <tfiga@chromium.org>
> 
> Due to complexity of the video encoding process, the V4L2 drivers of
> stateful encoder hardware require specific sequences of V4L2 API calls
> to be followed. These include capability enumeration, initialization,
> encoding, encode parameters change, drain and reset.
> 
> Specifics of the above have been discussed during Media Workshops at
> LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> originated at those events was later implemented by the drivers we already
> have merged in mainline, such as s5p-mfc or coda.
> 
> The only thing missing was the real specification included as a part of
> Linux Media documentation. Fix it now and document the encoder part of
> the Codec API.
> 
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
>  .../userspace-api/media/v4l/v4l2.rst          |   2 +
>  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
>  5 files changed, 767 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> 

<cut>

> +
> +Reset
> +=====
> +
> +The client may want to request the encoder to reinitialize the encoding, so
> +that the following stream data becomes independent from the stream data
> +generated before. Depending on the coded format, that may imply that:
> +
> +* encoded frames produced after the restart must not reference any frames
> +  produced before the stop, e.g. no long term references for H.264/HEVC,
> +
> +* any headers that must be included in a standalone stream must be produced
> +  again, e.g. SPS and PPS for H.264/HEVC.
> +
> +This can be achieved by performing the reset sequence.
> +
> +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> +   and respective buffers are dequeued.
> +
> +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> +   will return all currently queued ``CAPTURE`` buffers to the client, without
> +   valid frame data.
> +
> +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> +   continue with regular encoding sequence. The encoded frames produced into
> +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> +   decoded without the need for frames encoded before the reset sequence,
> +   starting at the first ``OUTPUT`` buffer queued after issuing the
> +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> +
> +This sequence may be also used to change encoding parameters for encoders
> +without the ability to change the parameters on the fly.

How the v4l2 client knows which parameters could be changed on the fly
and which cannot? Controls should return -EBUSY?

Also could that Reset be used to change the pixel format and probably
colorspace?

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-05-29  9:57   ` Stanimir Varbanov
@ 2020-05-29 10:17     ` Stanimir Varbanov
  2020-05-29 12:21     ` Tomasz Figa
  1 sibling, 0 replies; 23+ messages in thread
From: Stanimir Varbanov @ 2020-05-29 10:17 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter



On 5/29/20 12:57 PM, Stanimir Varbanov wrote:
> 
> 
> On 5/26/20 1:09 PM, Hans Verkuil wrote:
>> From: Tomasz Figa <tfiga@chromium.org>
>>
>> Due to complexity of the video encoding process, the V4L2 drivers of
>> stateful encoder hardware require specific sequences of V4L2 API calls
>> to be followed. These include capability enumeration, initialization,
>> encoding, encode parameters change, drain and reset.
>>
>> Specifics of the above have been discussed during Media Workshops at
>> LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
>> Conference Europe 2014 in Düsseldorf. The de facto Codec API that
>> originated at those events was later implemented by the drivers we already
>> have merged in mainline, such as s5p-mfc or coda.
>>
>> The only thing missing was the real specification included as a part of
>> Linux Media documentation. Fix it now and document the encoder part of
>> the Codec API.
>>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
>>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
>>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
>>  .../userspace-api/media/v4l/v4l2.rst          |   2 +
>>  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
>>  5 files changed, 767 insertions(+), 20 deletions(-)
>>  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
>>
> 
> <cut>
> 
>> +
>> +Reset
>> +=====
>> +
>> +The client may want to request the encoder to reinitialize the encoding, so
>> +that the following stream data becomes independent from the stream data
>> +generated before. Depending on the coded format, that may imply that:
>> +
>> +* encoded frames produced after the restart must not reference any frames
>> +  produced before the stop, e.g. no long term references for H.264/HEVC,
>> +
>> +* any headers that must be included in a standalone stream must be produced
>> +  again, e.g. SPS and PPS for H.264/HEVC.
>> +
>> +This can be achieved by performing the reset sequence.
>> +
>> +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
>> +   and respective buffers are dequeued.
>> +
>> +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
>> +   will return all currently queued ``CAPTURE`` buffers to the client, without
>> +   valid frame data.
>> +
>> +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
>> +   continue with regular encoding sequence. The encoded frames produced into
>> +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
>> +   decoded without the need for frames encoded before the reset sequence,
>> +   starting at the first ``OUTPUT`` buffer queued after issuing the
>> +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
>> +
>> +This sequence may be also used to change encoding parameters for encoders
>> +without the ability to change the parameters on the fly.
> 
> How the v4l2 client knows which parameters could be changed on the fly
> and which cannot? Controls should return -EBUSY?

Sorry, I re-read "Encoding Parameter Changes" paragraph.

> 
> Also could that Reset be used to change the pixel format and probably
> colorspace?
> 

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-05-29  9:57   ` Stanimir Varbanov
  2020-05-29 10:17     ` Stanimir Varbanov
@ 2020-05-29 12:21     ` Tomasz Figa
  1 sibling, 0 replies; 23+ messages in thread
From: Tomasz Figa @ 2020-05-29 12:21 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Hans Verkuil, Linux Media Mailing List, Nicolas Dufresne,
	Michael Tretter

On Fri, May 29, 2020 at 11:57 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
>
>
> On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > From: Tomasz Figa <tfiga@chromium.org>
> >
> > Due to complexity of the video encoding process, the V4L2 drivers of
> > stateful encoder hardware require specific sequences of V4L2 API calls
> > to be followed. These include capability enumeration, initialization,
> > encoding, encode parameters change, drain and reset.
> >
> > Specifics of the above have been discussed during Media Workshops at
> > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > originated at those events was later implemented by the drivers we already
> > have merged in mainline, such as s5p-mfc or coda.
> >
> > The only thing missing was the real specification included as a part of
> > Linux Media documentation. Fix it now and document the encoder part of
> > the Codec API.
> >
> > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> >  5 files changed, 767 insertions(+), 20 deletions(-)
> >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> >
>
> <cut>
>
> > +
> > +Reset
> > +=====
> > +
> > +The client may want to request the encoder to reinitialize the encoding, so
> > +that the following stream data becomes independent from the stream data
> > +generated before. Depending on the coded format, that may imply that:
> > +
> > +* encoded frames produced after the restart must not reference any frames
> > +  produced before the stop, e.g. no long term references for H.264/HEVC,
> > +
> > +* any headers that must be included in a standalone stream must be produced
> > +  again, e.g. SPS and PPS for H.264/HEVC.
> > +
> > +This can be achieved by performing the reset sequence.
> > +
> > +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> > +   and respective buffers are dequeued.
> > +
> > +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> > +   will return all currently queued ``CAPTURE`` buffers to the client, without
> > +   valid frame data.
> > +
> > +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> > +   continue with regular encoding sequence. The encoded frames produced into
> > +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> > +   decoded without the need for frames encoded before the reset sequence,
> > +   starting at the first ``OUTPUT`` buffer queued after issuing the
> > +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> > +
> > +This sequence may be also used to change encoding parameters for encoders
> > +without the ability to change the parameters on the fly.
>
> How the v4l2 client knows which parameters could be changed on the fly
> and which cannot? Controls should return -EBUSY?

Yes, but I guess you found the answer already. :) I guess we could add
a reference to the "Encoding Parameter Changes" section.

>
> Also could that Reset be used to change the pixel format and probably
> colorspace?

I guess it would indeed have to be used for any changes that would
affect the parameters of the encoded stream, e.g. coded resolution or
coded colorspace.

For any changes which wouldn't affect the encoded stream, e.g. a
source format change with the same crop size or a source colorspace
change while keeping the coded colorspace the same (i.e. hardware
doing the conversion), the OUTPUT queue could be reconfigured on its
own without the need to do anything to the CAPTURE queue.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 0/5] Stateful Encoding: final bits
  2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
                   ` (5 preceding siblings ...)
  2020-05-28  7:58 ` [PATCHv3 0/5] Stateful Encoding: final bits Michael Tretter
@ 2020-06-02  9:01 ` Tomasz Figa
  6 siblings, 0 replies; 23+ messages in thread
From: Tomasz Figa @ 2020-06-02  9:01 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List, Nicolas Dufresne, Michael Tretter

On Tue, May 26, 2020 at 12:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> This series adds the encoder spec and updates the VIDIOC_G/S_PARM
> documentation.
>
> This is a follow-up of the original "Stateful Encoding: final bits"
> series (1).
>
> The patches in that series that add support for V4L2_CID_MPEG_VIDEO_ENC_FRAME_RATE
> and V4L2_BUF_FLAG_TOO_SMALL have been dropped (the first is not necessary
> and the second can be skipped for now, see the irc discussion with
> Nicolas [3]).
>
> The encoder spec has been updated since [2] with the following
> changes:
>
> - Document the optional VIDIOC_ENUM_FRAMEINTERVALS ioctl.
>
> - Document how to use VIDIOC_S_PARM:
>
>   1) calling S_PARM for the OUTPUT queue sets both the raw frame interval
>      (this is a hint only) and the coded frame interval.
>
>   2) calling S_PARM for the CAPTURE queue sets the coded frame interval
>      only. This is optional and can be used for off-line encoding. In
>      that case the OUTPUT frame interval can be used by the driver to
>      schedule multiple encoders.
>
>   Ideally S_PARM for the OUTPUT queue would just provide a hint, but
>   existing encoder drivers all use S_PARM for the OUTPUT queue to
>   define the coded frame interval, and that can't be changed.
>
> - Added a note that if a capture buffer is too small it will be
>   returned with V4L2_BUF_FLAG_ERROR and that more work has to be
>   done to properly support this corner case.
>
> - Clarify in the 'Encoding' section that there are more reasons
>   why 'a buffer queued to OUTPUT may result in more than one buffer
>   produced on CAPTURE'.
>
> Added in v3:
>
> - Fix some minor typos.
>
> - Make it more explicit that setting S_PARM(OUTPUT) also sets the
>   CAPTURE frame interval.
>
> - Added a new V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag to indicate
>   that S_PARM(CAPTURE) can be set separately.
>
> I think that with these changes this stateful encoder spec is ready
> to be merged.

Acked-by: Tomasz Figa <tfiga@chromium.org>

Thanks Hans and everyone for the help with moving this forward.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
  2020-05-29  9:57   ` Stanimir Varbanov
@ 2020-06-05  7:19   ` Stanimir Varbanov
  2020-06-05 21:24     ` Nicolas Dufresne
  2020-06-23 10:36   ` Hans Verkuil
  2020-06-23 10:47   ` Mauro Carvalho Chehab
  3 siblings, 1 reply; 23+ messages in thread
From: Stanimir Varbanov @ 2020-06-05  7:19 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter



On 5/26/20 1:09 PM, Hans Verkuil wrote:
> From: Tomasz Figa <tfiga@chromium.org>
> 
> Due to complexity of the video encoding process, the V4L2 drivers of
> stateful encoder hardware require specific sequences of V4L2 API calls
> to be followed. These include capability enumeration, initialization,
> encoding, encode parameters change, drain and reset.
> 
> Specifics of the above have been discussed during Media Workshops at
> LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> originated at those events was later implemented by the drivers we already
> have merged in mainline, such as s5p-mfc or coda.
> 
> The only thing missing was the real specification included as a part of
> Linux Media documentation. Fix it now and document the encoder part of
> the Codec API.
> 
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
>  .../userspace-api/media/v4l/v4l2.rst          |   2 +
>  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
>  5 files changed, 767 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> 
> diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> new file mode 100644
> index 000000000000..aace7b812a9c
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> @@ -0,0 +1,728 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _encoder:
> +
> +*************************************************
> +Memory-to-Memory Stateful Video Encoder Interface
> +*************************************************
> +
> +A stateful video encoder takes raw video frames in display order and encodes
> +them into a bytestream. It generates complete chunks of the bytestream, including
> +all metadata, headers, etc. The resulting bytestream does not require any
> +further post-processing by the client.
> +
> +Performing software stream processing, header generation etc. in the driver
> +in order to support this interface is strongly discouraged. In case such
> +operations are needed, use of the Stateless Video Encoder Interface (in
> +development) is strongly advised.
> +

<cut>

> +Encoding Parameter Changes
> +==========================
> +
> +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> +parameters at any time. The availability of parameters is encoder-specific
> +and the client must query the encoder to find the set of available controls.
> +
> +The ability to change each parameter during encoding is encoder-specific, as
> +per the standard semantics of the V4L2 control interface. The client may
> +attempt to set a control during encoding and if the operation fails with the
> +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> +configuration change to be allowed. To do this, it may follow the `Drain`
> +sequence to avoid losing the already queued/encoded frames.
> +
> +The timing of parameter updates is encoder-specific, as per the standard
> +semantics of the V4L2 control interface. If the client needs to apply the
> +parameters exactly at specific frame, using the Request API
> +(:ref:`media-request-api`) should be considered, if supported by the encoder.

In request-api case does the control will return EBUSY immediately when
S_CTRL is called from the client? I'm asking in the context of the
controls which are not dynamic (cannot set during streaming and Reset
sequence is needed).

> +
> +Drain
> +=====
> +
> +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> +related ``CAPTURE`` buffers are given to the client, the client must follow the
> +drain sequence described below. After the drain sequence ends, the client has
> +received all encoded frames for all ``OUTPUT`` buffers queued before the
> +sequence was started.
> +
> +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> +
> +   * **Required fields:**
> +
> +     ``cmd``
> +         set to ``V4L2_ENC_CMD_STOP``.
> +
> +     ``flags``
> +         set to 0.
> +
> +     ``pts``
> +         set to 0.
> +
> +   .. warning::
> +
> +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> +      queues are streaming. For compatibility reasons, the call to
> +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> +      not streaming, but at the same time it will not initiate the `Drain`
> +      sequence and so the steps described below would not be applicable.
> +
> +2. Any ``OUTPUT`` buffers queued by the client before the
> +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> +   normal. The client must continue to handle both queues independently,
> +   similarly to normal encode operation. This includes:
> +
> +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> +
> +     .. warning::
> +
> +        The last buffer may be empty (with :c:type:`v4l2_buffer`
> +        ``bytesused`` = 0) and in that case it must be ignored by the client,
> +        as it does not contain an encoded frame.
> +
> +     .. note::
> +
> +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> +        :c:func:`VIDIOC_DQBUF`.
> +
> +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> +
> +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> +
> +   .. note::
> +
> +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> +      event when the last frame has been encoded and all frames are ready to be
> +      dequeued. It is deprecated behavior and the client must not rely on it.
> +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> +
> +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
> +   until the client issues any of the following operations:
> +
> +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> +     operation normally, with all the state from before the drain,
> +
> +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> +     and then resume encoding,
> +
> +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
> +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> +
> +.. note::
> +
> +   Once the drain sequence is initiated, the client needs to drive it to
> +   completion, as described by the steps above, unless it aborts the process by
> +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> +   will fail with -EBUSY error code if attempted.
> +
> +   For reference, handling of various corner cases is described below:
> +
> +   * In case of no buffer in the ``OUTPUT`` queue at the time the
> +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> +     ``V4L2_BUF_FLAG_LAST`` flag set.
> +
> +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
> +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> +     flag set.
> +
> +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> +     middle of the drain sequence, the drain sequence is canceled and all
> +     ``CAPTURE`` buffers are implicitly returned to the client.
> +
> +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> +     middle of the drain sequence, the drain sequence completes immediately and
> +     next ``CAPTURE`` buffer will be returned empty with the
> +     ``V4L2_BUF_FLAG_LAST`` flag set.
> +
> +   Although not mandatory, the availability of encoder commands may be queried
> +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> +
> +Reset
> +=====
> +
> +The client may want to request the encoder to reinitialize the encoding, so
> +that the following stream data becomes independent from the stream data
> +generated before. Depending on the coded format, that may imply that:
> +
> +* encoded frames produced after the restart must not reference any frames
> +  produced before the stop, e.g. no long term references for H.264/HEVC,
> +
> +* any headers that must be included in a standalone stream must be produced
> +  again, e.g. SPS and PPS for H.264/HEVC.

It seems that clients needs to know SPS/PPS (for muxers?) and thus they
will need to extract or parse the encoder output buffers to find them.
Maybe the spec should consider adding some buffer flag to mark such
buffers which contain SPS/PPS only.

[1] - see at "type AvcCBox struct"

Nicolas, how the gstreamer handle this?

> +
> +This can be achieved by performing the reset sequence.
> +
> +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> +   and respective buffers are dequeued.
> +
> +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> +   will return all currently queued ``CAPTURE`` buffers to the client, without
> +   valid frame data.
> +
> +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> +   continue with regular encoding sequence. The encoded frames produced into
> +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> +   decoded without the need for frames encoded before the reset sequence,
> +   starting at the first ``OUTPUT`` buffer queued after issuing the
> +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> +
> +This sequence may be also used to change encoding parameters for encoders
> +without the ability to change the parameters on the fly.
> +

<cut>

-- 
regards,
Stan

[1]
https://pkg.go.dev/github.com/Afrostream/afrostream-media-server/src/mp4?tab=doc

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-05  7:19   ` Stanimir Varbanov
@ 2020-06-05 21:24     ` Nicolas Dufresne
  2020-06-06  7:58       ` Stanimir Varbanov
  2020-06-10 12:28       ` Tomasz Figa
  0 siblings, 2 replies; 23+ messages in thread
From: Nicolas Dufresne @ 2020-06-05 21:24 UTC (permalink / raw)
  To: Stanimir Varbanov, Hans Verkuil, linux-media; +Cc: Tomasz Figa, Michael Tretter

Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
> 
> On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > From: Tomasz Figa <tfiga@chromium.org>
> > 
> > Due to complexity of the video encoding process, the V4L2 drivers of
> > stateful encoder hardware require specific sequences of V4L2 API calls
> > to be followed. These include capability enumeration, initialization,
> > encoding, encode parameters change, drain and reset.
> > 
> > Specifics of the above have been discussed during Media Workshops at
> > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > originated at those events was later implemented by the drivers we already
> > have merged in mainline, such as s5p-mfc or coda.
> > 
> > The only thing missing was the real specification included as a part of
> > Linux Media documentation. Fix it now and document the encoder part of
> > the Codec API.
> > 
> > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> >  5 files changed, 767 insertions(+), 20 deletions(-)
> >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > new file mode 100644
> > index 000000000000..aace7b812a9c
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > @@ -0,0 +1,728 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +.. _encoder:
> > +
> > +*************************************************
> > +Memory-to-Memory Stateful Video Encoder Interface
> > +*************************************************
> > +
> > +A stateful video encoder takes raw video frames in display order and encodes
> > +them into a bytestream. It generates complete chunks of the bytestream, including
> > +all metadata, headers, etc. The resulting bytestream does not require any
> > +further post-processing by the client.
> > +
> > +Performing software stream processing, header generation etc. in the driver
> > +in order to support this interface is strongly discouraged. In case such
> > +operations are needed, use of the Stateless Video Encoder Interface (in
> > +development) is strongly advised.
> > +
> 
> <cut>
> 
> > +Encoding Parameter Changes
> > +==========================
> > +
> > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > +parameters at any time. The availability of parameters is encoder-specific
> > +and the client must query the encoder to find the set of available controls.
> > +
> > +The ability to change each parameter during encoding is encoder-specific, as
> > +per the standard semantics of the V4L2 control interface. The client may
> > +attempt to set a control during encoding and if the operation fails with the
> > +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> > +configuration change to be allowed. To do this, it may follow the `Drain`
> > +sequence to avoid losing the already queued/encoded frames.
> > +
> > +The timing of parameter updates is encoder-specific, as per the standard
> > +semantics of the V4L2 control interface. If the client needs to apply the
> > +parameters exactly at specific frame, using the Request API
> > +(:ref:`media-request-api`) should be considered, if supported by the encoder.
> 
> In request-api case does the control will return EBUSY immediately when
> S_CTRL is called from the client? I'm asking in the context of the
> controls which are not dynamic (cannot set during streaming and Reset
> sequence is needed).

This is all hypothetical, as nothing of that has been implemented. But
I suppose there should be instant validation to allow that, even if
that means more plumbing inside the kernel. It would be better then
just running the request ignoring silently that control. Ideally
userspace should not have to go into trial and errors, so controls that
are means for the should be marked.

> 
> > +
> > +Drain
> > +=====
> > +
> > +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> > +related ``CAPTURE`` buffers are given to the client, the client must follow the
> > +drain sequence described below. After the drain sequence ends, the client has
> > +received all encoded frames for all ``OUTPUT`` buffers queued before the
> > +sequence was started.
> > +
> > +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> > +
> > +   * **Required fields:**
> > +
> > +     ``cmd``
> > +         set to ``V4L2_ENC_CMD_STOP``.
> > +
> > +     ``flags``
> > +         set to 0.
> > +
> > +     ``pts``
> > +         set to 0.
> > +
> > +   .. warning::
> > +
> > +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> > +      queues are streaming. For compatibility reasons, the call to
> > +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> > +      not streaming, but at the same time it will not initiate the `Drain`
> > +      sequence and so the steps described below would not be applicable.
> > +
> > +2. Any ``OUTPUT`` buffers queued by the client before the
> > +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> > +   normal. The client must continue to handle both queues independently,
> > +   similarly to normal encode operation. This includes:
> > +
> > +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> > +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> > +
> > +     .. warning::
> > +
> > +        The last buffer may be empty (with :c:type:`v4l2_buffer`
> > +        ``bytesused`` = 0) and in that case it must be ignored by the client,
> > +        as it does not contain an encoded frame.
> > +
> > +     .. note::
> > +
> > +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> > +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> > +        :c:func:`VIDIOC_DQBUF`.
> > +
> > +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> > +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> > +
> > +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> > +
> > +   .. note::
> > +
> > +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> > +      event when the last frame has been encoded and all frames are ready to be
> > +      dequeued. It is deprecated behavior and the client must not rely on it.
> > +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> > +
> > +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> > +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> > +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
> > +   until the client issues any of the following operations:
> > +
> > +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> > +     operation normally, with all the state from before the drain,
> > +
> > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> > +     and then resume encoding,
> > +
> > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
> > +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> > +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> > +
> > +.. note::
> > +
> > +   Once the drain sequence is initiated, the client needs to drive it to
> > +   completion, as described by the steps above, unless it aborts the process by
> > +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> > +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> > +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> > +   will fail with -EBUSY error code if attempted.
> > +
> > +   For reference, handling of various corner cases is described below:
> > +
> > +   * In case of no buffer in the ``OUTPUT`` queue at the time the
> > +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> > +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > +
> > +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> > +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
> > +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> > +     flag set.
> > +
> > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> > +     middle of the drain sequence, the drain sequence is canceled and all
> > +     ``CAPTURE`` buffers are implicitly returned to the client.
> > +
> > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> > +     middle of the drain sequence, the drain sequence completes immediately and
> > +     next ``CAPTURE`` buffer will be returned empty with the
> > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > +
> > +   Although not mandatory, the availability of encoder commands may be queried
> > +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> > +
> > +Reset
> > +=====
> > +
> > +The client may want to request the encoder to reinitialize the encoding, so
> > +that the following stream data becomes independent from the stream data
> > +generated before. Depending on the coded format, that may imply that:
> > +
> > +* encoded frames produced after the restart must not reference any frames
> > +  produced before the stop, e.g. no long term references for H.264/HEVC,
> > +
> > +* any headers that must be included in a standalone stream must be produced
> > +  again, e.g. SPS and PPS for H.264/HEVC.
> 
> It seems that clients needs to know SPS/PPS (for muxers?) and thus they
> will need to extract or parse the encoder output buffers to find them.
> Maybe the spec should consider adding some buffer flag to mark such
> buffers which contain SPS/PPS only.
> 
> [1] - see at "type AvcCBox struct"
> 
> Nicolas, how the gstreamer handle this?
> 
> > +
> > +This can be achieved by performing the reset sequence.
> > +
> > +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> > +   and respective buffers are dequeued.
> > +
> > +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> > +   will return all currently queued ``CAPTURE`` buffers to the client, without
> > +   valid frame data.
> > +
> > +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> > +   continue with regular encoding sequence. The encoded frames produced into
> > +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> > +   decoded without the need for frames encoded before the reset sequence,
> > +   starting at the first ``OUTPUT`` buffer queued after issuing the
> > +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> > +
> > +This sequence may be also used to change encoding parameters for encoders
> > +without the ability to change the parameters on the fly.
> > +
> 
> <cut>
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-05 21:24     ` Nicolas Dufresne
@ 2020-06-06  7:58       ` Stanimir Varbanov
  2020-06-07  0:27         ` Nicolas Dufresne
  2020-06-10 12:28       ` Tomasz Figa
  1 sibling, 1 reply; 23+ messages in thread
From: Stanimir Varbanov @ 2020-06-06  7:58 UTC (permalink / raw)
  To: Nicolas Dufresne, Hans Verkuil, linux-media; +Cc: Tomasz Figa, Michael Tretter

Hi Nicolas,

On 6/6/20 12:24 AM, Nicolas Dufresne wrote:
> Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
>>
>> On 5/26/20 1:09 PM, Hans Verkuil wrote:
>>> From: Tomasz Figa <tfiga@chromium.org>
>>>
>>> Due to complexity of the video encoding process, the V4L2 drivers of
>>> stateful encoder hardware require specific sequences of V4L2 API calls
>>> to be followed. These include capability enumeration, initialization,
>>> encoding, encode parameters change, drain and reset.
>>>
>>> Specifics of the above have been discussed during Media Workshops at
>>> LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
>>> Conference Europe 2014 in Düsseldorf. The de facto Codec API that
>>> originated at those events was later implemented by the drivers we already
>>> have merged in mainline, such as s5p-mfc or coda.
>>>
>>> The only thing missing was the real specification included as a part of
>>> Linux Media documentation. Fix it now and document the encoder part of
>>> the Codec API.
>>>
>>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>> ---
>>>  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
>>>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
>>>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
>>>  .../userspace-api/media/v4l/v4l2.rst          |   2 +
>>>  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
>>>  5 files changed, 767 insertions(+), 20 deletions(-)
>>>  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
>>>
>>> diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
>>> new file mode 100644
>>> index 000000000000..aace7b812a9c
>>> --- /dev/null
>>> +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
>>> @@ -0,0 +1,728 @@
>>> +.. SPDX-License-Identifier: GPL-2.0
>>> +
>>> +.. _encoder:
>>> +
>>> +*************************************************
>>> +Memory-to-Memory Stateful Video Encoder Interface
>>> +*************************************************
>>> +
>>> +A stateful video encoder takes raw video frames in display order and encodes
>>> +them into a bytestream. It generates complete chunks of the bytestream, including
>>> +all metadata, headers, etc. The resulting bytestream does not require any
>>> +further post-processing by the client.
>>> +
>>> +Performing software stream processing, header generation etc. in the driver
>>> +in order to support this interface is strongly discouraged. In case such
>>> +operations are needed, use of the Stateless Video Encoder Interface (in
>>> +development) is strongly advised.
>>> +
>>
>> <cut>
>>
>>> +Encoding Parameter Changes
>>> +==========================
>>> +
>>> +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
>>> +parameters at any time. The availability of parameters is encoder-specific
>>> +and the client must query the encoder to find the set of available controls.
>>> +
>>> +The ability to change each parameter during encoding is encoder-specific, as
>>> +per the standard semantics of the V4L2 control interface. The client may
>>> +attempt to set a control during encoding and if the operation fails with the
>>> +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
>>> +configuration change to be allowed. To do this, it may follow the `Drain`
>>> +sequence to avoid losing the already queued/encoded frames.
>>> +
>>> +The timing of parameter updates is encoder-specific, as per the standard
>>> +semantics of the V4L2 control interface. If the client needs to apply the
>>> +parameters exactly at specific frame, using the Request API
>>> +(:ref:`media-request-api`) should be considered, if supported by the encoder.
>>
>> In request-api case does the control will return EBUSY immediately when
>> S_CTRL is called from the client? I'm asking in the context of the
>> controls which are not dynamic (cannot set during streaming and Reset
>> sequence is needed).
> 
> This is all hypothetical, as nothing of that has been implemented. But
> I suppose there should be instant validation to allow that, even if
> that means more plumbing inside the kernel. It would be better then
> just running the request ignoring silently that control. Ideally
> userspace should not have to go into trial and errors, so controls that
> are means for the should be marked.

Meanwhile I looked into the controls code and found that at least
.try_ctrl will be called.

Do you have something to add in below subject for SPS/PPS?


> 
>>
>>> +
>>> +Drain
>>> +=====
>>> +
>>> +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
>>> +related ``CAPTURE`` buffers are given to the client, the client must follow the
>>> +drain sequence described below. After the drain sequence ends, the client has
>>> +received all encoded frames for all ``OUTPUT`` buffers queued before the
>>> +sequence was started.
>>> +
>>> +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
>>> +
>>> +   * **Required fields:**
>>> +
>>> +     ``cmd``
>>> +         set to ``V4L2_ENC_CMD_STOP``.
>>> +
>>> +     ``flags``
>>> +         set to 0.
>>> +
>>> +     ``pts``
>>> +         set to 0.
>>> +
>>> +   .. warning::
>>> +
>>> +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
>>> +      queues are streaming. For compatibility reasons, the call to
>>> +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
>>> +      not streaming, but at the same time it will not initiate the `Drain`
>>> +      sequence and so the steps described below would not be applicable.
>>> +
>>> +2. Any ``OUTPUT`` buffers queued by the client before the
>>> +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
>>> +   normal. The client must continue to handle both queues independently,
>>> +   similarly to normal encode operation. This includes:
>>> +
>>> +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
>>> +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
>>> +
>>> +     .. warning::
>>> +
>>> +        The last buffer may be empty (with :c:type:`v4l2_buffer`
>>> +        ``bytesused`` = 0) and in that case it must be ignored by the client,
>>> +        as it does not contain an encoded frame.
>>> +
>>> +     .. note::
>>> +
>>> +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
>>> +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
>>> +        :c:func:`VIDIOC_DQBUF`.
>>> +
>>> +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
>>> +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
>>> +
>>> +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
>>> +
>>> +   .. note::
>>> +
>>> +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
>>> +      event when the last frame has been encoded and all frames are ready to be
>>> +      dequeued. It is deprecated behavior and the client must not rely on it.
>>> +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
>>> +
>>> +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
>>> +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
>>> +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
>>> +   until the client issues any of the following operations:
>>> +
>>> +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
>>> +     operation normally, with all the state from before the drain,
>>> +
>>> +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
>>> +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
>>> +     and then resume encoding,
>>> +
>>> +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
>>> +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
>>> +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
>>> +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
>>> +
>>> +.. note::
>>> +
>>> +   Once the drain sequence is initiated, the client needs to drive it to
>>> +   completion, as described by the steps above, unless it aborts the process by
>>> +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
>>> +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
>>> +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
>>> +   will fail with -EBUSY error code if attempted.
>>> +
>>> +   For reference, handling of various corner cases is described below:
>>> +
>>> +   * In case of no buffer in the ``OUTPUT`` queue at the time the
>>> +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
>>> +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
>>> +     ``V4L2_BUF_FLAG_LAST`` flag set.
>>> +
>>> +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
>>> +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
>>> +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
>>> +     flag set.
>>> +
>>> +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
>>> +     middle of the drain sequence, the drain sequence is canceled and all
>>> +     ``CAPTURE`` buffers are implicitly returned to the client.
>>> +
>>> +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
>>> +     middle of the drain sequence, the drain sequence completes immediately and
>>> +     next ``CAPTURE`` buffer will be returned empty with the
>>> +     ``V4L2_BUF_FLAG_LAST`` flag set.
>>> +
>>> +   Although not mandatory, the availability of encoder commands may be queried
>>> +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
>>> +
>>> +Reset
>>> +=====
>>> +
>>> +The client may want to request the encoder to reinitialize the encoding, so
>>> +that the following stream data becomes independent from the stream data
>>> +generated before. Depending on the coded format, that may imply that:
>>> +
>>> +* encoded frames produced after the restart must not reference any frames
>>> +  produced before the stop, e.g. no long term references for H.264/HEVC,
>>> +
>>> +* any headers that must be included in a standalone stream must be produced
>>> +  again, e.g. SPS and PPS for H.264/HEVC.
>>
>> It seems that clients needs to know SPS/PPS (for muxers?) and thus they
>> will need to extract or parse the encoder output buffers to find them.
>> Maybe the spec should consider adding some buffer flag to mark such
>> buffers which contain SPS/PPS only.
>>
>> [1] - see at "type AvcCBox struct"
>>
>> Nicolas, how the gstreamer handle this?
>>
>>> +
>>> +This can be achieved by performing the reset sequence.
>>> +
>>> +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
>>> +   and respective buffers are dequeued.
>>> +
>>> +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
>>> +   will return all currently queued ``CAPTURE`` buffers to the client, without
>>> +   valid frame data.
>>> +
>>> +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
>>> +   continue with regular encoding sequence. The encoded frames produced into
>>> +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
>>> +   decoded without the need for frames encoded before the reset sequence,
>>> +   starting at the first ``OUTPUT`` buffer queued after issuing the
>>> +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
>>> +
>>> +This sequence may be also used to change encoding parameters for encoders
>>> +without the ability to change the parameters on the fly.
>>> +
>>
>> <cut>
>>
> 

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-06  7:58       ` Stanimir Varbanov
@ 2020-06-07  0:27         ` Nicolas Dufresne
  0 siblings, 0 replies; 23+ messages in thread
From: Nicolas Dufresne @ 2020-06-07  0:27 UTC (permalink / raw)
  To: Stanimir Varbanov, Hans Verkuil, linux-media; +Cc: Tomasz Figa, Michael Tretter

Le samedi 06 juin 2020 à 10:58 +0300, Stanimir Varbanov a écrit :
> Hi Nicolas,
> 
> On 6/6/20 12:24 AM, Nicolas Dufresne wrote:
> > Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
> > > On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > > > From: Tomasz Figa <tfiga@chromium.org>
> > > > 
> > > > Due to complexity of the video encoding process, the V4L2 drivers of
> > > > stateful encoder hardware require specific sequences of V4L2 API calls
> > > > to be followed. These include capability enumeration, initialization,
> > > > encoding, encode parameters change, drain and reset.
> > > > 
> > > > Specifics of the above have been discussed during Media Workshops at
> > > > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > > > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > > > originated at those events was later implemented by the drivers we already
> > > > have merged in mainline, such as s5p-mfc or coda.
> > > > 
> > > > The only thing missing was the real specification included as a part of
> > > > Linux Media documentation. Fix it now and document the encoder part of
> > > > the Codec API.
> > > > 
> > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > > ---
> > > >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> > > >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> > > >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> > > >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> > > >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> > > >  5 files changed, 767 insertions(+), 20 deletions(-)
> > > >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > 
> > > > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > new file mode 100644
> > > > index 000000000000..aace7b812a9c
> > > > --- /dev/null
> > > > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > @@ -0,0 +1,728 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +.. _encoder:
> > > > +
> > > > +*************************************************
> > > > +Memory-to-Memory Stateful Video Encoder Interface
> > > > +*************************************************
> > > > +
> > > > +A stateful video encoder takes raw video frames in display order and encodes
> > > > +them into a bytestream. It generates complete chunks of the bytestream, including
> > > > +all metadata, headers, etc. The resulting bytestream does not require any
> > > > +further post-processing by the client.
> > > > +
> > > > +Performing software stream processing, header generation etc. in the driver
> > > > +in order to support this interface is strongly discouraged. In case such
> > > > +operations are needed, use of the Stateless Video Encoder Interface (in
> > > > +development) is strongly advised.
> > > > +
> > > 
> > > <cut>
> > > 
> > > > +Encoding Parameter Changes
> > > > +==========================
> > > > +
> > > > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > > > +parameters at any time. The availability of parameters is encoder-specific
> > > > +and the client must query the encoder to find the set of available controls.
> > > > +
> > > > +The ability to change each parameter during encoding is encoder-specific, as
> > > > +per the standard semantics of the V4L2 control interface. The client may
> > > > +attempt to set a control during encoding and if the operation fails with the
> > > > +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> > > > +configuration change to be allowed. To do this, it may follow the `Drain`
> > > > +sequence to avoid losing the already queued/encoded frames.
> > > > +
> > > > +The timing of parameter updates is encoder-specific, as per the standard
> > > > +semantics of the V4L2 control interface. If the client needs to apply the
> > > > +parameters exactly at specific frame, using the Request API
> > > > +(:ref:`media-request-api`) should be considered, if supported by the encoder.
> > > 
> > > In request-api case does the control will return EBUSY immediately when
> > > S_CTRL is called from the client? I'm asking in the context of the
> > > controls which are not dynamic (cannot set during streaming and Reset
> > > sequence is needed).
> > 
> > This is all hypothetical, as nothing of that has been implemented. But
> > I suppose there should be instant validation to allow that, even if
> > that means more plumbing inside the kernel. It would be better then
> > just running the request ignoring silently that control. Ideally
> > userspace should not have to go into trial and errors, so controls that
> > are means for the should be marked.
> 
> Meanwhile I looked into the controls code and found that at least
> .try_ctrl will be called.
> 
> Do you have something to add in below subject for SPS/PPS?

My apology for incompletely reading your mail.

> 
> 
> > > > +
> > > > +Drain
> > > > +=====
> > > > +
> > > > +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> > > > +related ``CAPTURE`` buffers are given to the client, the client must follow the
> > > > +drain sequence described below. After the drain sequence ends, the client has
> > > > +received all encoded frames for all ``OUTPUT`` buffers queued before the
> > > > +sequence was started.
> > > > +
> > > > +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> > > > +
> > > > +   * **Required fields:**
> > > > +
> > > > +     ``cmd``
> > > > +         set to ``V4L2_ENC_CMD_STOP``.
> > > > +
> > > > +     ``flags``
> > > > +         set to 0.
> > > > +
> > > > +     ``pts``
> > > > +         set to 0.
> > > > +
> > > > +   .. warning::
> > > > +
> > > > +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> > > > +      queues are streaming. For compatibility reasons, the call to
> > > > +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> > > > +      not streaming, but at the same time it will not initiate the `Drain`
> > > > +      sequence and so the steps described below would not be applicable.
> > > > +
> > > > +2. Any ``OUTPUT`` buffers queued by the client before the
> > > > +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> > > > +   normal. The client must continue to handle both queues independently,
> > > > +   similarly to normal encode operation. This includes:
> > > > +
> > > > +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> > > > +
> > > > +     .. warning::
> > > > +
> > > > +        The last buffer may be empty (with :c:type:`v4l2_buffer`
> > > > +        ``bytesused`` = 0) and in that case it must be ignored by the client,
> > > > +        as it does not contain an encoded frame.
> > > > +
> > > > +     .. note::
> > > > +
> > > > +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> > > > +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> > > > +        :c:func:`VIDIOC_DQBUF`.
> > > > +
> > > > +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> > > > +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> > > > +
> > > > +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> > > > +
> > > > +   .. note::
> > > > +
> > > > +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> > > > +      event when the last frame has been encoded and all frames are ready to be
> > > > +      dequeued. It is deprecated behavior and the client must not rely on it.
> > > > +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> > > > +
> > > > +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> > > > +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> > > > +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
> > > > +   until the client issues any of the following operations:
> > > > +
> > > > +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> > > > +     operation normally, with all the state from before the drain,
> > > > +
> > > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > > +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> > > > +     and then resume encoding,
> > > > +
> > > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > > +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
> > > > +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> > > > +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> > > > +
> > > > +.. note::
> > > > +
> > > > +   Once the drain sequence is initiated, the client needs to drive it to
> > > > +   completion, as described by the steps above, unless it aborts the process by
> > > > +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> > > > +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> > > > +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> > > > +   will fail with -EBUSY error code if attempted.
> > > > +
> > > > +   For reference, handling of various corner cases is described below:
> > > > +
> > > > +   * In case of no buffer in the ``OUTPUT`` queue at the time the
> > > > +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> > > > +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > > +
> > > > +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> > > > +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
> > > > +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> > > > +     flag set.
> > > > +
> > > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> > > > +     middle of the drain sequence, the drain sequence is canceled and all
> > > > +     ``CAPTURE`` buffers are implicitly returned to the client.
> > > > +
> > > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> > > > +     middle of the drain sequence, the drain sequence completes immediately and
> > > > +     next ``CAPTURE`` buffer will be returned empty with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > > +
> > > > +   Although not mandatory, the availability of encoder commands may be queried
> > > > +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> > > > +
> > > > +Reset
> > > > +=====
> > > > +
> > > > +The client may want to request the encoder to reinitialize the encoding, so
> > > > +that the following stream data becomes independent from the stream data
> > > > +generated before. Depending on the coded format, that may imply that:
> > > > +
> > > > +* encoded frames produced after the restart must not reference any frames
> > > > +  produced before the stop, e.g. no long term references for H.264/HEVC,
> > > > +
> > > > +* any headers that must be included in a standalone stream must be produced
> > > > +  again, e.g. SPS and PPS for H.264/HEVC.
> > > 
> > > It seems that clients needs to know SPS/PPS (for muxers?) and thus they
> > > will need to extract or parse the encoder output buffers to find them.
> > > Maybe the spec should consider adding some buffer flag to mark such
> > > buffers which contain SPS/PPS only.
> > > 
> > > [1] - see at "type AvcCBox struct"

ISOMP4 stores the SPS/PPS out-of band, but the stream format also needs
to be in AVCc format (replacing start code by a nal size header).
Exposing this requires a different pixel format. It seams we have
V4L2_PIX_FMT_H264_NO_SC that seems similar, but this is poorly
documented, it not clear that no_sc would mean having a AVCc header.

Quick grep shows that it's up to you to decide as Venus is the only
driver using this. I haven't mapped it into GStreamer as the doc is too
obscure, and it does not define the semantic to retrieve the out-of-
band initial PPS/SPS.

> > > 
> > > Nicolas, how the gstreamer handle this?

Pretty much all our encoders we have support for producing AVCc stream
format (V4L2 being one of the exceptions), and most of them supports
both (start-code too). GStreamer has GstCaps that describe the format,
and we use this to negotiate and configure the encoder in the optimal
format. When there is no match, h264parse element will convert the
stream.

When using ACVc the approach taken is to store the pps/sps (called
codec-data) into the caps directly (this is very similar to FFMPEG,
which has the extra_data pointer in the context).

As for the software libraries, this is trivially delivered as an extra
pointer in the picture object. More generic API like OMX guaranty that
pps/sps will be stream first and separated from the visual content and
there is a flag that indicates the header only buffers. The OMX
component use a single buffer, or multiple buffer, we merge them in
GStreamer. I'm letting you know, as such a method seems suitable way of
passing initial SPS/PPS in V4L2, we simply need appropriate flag to
signal the header.

p.s. ffmpeg normalize the stream internally to AVCc as iterating NALs
is like iterating a linked list and does not requires reading a third
of the bytes (boyer-more scanning process).

> > > 
> > > > +
> > > > +This can be achieved by performing the reset sequence.
> > > > +
> > > > +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> > > > +   and respective buffers are dequeued.
> > > > +
> > > > +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> > > > +   will return all currently queued ``CAPTURE`` buffers to the client, without
> > > > +   valid frame data.
> > > > +
> > > > +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> > > > +   continue with regular encoding sequence. The encoded frames produced into
> > > > +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> > > > +   decoded without the need for frames encoded before the reset sequence,
> > > > +   starting at the first ``OUTPUT`` buffer queued after issuing the
> > > > +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> > > > +
> > > > +This sequence may be also used to change encoding parameters for encoders
> > > > +without the ability to change the parameters on the fly.
> > > > +
> > > 
> > > <cut>
> > > 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-05 21:24     ` Nicolas Dufresne
  2020-06-06  7:58       ` Stanimir Varbanov
@ 2020-06-10 12:28       ` Tomasz Figa
  2020-06-10 13:08         ` Nicolas Dufresne
  2020-06-10 13:58         ` Tomasz Figa
  1 sibling, 2 replies; 23+ messages in thread
From: Tomasz Figa @ 2020-06-10 12:28 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Stanimir Varbanov, Hans Verkuil, Linux Media Mailing List,
	Michael Tretter

On Fri, Jun 5, 2020 at 11:24 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
> >
> > On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > > From: Tomasz Figa <tfiga@chromium.org>
> > >
> > > Due to complexity of the video encoding process, the V4L2 drivers of
> > > stateful encoder hardware require specific sequences of V4L2 API calls
> > > to be followed. These include capability enumeration, initialization,
> > > encoding, encode parameters change, drain and reset.
> > >
> > > Specifics of the above have been discussed during Media Workshops at
> > > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > > originated at those events was later implemented by the drivers we already
> > > have merged in mainline, such as s5p-mfc or coda.
> > >
> > > The only thing missing was the real specification included as a part of
> > > Linux Media documentation. Fix it now and document the encoder part of
> > > the Codec API.
> > >
> > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > ---
> > >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> > >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> > >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> > >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> > >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> > >  5 files changed, 767 insertions(+), 20 deletions(-)
> > >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > new file mode 100644
> > > index 000000000000..aace7b812a9c
> > > --- /dev/null
> > > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > @@ -0,0 +1,728 @@
> > > +.. SPDX-License-Identifier: GPL-2.0
> > > +
> > > +.. _encoder:
> > > +
> > > +*************************************************
> > > +Memory-to-Memory Stateful Video Encoder Interface
> > > +*************************************************
> > > +
> > > +A stateful video encoder takes raw video frames in display order and encodes
> > > +them into a bytestream. It generates complete chunks of the bytestream, including
> > > +all metadata, headers, etc. The resulting bytestream does not require any
> > > +further post-processing by the client.
> > > +
> > > +Performing software stream processing, header generation etc. in the driver
> > > +in order to support this interface is strongly discouraged. In case such
> > > +operations are needed, use of the Stateless Video Encoder Interface (in
> > > +development) is strongly advised.
> > > +
> >
> > <cut>
> >
> > > +Encoding Parameter Changes
> > > +==========================
> > > +
> > > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > > +parameters at any time. The availability of parameters is encoder-specific
> > > +and the client must query the encoder to find the set of available controls.
> > > +
> > > +The ability to change each parameter during encoding is encoder-specific, as
> > > +per the standard semantics of the V4L2 control interface. The client may
> > > +attempt to set a control during encoding and if the operation fails with the
> > > +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> > > +configuration change to be allowed. To do this, it may follow the `Drain`
> > > +sequence to avoid losing the already queued/encoded frames.
> > > +
> > > +The timing of parameter updates is encoder-specific, as per the standard
> > > +semantics of the V4L2 control interface. If the client needs to apply the
> > > +parameters exactly at specific frame, using the Request API
> > > +(:ref:`media-request-api`) should be considered, if supported by the encoder.
> >
> > In request-api case does the control will return EBUSY immediately when
> > S_CTRL is called from the client? I'm asking in the context of the
> > controls which are not dynamic (cannot set during streaming and Reset
> > sequence is needed).
>
> This is all hypothetical, as nothing of that has been implemented. But
> I suppose there should be instant validation to allow that, even if
> that means more plumbing inside the kernel. It would be better then
> just running the request ignoring silently that control. Ideally
> userspace should not have to go into trial and errors, so controls that
> are means for the should be marked.
>

There isn't much to be implemented here, as it's the same standard
V4L2 behavior as existed for a while. If a device supports changing
the control on the fly, a call to S_CTRL succeeds, if not, it fails
with -EBUSY. This is regardless of whether requests are used or not.
Requests in this case simply synchronize applying the new control
values with the right frame.


> >
> > > +
> > > +Drain
> > > +=====
> > > +
> > > +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> > > +related ``CAPTURE`` buffers are given to the client, the client must follow the
> > > +drain sequence described below. After the drain sequence ends, the client has
> > > +received all encoded frames for all ``OUTPUT`` buffers queued before the
> > > +sequence was started.
> > > +
> > > +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> > > +
> > > +   * **Required fields:**
> > > +
> > > +     ``cmd``
> > > +         set to ``V4L2_ENC_CMD_STOP``.
> > > +
> > > +     ``flags``
> > > +         set to 0.
> > > +
> > > +     ``pts``
> > > +         set to 0.
> > > +
> > > +   .. warning::
> > > +
> > > +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> > > +      queues are streaming. For compatibility reasons, the call to
> > > +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> > > +      not streaming, but at the same time it will not initiate the `Drain`
> > > +      sequence and so the steps described below would not be applicable.
> > > +
> > > +2. Any ``OUTPUT`` buffers queued by the client before the
> > > +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> > > +   normal. The client must continue to handle both queues independently,
> > > +   similarly to normal encode operation. This includes:
> > > +
> > > +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> > > +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> > > +
> > > +     .. warning::
> > > +
> > > +        The last buffer may be empty (with :c:type:`v4l2_buffer`
> > > +        ``bytesused`` = 0) and in that case it must be ignored by the client,
> > > +        as it does not contain an encoded frame.
> > > +
> > > +     .. note::
> > > +
> > > +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> > > +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> > > +        :c:func:`VIDIOC_DQBUF`.
> > > +
> > > +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> > > +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> > > +
> > > +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> > > +
> > > +   .. note::
> > > +
> > > +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> > > +      event when the last frame has been encoded and all frames are ready to be
> > > +      dequeued. It is deprecated behavior and the client must not rely on it.
> > > +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> > > +
> > > +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> > > +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> > > +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
> > > +   until the client issues any of the following operations:
> > > +
> > > +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> > > +     operation normally, with all the state from before the drain,
> > > +
> > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> > > +     and then resume encoding,
> > > +
> > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
> > > +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> > > +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> > > +
> > > +.. note::
> > > +
> > > +   Once the drain sequence is initiated, the client needs to drive it to
> > > +   completion, as described by the steps above, unless it aborts the process by
> > > +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> > > +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> > > +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> > > +   will fail with -EBUSY error code if attempted.
> > > +
> > > +   For reference, handling of various corner cases is described below:
> > > +
> > > +   * In case of no buffer in the ``OUTPUT`` queue at the time the
> > > +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> > > +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > +
> > > +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> > > +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
> > > +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> > > +     flag set.
> > > +
> > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> > > +     middle of the drain sequence, the drain sequence is canceled and all
> > > +     ``CAPTURE`` buffers are implicitly returned to the client.
> > > +
> > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> > > +     middle of the drain sequence, the drain sequence completes immediately and
> > > +     next ``CAPTURE`` buffer will be returned empty with the
> > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > +
> > > +   Although not mandatory, the availability of encoder commands may be queried
> > > +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> > > +
> > > +Reset
> > > +=====
> > > +
> > > +The client may want to request the encoder to reinitialize the encoding, so
> > > +that the following stream data becomes independent from the stream data
> > > +generated before. Depending on the coded format, that may imply that:
> > > +
> > > +* encoded frames produced after the restart must not reference any frames
> > > +  produced before the stop, e.g. no long term references for H.264/HEVC,
> > > +
> > > +* any headers that must be included in a standalone stream must be produced
> > > +  again, e.g. SPS and PPS for H.264/HEVC.
> >
> > It seems that clients needs to know SPS/PPS (for muxers?) and thus they
> > will need to extract or parse the encoder output buffers to find them.
> > Maybe the spec should consider adding some buffer flag to mark such
> > buffers which contain SPS/PPS only.
> >
> > [1] - see at "type AvcCBox struct"
> >
> > Nicolas, how the gstreamer handle this?
> >
> > > +
> > > +This can be achieved by performing the reset sequence.
> > > +
> > > +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> > > +   and respective buffers are dequeued.
> > > +
> > > +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> > > +   will return all currently queued ``CAPTURE`` buffers to the client, without
> > > +   valid frame data.
> > > +
> > > +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> > > +   continue with regular encoding sequence. The encoded frames produced into
> > > +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> > > +   decoded without the need for frames encoded before the reset sequence,
> > > +   starting at the first ``OUTPUT`` buffer queued after issuing the
> > > +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> > > +
> > > +This sequence may be also used to change encoding parameters for encoders
> > > +without the ability to change the parameters on the fly.
> > > +
> >
> > <cut>
> >
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-10 12:28       ` Tomasz Figa
@ 2020-06-10 13:08         ` Nicolas Dufresne
  2020-06-10 13:55           ` Tomasz Figa
  2020-06-10 13:58         ` Tomasz Figa
  1 sibling, 1 reply; 23+ messages in thread
From: Nicolas Dufresne @ 2020-06-10 13:08 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Stanimir Varbanov, Hans Verkuil, Linux Media Mailing List,
	Michael Tretter

Le mercredi 10 juin 2020 à 14:28 +0200, Tomasz Figa a écrit :
> On Fri, Jun 5, 2020 at 11:24 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
> > > On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > > > From: Tomasz Figa <tfiga@chromium.org>
> > > > 
> > > > Due to complexity of the video encoding process, the V4L2 drivers of
> > > > stateful encoder hardware require specific sequences of V4L2 API calls
> > > > to be followed. These include capability enumeration, initialization,
> > > > encoding, encode parameters change, drain and reset.
> > > > 
> > > > Specifics of the above have been discussed during Media Workshops at
> > > > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > > > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > > > originated at those events was later implemented by the drivers we already
> > > > have merged in mainline, such as s5p-mfc or coda.
> > > > 
> > > > The only thing missing was the real specification included as a part of
> > > > Linux Media documentation. Fix it now and document the encoder part of
> > > > the Codec API.
> > > > 
> > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > > ---
> > > >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> > > >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> > > >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> > > >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> > > >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> > > >  5 files changed, 767 insertions(+), 20 deletions(-)
> > > >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > 
> > > > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > new file mode 100644
> > > > index 000000000000..aace7b812a9c
> > > > --- /dev/null
> > > > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > @@ -0,0 +1,728 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +.. _encoder:
> > > > +
> > > > +*************************************************
> > > > +Memory-to-Memory Stateful Video Encoder Interface
> > > > +*************************************************
> > > > +
> > > > +A stateful video encoder takes raw video frames in display order and encodes
> > > > +them into a bytestream. It generates complete chunks of the bytestream, including
> > > > +all metadata, headers, etc. The resulting bytestream does not require any
> > > > +further post-processing by the client.
> > > > +
> > > > +Performing software stream processing, header generation etc. in the driver
> > > > +in order to support this interface is strongly discouraged. In case such
> > > > +operations are needed, use of the Stateless Video Encoder Interface (in
> > > > +development) is strongly advised.
> > > > +
> > > 
> > > <cut>
> > > 
> > > > +Encoding Parameter Changes
> > > > +==========================
> > > > +
> > > > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > > > +parameters at any time. The availability of parameters is encoder-specific
> > > > +and the client must query the encoder to find the set of available controls.
> > > > +
> > > > +The ability to change each parameter during encoding is encoder-specific, as
> > > > +per the standard semantics of the V4L2 control interface. The client may
> > > > +attempt to set a control during encoding and if the operation fails with the
> > > > +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> > > > +configuration change to be allowed. To do this, it may follow the `Drain`
> > > > +sequence to avoid losing the already queued/encoded frames.
> > > > +
> > > > +The timing of parameter updates is encoder-specific, as per the standard
> > > > +semantics of the V4L2 control interface. If the client needs to apply the
> > > > +parameters exactly at specific frame, using the Request API
> > > > +(:ref:`media-request-api`) should be considered, if supported by the encoder.
> > > 
> > > In request-api case does the control will return EBUSY immediately when
> > > S_CTRL is called from the client? I'm asking in the context of the
> > > controls which are not dynamic (cannot set during streaming and Reset
> > > sequence is needed).
> > 
> > This is all hypothetical, as nothing of that has been implemented. But
> > I suppose there should be instant validation to allow that, even if
> > that means more plumbing inside the kernel. It would be better then
> > just running the request ignoring silently that control. Ideally
> > userspace should not have to go into trial and errors, so controls that
> > are means for the should be marked.
> > 
> 
> There isn't much to be implemented here, as it's the same standard
> V4L2 behavior as existed for a while. If a device supports changing
> the control on the fly, a call to S_CTRL succeeds, if not, it fails
> with -EBUSY. This is regardless of whether requests are used or not.

Are you against introducing flags to signal generically when a control
will have live effect, delayed effect or will simply be rejected (-
EBUSY) if set while streaming ? I'm particularly concern for cases
where the same control may work live for one driver, and not for
another. It would be very disappointing to get an EBUSY on a bitrate
control of an encoder as an example. So if someone foresee having
bitrate control that cannot be adapted while streaming (or that will
only be applied if a key frame is generated), I'd rather prefer to know
in advance, and likely not select this encoder for WebRTC purpose as an
example. Having to do all the steps to get the driver in streaming in
order to set the control and discover that it's not suitable for the
purpose seems quite sub-optimal.

I'm just saying, but the "standard" that existed for a while has always
been hardcoded to the control (through control documentation, and
that's fine with me) or the HW driver being used (which isn't fine in
my opinion).

> Requests in this case simply synchronize applying the new control
> values with the right frame.
> 
> 
> > > > +
> > > > +Drain
> > > > +=====
> > > > +
> > > > +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> > > > +related ``CAPTURE`` buffers are given to the client, the client must follow the
> > > > +drain sequence described below. After the drain sequence ends, the client has
> > > > +received all encoded frames for all ``OUTPUT`` buffers queued before the
> > > > +sequence was started.
> > > > +
> > > > +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> > > > +
> > > > +   * **Required fields:**
> > > > +
> > > > +     ``cmd``
> > > > +         set to ``V4L2_ENC_CMD_STOP``.
> > > > +
> > > > +     ``flags``
> > > > +         set to 0.
> > > > +
> > > > +     ``pts``
> > > > +         set to 0.
> > > > +
> > > > +   .. warning::
> > > > +
> > > > +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> > > > +      queues are streaming. For compatibility reasons, the call to
> > > > +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> > > > +      not streaming, but at the same time it will not initiate the `Drain`
> > > > +      sequence and so the steps described below would not be applicable.
> > > > +
> > > > +2. Any ``OUTPUT`` buffers queued by the client before the
> > > > +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> > > > +   normal. The client must continue to handle both queues independently,
> > > > +   similarly to normal encode operation. This includes:
> > > > +
> > > > +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> > > > +
> > > > +     .. warning::
> > > > +
> > > > +        The last buffer may be empty (with :c:type:`v4l2_buffer`
> > > > +        ``bytesused`` = 0) and in that case it must be ignored by the client,
> > > > +        as it does not contain an encoded frame.
> > > > +
> > > > +     .. note::
> > > > +
> > > > +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> > > > +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> > > > +        :c:func:`VIDIOC_DQBUF`.
> > > > +
> > > > +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> > > > +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> > > > +
> > > > +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> > > > +
> > > > +   .. note::
> > > > +
> > > > +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> > > > +      event when the last frame has been encoded and all frames are ready to be
> > > > +      dequeued. It is deprecated behavior and the client must not rely on it.
> > > > +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> > > > +
> > > > +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> > > > +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> > > > +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
> > > > +   until the client issues any of the following operations:
> > > > +
> > > > +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> > > > +     operation normally, with all the state from before the drain,
> > > > +
> > > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > > +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> > > > +     and then resume encoding,
> > > > +
> > > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > > +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
> > > > +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> > > > +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> > > > +
> > > > +.. note::
> > > > +
> > > > +   Once the drain sequence is initiated, the client needs to drive it to
> > > > +   completion, as described by the steps above, unless it aborts the process by
> > > > +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> > > > +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> > > > +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> > > > +   will fail with -EBUSY error code if attempted.
> > > > +
> > > > +   For reference, handling of various corner cases is described below:
> > > > +
> > > > +   * In case of no buffer in the ``OUTPUT`` queue at the time the
> > > > +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> > > > +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > > +
> > > > +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> > > > +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
> > > > +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> > > > +     flag set.
> > > > +
> > > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> > > > +     middle of the drain sequence, the drain sequence is canceled and all
> > > > +     ``CAPTURE`` buffers are implicitly returned to the client.
> > > > +
> > > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> > > > +     middle of the drain sequence, the drain sequence completes immediately and
> > > > +     next ``CAPTURE`` buffer will be returned empty with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > > +
> > > > +   Although not mandatory, the availability of encoder commands may be queried
> > > > +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> > > > +
> > > > +Reset
> > > > +=====
> > > > +
> > > > +The client may want to request the encoder to reinitialize the encoding, so
> > > > +that the following stream data becomes independent from the stream data
> > > > +generated before. Depending on the coded format, that may imply that:
> > > > +
> > > > +* encoded frames produced after the restart must not reference any frames
> > > > +  produced before the stop, e.g. no long term references for H.264/HEVC,
> > > > +
> > > > +* any headers that must be included in a standalone stream must be produced
> > > > +  again, e.g. SPS and PPS for H.264/HEVC.
> > > 
> > > It seems that clients needs to know SPS/PPS (for muxers?) and thus they
> > > will need to extract or parse the encoder output buffers to find them.
> > > Maybe the spec should consider adding some buffer flag to mark such
> > > buffers which contain SPS/PPS only.
> > > 
> > > [1] - see at "type AvcCBox struct"
> > > 
> > > Nicolas, how the gstreamer handle this?
> > > 
> > > > +
> > > > +This can be achieved by performing the reset sequence.
> > > > +
> > > > +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> > > > +   and respective buffers are dequeued.
> > > > +
> > > > +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> > > > +   will return all currently queued ``CAPTURE`` buffers to the client, without
> > > > +   valid frame data.
> > > > +
> > > > +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> > > > +   continue with regular encoding sequence. The encoded frames produced into
> > > > +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> > > > +   decoded without the need for frames encoded before the reset sequence,
> > > > +   starting at the first ``OUTPUT`` buffer queued after issuing the
> > > > +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> > > > +
> > > > +This sequence may be also used to change encoding parameters for encoders
> > > > +without the ability to change the parameters on the fly.
> > > > +
> > > 
> > > <cut>
> > > 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-10 13:08         ` Nicolas Dufresne
@ 2020-06-10 13:55           ` Tomasz Figa
  0 siblings, 0 replies; 23+ messages in thread
From: Tomasz Figa @ 2020-06-10 13:55 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Stanimir Varbanov, Hans Verkuil, Linux Media Mailing List,
	Michael Tretter

On Wed, Jun 10, 2020 at 3:08 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le mercredi 10 juin 2020 à 14:28 +0200, Tomasz Figa a écrit :
> > On Fri, Jun 5, 2020 at 11:24 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > > Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
> > > > On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > > > > From: Tomasz Figa <tfiga@chromium.org>
> > > > >
> > > > > Due to complexity of the video encoding process, the V4L2 drivers of
> > > > > stateful encoder hardware require specific sequences of V4L2 API calls
> > > > > to be followed. These include capability enumeration, initialization,
> > > > > encoding, encode parameters change, drain and reset.
> > > > >
> > > > > Specifics of the above have been discussed during Media Workshops at
> > > > > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > > > > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > > > > originated at those events was later implemented by the drivers we already
> > > > > have merged in mainline, such as s5p-mfc or coda.
> > > > >
> > > > > The only thing missing was the real specification included as a part of
> > > > > Linux Media documentation. Fix it now and document the encoder part of
> > > > > the Codec API.
> > > > >
> > > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > > > ---
> > > > >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> > > > >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> > > > >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> > > > >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> > > > >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> > > > >  5 files changed, 767 insertions(+), 20 deletions(-)
> > > > >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > >
> > > > > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > > new file mode 100644
> > > > > index 000000000000..aace7b812a9c
> > > > > --- /dev/null
> > > > > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > > @@ -0,0 +1,728 @@
> > > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > > +
> > > > > +.. _encoder:
> > > > > +
> > > > > +*************************************************
> > > > > +Memory-to-Memory Stateful Video Encoder Interface
> > > > > +*************************************************
> > > > > +
> > > > > +A stateful video encoder takes raw video frames in display order and encodes
> > > > > +them into a bytestream. It generates complete chunks of the bytestream, including
> > > > > +all metadata, headers, etc. The resulting bytestream does not require any
> > > > > +further post-processing by the client.
> > > > > +
> > > > > +Performing software stream processing, header generation etc. in the driver
> > > > > +in order to support this interface is strongly discouraged. In case such
> > > > > +operations are needed, use of the Stateless Video Encoder Interface (in
> > > > > +development) is strongly advised.
> > > > > +
> > > >
> > > > <cut>
> > > >
> > > > > +Encoding Parameter Changes
> > > > > +==========================
> > > > > +
> > > > > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > > > > +parameters at any time. The availability of parameters is encoder-specific
> > > > > +and the client must query the encoder to find the set of available controls.
> > > > > +
> > > > > +The ability to change each parameter during encoding is encoder-specific, as
> > > > > +per the standard semantics of the V4L2 control interface. The client may
> > > > > +attempt to set a control during encoding and if the operation fails with the
> > > > > +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> > > > > +configuration change to be allowed. To do this, it may follow the `Drain`
> > > > > +sequence to avoid losing the already queued/encoded frames.
> > > > > +
> > > > > +The timing of parameter updates is encoder-specific, as per the standard
> > > > > +semantics of the V4L2 control interface. If the client needs to apply the
> > > > > +parameters exactly at specific frame, using the Request API
> > > > > +(:ref:`media-request-api`) should be considered, if supported by the encoder.
> > > >
> > > > In request-api case does the control will return EBUSY immediately when
> > > > S_CTRL is called from the client? I'm asking in the context of the
> > > > controls which are not dynamic (cannot set during streaming and Reset
> > > > sequence is needed).
> > >
> > > This is all hypothetical, as nothing of that has been implemented. But
> > > I suppose there should be instant validation to allow that, even if
> > > that means more plumbing inside the kernel. It would be better then
> > > just running the request ignoring silently that control. Ideally
> > > userspace should not have to go into trial and errors, so controls that
> > > are means for the should be marked.
> > >
> >
> > There isn't much to be implemented here, as it's the same standard
> > V4L2 behavior as existed for a while. If a device supports changing
> > the control on the fly, a call to S_CTRL succeeds, if not, it fails
> > with -EBUSY. This is regardless of whether requests are used or not.
>
> Are you against introducing flags to signal generically when a control
> will have live effect, delayed effect or will simply be rejected (-
> EBUSY) if set while streaming ? I'm particularly concern for cases
> where the same control may work live for one driver, and not for
> another. It would be very disappointing to get an EBUSY on a bitrate
> control of an encoder as an example. So if someone foresee having
> bitrate control that cannot be adapted while streaming (or that will
> only be applied if a key frame is generated), I'd rather prefer to know
> in advance, and likely not select this encoder for WebRTC purpose as an
> example. Having to do all the steps to get the driver in streaming in
> order to set the control and discover that it's not suitable for the
> purpose seems quite sub-optimal.
>
> I'm just saying, but the "standard" that existed for a while has always
> been hardcoded to the control (through control documentation, and
> that's fine with me) or the HW driver being used (which isn't fine in
> my opinion).

I was just clarifying the behavior of S_(EXT_)CTRL(S).

A way to query the behavior in advance is an orthogonal problem, which
I agree that should be solved and I guess a flag returned by
VIDIOC_QUERY_EXT_CTRLS could do.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-10 12:28       ` Tomasz Figa
  2020-06-10 13:08         ` Nicolas Dufresne
@ 2020-06-10 13:58         ` Tomasz Figa
  1 sibling, 0 replies; 23+ messages in thread
From: Tomasz Figa @ 2020-06-10 13:58 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Stanimir Varbanov, Hans Verkuil, Linux Media Mailing List,
	Michael Tretter

On Wed, Jun 10, 2020 at 2:28 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> On Fri, Jun 5, 2020 at 11:24 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> >
> > Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
> > >
> > > On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > > > From: Tomasz Figa <tfiga@chromium.org>
> > > >
> > > > Due to complexity of the video encoding process, the V4L2 drivers of
> > > > stateful encoder hardware require specific sequences of V4L2 API calls
> > > > to be followed. These include capability enumeration, initialization,
> > > > encoding, encode parameters change, drain and reset.
> > > >
> > > > Specifics of the above have been discussed during Media Workshops at
> > > > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > > > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > > > originated at those events was later implemented by the drivers we already
> > > > have merged in mainline, such as s5p-mfc or coda.
> > > >
> > > > The only thing missing was the real specification included as a part of
> > > > Linux Media documentation. Fix it now and document the encoder part of
> > > > the Codec API.
> > > >
> > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > > ---
> > > >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> > > >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> > > >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> > > >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> > > >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> > > >  5 files changed, 767 insertions(+), 20 deletions(-)
> > > >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > new file mode 100644
> > > > index 000000000000..aace7b812a9c
> > > > --- /dev/null
> > > > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > > > @@ -0,0 +1,728 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +.. _encoder:
> > > > +
> > > > +*************************************************
> > > > +Memory-to-Memory Stateful Video Encoder Interface
> > > > +*************************************************
> > > > +
> > > > +A stateful video encoder takes raw video frames in display order and encodes
> > > > +them into a bytestream. It generates complete chunks of the bytestream, including
> > > > +all metadata, headers, etc. The resulting bytestream does not require any
> > > > +further post-processing by the client.
> > > > +
> > > > +Performing software stream processing, header generation etc. in the driver
> > > > +in order to support this interface is strongly discouraged. In case such
> > > > +operations are needed, use of the Stateless Video Encoder Interface (in
> > > > +development) is strongly advised.
> > > > +
> > >
> > > <cut>
> > >
> > > > +Encoding Parameter Changes
> > > > +==========================
> > > > +
> > > > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > > > +parameters at any time. The availability of parameters is encoder-specific
> > > > +and the client must query the encoder to find the set of available controls.
> > > > +
> > > > +The ability to change each parameter during encoding is encoder-specific, as
> > > > +per the standard semantics of the V4L2 control interface. The client may
> > > > +attempt to set a control during encoding and if the operation fails with the
> > > > +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> > > > +configuration change to be allowed. To do this, it may follow the `Drain`
> > > > +sequence to avoid losing the already queued/encoded frames.
> > > > +
> > > > +The timing of parameter updates is encoder-specific, as per the standard
> > > > +semantics of the V4L2 control interface. If the client needs to apply the
> > > > +parameters exactly at specific frame, using the Request API
> > > > +(:ref:`media-request-api`) should be considered, if supported by the encoder.
> > >
> > > In request-api case does the control will return EBUSY immediately when
> > > S_CTRL is called from the client? I'm asking in the context of the
> > > controls which are not dynamic (cannot set during streaming and Reset
> > > sequence is needed).
> >
> > This is all hypothetical, as nothing of that has been implemented. But
> > I suppose there should be instant validation to allow that, even if
> > that means more plumbing inside the kernel. It would be better then
> > just running the request ignoring silently that control. Ideally
> > userspace should not have to go into trial and errors, so controls that
> > are means for the should be marked.
> >
>
> There isn't much to be implemented here, as it's the same standard
> V4L2 behavior as existed for a while. If a device supports changing
> the control on the fly, a call to S_CTRL succeeds, if not, it fails
> with -EBUSY. This is regardless of whether requests are used or not.
> Requests in this case simply synchronize applying the new control
> values with the right frame.
>

Maybe one more thing that needs to be clarified about the Request API:
The operations are still validated at the time the commands are
executed, not when the request is queued or executed. So calling
S_EXT_CTRLS with a request would instantly fail with -EBUSY if the
driver doesn't support changing the control on the fly.

>
> > >
> > > > +
> > > > +Drain
> > > > +=====
> > > > +
> > > > +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> > > > +related ``CAPTURE`` buffers are given to the client, the client must follow the
> > > > +drain sequence described below. After the drain sequence ends, the client has
> > > > +received all encoded frames for all ``OUTPUT`` buffers queued before the
> > > > +sequence was started.
> > > > +
> > > > +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> > > > +
> > > > +   * **Required fields:**
> > > > +
> > > > +     ``cmd``
> > > > +         set to ``V4L2_ENC_CMD_STOP``.
> > > > +
> > > > +     ``flags``
> > > > +         set to 0.
> > > > +
> > > > +     ``pts``
> > > > +         set to 0.
> > > > +
> > > > +   .. warning::
> > > > +
> > > > +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> > > > +      queues are streaming. For compatibility reasons, the call to
> > > > +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> > > > +      not streaming, but at the same time it will not initiate the `Drain`
> > > > +      sequence and so the steps described below would not be applicable.
> > > > +
> > > > +2. Any ``OUTPUT`` buffers queued by the client before the
> > > > +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> > > > +   normal. The client must continue to handle both queues independently,
> > > > +   similarly to normal encode operation. This includes:
> > > > +
> > > > +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> > > > +
> > > > +     .. warning::
> > > > +
> > > > +        The last buffer may be empty (with :c:type:`v4l2_buffer`
> > > > +        ``bytesused`` = 0) and in that case it must be ignored by the client,
> > > > +        as it does not contain an encoded frame.
> > > > +
> > > > +     .. note::
> > > > +
> > > > +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> > > > +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> > > > +        :c:func:`VIDIOC_DQBUF`.
> > > > +
> > > > +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> > > > +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> > > > +
> > > > +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> > > > +
> > > > +   .. note::
> > > > +
> > > > +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> > > > +      event when the last frame has been encoded and all frames are ready to be
> > > > +      dequeued. It is deprecated behavior and the client must not rely on it.
> > > > +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> > > > +
> > > > +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> > > > +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> > > > +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
> > > > +   until the client issues any of the following operations:
> > > > +
> > > > +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> > > > +     operation normally, with all the state from before the drain,
> > > > +
> > > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > > +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> > > > +     and then resume encoding,
> > > > +
> > > > +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > > > +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
> > > > +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> > > > +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> > > > +
> > > > +.. note::
> > > > +
> > > > +   Once the drain sequence is initiated, the client needs to drive it to
> > > > +   completion, as described by the steps above, unless it aborts the process by
> > > > +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> > > > +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> > > > +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> > > > +   will fail with -EBUSY error code if attempted.
> > > > +
> > > > +   For reference, handling of various corner cases is described below:
> > > > +
> > > > +   * In case of no buffer in the ``OUTPUT`` queue at the time the
> > > > +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> > > > +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > > +
> > > > +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> > > > +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
> > > > +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> > > > +     flag set.
> > > > +
> > > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> > > > +     middle of the drain sequence, the drain sequence is canceled and all
> > > > +     ``CAPTURE`` buffers are implicitly returned to the client.
> > > > +
> > > > +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> > > > +     middle of the drain sequence, the drain sequence completes immediately and
> > > > +     next ``CAPTURE`` buffer will be returned empty with the
> > > > +     ``V4L2_BUF_FLAG_LAST`` flag set.
> > > > +
> > > > +   Although not mandatory, the availability of encoder commands may be queried
> > > > +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> > > > +
> > > > +Reset
> > > > +=====
> > > > +
> > > > +The client may want to request the encoder to reinitialize the encoding, so
> > > > +that the following stream data becomes independent from the stream data
> > > > +generated before. Depending on the coded format, that may imply that:
> > > > +
> > > > +* encoded frames produced after the restart must not reference any frames
> > > > +  produced before the stop, e.g. no long term references for H.264/HEVC,
> > > > +
> > > > +* any headers that must be included in a standalone stream must be produced
> > > > +  again, e.g. SPS and PPS for H.264/HEVC.
> > >
> > > It seems that clients needs to know SPS/PPS (for muxers?) and thus they
> > > will need to extract or parse the encoder output buffers to find them.
> > > Maybe the spec should consider adding some buffer flag to mark such
> > > buffers which contain SPS/PPS only.
> > >
> > > [1] - see at "type AvcCBox struct"
> > >
> > > Nicolas, how the gstreamer handle this?
> > >
> > > > +
> > > > +This can be achieved by performing the reset sequence.
> > > > +
> > > > +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> > > > +   and respective buffers are dequeued.
> > > > +
> > > > +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> > > > +   will return all currently queued ``CAPTURE`` buffers to the client, without
> > > > +   valid frame data.
> > > > +
> > > > +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> > > > +   continue with regular encoding sequence. The encoded frames produced into
> > > > +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> > > > +   decoded without the need for frames encoded before the reset sequence,
> > > > +   starting at the first ``OUTPUT`` buffer queued after issuing the
> > > > +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> > > > +
> > > > +This sequence may be also used to change encoding parameters for encoders
> > > > +without the ability to change the parameters on the fly.
> > > > +
> > >
> > > <cut>
> > >
> >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
  2020-05-29  9:57   ` Stanimir Varbanov
  2020-06-05  7:19   ` Stanimir Varbanov
@ 2020-06-23 10:36   ` Hans Verkuil
  2020-06-23 11:47     ` Tomasz Figa
  2020-06-23 10:47   ` Mauro Carvalho Chehab
  3 siblings, 1 reply; 23+ messages in thread
From: Hans Verkuil @ 2020-06-23 10:36 UTC (permalink / raw)
  To: linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter

Hi Tomasz,

On 26/05/2020 12:09, Hans Verkuil wrote:
> From: Tomasz Figa <tfiga@chromium.org>
> 
> Due to complexity of the video encoding process, the V4L2 drivers of
> stateful encoder hardware require specific sequences of V4L2 API calls
> to be followed. These include capability enumeration, initialization,
> encoding, encode parameters change, drain and reset.
> 
> Specifics of the above have been discussed during Media Workshops at
> LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> originated at those events was later implemented by the drivers we already
> have merged in mainline, such as s5p-mfc or coda.
> 
> The only thing missing was the real specification included as a part of
> Linux Media documentation. Fix it now and document the encoder part of
> the Codec API.
> 
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
>  .../userspace-api/media/v4l/v4l2.rst          |   2 +
>  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
>  5 files changed, 767 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> 
> diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> new file mode 100644
> index 000000000000..aace7b812a9c
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> @@ -0,0 +1,728 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +

New rst docs should be dual licensed:

SPDX-License-Identifier: GPL-2.0 OR GFDL-1.1-or-later-no-invariants

It is OK to make this change? No need to repost, if you agree with this
I or Mauro will make the change.

If you agree, just reply with your Signed-off-by line.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
                     ` (2 preceding siblings ...)
  2020-06-23 10:36   ` Hans Verkuil
@ 2020-06-23 10:47   ` Mauro Carvalho Chehab
  3 siblings, 0 replies; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2020-06-23 10:47 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Nicolas Dufresne, Tomasz Figa, Michael Tretter

Em Tue, 26 May 2020 12:09:28 +0200
Hans Verkuil <hverkuil-cisco@xs4all.nl> escreveu:

There are some things here that it should be fine for now, but I would like
to be added on our TODO:

> +Conventions and Notations Used in This Document
> +===============================================

...

> +
> +2. The meaning of words "must", "may", "should", etc. is as per `RFC
> +   2119 <https://tools.ietf.org/html/rfc2119>`_.
> +
> +3. All steps not marked "optional" are required.

Those should be moved in the future to the media uAPI spec as a hole.

> +
> +4. :c:func:`VIDIOC_G_EXT_CTRLS` and :c:func:`VIDIOC_S_EXT_CTRLS` may be used
> +   interchangeably with :c:func:`VIDIOC_G_CTRL` and :c:func:`VIDIOC_S_CTRL`,
> +   unless specified otherwise.
> +
> +5. Single-planar API (see :ref:`planar-apis`) and applicable structures may be
> +   used interchangeably with multi-planar API, unless specified otherwise,
> +   depending on encoder capabilities and following the general V4L2 guidelines.

Probably those too.

> +Glossary
> +========
> +
> +Refer to :ref:`decoder-glossary`.

I have some patches (yet to be merged) for a media glossary. We should move
the decoder one to be at the same place.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
  2020-06-23 10:36   ` Hans Verkuil
@ 2020-06-23 11:47     ` Tomasz Figa
  0 siblings, 0 replies; 23+ messages in thread
From: Tomasz Figa @ 2020-06-23 11:47 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List, Nicolas Dufresne, Michael Tretter

On Tue, Jun 23, 2020 at 12:37 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> Hi Tomasz,
>
> On 26/05/2020 12:09, Hans Verkuil wrote:
> > From: Tomasz Figa <tfiga@chromium.org>
> >
> > Due to complexity of the video encoding process, the V4L2 drivers of
> > stateful encoder hardware require specific sequences of V4L2 API calls
> > to be followed. These include capability enumeration, initialization,
> > encoding, encode parameters change, drain and reset.
> >
> > Specifics of the above have been discussed during Media Workshops at
> > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > originated at those events was later implemented by the drivers we already
> > have merged in mainline, such as s5p-mfc or coda.
> >
> > The only thing missing was the real specification included as a part of
> > Linux Media documentation. Fix it now and document the encoder part of
> > the Codec API.
> >
> > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> >  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
> >  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
> >  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
> >  .../userspace-api/media/v4l/v4l2.rst          |   2 +
> >  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
> >  5 files changed, 767 insertions(+), 20 deletions(-)
> >  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> >
> > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > new file mode 100644
> > index 000000000000..aace7b812a9c
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > @@ -0,0 +1,728 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
>
> New rst docs should be dual licensed:
>
> SPDX-License-Identifier: GPL-2.0 OR GFDL-1.1-or-later-no-invariants
>
> It is OK to make this change? No need to repost, if you agree with this
> I or Mauro will make the change.
>
> If you agree, just reply with your Signed-off-by line.

Fine with me.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-06-23 11:47 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 10:09 [PATCHv3 0/5] Stateful Encoding: final bits Hans Verkuil
2020-05-26 10:09 ` [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface Hans Verkuil
2020-05-29  9:57   ` Stanimir Varbanov
2020-05-29 10:17     ` Stanimir Varbanov
2020-05-29 12:21     ` Tomasz Figa
2020-06-05  7:19   ` Stanimir Varbanov
2020-06-05 21:24     ` Nicolas Dufresne
2020-06-06  7:58       ` Stanimir Varbanov
2020-06-07  0:27         ` Nicolas Dufresne
2020-06-10 12:28       ` Tomasz Figa
2020-06-10 13:08         ` Nicolas Dufresne
2020-06-10 13:55           ` Tomasz Figa
2020-06-10 13:58         ` Tomasz Figa
2020-06-23 10:36   ` Hans Verkuil
2020-06-23 11:47     ` Tomasz Figa
2020-06-23 10:47   ` Mauro Carvalho Chehab
2020-05-26 10:09 ` [PATCHv3 2/5] vidioc-g-parm.rst: update the VIDIOC_G/S_PARM documentation Hans Verkuil
2020-05-28  7:54   ` Michael Tretter
2020-05-26 10:09 ` [PATCHv3 3/5] dev-decoder.rst: small fixes Hans Verkuil
2020-05-26 10:09 ` [PATCHv3 4/5] videodev2.h: add V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag Hans Verkuil
2020-05-26 10:09 ` [PATCHv3 5/5] dev-encoder.rst: add reference to V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL Hans Verkuil
2020-05-28  7:58 ` [PATCHv3 0/5] Stateful Encoding: final bits Michael Tretter
2020-06-02  9:01 ` Tomasz Figa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.