Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181022144901.113852-1-tfiga@chromium.org> <20181022144901.113852-2-tfiga@chromium.org>
 <cf0fc2fc-72c6-dbca-68f7-a349879a3a14@xs4all.nl> <CAAFQd5AORjMjHdavdr3zM13BnyFnKnEb-0aKNjvwbB_xJEnxgQ@mail.gmail.com>
 <9b7c1385-d482-6e92-2222-2daa835dbc91@xs4all.nl> <CAAFQd5DwjLt8UeDohzrMausaLGnOStvrmp5p7frYbG1hbGjx3Q@mail.gmail.com>
 <CAAFQd5BPJv3cbJOWrziEjz_yE32DhfZv9vb-pG1Ltx-KS2=PQg@mail.gmail.com> <3ea3bf5bf9904ce877142c41f595207752172d27.camel@ndufresne.ca>
In-Reply-To: <3ea3bf5bf9904ce877142c41f595207752172d27.camel@ndufresne.ca>
From:   Tomasz Figa <tfiga@chromium.org>
Date:   Fri, 25 Jan 2019 12:27:52 +0900
Message-ID: <CAAFQd5C_OD=bvAxG0B_G+T6bnWddPHuiVZApj_8_+4xpMjH9+g@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] media: docs-rst: Document memory-to-memory video
 decoder interface
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>,
        =?UTF-8?B?VGlmZmFueSBMaW4gKOael+aFp+ePiik=?= 
        <tiffany.lin@mediatek.com>,
        =?UTF-8?B?QW5kcmV3LUNUIENoZW4gKOmZs+aZuui/qik=?= 
        <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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 25, 2019 at 4:55 AM Nicolas Dufresne <nicolas@ndufresne.ca> wro=
te:
>
> Le jeudi 24 janvier 2019 =C3=A0 18:06 +0900, Tomasz Figa a =C3=A9crit :
> > > 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 jus=
t
> > > 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).

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.

Best regards,
Tomasz