lkml.org 
[lkml]   [2021]   [Nov]   [24]   [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
    On 03/11/2021 23:58, Alexandre Courbot wrote:
    > Thanks for the comments. It looks like we are having a consensus that
    > the described behavior is the current (untold) expectation, and how a
    > client should behave if it wants to support all V4L2 decoders. OTOH it
    > would also be nice to be able to signal the client that CAPTURE
    > buffers are not used by the hardware and can thus be overwritten/freed
    > at will.
    >
    > I have discussed a bit with Nicolas on IRC and we were wondering where
    > such a flag signaling that capability should be. We could have:
    >
    > 1) Something global to the currently set format, i.e. maybe take some
    > space from the reserved area of v4l2_pix_format_mplane to add a flag.
    > The property would then be global to all buffers, and apply between
    > calls to STREAMON and STREAMOFF.

    VIDIOC_ENUM_FMT is already used to signal format flags, so it could be
    put there.

    Regards,

    Hans

    >
    > 2) An additional flag to the v4l2_buffer structure that would signal
    > whether the buffer is currently writable. This means the writable
    > property of buffers can be signaled on a finer grain (i.e. reference
    > frames would not be writable, but non-reference ones would be), and we
    > can even update the status of a given buffer after it is not used as a
    > reference (the client would have to QUERYBUF to get the updated status
    > though). OTOH a client that needs to know whether the buffers are
    > writable before streaming would need to query them all one-by-one.
    >
    > I am not sure whether we need something as precise as 2), or whether
    > 1) would be enough and globally simpler. Any more thoughts?
    >
    > Cheers,
    > Alex.
    >
    > On Mon, Nov 1, 2021 at 11:52 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
    >>
    >> 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-24 12:45    [W:3.605 / U:0.428 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site