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

[Nicolas Dufresne]

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).

Nicolas

Attachment: signature.asc
Description: This is a digitally signed message part