lkml.org 
[lkml]   [2021]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [EXT] Re: [PATCH] media: docs: dev-decoder: add restrictions about CAPTURE buffers
From
Date
Le vendredi 29 octobre 2021 à 10:28 +0300, Stanimir Varbanov a écrit :
>
> On 10/29/21 5:10 AM, Ming Qian wrote:
> > > -----Original Message-----
> > > From: Nicolas Dufresne [mailto:nicolas@ndufresne.ca]
> > > Sent: Tuesday, October 26, 2021 10:12 PM
> > > To: Alexandre Courbot <acourbot@chromium.org>; Mauro Carvalho Chehab
> > > <mchehab@kernel.org>; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Tomasz Figa
> > > <tfiga@chromium.org>
> > > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: [EXT] Re: [PATCH] media: docs: dev-decoder: add restrictions about
> > > CAPTURE buffers
> > >
> > > Caution: EXT Email
> > >
> > > Le lundi 18 octobre 2021 à 18:14 +0900, Alexandre Courbot a écrit :
> > > > CAPTURE buffers might be read by the hardware after they are dequeued,
> > > > which goes against the general idea that userspace has full control
> > > > over dequeued buffers. Explain why and document the restrictions that
> > > > this implies for userspace.
> > > >
> > > > Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
> > > > ---
> > > > .../userspace-api/media/v4l/dev-decoder.rst | 17
> > > +++++++++++++++++
> > > > 1 file changed, 17 insertions(+)
> > > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/dev-decoder.rst
> > > > b/Documentation/userspace-api/media/v4l/dev-decoder.rst
> > > > index 5b9b83feeceb..3cf2b496f2d0 100644
> > > > --- a/Documentation/userspace-api/media/v4l/dev-decoder.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/dev-decoder.rst
> > > > @@ -752,6 +752,23 @@ available to dequeue. Specifically:
> > > > buffers are out-of-order compared to the ``OUTPUT`` buffers):
> > > ``CAPTURE``
> > > > timestamps will not retain the order of ``OUTPUT`` timestamps.
> > > >
> > > > +.. note::
> > > > +
> > > > + The backing memory of ``CAPTURE`` buffers that are used as reference
> > > frames
> > > > + by the stream may be read by the hardware even after they are
> > > dequeued.
> > > > + Consequently, the client should avoid writing into this memory while the
> > > > + ``CAPTURE`` queue is streaming. Failure to observe this may result in
> > > > + corruption of decoded frames.
> > > > +
> > > > + Similarly, when using a memory type other than
> > > ``V4L2_MEMORY_MMAP``, the
> > > > + client should make sure that each ``CAPTURE`` buffer is always queued
> > > with
> > > > + the same backing memory for as long as the ``CAPTURE`` queue is
> > > streaming.
> > > > + The reason for this is that V4L2 buffer indices can be used by drivers to
> > > > + identify frames. Thus, if the backing memory of a reference frame is
> > > > + submitted under a different buffer ID, the driver may misidentify it and
> > > > + decode a new frame into it while it is still in use, resulting in corruption
> > > > + of the following frames.
> > > > +
> > >
> > > I think this is nice addition, but insufficient. We should extend the API with a
> > > flags that let application know if the buffers are reference or secondary. For the
> > > context, we have a mix of CODEC that will output usable reference frames and
> > > needs careful manipulation and many other drivers where the buffers *maybe*
> > > secondary, meaning they may have been post-processed and modifying these
> > > in- place may have no impact.
> > >
> > > The problem is the "may", that will depends on the chosen CAPTURE format. I
> > > believe we should flag this, this flag should be set by the driver, on CAPTURE
> > > queue. The information is known after S_FMT, so Format Flag, Reqbufs
> > > capabilities or querybuf flags are candidates. I think the buffer flags are the
> > > best named flag, though we don't expect this to differ per buffer. Though,
> > > userspace needs to call querybuf for all buf in order to export or map them.
> > >
> > > What userspace can do with this is to export the DMABuf as read-only, and
> > > signal this internally in its own context. This is great to avoid any unwanted
> > > side effect described here.
> >
> > I think a flag should be add to tell a buffer is reference or secondary.
> > But for some codec, it's hard to determine the buffer flag when reqbufs.
> > The buffer flag should be dynamically updated by driver.
> > User can check the flag after dqbuf every time.
>
> +1
>
> I'm not familiar with stateless decoders where on the reqbuf time it
> could work, debut for stateful coders it should be a dynamic flag on
> every capture buffer.

This isn't very clear request here, on which C structure to you say you would
prefer this ?

There is no difference for stateful/stateless for this regard. The capture
format must have been configured prior to reqbuf, so nothing post S_FMT(CAPTURE)
can every be very dynamic. I think the flag in S_FMT is miss-named and would
create confusion. struct v4l2_reqbufs vs struc v4l2_buffer are equivalent. The
seconds opens for flexibility.

In fact, for some certain CODEC, there exist B-Frames that are never used as
references, so these could indeed can be written to even if the buffer are not
secondary. I think recommending to flag this in v4l2_buffer, and read through
VIDIOC_QUERYBUF would be the best choice.

>
> >
> > >
> > > > During the decoding, the decoder may initiate one of the special
> > > > sequences, as listed below. The sequences will result in the decoder
> > > > returning all the ``CAPTURE`` buffers that originated from all the
> > > > ``OUTPUT`` buffers processed
> > >
> >
>


\
 
 \ /
  Last update: 2021-11-01 15:53    [W:0.062 / U:0.768 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site