Hi Laurent, Please refer to the comments below.
On 03/22/2012 12:16 PM, Laurent Pinchart wrote:
Hi Tomasz,
Thanks for the patch.
On Tuesday 13 March 2012 11:17:02 Tomasz Stanislawski wrote:
This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success.
I know code is more fun to write than documentation, but Documentation/DocBook/media/v4l will be sad if this patch is merged as-is ;-)
Ok. I will prepare the documentation just after there will be consensus about shape of buffer exporting API :).
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
drivers/media/video/v4l2-compat-ioctl32.c | 1 + drivers/media/video/v4l2-ioctl.c | 11 +++++++++++ include/linux/videodev2.h | 20 ++++++++++++++++++++ include/media/v4l2-ioctl.h | 2 ++ 4 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c index e6f67aa..fd157cb 100644 --- a/drivers/media/video/v4l2-compat-ioctl32.c +++ b/drivers/media/video/v4l2-compat-ioctl32.c @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg) case VIDIOC_S_FBUF32: case VIDIOC_OVERLAY32: case VIDIOC_QBUF32:
- case VIDIOC_EXPBUF: case VIDIOC_DQBUF32: case VIDIOC_STREAMON32: case VIDIOC_STREAMOFF32:
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index 74cab51..a125016 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = { [_IOC_NR(VIDIOC_S_FBUF)] = "VIDIOC_S_FBUF", [_IOC_NR(VIDIOC_OVERLAY)] = "VIDIOC_OVERLAY", [_IOC_NR(VIDIOC_QBUF)] = "VIDIOC_QBUF",
- [_IOC_NR(VIDIOC_EXPBUF)] = "VIDIOC_EXPBUF", [_IOC_NR(VIDIOC_DQBUF)] = "VIDIOC_DQBUF", [_IOC_NR(VIDIOC_STREAMON)] = "VIDIOC_STREAMON", [_IOC_NR(VIDIOC_STREAMOFF)] = "VIDIOC_STREAMOFF",
@@ -938,6 +939,16 @@ static long __video_do_ioctl(struct file *file, dbgbuf(cmd, vfd, p); break; }
- case VIDIOC_EXPBUF:
- {
struct v4l2_exportbuffer *p = arg;if (!ops->vidioc_expbuf)break;ret = ops->vidioc_expbuf(file, fh, p);You can pass arg to ops->vidioc_expbuf() directly, there's no need to create a struct v4l2_exportbuffer *p variable.
No problem. I tried to follow style of other ioctls. Notice that adding this temporary variable provides some form of type checking. I mean using a proper structure for a proper callback.
break;- } case VIDIOC_DQBUF: { struct v4l2_buffer *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index bb6844e..e71c787 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -680,6 +680,25 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
+/**
- struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor
- @fd: file descriptor associated with DMABUF (set by driver)
- @mem_offset: for non-multiplanar buffers with memory ==
V4L2_MEMORY_MMAP;
I don't think we will ever support exporting anything else than V4L2_MEMORY_MMAP buffers. What will happen for multiplanar buffers ?
Every plane is described by its separate mem-offset. So planes are exported as separate DMABUF. I will update field description.
offset from the start of the device memory for this plane,
(or a "cookie" that should be passed to mmap() as offset)
Shouldn't the mem_offset field always be set to the mmap cookie value ? I'm a bit confused by the "or" part, it seems to have been copied from the v4l2_buffer documentation directly. We should clarify that.
Ok. I agree.
- Contains data used for exporting a video buffer as DMABUF file
- descriptor. Uses the same 'cookie' as mmap() syscall. All reserved
fields
- must be set to zero.
- */
+struct v4l2_exportbuffer {
- __u32 fd;
- __u32 reserved0;
Why is there a reserved field here ?
I expected that struct v4l2_requestbuffer might allow exporting buffers described by something else than 'mmap cookie'. Union could be used for different schemes i.e.
struct v4l2_exportbuffer { __u32 fd; __u32 type; union { __u32 mem_offset; __u32 reserved[14]; /* other descriptions */ } m; };
Adding reserved0 field will allow to introduce field type, and keep the union at 64-bit aligned address to keep compatibility between 32/64bit libraries. Field type == 0 would indicate description of a buffer using 'mmap offset'.
- __u32 mem_offset;
- __u32 reserved[13];
+};
/*
- O V E R L A Y P R E V I E W
*/ @@ -2303,6 +2322,7 @@ struct v4l2_create_buffers { #define VIDIOC_S_FBUF _IOW('V', 11, struct v4l2_framebuffer) #define VIDIOC_OVERLAY _IOW('V', 14, int) #define VIDIOC_QBUF _IOWR('V', 15, struct v4l2_buffer) +#define VIDIOC_EXPBUF _IOWR('V', 16, struct v4l2_exportbuffer) #define VIDIOC_DQBUF _IOWR('V', 17, struct v4l2_buffer) #define VIDIOC_STREAMON _IOW('V', 18, int) #define VIDIOC_STREAMOFF _IOW('V', 19, int) diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index 4df031a..d8716c6f 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -120,6 +120,8 @@ struct v4l2_ioctl_ops { int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b); int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b); int (*vidioc_qbuf) (struct file *file, void *fh, struct v4l2_buffer *b);
- int (*vidioc_expbuf) (struct file *file, void *fh,
struct v4l2_exportbuffer *e);*b);
int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);
Regards, Tomasz Stanislawski