[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6cb4ab4f-1132-f386-6dd0-bf4eeab4433c@collabora.com>
Date: Wed, 15 Jun 2022 12:18:19 +0200
From: Benjamin Gaignard <benjamin.gaignard@...labora.com>
To: Hans Verkuil <hverkuil@...all.nl>, mchehab@...nel.org,
ezequiel@...guardiasur.com.ar, p.zabel@...gutronix.de,
gregkh@...uxfoundation.org, mripard@...nel.org,
paul.kocialkowski@...tlin.com, wens@...e.org,
jernej.skrabec@...il.com, samuel@...lland.org,
nicolas.dufresne@...labora.com, andrzej.p@...labora.com
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-rockchip@...ts.infradead.org, linux-staging@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
kernel@...labora.com, Hans Verkuil <hverkuil-cisco@...all.nl>
Subject: Re: [PATCH v8 01/17] videodev2.h: add V4L2_CTRL_FLAG_DYNAMIC_ARRAY
Le 15/06/2022 à 11:33, Hans Verkuil a écrit :
> Hi Benjamin,
>
> On 6/14/22 10:35, Benjamin Gaignard wrote:
>> From: Hans Verkuil <hverkuil-cisco@...all.nl>
>>
>> Add a new flag that indicates that this control is a dynamically sized
>> array. Also document this flag.
>>
>> Currently dynamically sized arrays are limited to one dimensional arrays,
>> but that might change in the future if there is a need for it.
>>
>> The initial use-case of dynamic arrays are stateless codecs. A frame
>> can be divided in many slices, so you want to provide an array containing
>> slice information for each slice. Typically the number of slices is small,
>> but the standard allow for hundreds or thousands of slices. Dynamic arrays
>> are a good solution since sizing the array for the worst case would waste
>> substantial amounts of memory.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@...all.nl>
>> ---
>> .../userspace-api/media/v4l/vidioc-queryctrl.rst | 8 ++++++++
>> include/uapi/linux/videodev2.h | 1 +
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
>> index 88f630252d98..a20dfa2a933b 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
>> @@ -625,6 +625,14 @@ See also the examples in :ref:`control`.
>> ``V4L2_CTRL_FLAG_GRABBED`` flag when buffers are allocated or
>> streaming is in progress since most drivers do not support changing
>> the format in that case.
>> + * - ``V4L2_CTRL_FLAG_DYNAMIC_ARRAY``
>> + - 0x0800
>> + - This control is a dynamically sized 1-dimensional array. It
>> + behaves the same as a regular array, except that the number
>> + of elements as reported by the ``elems`` field is between 1 and
>> + ``dims[0]``. So setting the control with a differently sized
>> + array will change the ``elems`` field when the control is
>> + queried afterwards.
> I am proposing a change to the dynamic array implementation: initially
> dynamic array controls start off as empty arrays (0 elements). This also
> allows userspace to set a dynamic array control to an empty array.
>
> It probably would also make sense to add a min_dyn_elems to set the minimum
> allowed number of elements for a dynamic array. This would most likely be
> either 0 or 1.
>
> In the context of this HEVC series, does it help to allow empty dynamic arrays?
>
> For V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS in particular it looks like
> it would make sense since I think (correct me if I am wrong) there can be 0
> entry point offsets. So with empty arrays supported would the field
> num_entry_point_offsets still be needed?
yes it possible to have 0 entry point offsets in bitstream but in this case
I think the stack won't use V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS because
it have not data to send.
I have introduce num_entry_point_offsets field because most of the drivers
only need this information and not the full offsets array so I think it is needed.
For now none of the drivers use V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS
control, it is here to be future proof.
Regards,
Benjamin
>
> If you want to test, then let me know and I mail a very simple patch adding
> support for empty arrays (not yet min_dyn_elems, though).
>
> Regards,
>
> Hans
>
>>
>> Return Value
>> ============
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 5311ac4fde35..9018aa984db3 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1890,6 +1890,7 @@ struct v4l2_querymenu {
>> #define V4L2_CTRL_FLAG_HAS_PAYLOAD 0x0100
>> #define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE 0x0200
>> #define V4L2_CTRL_FLAG_MODIFY_LAYOUT 0x0400
>> +#define V4L2_CTRL_FLAG_DYNAMIC_ARRAY 0x0800
>>
>> /* Query flags, to be ORed with the control ID */
>> #define V4L2_CTRL_FLAG_NEXT_CTRL 0x80000000
Powered by blists - more mailing lists