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: <18f834cb4885357ce6274115096911d90b9c17ea.camel@ndufresne.ca>
Subject: Re: [PATCH v2 00/11] Venus stateful Codec API
From:   Nicolas Dufresne <nicolas@ndufresne.ca>
To:     Hans Verkuil <hverkuil@xs4all.nl>,
        Stanimir Varbanov <stanimir.varbanov@linaro.org>,
        linux-media@vger.kernel.org
Cc:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        Vikash Garodia <vgarodia@codeaurora.org>,
        Tomasz Figa <tfiga@chromium.org>,
        Alexandre Courbot <acourbot@chromium.org>
Date:   Fri, 28 Jun 2019 10:31:50 -0400
In-Reply-To: <eba506ce-4d63-649a-80e6-efad20125263@xs4all.nl>
References: <20190628130002.24293-1-stanimir.varbanov@linaro.org>
         <9c3399a8-4fc6-3117-10ee-3395cee034da@xs4all.nl>
         <997204c7-c702-868c-9a49-52fefc9ab3d2@linaro.org>
         <eba506ce-4d63-649a-80e6-efad20125263@xs4all.nl>
Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature";
        boundary="=-7GNINHKy+QfnqJkWj32i"
User-Agent: Evolution 3.32.2 (3.32.2-1.fc30) 
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--=-7GNINHKy+QfnqJkWj32i
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Le vendredi 28 juin 2019 =C3=A0 16:25 +0200, Hans Verkuil a =C3=A9crit :
> On 6/28/19 4:23 PM, Stanimir Varbanov wrote:
> > Hi Hans,
> >=20
> > On 6/28/19 4:37 PM, Hans Verkuil wrote:
> > > On 6/28/19 2:59 PM, Stanimir Varbanov wrote:
> > > > Hello,
> > > >=20
> > > > Here is v2 of the Venus transition to stateful codec API
> > > > compliance. The v2 can be found at [1].
> > > >=20
> > > > Changes since v1:
> > > >  * codec_state is now enum
> > > >  * dropped IS_OUT and IS_CAP macros and use vb2_start_streaming_cal=
led()
> > > >  * corrected g_fmt and reconfig logic
> > > >  * s/vdec_dst_buffers_done/vdec_cancel_dst_buffers
> > > >  * use v4l2_m2m_ioctl_try_decoder_cmd M2M helper
> > > >  * various fixes to make v4l2-compliance pass the streaming test
> > > >=20
> > > > To test the streaming with --stream-from-hdr v4l2-compliance option=
 I have
> > > > to make the following hack (it is needed because the size of decode=
r input
> > > > buffers (OUTPUT queue) is not enough for the h264 bitstream, i.e th=
e driver
> > > > default resolution is 64x64 but the h264 stream is 320x240):
> > > >=20
> > > > diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4=
l2-compliance/v4l2-test-buffers.cpp
> > > > index c71dcf65b721..dc0fcf20d3e4 100644
> > > > --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> > > > +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> > > > @@ -1294,6 +1294,11 @@ int testMmap(struct node *node, unsigned fra=
me_count, enum poll_mode pollmode)
> > > >                                         fmt.s_sizeimage(fmt.g_sizei=
mage(p) * 2, p);
> > > >                         }
> > > >                         fail_on_test(q.create_bufs(node, 1, &fmt));
> > > > +
> > > > +                       for (unsigned p =3D 0; p < fmt.g_num_planes=
(); p++)
> > > > +                               fmt.s_sizeimage(fmt.g_sizeimage(p) =
* 2, p);
> > > > +                       node->s_fmt(fmt);
> > > > +
> > > >                         fail_on_test(q.reqbufs(node, 2));
> > > >                 }
> > > >                 if (v4l_type_is_output(type))
> > >=20
> > > Does the venus driver set sizeimage based on the given output resolut=
ion?
> >=20
> > Yes.
> >=20
> > > E.g. if v4l2-compliance would first set the output resolution to 320x=
240,
> > > is the returned sizeimage value OK in that case?
> >=20
> > Yes.
> >=20
> > Here are few options to me:
> >  - set the correct resolution
> >  - set 0x0 and sizeimage at some arbitrary value (1 or 2MB). Despite if
> > the bitstream is 4K it will not be enough if the bitrate is huge.
> >  - invent some mechanism to trigger reconfiguration on the OUTPUT queue
> > as well (similar to the CAPTURE queue)
> >=20
> > > And this also means that the venus driver requires each buffer to hav=
e
> > > a single compressed frame, right? I.e. it can't be spread over multip=
le
> > > OUTPUT buffers.
> >=20
> > I cannot say for sure but that is how all downstream cases uses it i.e.
> > one compressed frame per input buffer. I wonder if you fill input
> > decoder buffer with many compressed frames in one input decoder buffer
> > how you pass the timestamp for every packet?
> >=20
> > > We really need to let userspace know about such restrictions.
> > >=20
> > > Stanimir, can you list the restrictions of the decoder for the variou=
s
> > > codecs?
> >=20
> > What you mean? Restrictions like "one compressed frame per input buffer=
"?
> >=20
>=20
> Yes :-)

I think I just had the same discussions through some RPi patches
reviews. All the stateless codec drivers we have so far assumes full
frames and some wording iirc specified this in the spec at some point.
That removes ambiguity for timestamps application. Even though,
timestamps in V4L2 are useless for B-Frame enabled streams (ffmpeg and
GStreamer will ignore them).

Nicolas

--=-7GNINHKy+QfnqJkWj32i
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+hb5vvd45xUwItrAaoHAUCXRYk1gAKCRBxUwItrAao
HEd7AJoD6UTL2Ch2lIuU3By7HzfYUqaOmgCgu6kMeEhOmohXcnAqEsjr9N8otzU=
=y1YL
-----END PGP SIGNATURE-----

--=-7GNINHKy+QfnqJkWj32i--