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

From: Tomasz Figa <tfiga@chromium.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: "Linux Media Mailing List" <linux-media@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Stanimir Varbanov" <stanimir.varbanov@linaro.org>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Hans Verkuil" <hverkuil@xs4all.nl>,
	"Pawel Osciak" <posciak@chromium.org>,
	"Alexandre Courbot" <acourbot@chromium.org>,
	kamil@wypas.org, a.hajda@samsung.com,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	jtp.park@samsung.com, "Philipp Zabel" <p.zabel@pengutronix.de>,
	"Tiffany Lin (林慧珊)" <tiffany.lin@mediatek.com>,
	"Andrew-CT Chen (陳智迪)" <andrew-ct.chen@mediatek.com>,
	todor.tomov@linaro.org, nicolas@ndufresne.ca,
	"Paul Kocialkowski" <paul.kocialkowski@bootlin.com>,
	dave.stevenson@raspberrypi.org,
	"Ezequiel Garcia" <ezequiel@collabora.com>
Subject: Re: [PATCH 1/2] media: docs-rst: Document memory-to-memory video decoder interface
Date: Sun, 21 Oct 2018 00:39:42 +0900	[thread overview]
Message-ID: <CAAFQd5CdjZKEeZmowMKVLVKhiWPxEK2f7H+xNhhXAPn7O-S33g@mail.gmail.com> (raw)
In-Reply-To: <CAAFQd5Bb2v0iU6GAwOKtc-4kv1V+P_4Co7=OUYg77wfmKQQ=kA@mail.gmail.com>

On Thu, Oct 18, 2018 at 7:03 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> Hi Laurent,
>
> On Wed, Oct 17, 2018 at 10:34 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Tomasz,
> >
> > Thank you for the patch.
>
> Thanks for your comments! Please see my replies inline.
>
> >
> > On Tuesday, 24 July 2018 17:06:20 EEST Tomasz Figa wrote:
[snip]
> > > The driver must also set
> > > +     ``V4L2_BUF_FLAG_LAST`` in :c:type:`v4l2_buffer` ``flags`` field on the
> > > +     buffer on the ``CAPTURE`` queue containing the last frame (if any)
> > > +     produced as a result of processing the ``OUTPUT`` buffers queued
> > > +     before ``V4L2_DEC_CMD_STOP``. If no more frames are left to be
> > > +     returned at the point of handling ``V4L2_DEC_CMD_STOP``, the driver
> > > +     must return an empty buffer (with :c:type:`v4l2_buffer`
> > > +     ``bytesused`` = 0) as the last buffer with ``V4L2_BUF_FLAG_LAST`` set
> > > +     instead. Any attempts to dequeue more buffers beyond the buffer marked
> > > +     with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> > > +     :c:func:`VIDIOC_DQBUF`.
> > > +
> > > +   * If the ``CAPTURE`` queue is NOT streaming, no action is necessary for
> > > +     ``CAPTURE`` queue and the driver must send a ``V4L2_EVENT_EOS``
> > > +     immediately after all ``OUTPUT`` buffers in question have been
> > > +     processed.
> >
> > What is the use case for this ? Can't we just return an error if decoder isn't
> > streaming ?
> >
>
> Actually this is wrong. We want the queued OUTPUT buffers to be
> processed and decoded, so if the CAPTURE queue is not yet set up
> (initialization sequence not completed yet), handling the
> initialization sequence first will be needed as a part of the drain
> sequence. I've updated the document with that.

I might want to take this back. The client could just drive the
initialization to completion on its own and start the drain sequence
after that. Let me think if it makes anything easier. For reference, I
don't see any compatibility constraint here, since the existing user
space already works like that.

Best regards,
Tomasz