Re: [PATCH v2 2/2] media: docs-rst: Document memory-to-memory video encoder interface

From: Nicolas Dufresne <nicolas@ndufresne.ca>
To: Tomasz Figa <tfiga@chromium.org>
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>,
	"Paweł Ościak" <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 2/2] media: docs-rst: Document memory-to-memory video encoder interface
Date: Thu, 24 Jan 2019 15:02:02 -0500	[thread overview]
Message-ID: <74aef8bfac022ba8ea875b1c69538ad91bf00a0b.camel@ndufresne.ca> (raw)
In-Reply-To: <CAAFQd5B5EhdK78uzDE=t-ZHYDVYrrUo9t2_HQH+bKjRp6HfSOw@mail.gmail.com>

Le mercredi 23 janvier 2019 à 19:02 +0900, Tomasz Figa a écrit :
> On Sun, Nov 18, 2018 at 10:34 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > Le samedi 17 novembre 2018 à 12:37 +0100, Hans Verkuil a écrit :
> > > > > Does V4L2_CID_MIN_BUFFERS_FOR_CAPTURE make any sense for encoders?
> > > > 
> > > > We do account for it in GStreamer (the capture/output handling is
> > > > generic), but I don't know if it's being used anywhere.
> > > 
> > > Do you use this value directly for REQBUFS, or do you use it as the minimum
> > > value but in practice use more buffers?
> > 
> > We add more buffers to that value. We assume this value is what will be
> > held by the driver, hence without adding some buffers, the driver would
> > go idle as soon as one is dequeued. We also need to allocate for the
> > importing driver.
> > 
> > In general, if we have a pipeline with Driver A sending to Driver B,
> > both driver will require a certain amount of buffers to operate. E.g.
> > with DRM display, the driver will hold on 1 buffer (the scannout
> > buffer).
> > 
> > In GStreamer, it's implemented generically, so we do:
> > 
> >   MIN_BUFFERS_FOR + remote_min + 1
> > 
> > If only MIN_BUFFERS_FOR was allocated, ignoring remote driver
> > requirement, the streaming will likely get stuck.
> 
> What happens if the driver doesn't report it?

If the driver does not report it because it does not use it (I think
CODA decoder is like that), there is no issue. If the driver do not
report but needs extra, the driver will end up growing count in
REQBUFS, so the end result will be under-allocation since the remote
requirement won't be accounted. Streaming will hang.

A good example is transcoding, you encoder will never have enough
frames to reproduce an output, because the decoder is waiting for his
frame to come back. Only solution to that would be a memcpy(), or
double allocation (redoing REQBUFS later on). The MIN_BUFFERS_FOR
announcement is the optimal way, avoiding copies and allocating twice.

> 
> Best regards,
> Tomasz