Re: [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API

From: Tomasz Figa <tfiga@chromium.org>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Nicolas Dufresne <nicolas@ndufresne.ca>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Vikash Garodia <vgarodia@codeaurora.org>,
	Alexandre Courbot <acourbot@chromium.org>,
	Malathi Gottam <mgottam@codeaurora.org>
Subject: Re: [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API
Date: Thu, 31 Jan 2019 22:34:48 +0900	[thread overview]
Message-ID: <CAAFQd5Aih7cWu-cfwBvNdwhHHYEaMF0SFebrYfdNXD9qKu8fxw@mail.gmail.com> (raw)
In-Reply-To: <1548938556.4585.1.camel@pengutronix.de>

On Thu, Jan 31, 2019 at 9:42 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> Hi Nicolas,
>
> On Wed, 2019-01-30 at 10:32 -0500, Nicolas Dufresne wrote:
> > Le mercredi 30 janvier 2019 à 15:17 +0900, Tomasz Figa a écrit :
> > > > I don't remember saying that, maybe I meant to say there might be a
> > > > workaround ?
> > > >
> > > > For the fact, here we queue the headers (or first frame):
> > > >
> > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/blob/master/sys/v4l2/gstv4l2videodec.c#L624
> > > >
> > > > Then few line below this helper does G_FMT internally:
> > > >
> > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/blob/master/sys/v4l2/gstv4l2videodec.c#L634
> > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/blob/master/sys/v4l2/gstv4l2object.c#L3907
> > > >
> > > > And just plainly fails if G_FMT returns an error of any type. This was
> > > > how Kamil designed it initially for MFC driver. There was no other
> > > > alternative back then (no EAGAIN yet either).
> > >
> > > Hmm, was that ffmpeg then?
> > >
> > > So would it just set the OUTPUT width and height to 0? Does it mean
> > > that gstreamer doesn't work with coda and mtk-vcodec, which don't have
> > > such wait in their g_fmt implementations?
> >
> > I don't know for MTK, I don't have the hardware and didn't integrate
> > their vendor pixel format. For the CODA, I know it works, if there is
> > no wait in the G_FMT, then I suppose we are being really lucky with the
> > timing (it would be that the drivers process the SPS/PPS synchronously,
> > and a simple lock in the G_FMT call is enough to wait). Adding Philipp
> > in CC, he could explain how this works, I know they use GStreamer in
> > production, and he would have fixed GStreamer already if that was
> > causing important issue.
>
> CODA predates the width/height=0 rule on the coded/OUTPUT queue.
> It currently behaves more like a traditional mem2mem device.

The rule in the latest spec is that if width/height is 0 then CAPTURE
format is determined only after the stream is parsed. Otherwise it's
instantly deduced from the OUTPUT resolution.

>
> When width/height is set via S_FMT(OUT) or output crop selection, the
> driver will believe it and set the same (rounded up to macroblock
> alignment) on the capture queue without ever having seen the SPS.

That's why I asked whether gstreamer sets width and height of OUTPUT
to non-zero values. If so, there is no regression, as the specs mimic
the coda behavior.

>
> The source change event after SPS parsing that the spec requires isn't
> even implemented yet.
>
> regards
> Philipp