Re: [PATCH v2 00/11] Venus stateful Codec API

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- Attachment #1: Type: text/plain, Size: 4151 bytes --]

Le vendredi 28 juin 2019 à 10:31 -0400, Nicolas Dufresne a écrit :
> Le vendredi 28 juin 2019 à 16:25 +0200, Hans Verkuil a écrit :
> > On 6/28/19 4:23 PM, Stanimir Varbanov wrote:
> > > Hi Hans,
> > > 
> > > On 6/28/19 4:37 PM, Hans Verkuil wrote:
> > > > On 6/28/19 2:59 PM, Stanimir Varbanov wrote:
> > > > > Hello,
> > > > > 
> > > > > Here is v2 of the Venus transition to stateful codec API
> > > > > compliance. The v2 can be found at [1].
> > > > > 
> > > > > Changes since v1:
> > > > >  * codec_state is now enum
> > > > >  * dropped IS_OUT and IS_CAP macros and use vb2_start_streaming_called()
> > > > >  * 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
> > > > > 
> > > > > 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 decoder input
> > > > > buffers (OUTPUT queue) is not enough for the h264 bitstream, i.e the driver
> > > > > default resolution is 64x64 but the h264 stream is 320x240):
> > > > > 
> > > > > diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-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 frame_count, enum poll_mode pollmode)
> > > > >                                         fmt.s_sizeimage(fmt.g_sizeimage(p) * 2, p);
> > > > >                         }
> > > > >                         fail_on_test(q.create_bufs(node, 1, &fmt));
> > > > > +
> > > > > +                       for (unsigned p = 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))
> > > > 
> > > > Does the venus driver set sizeimage based on the given output resolution?
> > > 
> > > Yes.
> > > 
> > > > E.g. if v4l2-compliance would first set the output resolution to 320x240,
> > > > is the returned sizeimage value OK in that case?
> > > 
> > > Yes.
> > > 
> > > 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)
> > > 
> > > > And this also means that the venus driver requires each buffer to have
> > > > a single compressed frame, right? I.e. it can't be spread over multiple
> > > > OUTPUT buffers.
> > > 
> > > 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?
> > > 
> > > > We really need to let userspace know about such restrictions.
> > > > 
> > > > Stanimir, can you list the restrictions of the decoder for the various
> > > > codecs?
> > > 
> > > What you mean? Restrictions like "one compressed frame per input buffer"?
> > > 
> > 
> > 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).

Sorry, I mean they are not used as timestamp, only for frame matching.

> 
> Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]