From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Dufresne Subject: Re: [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API Date: Thu, 31 Jan 2019 10:18:17 -0500 Message-ID: <1f8485785a21c0b0e071a3a766ed2cbc727e47f6.camel@ndufresne.ca> References: <20190117162008.25217-1-stanimir.varbanov@linaro.org> <20190117162008.25217-11-stanimir.varbanov@linaro.org> <28069a44-b188-6b89-2687-542fa762c00e@linaro.org> <57419418d377f32d0e6978f4e4171c0da7357cbb.camel@ndufresne.ca> <1548938556.4585.1.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-FzoxvhNtcf6eebpqy6W3" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Tomasz Figa , Philipp Zabel Cc: Stanimir Varbanov , Linux Media Mailing List , Mauro Carvalho Chehab , Hans Verkuil , Linux Kernel Mailing List , linux-arm-msm , Vikash Garodia , Alexandre Courbot , Malathi Gottam List-Id: linux-arm-msm@vger.kernel.org --=-FzoxvhNtcf6eebpqy6W3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Le jeudi 31 janvier 2019 =C3=A0 22:34 +0900, Tomasz Figa a =C3=A9crit : > On Thu, Jan 31, 2019 at 9:42 PM Philipp Zabel wr= ote: > > Hi Nicolas, > >=20 > > On Wed, 2019-01-30 at 10:32 -0500, Nicolas Dufresne wrote: > > > Le mercredi 30 janvier 2019 =C3=A0 15:17 +0900, Tomasz Figa a =C3=A9c= rit : > > > > > I don't remember saying that, maybe I meant to say there might be= a > > > > > workaround ? > > > > >=20 > > > > > For the fact, here we queue the headers (or first frame): > > > > >=20 > > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/blob/ma= ster/sys/v4l2/gstv4l2videodec.c#L624 > > > > >=20 > > > > > Then few line below this helper does G_FMT internally: > > > > >=20 > > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/blob/ma= ster/sys/v4l2/gstv4l2videodec.c#L634 > > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/blob/ma= ster/sys/v4l2/gstv4l2object.c#L3907 > > > > >=20 > > > > > And just plainly fails if G_FMT returns an error of any type. Thi= s was > > > > > how Kamil designed it initially for MFC driver. There was no othe= r > > > > > alternative back then (no EAGAIN yet either). > > > >=20 > > > > Hmm, was that ffmpeg then? > > > >=20 > > > > 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 h= ave > > > > such wait in their g_fmt implementations? > > >=20 > > > 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 t= he > > > timing (it would be that the drivers process the SPS/PPS synchronousl= y, > > > and a simple lock in the G_FMT call is enough to wait). Adding Philip= p > > > 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. > >=20 > > CODA predates the width/height=3D0 rule on the coded/OUTPUT queue. > > It currently behaves more like a traditional mem2mem device. >=20 > 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. >=20 > > 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. >=20 > 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. I see, with Philipp's answer it explains why it works. Note that GStreamer sets the display size on the OUTPUT format (in fact we pass as much information as we have, because a) it's generic code and b) it will be needed someday when we enable pre-allocation (REQBUFS before SPS/PPS is passed, to avoid the setup delay introduce by allocation, mostly seen with CMA base decoder). In any case, the driver reported display size should always be ignored in GStreamer, the only information we look at is the G_SELECTION for the case the x/y or the cropping rectangle is non-zero. Note this can only work if the capture queue is not affected by the coded size, or if the round-up made by the driver is bigger or equal to that coded size. I believe CODA falls into the first category, since the decoding happens in a separate set of buffers and are then de-tiled=20 into the capture buffers (if understood correctly). I would say, best is just to test the updated Venus driver, which is in my queue. >=20 > > The source change event after SPS parsing that the spec requires isn't > > even implemented yet. Just to make sure, if I try and register that event on CODA with the current driver, it will simply fail immediately right ? I don't need any other magic to detect that this isn't supported ? > >=20 > > regards > > Philipp --=-FzoxvhNtcf6eebpqy6W3 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iF0EABECAB0WIQSScpfJiL+hb5vvd45xUwItrAaoHAUCXFMRuQAKCRBxUwItrAao HNHrAJ44pwdbaPbPYPgDWNbhxMRrBAqkQACfeSfsnTuENX53E5nD9wBJl+8UJS4= =seHL -----END PGP SIGNATURE----- --=-FzoxvhNtcf6eebpqy6W3--