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

[John Cox <jc@kynesim.co.uk>]

Hi Hans

Thanks for looking at this

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

I would like to say that you have spotted my deliberate mistake, but no
- I was just wrong and you are right.

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

Sounds good and has obvious extensions for (later) multi-dimensional
stuff.

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

I look forward to our brave new cleaner dynamic world :-)

Many thanks

John Cox