From: Hans Verkuil <hansverk@cisco.com>
To: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
Sakari Ailus <sakari.ailus@iki.fi>,
Hans Verkuil <hverkuil@xs4all.nl>,
Pawel Osciak <pawel@osciak.com>,
Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Mauro Carvalho Chehab <mchehab@infradead.org>
Subject: Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
Date: Mon, 8 Aug 2011 11:16:41 +0200 [thread overview]
Message-ID: <201108081116.41126.hansverk@cisco.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1108050908590.26715@axis700.grange>
Hi Guennadi!
On Friday, August 05, 2011 09:47:13 Guennadi Liakhovetski wrote:
> A possibility to preallocate and initialise buffers of different sizes
> in V4L2 is required for an efficient implementation of asnapshot mode.
> This patch adds two new ioctl()s: VIDIOC_CREATE_BUFS and
> VIDIOC_PREPARE_BUF and defines respective data structures.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> v4:
>
> 1. CREATE_BUFS now takes an array of plane sizes and a fourcc code in its
> argument, instead of a frame format specification, including
> documentation update
> 2. documentation improvements, as suggested by Hans
> 3. increased reserved fields to 18, as suggested by Sakari
>
> Documentation/DocBook/media/v4l/io.xml | 17 ++
> Documentation/DocBook/media/v4l/v4l2.xml | 2 +
> .../DocBook/media/v4l/vidioc-create-bufs.xml | 161
++++++++++++++++++++
> .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++
> drivers/media/video/v4l2-compat-ioctl32.c | 6 +
> drivers/media/video/v4l2-ioctl.c | 26 +++
> include/linux/videodev2.h | 18 +++
> include/media/v4l2-ioctl.h | 2 +
> 8 files changed, 328 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
> create mode 100644 Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml
>
<snip>
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index fca24cc..3cd0cb3 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -653,6 +653,9 @@ struct v4l2_buffer {
> #define V4L2_BUF_FLAG_ERROR 0x0040
> #define V4L2_BUF_FLAG_TIMECODE 0x0100 /* timecode field is valid */
> #define V4L2_BUF_FLAG_INPUT 0x0200 /* input field is valid */
> +/* Cache handling flags */
> +#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0400
> +#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x0800
>
> /*
> * O V E R L A Y P R E V I E W
> @@ -2092,6 +2095,18 @@ struct v4l2_dbg_chip_ident {
> __u32 revision; /* chip revision, chip specific */
> } __attribute__ ((packed));
>
> +/* VIDIOC_CREATE_BUFS */
> +struct v4l2_create_buffers {
> + __u32 index; /* output: buffers index...index + count - 1 have been
created */
> + __u32 count;
> + __u32 type;
> + __u32 memory;
> + __u32 fourcc;
> + __u32 num_planes;
> + __u32 sizes[VIDEO_MAX_PLANES];
> + __u32 reserved[18];
> +};
I know you are going to hate me for this, but I've changed my mind: I think
this should use a struct v4l2_format after all.
This change of heart came out of discussions during the V4L2 brainstorm
meeting last week. The only way to be sure the buffers are allocated optimally
is if the driver has all the information. The easiest way to do that is by
passing struct v4l2_format. This is also consistent with REQBUFS since that
uses the information from the currently selected format (i.e. what you get
back from VIDIOC_G_FMT).
There can be subtle behaviors such as allocating from different memory back
based on the fourcc and the size of the image.
One reason why I liked passing sizes directly is that it allows the caller to
ask for more memory than is strictly necessary.
However, while brainstorming last week the suggestion was made that there is
no reason why the user can't set the sizeimage field in
v4l2_pix_format(_mplane) to something higher. The S/TRY_FMT spec explicitly
mentions that the sizeimage field is set by the driver, but for the new
CREATEBUFS ioctl no such limitation has to be placed. The only thing necessary
is to ensure that sizeimage is not too small (and you probably want some
sanity check against crazy values as well).
This way the decision on how to allocate memory is the same between REQBUFS
and CREATEBUFS (i.e. both use v4l2_format information), but there is no need
for a union as we had in the initial proposal since apps can set the sizeimage
to something larger than strictly necessary (or just leave it to 0 to get the
smallest size).
Regards,
Hans
> +
> /*
> * 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
> *
> @@ -2182,6 +2197,9 @@ struct v4l2_dbg_chip_ident {
> #define VIDIOC_SUBSCRIBE_EVENT _IOW('V', 90, struct
v4l2_event_subscription)
> #define VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91, struct
v4l2_event_subscription)
>
> +#define VIDIOC_CREATE_BUFS _IOWR('V', 92, struct v4l2_create_buffers)
> +#define VIDIOC_PREPARE_BUF _IOW('V', 93, struct v4l2_buffer)
> +
> /* Reminder: when adding new ioctls please add support for them to
> drivers/media/video/v4l2-compat-ioctl32.c as well! */
>
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index dd9f1e7..4d1c74a 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -122,6 +122,8 @@ struct v4l2_ioctl_ops {
> int (*vidioc_qbuf) (struct file *file, void *fh, struct v4l2_buffer
*b);
> int (*vidioc_dqbuf) (struct file *file, void *fh, struct v4l2_buffer
*b);
>
> + int (*vidioc_create_bufs)(struct file *file, void *fh, struct
v4l2_create_buffers *b);
> + int (*vidioc_prepare_buf)(struct file *file, void *fh, struct v4l2_buffer
*b);
>
> int (*vidioc_overlay) (struct file *file, void *fh, unsigned int i);
> int (*vidioc_g_fbuf) (struct file *file, void *fh,
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
next prev parent reply other threads:[~2011-08-08 9:19 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-05 7:47 [PATCH 0/6 v4] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management Guennadi Liakhovetski
2011-08-06 18:42 ` Sakari Ailus
2011-08-17 8:41 ` Guennadi Liakhovetski
2011-08-17 12:13 ` Sakari Ailus
2011-08-06 21:51 ` Pawel Osciak
2011-08-08 9:16 ` Hans Verkuil [this message]
2011-08-08 11:40 ` Laurent Pinchart
2011-08-08 12:40 ` Hans Verkuil
2011-08-08 22:06 ` Laurent Pinchart
2011-08-08 22:46 ` Sakari Ailus
2011-08-09 7:26 ` Hans Verkuil
2011-08-09 23:37 ` Sakari Ailus
2011-08-10 6:25 ` Hans Verkuil
2011-08-11 11:09 ` Sakari Ailus
2011-08-15 11:28 ` Guennadi Liakhovetski
2011-08-15 11:36 ` Hans Verkuil
2011-08-15 13:45 ` Guennadi Liakhovetski
2011-08-16 13:13 ` Guennadi Liakhovetski
2011-08-16 16:14 ` Pawel Osciak
2011-08-17 9:11 ` Guennadi Liakhovetski
2011-08-17 9:14 ` Laurent Pinchart
2011-08-17 15:29 ` Pawel Osciak
2011-08-22 10:06 ` Hans Verkuil
2011-08-22 10:40 ` Guennadi Liakhovetski
2011-08-22 11:16 ` Hans Verkuil
2011-08-22 13:54 ` Guennadi Liakhovetski
2011-08-22 14:01 ` Hans Verkuil
2011-08-22 15:42 ` Laurent Pinchart
2011-08-22 15:52 ` Hans Verkuil
2011-08-22 17:21 ` Laurent Pinchart
2011-08-23 6:31 ` Hans Verkuil
2011-08-24 4:05 ` Pawel Osciak
2011-08-24 6:44 ` Hans Verkuil
2011-08-17 13:22 ` Marek Szyprowski
2011-08-17 14:57 ` Pawel Osciak
2011-08-18 5:49 ` Marek Szyprowski
2011-08-05 7:47 ` [PATCH 2/6 v4] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 3/6 v4] V4L: vb2: change .queue_setup() argument to unsigned int Guennadi Liakhovetski
2011-08-05 21:36 ` [PATCH 3/6 v5] V4L: vb2: prepare to support multi-size buffers Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 4/6 v4] V4L: vb2: add support for buffers of different sizes on a single queue Guennadi Liakhovetski
2011-08-06 18:56 ` Sakari Ailus
2011-08-17 8:44 ` Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 5/6 v4] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers Guennadi Liakhovetski
2011-08-05 21:39 ` [PATCH 5/6 v5] " Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 6/6 v4] V4L: soc-camera: add 2 new ioctl() handlers Guennadi Liakhovetski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201108081116.41126.hansverk@cisco.com \
--to=hansverk@cisco.com \
--cc=g.liakhovetski@gmx.de \
--cc=hverkuil@xs4all.nl \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@infradead.org \
--cc=pawel@osciak.com \
--cc=sakari.ailus@iki.fi \
--cc=sakari.ailus@maxwell.research.nokia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.