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

From: Tomasz Figa <tfiga@chromium.org>
To: Nicolas Dufresne <nicolas@ndufresne.ca>
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: Wed, 30 Jan 2019 15:17:46 +0900	[thread overview]
Message-ID: <CAAFQd5DFfQRd1VoN7itVXnWGKW_WBKU-sm6vo5CdgjkzjEEkFg@mail.gmail.com> (raw)
In-Reply-To: <e8a90694c306fde24928a569b7bcb231b86ec73b.camel@ndufresne.ca>

On Wed, Jan 30, 2019 at 1:21 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le mercredi 30 janvier 2019 à 12:38 +0900, Tomasz Figa a écrit :
> > > Yes, unfortunately, GStreamer still rely on G_FMT waiting a minimal
> > > amount of time of the headers to be processed. This was how things was
> > > created back in 2011, I could not program GStreamer for the future. If
> > > we stop doing this, we do break GStreamer as a valid userspace
> > > application.
> >
> > Does it? Didn't you say earlier that you end up setting the OUTPUT
> > format with the stream resolution as parsed on your own? If so, that
> > would actually expose a matching framebuffer format on the CAPTURE
> > queue, so there is no need to wait for the real parsing to happen.
>
> 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?

>
> Nicolas
>
> p.s. it's still in my todo's to implement source change event as I
> believe it is a better mechanism (specially if you header happened to
> be corrupted, then the driver can consume the stream until it finds a
> sync). So these sleep or normally wait exist all over to support this
> legacy thing. It is unfortunate, the question is do you want to break
> userspace now ? Without having first placed a patch that would maybe
> warn or something for a while ?
>

I don't want and my understanding was that we could workaround it by
the propagation of format from OUTPUT to CAPTURE. Also see above.

Best regards,
Tomasz