Hello Hans,
On Mon, 18 Nov 2019 14:06:40 +0100
Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> Here is a proposal for a new VIDIOC_DESTROY_BUFS ioctl:
Thanks for sending this RFC.
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index c7c1179eea65..1a80d1119768 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2423,6 +2423,19 @@ struct v4l2_create_buffers {
> __u32 reserved[7];
> };
>
> +/**
> + * struct v4l2_destroy_buffers - VIDIOC_DESTROY_BUFS argument
> + * @type: stream type
> + * @index: index of the first buffer to destroy
> + * @count: number of consecutive buffers starting from @index to destroy
> + */
> +struct v4l2_destroy_buffers {
> + __u32 type;
> + __u32 index;
> + __u32 count;
> +};
> +
> +
> /*
> * I O C T L C O D E S F O R V I D E O D E V I C E S
> *
> @@ -2522,6 +2535,7 @@ struct v4l2_create_buffers {
> #define VIDIOC_DBG_G_CHIP_INFO _IOWR('V', 102, struct v4l2_dbg_chip_info)
>
> #define VIDIOC_QUERY_EXT_CTRL _IOWR('V', 103, struct v4l2_query_ext_ctrl)
> +#define VIDIOC_DESTROY_BUFS _IOW ('V', 104, struct v4l2_destroy_buffers)
>
> /* Reminder: when adding new ioctls please add support for them to
> drivers/media/v4l2-core/v4l2-compat-ioctl32.c as well! */
>
>
>
> So this basically just destroys buffers [index..index+count-1]. Does nothing if
> count == 0. All buffers in the sequence must be dequeued or it will return
> -EBUSY and do nothing.
>
> If some of the buffers in that range are already destroyed, or in fact were
> never created, then they will be ignored. I.e., DESTROY_BUFS won't return
> an error in that case.
Sounds good to me.
>
>
> VIDIOC_CREATE_BUFS will need a few changes:
>
> CREATE_BUFS will try to find a range of <count> free consecutive buffers.
> If that's not available, then it will reduce <count> to the count of the
> maximum freely available consecutive buffers. If <count> is 0, then it
> will set <index> to the maximum index of an existing buffer + 1.
>
> As long as DESTROY_BUFS isn't used, then CREATE_BUFS acts exactly the same
> as it does today.
Sounds good too.
>
> I would also like to extend struct v4l2_create_buffers with a new field:
> __u32 max_index. This is a maximum index possible, typically VIDEO_MAX_FRAME-1.
Shouldn't max_buffers be a property of the queue, set through a separate
ioctl()? BTW, how would you decrease the queue depth?
CREATE_BUFS.{count=0,max_index=<new-depth>}?
Thanks,
Boris
