Re: [RFC] media: v4l2: Variable length controls (for h265 slice headers)

[Hans Verkuil <hverkuil@xs4all.nl>]

Hi John,

On 23/03/2021 15:12, John Cox wrote:
> Hi all
> 
> I am developing a H265 V4L2 stateless decoder. After some
> experimentation it looks like the best way to achieve performance would
> be to submit bitstream data for an entire frame in a single buffer with
> an array of slice headers that point into it. The number of slices in a
> frame can be very variable, often there will be just one, in nearly all
> cases there will be less than 16 but the worst case could be hundreds
> (actually theoretically it could be thousands but I'm prepared to, and
> it is probably sensible to, reject any stream that looks like this).
> 
> Given the large range of possible array sizes a (large) fixed length
> array is very wasteful and probably slow in nearly all cases. As it
> stands V4L2 has no variable length structure so there is a problem here.
> My experience with v4l2 controls in minimal so trying to add a variable
> length array control myself seems brave.  Luckily (in other channels) I
> was told "Hans offered multiple times to implement variable array
> controls himself, he just needs someone to send an RFC with details on
> what's needed." so here I am.
> 
> So as a suggestion for the interface:
> 
> From the user point of view:
> 
> Only the last dimension of the array can be dynamic (like a C array "int
> a[10][15][];")  Otherwise we add a lot of complexity.

I would limit this to single dimensional arrays for now.

I think it is the first (not the last) dimension that can be dynamic: if
you want 8 3x3 matrices, then you would set dims to [8][3][3]. So a variable
length array of 3x3 matrices would have the first dimension as the variable
one.

> 
> VIDIOC_S_EXT_CTRLS
> In v4l2_ext_control the user can pass in any size that is a multiple of
> the element size.  If greater than the max then .size is set to the max
> by the ioctl on return.
> 
> VIDIOC_G_EXT_CTRLS
> On entry .size contains the buffer size to receive the values and on
> return it contains the size actually wanted - if the buffer can contain
> the data then it is also the number filled in.
> 
> VIDIOC_QUERY_EXT_CTRLS
> Add a flag to indicate variable length and either use .maximum/.minimum
> or some of the currently reserved structure to give max/min sizes

dims[0] can set the maximum size of the array.

> 
> From the driver point of view - frankly anything will do as long as I
> can find out how many headers I have. I think it is probably a good idea
> to dynamically allocate the storage for such an array rather than having
> a fixed size block on the end of the ctrl structure to avoid unnecessary
> overallocation.

The hevc slice structures are quite large, so it definitely has to be a
dynamic allocation.

> 
> I imagine that I've missed many important details in the sketch above,
> but probably good to start the discussion and Hans, am I trying to take
> you up on an offer you didn't actually make?

So the uAPI part is fairly simple, the biggest problem is in the internal
implementation. As the control framework becomes ever more complex (esp.
with the requests support) it is getting harder and harder to add new features.

I think that this might be a good time to start refactoring code, but for
that I also need to add better testing in v4l2-compliance of esp. requests.

It will also make it more time consuming, but I don't feel comfortable
to continue hacking on the code without doing a cleanup first.

Regards,

	Hans