Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <a7c72589f55594069160afbf94613d322b55470a.camel@ndufresne.ca>
Subject: Re: [EXT] Re: [PATCH 0/5] Add V4L2 driver for i.MX8 JPEG
 Encoder/Decoder
From:   Nicolas Dufresne <nicolas@ndufresne.ca>
To:     Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Mirela Rabulea <mirela.rabulea@nxp.com>,
        "mchehab@kernel.org" <mchehab@kernel.org>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>
Cc:     dl-linux-imx <linux-imx@nxp.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "laurent.pinchart+renesas@ideasonboard.com" 
        <laurent.pinchart+renesas@ideasonboard.com>,
        Aisheng Dong <aisheng.dong@nxp.com>,
        Laurentiu Palcu <laurentiu.palcu@nxp.com>,
        "paul.kocialkowski@bootlin.com" <paul.kocialkowski@bootlin.com>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
        Robert Chiras <robert.chiras@nxp.com>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
        "niklas.soderlund+renesas@ragnatech.se" 
        <niklas.soderlund+renesas@ragnatech.se>,
        Daniel Baluta <daniel.baluta@nxp.com>,
        Leonard Crestez <leonard.crestez@nxp.com>,
        "dafna.hirschfeld@collabora.com" <dafna.hirschfeld@collabora.com>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>
Date:   Mon, 11 Nov 2019 14:27:17 -0500
In-Reply-To: <ffbc7501-bb56-b5f2-cd43-209f099be594@xs4all.nl>
References: <1573053633-21437-1-git-send-email-mirela.rabulea@nxp.com>
         <87d80882-b032-f332-0808-d669600f844f@xs4all.nl>
         <1573058741.16477.18.camel@nxp.com>
         <2ad1718b-2556-ea7c-27e0-9e60445a5b71@xs4all.nl>
         <1573220200.16477.32.camel@nxp.com>
         <ffbc7501-bb56-b5f2-cd43-209f099be594@xs4all.nl>
Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature";
        boundary="=-VUH0YJwbahd5gI0MVm8e"
User-Agent: Evolution 3.32.4 (3.32.4-1.fc30) 
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--=-VUH0YJwbahd5gI0MVm8e
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Le vendredi 08 novembre 2019 =C3=A0 15:06 +0100, Hans Verkuil a =C3=A9crit =
:
> On 11/8/19 2:36 PM, Mirela Rabulea wrote:
> > Hi Hans,
> >=20
> > On Mi, 2019-11-06 at 17:52 +0100, Hans Verkuil wrote:
> > >       test Scaling: OK
> > > The presence of a scaler is suspicious: is the encoder indeed
> > > capable of scaling? I suspect this is a bug.
> >=20
> > No, it's not capable of scaling. You suspect a bug in the driver or the
> > tests?
>=20
> Actually, I think that's an outstanding bug in v4l-utils. It doesn't
> correctly handle the m2m case with respect to scaling. I think. I'll
> look into this a bit more.
>=20
> > > Codec ioctls:
> > > >       test VIDIOC_(TRY_)ENCODER_CMD: OK
> > > The presence of this...
> > >=20
> > > >       test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> > > >       test VIDIOC_(TRY_)DECODER_CMD: OK
> > > ...and this is also strange for a JPEG codec. These ioctls are
> > > typically only
> > > needed for MPEG/H264/etc. codecs, and not for a simple JPEG codec.
> > >=20
> > > The same issues are found for the JPEG decoder.
> >=20
> > I implemented the CMD_STOP for both encoder & decoder, because it was
> > requested by our developer for gstreamer plugin for this codec.
> > The context in which this was requested was for playing MJPEG videos (a
> > concatenation of JPEG frames).
>=20
> This ioctl makes no sense for JPEG codecs, and in fact jpeg drivers like
> s5p-jpeg or mtk-jpeg do not implement this. This sounds like a gstreamer =
bug.
> Nicolas, do you know anything about this?

This code was added in generic bits of GStreamer. So we just always do
CMD_STOP to drain the remaining buffers and expect EPIPE later on when
done. We can special case this, but at the same time why ? This could
be buried into the m2m helpers with a default implementation.

If you feel like this is completely wrong and userspace should do more
case-by-case code path for different CODEC handling, then someone will
have to update GStreamer accordingly. This is all moving target at the
spec just got merged.

>=20
> > > Streaming ioctls:
> > > >       test read/write: OK (Not Supported)
> > > >       test blocking wait: OK
> > > >               fail: v4l2-test-buffers.cpp(254): g_field() =3D=3D
> > > > V4L2_FIELD_ANY
> > > The driver shall never return FIELD_ANY. This needs to be FIELD_NONE.
> >=20
> > Is there a "good example" of a v4l m2m driver that passes these
>=20
> vim2m. Also drivers/media/platform/mtk-jpeg/ (although I'm not sure when
> it was last tested with v4l2-compliance, so it might be a bit out of date=
).
>=20
> > streaming tests? That would save some time on my side.
> > For the FIELD_ANY issue, I got inspired from your commit:
> > ab7afaf3 media: vim2m: add buf_out_validate callback
> > But there's a lot more to go...
> >=20
> > Thanks,
> > Mirela
> >=20
>=20
> Regards,
>=20
> 	Hans

--=-VUH0YJwbahd5gI0MVm8e
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+hb5vvd45xUwItrAaoHAUCXcm2FQAKCRBxUwItrAao
HH/yAJ4i/zn4lovHANSysp2/fBJ+90qJgQCgp6bTO443f6uWk4bazz8NHgVEmy0=
=zbkH
-----END PGP SIGNATURE-----

--=-VUH0YJwbahd5gI0MVm8e--