[PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
allow user-space applications to allocate video buffers of different
sizes and hand them over to the driver for fast switching between
different frame formats. This patch adds support for buffers of different
sizes on the same buffer-queue to vb2.
Signed-off-by: Guennadi Liakhovetski
Cc: Hans Verkuil
Cc: Laurent Pinchart
Cc: Marek Szyprowski
Cc: Mauro Carvalho Chehab
Cc: Pawel Osciak
Cc: Sakari Ailus
---
drivers/media/video/videobuf2-core.c | 278 --
include/media/videobuf2-core.h | 31 +++--
2 files changed, 252 insertions(+), 57 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c
b/drivers/media/video/videobuf2-core.c
index 8a81a89..fed6f2d 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -44,7 +44,7 @@ module_param(debug, int, 0644);
* __vb2_buf_mem_alloc() - allocate video memory for the given buffer
*/
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb,
- unsigned long *plane_sizes)
+ const unsigned long *plane_sizes)
{
struct vb2_queue *q = vb->vb2_queue;
void *mem_priv;
@@ -110,13 +110,22 @@ static void __vb2_buf_userptr_put(struct vb2_buffer *vb)
* __setup_offsets() - setup unique offsets ("cookies") for every plane in
* every buffer on the queue
*/
-static void __setup_offsets(struct vb2_queue *q)
+static void __setup_offsets(struct vb2_queue *q, unsigned int n)
{
unsigned int buffer, plane;
struct vb2_buffer *vb;
- unsigned long off = 0;
+ unsigned long off;
- for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+ if (q->num_buffers) {
+ struct v4l2_plane *p;
+ vb = q->bufs[q->num_buffers - 1];
+ p = &vb->v4l2_planes[vb->num_planes - 1];
+ off = PAGE_ALIGN(p->m.mem_offset + p->length);
+ } else {
+ off = 0;
+ }
+
+ for (buffer = q->num_buffers; buffer < q->num_buffers + n; ++buffer) {
vb = q->bufs[buffer];
if (!vb)
continue;
@@ -142,7 +151,7 @@ static void __setup_offsets(struct vb2_queue *q)
*/
static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
unsigned int num_buffers, unsigned int num_planes,
-unsigned long plane_sizes[])
+const unsigned long *plane_sizes)
{
unsigned int buffer;
struct vb2_buffer *vb;
@@ -163,7 +172,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
v4l2_memory memory,
vb->state = VB2_BUF_STATE_DEQUEUED;
vb->vb2_queue = q;
vb->num_planes = num_planes;
- vb->v4l2_buf.index = buffer;
+ vb->v4l2_buf.index = q->num_buffers + buffer;
vb->v4l2_buf.type = q->type;
vb->v4l2_buf.memory = memory;
@@ -191,15 +200,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
v4l2_memory memory,
}
}
- q->bufs[buffer] = vb;
+ q->bufs[q->num_buffers + buffer] = vb;
}
- q->num_buffers = buffer;
-
- __setup_offsets(q);
+ __setup_offsets(q, buffer);
dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
- q->num_buffers, num_planes);
+ buffer, num_planes);
return buffer;
}
@@ -207,12 +214,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
v4l2_memory memory,
/**
* __vb2_free_mem() - release all video buffer memory for a given queue
*/
-static void __vb2_free_mem(struct vb2_queue *q)
+static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers)
{
unsigned int buffer;
struct vb2_buffer *vb;
- for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+ for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+++buffer) {
vb = q->bufs[buffer];
if (!vb)
continue;
@@ -226,17 +234,18 @@ static void __vb2_free_mem(struct vb2_queue *q)
}
/**
- * __vb2_queue_free() - free the queue - video memory and related information
- * and return the queue to an uninitialized state. Might be called even if the
- * queue has already been freed.
+ * __vb2_queue_free() - free buffers at the end of the queue - video memory and
+ * related information, if no buffers are left return the queue to an
+ * uninitialized state. Might be called even if the queue has already been
freed.
*/
-static void __vb2_queue_free(struct vb2_queue *q)
+static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
{
unsigned int buffer;
/* Call driver-provided cleanup function for each buffer, if provided */
if (q->o
Re: [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
Hi Pawel
On Mon, 29 Aug 2011, Guennadi Liakhovetski wrote:
> On Sun, 28 Aug 2011, Pawel Osciak wrote:
[snip]
> > Could you share the userspace code that you used for testing this? I
> > just wanted to get a feel of how those new ioctls fall into place
> > together.
>
> Theoretically - yes, just will have to clean up the code a bit;-) Will try
> to find some time for this in the next couple of days.
Below is a patch to the current capture-example.c. It is not extremely
intelligent, many things are hard-coded, but you should get the idea.
> > Also, did you try multiple CREATE_BUFS calls?
>
> Yes, it currently does two CREATE_BUFS, I might try to mix REQBUFS with
> CREATE_BUFS too though.
With this patch you can use the "-q" flag to choose, whether you want your
first buffers to be allocated per REQBUFS or CREATE_BUFS. Both options
work.
Locally I have an even much dirtier, but more useful version of this
patch, that draws one buffer size on the framebuffer, and stores the other
one in .pgm files, all works well.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
diff --git a/contrib/test/capture-example.c b/contrib/test/capture-example.c
index 417615a..ab0d8fb 100644
--- a/contrib/test/capture-example.c
+++ b/contrib/test/capture-example.c
@@ -38,14 +38,23 @@ struct buffer {
size_t length;
};
+enum {
+ false = 0,
+ true= 1
+};
+
static char*dev_name;
static enum io_method io = IO_METHOD_MMAP;
static int fd = -1;
struct buffer *buffers;
-static unsigned int n_buffers;
+static unsigned int n_buffers, n_buffers_alt;
static int out_buf;
static int force_format;
static int frame_count = 70;
+static unsigned intwidth, height, fourcc, colorspace;
+static unsigned intwidth_alt = 320, height_alt = 240,
+ fourcc_alt = V4L2_PIX_FMT_NV12, colorspace_alt = V4L2_COLORSPACE_JPEG;
+static _Bool use_request;
static void errno_exit(const char *s)
{
@@ -64,6 +73,61 @@ static int xioctl(int fh, unsigned long int request, void
*arg)
return r;
}
+static int create_buffers(unsigned int *width, unsigned int *height,
+ unsigned int fcc, unsigned int clrspc, int n)
+{
+ int i, ret;
+ struct v4l2_create_buffers create = {
+ .memory = V4L2_MEMORY_MMAP,
+ .count = n,
+ .format = {
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+ .fmt.pix = {
+ .width = *width,
+ .height = *height,
+ .pixelformat = fcc,
+ .field = V4L2_FIELD_ANY,
+ .colorspace = clrspc,
+ },
+ },
+ };
+ struct v4l2_buffer buf = {
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+ .memory = V4L2_MEMORY_MMAP,
+ .field = V4L2_FIELD_ANY,
+ };
+ struct v4l2_pix_format *pix = &create.format.fmt.pix;
+
+ ret = xioctl(fd, VIDIOC_TRY_FMT, &create.format);
+ if (ret < 0) {
+ fprintf(stderr, "TRY_FMT should never fail!: %d\n", ret);
+ errno_exit("VIDIOC_TRY_FMT");
+ }
+ fprintf(stderr, "TRY_FMT(%ux%u:%x) -> %u\n", pix->width, pix->height,
+pix->pixelformat, pix->sizeimage);
+
+ *width = pix->width;
+ *height = pix->height;
+
+ ret = xioctl(fd, VIDIOC_CREATE_BUFS, &create);
+ if (ret < 0)
+ errno_exit("VIDIOC_CREATE_BUFS");
+
+ fprintf(stderr, "CREATE_BUFS(%d@%d)\n", create.count, create.index);
+
+ for (i = create.index; i < create.index + create.count; i++) {
+ buf.index = i;
+ ret = xioctl(fd, VIDIOC_PREPARE_BUF, &buf);
+ if (ret < 0)
+ errno_exit("VIDIOC_PREPARE_BUF");
+ }
+
+ if (i > create.index)
+ return i - create.index;
+
+ return ret;
+}
+
static void process_image(const void *p, int size)
{
if (out_buf)
@@ -74,7 +138,7 @@ static void process_image(const void *p, int size)
fflush(stdout);
}
-static int read_frame(void)
+static int read_frame(_Bool do_queue)
{
struct v4l2_buffer buf;
unsigned int i;
@@ -120,11 +184,11 @@ static int read_frame(void)
}
}
- assert(buf.index < n_buffers);
+ assert(buf.index < n_buffers + n_buffers_alt);
process_image(buffers[buf.index].start, buf.bytesused);
- if (-1 == xioctl(fd, VIDIOC_QBUF, &buf))
+ if (do_queue && -1 == xioctl(fd, VIDIOC_QBUF, &buf))
errno_exit("VIDIOC_QBUF");
break;
@@ -154,11 +218,11 @@ static int read_frame(void)
Re: [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
Hi Guennadi,
On Wed, Aug 24, 2011 at 11:41, Guennadi Liakhovetski
wrote:
> The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
> allow user-space applications to allocate video buffers of different
> sizes and hand them over to the driver for fast switching between
> different frame formats. This patch adds support for buffers of different
> sizes on the same buffer-queue to vb2.
>
> Signed-off-by: Guennadi Liakhovetski
> Cc: Hans Verkuil
> Cc: Laurent Pinchart
> Cc: Marek Szyprowski
> Cc: Mauro Carvalho Chehab
> Cc: Pawel Osciak
> Cc: Sakari Ailus
> ---
> drivers/media/video/videobuf2-core.c | 278
> --
> include/media/videobuf2-core.h | 31 +++--
> 2 files changed, 252 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index 8a81a89..fed6f2d 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -454,7 +465,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
> */
> int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> {
> - unsigned int num_buffers, num_planes;
> + unsigned int num_buffers, allocated_buffers, num_planes = 0;
> unsigned long plane_sizes[VIDEO_MAX_PLANES];
> int ret = 0;
>
> @@ -503,7 +514,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
> return -EBUSY;
> }
>
> - __vb2_queue_free(q);
> + __vb2_queue_free(q, q->num_buffers);
>
> /*
> * In case of REQBUFS(0) return immediately without calling
> @@ -538,44 +549,166 @@ int vb2_reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
> return -ENOMEM;
> }
>
> + allocated_buffers = ret;
> +
> /*
> * Check if driver can handle the allocated number of buffers.
> */
> - if (ret < num_buffers) {
> - unsigned int orig_num_buffers;
> + if (allocated_buffers < num_buffers) {
> + num_buffers = allocated_buffers;
>
> - orig_num_buffers = num_buffers = ret;
> ret = call_qop(q, queue_setup, q, NULL, &num_buffers,
> &num_planes, plane_sizes, q->alloc_ctx);
> - if (ret)
> - goto free_mem;
>
> - if (orig_num_buffers < num_buffers) {
> + if (!ret && allocated_buffers < num_buffers)
> ret = -ENOMEM;
> - goto free_mem;
> - }
>
> /*
> - * Ok, driver accepted smaller number of buffers.
> + * Either the driver has accepted a smaller number of buffers,
> + * or .queue_setup() returned an error
> */
> - ret = num_buffers;
> + }
> +
> + q->num_buffers = allocated_buffers;
> +
> + if (ret < 0) {
> + __vb2_queue_free(q, allocated_buffers);
> + return ret;
> }
>
> /*
> * Return the number of successfully allocated buffers
> * to the userspace.
> */
> - req->count = ret;
> + req->count = allocated_buffers;
>
> return 0;
> -
> -free_mem:
> - __vb2_queue_free(q);
> - return ret;
> }
> EXPORT_SYMBOL_GPL(vb2_reqbufs);
>
> /**
> + * vb2_create_bufs() - Allocate buffers and any required auxiliary structs
> + * @q: videobuf2 queue
> + * @create: creation parameters, passed from userspace to
> vidioc_create_bufs
> + * handler in driver
> + *
> + * Should be called from vidioc_create_bufs ioctl handler of a driver.
> + * This function:
> + * 1) verifies parameter sanity
> + * 2) calls the .queue_setup() queue operation
> + * 3) performs any necessary memory allocations
> + *
> + * The return values from this function are intended to be directly returned
> + * from vidioc_create_bufs handler in driver.
> + */
> +int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
> +{
> + unsigned int num_planes, num_buffers = create->count,
> allocated_buffers;
> + unsigned long plane_sizes[VIDEO_MAX_PLANES];
> + int ret = 0;
> +
> + if (q->fileio) {
> + dprintk(1, "%s(): file io in progress\n", __func__);
> + return -EBUSY;
> + }
> +
> + if (create->memory != V4L2_MEMORY_MMAP
> + && create->memory != V4L2_MEMORY_USERPTR) {
> + dprintk(1, "%s(): unsupported memory type\n", __func__);
> + return -EINVAL;
> + }
> +
> + if (create->format.type != q->type) {
> + dprintk(1, "%s(): requested type is incorrect\n", __func__);
> + return -EINVAL;
> + }
> +
> + /*
> + * Make sure all the required memory ops for given memory type
> +
Re: [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
Hi Pawel
Thanks for the review.
On Sun, 28 Aug 2011, Pawel Osciak wrote:
> Hi Guennadi,
>
> On Wed, Aug 24, 2011 at 11:41, Guennadi Liakhovetski
> wrote:
> > The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
> > allow user-space applications to allocate video buffers of different
> > sizes and hand them over to the driver for fast switching between
> > different frame formats. This patch adds support for buffers of different
> > sizes on the same buffer-queue to vb2.
> >
> > Signed-off-by: Guennadi Liakhovetski
> > Cc: Hans Verkuil
> > Cc: Laurent Pinchart
> > Cc: Marek Szyprowski
> > Cc: Mauro Carvalho Chehab
> > Cc: Pawel Osciak
> > Cc: Sakari Ailus
> > ---
> > drivers/media/video/videobuf2-core.c | 278
> > --
> > include/media/videobuf2-core.h | 31 +++--
> > 2 files changed, 252 insertions(+), 57 deletions(-)
> >
> > diff --git a/drivers/media/video/videobuf2-core.c
> > b/drivers/media/video/videobuf2-core.c
> > index 8a81a89..fed6f2d 100644
> > --- a/drivers/media/video/videobuf2-core.c
> > +++ b/drivers/media/video/videobuf2-core.c
[snip]
> > /**
> > + * vb2_create_bufs() - Allocate buffers and any required auxiliary structs
> > + * @q: videobuf2 queue
> > + * @create: creation parameters, passed from userspace to
> > vidioc_create_bufs
> > + * handler in driver
> > + *
> > + * Should be called from vidioc_create_bufs ioctl handler of a driver.
> > + * This function:
> > + * 1) verifies parameter sanity
> > + * 2) calls the .queue_setup() queue operation
> > + * 3) performs any necessary memory allocations
> > + *
> > + * The return values from this function are intended to be directly
> > returned
> > + * from vidioc_create_bufs handler in driver.
> > + */
> > +int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers
> > *create)
> > +{
> > + unsigned int num_planes, num_buffers = create->count,
> > allocated_buffers;
> > + unsigned long plane_sizes[VIDEO_MAX_PLANES];
> > + int ret = 0;
> > +
> > + if (q->fileio) {
> > + dprintk(1, "%s(): file io in progress\n", __func__);
> > + return -EBUSY;
> > + }
> > +
> > + if (create->memory != V4L2_MEMORY_MMAP
> > + && create->memory != V4L2_MEMORY_USERPTR) {
> > + dprintk(1, "%s(): unsupported memory type\n", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + if (create->format.type != q->type) {
> > + dprintk(1, "%s(): requested type is incorrect\n", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > + * Make sure all the required memory ops for given memory type
> > + * are available.
> > + */
> > + if (create->memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
> > + dprintk(1, "%s(): MMAP for current setup unsupported\n",
> > __func__);
> > + return -EINVAL;
> > + }
> > +
> > + if (create->memory == V4L2_MEMORY_USERPTR &&
> > __verify_userptr_ops(q)) {
> > + dprintk(1, "%s(): USERPTR for current setup unsupported\n",
> > __func__);
> > + return -EINVAL;
> > + }
> > +
> > + if (q->num_buffers == VIDEO_MAX_FRAME) {
> > + dprintk(1, "%s(): maximum number of buffers already
> > allocated\n",
> > + __func__);
> > + return -ENOBUFS;
> > + }
>
> I think we should be verifying that q->num_buffers + create->count <=
> VIDEO_MAX_FRAME.
Yeah, something like that, thanks. Some of us didn't like this
VIDEO_MAX_FRAME limit at all, but as long as we have it, e-g-. as long as
struct vb2_queue::bufs[] is a fixed-size array with exactly that many
elements, we have to check for it. Increasing / eliminating it could be a
separate patch.
>
> > +
> > + create->index = q->num_buffers;
> > +
> > + if (!q->num_buffers) {
> > + memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> > + q->memory = create->memory;
> > + }
> > +
> > + /*
> > + * Ask the driver, whether the requested number of buffers, planes
> > per
> > + * buffer and their sizes are acceptable
> > + */
> > + ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
> > + &num_planes, plane_sizes, q->alloc_ctx);
>
> I don't see you zeroing neither num_planes nor plane_sizes[] in
> vb2_create_bufs and vb2_reqbufs. Since instead of always requiring the
> driver to fill them, you've introduced the "non-zero num_planes and/or
> plane_sizes" case (see my comment for queue_setup() documentation in
> videobuf2-core.h), it looks to me that the drivers will be getting
> random values in num_planes and plane_sizes in queue_setup() and will
> have to attempt to use them. Ditto for all other qop calls to
> queue_setup in this file (in vb2_reqbufs as well).
