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

From: Philipp Zabel <p.zabel@pengutronix.de>
To: Nicolas Dufresne <nicolas@ndufresne.ca>,
	Tomasz Figa <tfiga@chromium.org>
Cc: 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 13:42:36 +0100	[thread overview]
Message-ID: <1548938556.4585.1.camel@pengutronix.de> (raw)
In-Reply-To: <57419418d377f32d0e6978f4e4171c0da7357cbb.camel@ndufresne.ca>

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.

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.

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

regards
Philipp