From: Tomasz Figa <tfiga@chromium.org>
To: Nicolas Dufresne <nicolas@ndufresne.ca>
Cc: "Hans Verkuil" <hverkuil@xs4all.nl>,
"Linux Media Mailing List" <linux-media@vger.kernel.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Pawel Osciak" <posciak@chromium.org>,
"Alexandre Courbot" <acourbot@chromium.org>,
"Kamil Debski" <kamil@wypas.org>,
"Andrzej Hajda" <a.hajda@samsung.com>,
"Kyungmin Park" <kyungmin.park@samsung.com>,
"Jeongtae Park" <jtp.park@samsung.com>,
"Philipp Zabel" <p.zabel@pengutronix.de>,
"Tiffany Lin (林慧珊)" <tiffany.lin@mediatek.com>,
"Andrew-CT Chen (陳智迪)" <andrew-ct.chen@mediatek.com>,
"Stanimir Varbanov" <stanimir.varbanov@linaro.org>,
"Todor Tomov" <todor.tomov@linaro.org>,
"Paul Kocialkowski" <paul.kocialkowski@bootlin.com>,
"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
dave.stevenson@raspberrypi.org,
"Ezequiel Garcia" <ezequiel@collabora.com>,
"Maxime Jourdan" <maxi.jourdan@wanadoo.fr>
Subject: Re: [PATCH v2 1/2] media: docs-rst: Document memory-to-memory video decoder interface
Date: Wed, 6 Feb 2019 14:35:51 +0900 [thread overview]
Message-ID: <CAAFQd5D4AP+xj_EkcsB84UqxCLBBEGComqGaMCm2zHSMnpqOKw@mail.gmail.com> (raw)
In-Reply-To: <c145fbf21301d03bdfdd8bf6613f0f68576e66be.camel@ndufresne.ca>
On Wed, Jan 30, 2019 at 1:02 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le vendredi 25 janvier 2019 à 12:27 +0900, Tomasz Figa a écrit :
> > On Fri, Jan 25, 2019 at 4:55 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > > Le jeudi 24 janvier 2019 à 18:06 +0900, Tomasz Figa a écrit :
> > > > > Actually I just realized the last point might not even be achievable
> > > > > for some of the decoders (s5p-mfc, mtk-vcodec), as they don't report
> > > > > which frame originates from which bitstream buffer and the driver just
> > > > > picks the most recently consumed OUTPUT buffer to copy the timestamp
> > > > > from. (s5p-mfc actually "forgets" to set the timestamp in some cases
> > > > > too...)
> > > > >
> > > > > I need to think a bit more about this.
> > > >
> > > > Actually I misread the code. Both s5p-mfc and mtk-vcodec seem to
> > > > correctly match the buffers.
> > >
> > > Ok good, since otherwise it would have been a regression in MFC driver.
> > > This timestamp passing thing could in theory be made optional though,
> > > it lives under some COPY_TIMESTAMP kind of flag. What that means though
> > > is that a driver without such a capability would need to signal dropped
> > > frames using some other mean.
> > >
> > > In userspace, the main use is to match the produced frame against a
> > > userspace specific list of frames. At least this seems to be the case
> > > in Gst and Chromium, since the userspace list contains a superset of
> > > the metadata found in the v4l2_buffer.
> > >
> > > Now, using the produced timestamp, userspace can deduce frame that the
> > > driver should have produced but didn't (could be a deadline case codec,
> > > or simply the frames where corrupted). It's quite normal for a codec to
> > > just keep parsing until it finally find something it can decode.
> > >
> > > That's at least one way to do it, but there is other possible
> > > mechanism. The sequence number could be used, or even producing buffers
> > > with the ERROR flag set. What matters is just to give userspace a way
> > > to clear these frames, which would simply grow userspace memory usage
> > > over time.
> >
> > Is it just me or we were missing some consistent error handling then?
> >
> > I feel like the drivers should definitely return the bitstream buffers
> > with the ERROR flag, if there is a decode failure of data in the
> > buffer. Still, that could become more complicated if there is more
> > than 1 frame in that piece of bitstream, but only 1 frame is corrupted
> > (or whatever).
>
> I agree, but it might be more difficult then it looks (even FFMPEG does
> not do that). I believe the code that is processing the bitstream in
> stateful codecs is mostly unrelated from the code actually doing the
> decoding. So what might happen is that the decoding part will never
> actually allocate a buffer for the skipped / corrupted part of the
> bitstream. Also, the notion of a skipped frame is not always evident in
> when parsing H264 or HEVC NALs. There is still a full page of text just
> to explain how to detect that start of a new frame.
Right. I don't think we can guarantee that we can always correlate the
errors with exact buffers and so I phrased the paragraph about errors
in v3 in a bit more conservative way:
See the snapshot hosted by Hans (thanks!):
https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-decoder.html#decoding
>
> Yet, it would be interesting to study the firmwares we have and see
> what they provide that would help making decode errors more explicit.
>
Agreed.
> >
> > Another case is when the bitstream, even if corrupted, is still enough
> > to produce some output. My intuition tells me that such CAPTURE buffer
> > should be then returned with the ERROR flag. That wouldn't still be
> > enough for any more sophisticated userspace error concealment, but
> > could still let the userspace know to perhaps drop the frame.
>
> You mean if a frame was concealed (typically the frame was decoded from
> a closed by reference instead of the expected reference). That is
> something signalled by FFPEG. We should document this possibility. I
> actually have something implemented in GStreamer. Basically if we have
> the ERROR flag with a payload size smaller then expected, I drop the
> frame and produce a drop event message, while if I have a frame with
> ERROR flag but of the right payload size, I assume it is corrupted, and
> simply flag it as corrupted, leaving to the application the decision to
> display it or not. This is a case that used to happen with some UVC
> cameras (though some have been fixed, and the UVC camera should drop
> smaller payload size buffers now).
I think it's a behavior that makes the most sense indeed.
Technically one could also consider the case of 0 < bytesused <
sizeimage, which could mean that only a part of the frame is in the
buffer. An application could try to blend it with previous frame using
some concealing algorithms. I haven't seen an app that could do such
thing, though.
Best regards,
Tomasz
next prev parent reply other threads:[~2019-02-06 5:43 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-22 14:48 [PATCH v2 0/2] Document memory-to-memory video codec interfaces Tomasz Figa
2018-10-22 14:48 ` [PATCH v2 1/2] media: docs-rst: Document memory-to-memory video decoder interface Tomasz Figa
2018-10-29 9:45 ` Stanimir Varbanov
2018-10-29 10:06 ` Tomasz Figa
2018-10-29 10:07 ` Tomasz Figa
2018-11-12 11:37 ` Hans Verkuil
2019-01-22 10:02 ` Tomasz Figa
2019-01-22 14:47 ` Hans Verkuil
2019-01-23 5:27 ` Tomasz Figa
2019-01-23 8:10 ` Hans Verkuil
2019-01-24 9:06 ` Tomasz Figa
2019-01-24 19:55 ` Nicolas Dufresne
2019-01-25 3:27 ` Tomasz Figa
2019-01-30 4:02 ` Nicolas Dufresne
2019-02-06 5:35 ` Tomasz Figa [this message]
2019-04-09 9:47 ` Tomasz Figa
2019-04-10 9:26 ` Hans Verkuil
2018-11-12 15:04 ` Stanimir Varbanov
2018-11-15 14:34 ` Hans Verkuil
2018-11-17 4:31 ` Nicolas Dufresne
2018-11-17 11:43 ` Hans Verkuil
2018-11-18 1:25 ` Nicolas Dufresne
2018-10-22 14:49 ` [PATCH v2 2/2] media: docs-rst: Document memory-to-memory video encoder interface Tomasz Figa
2018-11-12 13:23 ` Hans Verkuil
2018-11-17 4:18 ` Nicolas Dufresne
2018-11-17 11:37 ` Hans Verkuil
2018-11-18 1:34 ` Nicolas Dufresne
2019-01-23 10:02 ` Tomasz Figa
2019-01-24 20:02 ` Nicolas Dufresne
2019-01-23 10:00 ` Tomasz Figa
2019-01-23 11:28 ` Hans Verkuil
2019-01-24 20:04 ` Nicolas Dufresne
2019-01-25 3:29 ` Tomasz Figa
2019-01-23 9:52 ` Tomasz Figa
2019-01-23 13:04 ` Hans Verkuil
2019-01-24 20:14 ` Nicolas Dufresne
2019-01-25 3:59 ` Tomasz Figa
2019-01-30 15:06 ` Nicolas Dufresne
2019-02-06 5:49 ` Tomasz Figa
2018-10-22 15:41 ` [PATCH v2 0/2] Document memory-to-memory video codec interfaces Hans Verkuil
2018-10-23 0:54 ` Tomasz Figa
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=CAAFQd5D4AP+xj_EkcsB84UqxCLBBEGComqGaMCm2zHSMnpqOKw@mail.gmail.com \
--to=tfiga@chromium.org \
--cc=a.hajda@samsung.com \
--cc=acourbot@chromium.org \
--cc=andrew-ct.chen@mediatek.com \
--cc=dave.stevenson@raspberrypi.org \
--cc=ezequiel@collabora.com \
--cc=hverkuil@xs4all.nl \
--cc=jtp.park@samsung.com \
--cc=kamil@wypas.org \
--cc=kyungmin.park@samsung.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=maxi.jourdan@wanadoo.fr \
--cc=mchehab@kernel.org \
--cc=nicolas@ndufresne.ca \
--cc=p.zabel@pengutronix.de \
--cc=paul.kocialkowski@bootlin.com \
--cc=posciak@chromium.org \
--cc=stanimir.varbanov@linaro.org \
--cc=tiffany.lin@mediatek.com \
--cc=todor.tomov@linaro.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).