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

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

[John Cox]

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.

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

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.

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?

Regards

John Cox

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

[Hans Verkuil]

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

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

[John Cox]

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