Re: [PATCH 1/2] media: docs-rst: Document memory-to-memory video decoder interface

[Tomasz Figa]

Hi Maxime,

On Tue, Aug 7, 2018 at 5:32 AM Maxime Jourdan <maxi.jourdan@xxxxxxxxxx> wrote:
>
> Hi Tomasz,
>
> Sorry for sending this email only to you, I subscribed to linux-media
> after you posted this and I'm not sure how to respond to everybody.
>

No worries. Let me reply with other recipients added back. Thanks for
your comments.

> I'm currently developing a V4L2 M2M decoder driver for Amlogic SoCs so
> my comments are somewhat biased towards it
> (https://github.com/Elyotna/linux)
>
> > +Seek
> > +====
> > +
> > +Seek is controlled by the ``OUTPUT`` queue, as it is the source of
> > +bitstream data. ``CAPTURE`` queue remains unchanged/unaffected.
> > +
> > +1. Stop the ``OUTPUT`` queue to begin the seek sequence via
> > +   :c:func:`VIDIOC_STREAMOFF`.
> > +
> > +   * **Required fields:**
> > +
> > +     ``type``
> > +         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``
> > +
> > +   * The driver must drop all the pending ``OUTPUT`` buffers and they are
> > +     treated as returned to the client (following standard semantics).
> > +
> > +2. Restart the ``OUTPUT`` queue via :c:func:`VIDIOC_STREAMON`
> > +
> > +   * **Required fields:**
> > +
> > +     ``type``
> > +         a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT``
> > +
> > +   * The driver must be put in a state after seek and be ready to
> > +     accept new source bitstream buffers.
> > +
> > +3. Start queuing buffers to ``OUTPUT`` queue containing stream data after
> > +   the seek until a suitable resume point is found.
> > +
> > +   .. note::
> > +
> > +      There is no requirement to begin queuing stream starting exactly from
> > +      a resume point (e.g. SPS or a keyframe). The driver must handle any
> > +      data queued and must keep processing the queued buffers until it
> > +      finds a suitable resume point. While looking for a resume point, the
> > +      driver processes ``OUTPUT`` buffers and returns them to the client
> > +      without producing any decoded frames.
> > +
> > +      For hardware known to be mishandling seeks to a non-resume point,
> > +      e.g. by returning corrupted decoded frames, the driver must be able
> > +      to handle such seeks without a crash or any fatal decode error.
>
> This is unfortunately my case, apart from parsing the bitstream
> manually - which is a no-no -, there is no way to know when I'll be
> writing in an IDR frame to the HW bitstream parser. I think it would
> be much preferable that the client starts sending in an IDR frame for
> sure.

Most of the hardware, which have upstream drivers, deal with this
correctly and there is existing user space that relies on this, so we
cannot simply add such requirement. However, when sending your driver
upstream, feel free to include a patch that adds a read-only control
that tells the user space that it needs to do seeks to resume points.
Obviously this will work only with user space aware of this
requirement, but I don't think we can do anything better here.

>
> > +4. After a resume point is found, the driver will start returning
> > +   ``CAPTURE`` buffers with decoded frames.
> > +
> > +   * There is no precise specification for ``CAPTURE`` queue of when it
> > +     will start producing buffers containing decoded data from buffers
> > +     queued after the seek, as it operates independently
> > +     from ``OUTPUT`` queue.
> > +
> > +     * The driver is allowed to and may return a number of remaining
> > +       ``CAPTURE`` buffers containing decoded frames from before the seek
> > +       after the seek sequence (STREAMOFF-STREAMON) is performed.
> > +
> > +     * The driver is also allowed to and may not return all decoded frames
> > +       queued but not decode before the seek sequence was initiated. For
> > +       example, given an ``OUTPUT`` queue sequence: QBUF(A), QBUF(B),
> > +       STREAMOFF(OUT), STREAMON(OUT), QBUF(G), QBUF(H), any of the
> > +       following results on the ``CAPTURE`` queue is allowed: {Aâ, Bâ, Gâ,
> > +       Hâ}, {Aâ, Gâ, Hâ}, {Gâ, Hâ}.
> > +
> > +   .. note::
> > +
> > +      To achieve instantaneous seek, the client may restart streaming on
> > +      ``CAPTURE`` queue to discard decoded, but not yet dequeued buffers.
>
> Overall, I think Drain followed by V4L2_DEC_CMD_START is a more
> applicable scenario for seeking.
> Heck, simply starting to queue buffers at the seek - starting with an
> IDR - without doing any kind of streamon/off or cmd_start(stop) will
> do the trick.

Why do you think so?

For a seek, as expected by a typical device user, the result should be
discarding anything already queued and just start decoding new frames
as soon as possible.

Actually, this section doesn't describe any specific sequence, just
possible ways to do a seek using existing primitives.

Best regards,
Tomasz