From: Stanimir Varbanov <stanimir.varbanov@linaro.org>
To: Ming Qian <ming.qian@nxp.com>,
Nicolas Dufresne <nicolas@ndufresne.ca>,
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-media@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [EXT] Re: [PATCH] media: docs: dev-decoder: add restrictions about CAPTURE buffers
Date: Fri, 29 Oct 2021 10:28:15 +0300 [thread overview]
Message-ID: <dc7496db-9ba3-fa7b-8563-1157b63c9b0d@linaro.org> (raw)
In-Reply-To: <AM6PR04MB634130FEB433CCA352CE98FBE7879@AM6PR04MB6341.eurprd04.prod.outlook.com>
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.
>
>>
>>> 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
>>
>
--
regards,
Stan
next prev parent reply other threads:[~2021-10-29 7:28 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-18 9:14 [PATCH] media: docs: dev-decoder: add restrictions about CAPTURE buffers Alexandre Courbot
2021-10-21 8:21 ` Tomasz Figa
2021-10-26 14:11 ` Nicolas Dufresne
2021-10-29 2:10 ` [EXT] " Ming Qian
2021-10-29 7:28 ` Stanimir Varbanov [this message]
2021-11-01 14:52 ` Nicolas Dufresne
2021-11-03 22:58 ` Alexandre Courbot
2021-11-24 11:44 ` Hans Verkuil
2021-10-29 3:04 ` Tomasz Figa
2021-11-01 14:45 ` Nicolas Dufresne
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dc7496db-9ba3-fa7b-8563-1157b63c9b0d@linaro.org \
--to=stanimir.varbanov@linaro.org \
--cc=acourbot@chromium.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=ming.qian@nxp.com \
--cc=nicolas@ndufresne.ca \
--cc=tfiga@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).