[PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Sakari Ailus]

Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
tells the former is valid. The flag is used by no driver currently.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
Hi all,

I thought this would be a good time to get rid of the input field in
v4l2_buffer to avoid writing more useless compat code for it --- the enum
compat code.

Comments are welcome. This patch is compile tested on videobuf and
videobuf2.

 drivers/media/video/v4l2-compat-ioctl32.c |    8 +++-----
 drivers/media/video/videobuf-core.c       |   16 ----------------
 drivers/media/video/videobuf2-core.c      |    4 +---
 include/linux/videodev2.h                 |    4 +---
 include/media/videobuf-core.h             |    2 --
 5 files changed, 5 insertions(+), 29 deletions(-)

diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index 2829d25..a2ddc37 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -387,8 +387,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		get_user(kp->index, &up->index) ||
 		get_user(kp->type, &up->type) ||
 		get_user(kp->flags, &up->flags) ||
-		get_user(kp->memory, &up->memory) ||
-		get_user(kp->input, &up->input))
+		get_user(kp->memory, &up->memory)
 			return -EFAULT;
 
 	if (V4L2_TYPE_IS_OUTPUT(kp->type))
@@ -472,8 +471,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->index, &up->index) ||
 		put_user(kp->type, &up->type) ||
 		put_user(kp->flags, &up->flags) ||
-		put_user(kp->memory, &up->memory) ||
-		put_user(kp->input, &up->input))
+		put_user(kp->memory, &up->memory)
 			return -EFAULT;
 
 	if (put_user(kp->bytesused, &up->bytesused) ||
@@ -482,7 +480,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) ||
 		copy_to_user(&up->timecode, &kp->timecode, sizeof(struct v4l2_timecode)) ||
 		put_user(kp->sequence, &up->sequence) ||
-		put_user(kp->reserved, &up->reserved))
+		copy_to_user(&kp->reserved, &up->reserved, sizeof(kp->reserved))
 			return -EFAULT;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index ffdf59c..bf7a326 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -359,11 +359,6 @@ static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b,
 		break;
 	}
 
-	if (vb->input != UNSET) {
-		b->flags |= V4L2_BUF_FLAG_INPUT;
-		b->input  = vb->input;
-	}
-
 	b->field     = vb->field;
 	b->timestamp = vb->ts;
 	b->bytesused = vb->size;
@@ -402,7 +397,6 @@ int __videobuf_mmap_setup(struct videobuf_queue *q,
 			break;
 
 		q->bufs[i]->i      = i;
-		q->bufs[i]->input  = UNSET;
 		q->bufs[i]->memory = memory;
 		q->bufs[i]->bsize  = bsize;
 		switch (memory) {
@@ -566,16 +560,6 @@ int videobuf_qbuf(struct videobuf_queue *q, struct v4l2_buffer *b)
 		goto done;
 	}
 
-	if (b->flags & V4L2_BUF_FLAG_INPUT) {
-		if (b->input >= q->inputs) {
-			dprintk(1, "qbuf: wrong input.\n");
-			goto done;
-		}
-		buf->input = b->input;
-	} else {
-		buf->input = UNSET;
-	}
-
 	switch (b->memory) {
 	case V4L2_MEMORY_MMAP:
 		if (0 == buf->baddr) {
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 3786d88..0daaec7 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -338,8 +338,7 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 
 	/* Copy back data such as timestamp, flags, input, etc. */
 	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
-	b->input = vb->v4l2_buf.input;
-	b->reserved = vb->v4l2_buf.reserved;
+	memcpy(b->reserved, vb->v4l2_buf.reserved, sizeof(b->reserved));
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
 		ret = __verify_planes_array(vb, b);
@@ -860,7 +859,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
 
 	vb->v4l2_buf.field = b->field;
 	vb->v4l2_buf.timestamp = b->timestamp;
-	vb->v4l2_buf.input = b->input;
 	vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS;
 
 	return 0;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 5a09ac3..ae3062d 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -652,8 +652,7 @@ struct v4l2_buffer {
 		struct v4l2_plane *planes;
 	} m;
 	__u32			length;
-	__u32			input;
-	__u32			reserved;
+	__u32			reserved[2];
 };
 
 /*  Flags for 'flags' field */
@@ -666,7 +665,6 @@ struct v4l2_buffer {
 /* Buffer is ready, but the data contained within is corrupted. */
 #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 */
 #define V4L2_BUF_FLAG_PREPARED	0x0400	/* Buffer is prepared for queuing */
 /* Cache handling flags */
 #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
diff --git a/include/media/videobuf-core.h b/include/media/videobuf-core.h
index 90ed895..4511d75 100644
--- a/include/media/videobuf-core.h
+++ b/include/media/videobuf-core.h
@@ -19,8 +19,6 @@
 #include <linux/poll.h>
 #include <linux/videodev2.h>
 
-#define UNSET (-1U)
-
 
 struct videobuf_buffer;
 struct videobuf_queue;
-- 
1.7.2.5

Re: [PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Laurent Pinchart]

Hi Sakari,

On Monday 30 April 2012 15:40:24 Sakari Ailus wrote:
> Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
> tells the former is valid. The flag is used by no driver currently.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
> Hi all,
> 
> I thought this would be a good time to get rid of the input field in
> v4l2_buffer to avoid writing more useless compat code for it --- the enum
> compat code.
> 
> Comments are welcome. This patch is compile tested on videobuf and
> videobuf2.

I'm all for this. As far as I know, the field was only useful for a single 
out-of-tree driver which is long dead now.

>  drivers/media/video/v4l2-compat-ioctl32.c |    8 +++-----
>  drivers/media/video/videobuf-core.c       |   16 ----------------
>  drivers/media/video/videobuf2-core.c      |    4 +---
>  include/linux/videodev2.h                 |    4 +---
>  include/media/videobuf-core.h             |    2 --

A quick grep through the code shows that you've missed the cpia2 driver which 
sets the input field to 0 in cpia2_dqbuf(). Please try to compile as many 
drivers as possible with this patch. Using coccinelle 
(http://coccinelle.lip6.fr/) could help finding other accesses to the input 
field.

>  5 files changed, 5 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> b/drivers/media/video/v4l2-compat-ioctl32.c index 2829d25..a2ddc37 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -387,8 +387,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user get_user(kp->index, &up->index) ||
>  		get_user(kp->type, &up->type) ||
>  		get_user(kp->flags, &up->flags) ||
> -		get_user(kp->memory, &up->memory) ||
> -		get_user(kp->input, &up->input))
> +		get_user(kp->memory, &up->memory)
>  			return -EFAULT;
> 
>  	if (V4L2_TYPE_IS_OUTPUT(kp->type))
> @@ -472,8 +471,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user put_user(kp->index, &up->index) ||
>  		put_user(kp->type, &up->type) ||
>  		put_user(kp->flags, &up->flags) ||
> -		put_user(kp->memory, &up->memory) ||
> -		put_user(kp->input, &up->input))
> +		put_user(kp->memory, &up->memory)
>  			return -EFAULT;
> 
>  	if (put_user(kp->bytesused, &up->bytesused) ||
> @@ -482,7 +480,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user put_user(kp->timestamp.tv_usec,
> &up->timestamp.tv_usec) ||
>  		copy_to_user(&up->timecode, &kp->timecode, sizeof(struct 
v4l2_timecode))
> || put_user(kp->sequence, &up->sequence) ||
> -		put_user(kp->reserved, &up->reserved))
> +		copy_to_user(&kp->reserved, &up->reserved, sizeof(kp->reserved))
>  			return -EFAULT;
> 
>  	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
> diff --git a/drivers/media/video/videobuf-core.c
> b/drivers/media/video/videobuf-core.c index ffdf59c..bf7a326 100644
> --- a/drivers/media/video/videobuf-core.c
> +++ b/drivers/media/video/videobuf-core.c
> @@ -359,11 +359,6 @@ static void videobuf_status(struct videobuf_queue *q,
> struct v4l2_buffer *b, break;
>  	}
> 
> -	if (vb->input != UNSET) {
> -		b->flags |= V4L2_BUF_FLAG_INPUT;
> -		b->input  = vb->input;
> -	}
> -
>  	b->field     = vb->field;
>  	b->timestamp = vb->ts;
>  	b->bytesused = vb->size;
> @@ -402,7 +397,6 @@ int __videobuf_mmap_setup(struct videobuf_queue *q,
>  			break;
> 
>  		q->bufs[i]->i      = i;
> -		q->bufs[i]->input  = UNSET;
>  		q->bufs[i]->memory = memory;
>  		q->bufs[i]->bsize  = bsize;
>  		switch (memory) {
> @@ -566,16 +560,6 @@ int videobuf_qbuf(struct videobuf_queue *q, struct
> v4l2_buffer *b) goto done;
>  	}
> 
> -	if (b->flags & V4L2_BUF_FLAG_INPUT) {
> -		if (b->input >= q->inputs) {
> -			dprintk(1, "qbuf: wrong input.\n");
> -			goto done;
> -		}
> -		buf->input = b->input;
> -	} else {
> -		buf->input = UNSET;
> -	}
> -
>  	switch (b->memory) {
>  	case V4L2_MEMORY_MMAP:
>  		if (0 == buf->baddr) {
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c index 3786d88..0daaec7 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -338,8 +338,7 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb,
> struct v4l2_buffer *b)
> 
>  	/* Copy back data such as timestamp, flags, input, etc. */
>  	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
> -	b->input = vb->v4l2_buf.input;
> -	b->reserved = vb->v4l2_buf.reserved;
> +	memcpy(b->reserved, vb->v4l2_buf.reserved, sizeof(b->reserved));
> 
>  	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
>  		ret = __verify_planes_array(vb, b);
> @@ -860,7 +859,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> const struct v4l2_buffer *b,
> 
>  	vb->v4l2_buf.field = b->field;
>  	vb->v4l2_buf.timestamp = b->timestamp;
> -	vb->v4l2_buf.input = b->input;
>  	vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS;
> 
>  	return 0;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 5a09ac3..ae3062d 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -652,8 +652,7 @@ struct v4l2_buffer {
>  		struct v4l2_plane *planes;
>  	} m;
>  	__u32			length;
> -	__u32			input;
> -	__u32			reserved;
> +	__u32			reserved[2];
>  };
> 
>  /*  Flags for 'flags' field */
> @@ -666,7 +665,6 @@ struct v4l2_buffer {
>  /* Buffer is ready, but the data contained within is corrupted. */
>  #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 */
>  #define V4L2_BUF_FLAG_PREPARED	0x0400	/* Buffer is prepared for queuing 
*/
>  /* Cache handling flags */
>  #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
> diff --git a/include/media/videobuf-core.h b/include/media/videobuf-core.h
> index 90ed895..4511d75 100644
> --- a/include/media/videobuf-core.h
> +++ b/include/media/videobuf-core.h
> @@ -19,8 +19,6 @@
>  #include <linux/poll.h>
>  #include <linux/videodev2.h>
> 
> -#define UNSET (-1U)
> -
> 
>  struct videobuf_buffer;
>  struct videobuf_queue;
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Sakari Ailus]

Hi Laurent,

On Mon, Apr 30, 2012 at 02:48:23PM +0200, Laurent Pinchart wrote:
> On Monday 30 April 2012 15:40:24 Sakari Ailus wrote:
> > Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
> > tells the former is valid. The flag is used by no driver currently.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> > Hi all,
> > 
> > I thought this would be a good time to get rid of the input field in
> > v4l2_buffer to avoid writing more useless compat code for it --- the enum
> > compat code.
> > 
> > Comments are welcome. This patch is compile tested on videobuf and
> > videobuf2.
> 
> I'm all for this. As far as I know, the field was only useful for a single 
> out-of-tree driver which is long dead now.
> 
> >  drivers/media/video/v4l2-compat-ioctl32.c |    8 +++-----
> >  drivers/media/video/videobuf-core.c       |   16 ----------------
> >  drivers/media/video/videobuf2-core.c      |    4 +---
> >  include/linux/videodev2.h                 |    4 +---
> >  include/media/videobuf-core.h             |    2 --
> 
> A quick grep through the code shows that you've missed the cpia2 driver which 
> sets the input field to 0 in cpia2_dqbuf(). Please try to compile as many 
> drivers as possible with this patch. Using coccinelle 
> (http://coccinelle.lip6.fr/) could help finding other accesses to the input 
> field.

I grepped them but forgot to give -r option to grep... I found no further
uses of the input field by quick looking. I'll compile what I can and resend
the patch.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

[PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Sakari Ailus]

Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
tells the former is valid. The flag is used by no driver currently.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
Hi all,

I thought this would be a good time to get rid of the input field in
v4l2_buffer to avoid writing more useless compat code for it --- the enum
compat code.

Comments are welcome. This patch is compile tested on videobuf and
videobuf2.

Update: Fixes according to Laurent's comments. All drivers available on x86
and arm (N9) have been compile tested. Changes to the previous patch:

- videobuf2 comment fix (the input field is no longer there).
- cpia driver still contained a reference to input field and also needed
  changes to handling of the reserved field.
- Bring back UNSET. While this is no longer used in videobuf as the result
  of the removal of the input field, it is quite widely misused outside
  videobuf, and removing it should be a separate patch.

 drivers/media/video/cpia2/cpia2_v4l.c     |    3 +--
 drivers/media/video/v4l2-compat-ioctl32.c |    8 +++-----
 drivers/media/video/videobuf-core.c       |   16 ----------------
 drivers/media/video/videobuf2-core.c      |    6 ++----
 include/linux/videodev2.h                 |    4 +---
 5 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/drivers/media/video/cpia2/cpia2_v4l.c b/drivers/media/video/cpia2/cpia2_v4l.c
index 077eb1d..1ed633f 100644
--- a/drivers/media/video/cpia2/cpia2_v4l.c
+++ b/drivers/media/video/cpia2/cpia2_v4l.c
@@ -1289,8 +1289,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
 	buf->sequence = cam->buffers[buf->index].seq;
 	buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
 	buf->length = cam->frame_size;
-	buf->input = 0;
-	buf->reserved = 0;
+	memset(buf->reserved, 0, sizeof(buf->reserved));
 	memset(&buf->timecode, 0, sizeof(buf->timecode));
 
 	DBG("DQBUF #%d status:%d seq:%d length:%d\n", buf->index,
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index 2829d25..a2ddc37 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -387,8 +387,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		get_user(kp->index, &up->index) ||
 		get_user(kp->type, &up->type) ||
 		get_user(kp->flags, &up->flags) ||
-		get_user(kp->memory, &up->memory) ||
-		get_user(kp->input, &up->input))
+		get_user(kp->memory, &up->memory)
 			return -EFAULT;
 
 	if (V4L2_TYPE_IS_OUTPUT(kp->type))
@@ -472,8 +471,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->index, &up->index) ||
 		put_user(kp->type, &up->type) ||
 		put_user(kp->flags, &up->flags) ||
-		put_user(kp->memory, &up->memory) ||
-		put_user(kp->input, &up->input))
+		put_user(kp->memory, &up->memory)
 			return -EFAULT;
 
 	if (put_user(kp->bytesused, &up->bytesused) ||
@@ -482,7 +480,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) ||
 		copy_to_user(&up->timecode, &kp->timecode, sizeof(struct v4l2_timecode)) ||
 		put_user(kp->sequence, &up->sequence) ||
-		put_user(kp->reserved, &up->reserved))
+		copy_to_user(&kp->reserved, &up->reserved, sizeof(kp->reserved))
 			return -EFAULT;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index ffdf59c..bf7a326 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -359,11 +359,6 @@ static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b,
 		break;
 	}
 
-	if (vb->input != UNSET) {
-		b->flags |= V4L2_BUF_FLAG_INPUT;
-		b->input  = vb->input;
-	}
-
 	b->field     = vb->field;
 	b->timestamp = vb->ts;
 	b->bytesused = vb->size;
@@ -402,7 +397,6 @@ int __videobuf_mmap_setup(struct videobuf_queue *q,
 			break;
 
 		q->bufs[i]->i      = i;
-		q->bufs[i]->input  = UNSET;
 		q->bufs[i]->memory = memory;
 		q->bufs[i]->bsize  = bsize;
 		switch (memory) {
@@ -566,16 +560,6 @@ int videobuf_qbuf(struct videobuf_queue *q, struct v4l2_buffer *b)
 		goto done;
 	}
 
-	if (b->flags & V4L2_BUF_FLAG_INPUT) {
-		if (b->input >= q->inputs) {
-			dprintk(1, "qbuf: wrong input.\n");
-			goto done;
-		}
-		buf->input = b->input;
-	} else {
-		buf->input = UNSET;
-	}
-
 	switch (b->memory) {
 	case V4L2_MEMORY_MMAP:
 		if (0 == buf->baddr) {
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 3786d88..61a89fb 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -336,10 +336,9 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 	struct vb2_queue *q = vb->vb2_queue;
 	int ret;
 
-	/* Copy back data such as timestamp, flags, input, etc. */
+	/* Copy back data such as timestamp, flags, etc. */
 	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
-	b->input = vb->v4l2_buf.input;
-	b->reserved = vb->v4l2_buf.reserved;
+	memcpy(b->reserved, vb->v4l2_buf.reserved, sizeof(b->reserved));
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
 		ret = __verify_planes_array(vb, b);
@@ -860,7 +859,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
 
 	vb->v4l2_buf.field = b->field;
 	vb->v4l2_buf.timestamp = b->timestamp;
-	vb->v4l2_buf.input = b->input;
 	vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS;
 
 	return 0;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 5a09ac3..ae3062d 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -652,8 +652,7 @@ struct v4l2_buffer {
 		struct v4l2_plane *planes;
 	} m;
 	__u32			length;
-	__u32			input;
-	__u32			reserved;
+	__u32			reserved[2];
 };
 
 /*  Flags for 'flags' field */
@@ -666,7 +665,6 @@ struct v4l2_buffer {
 /* Buffer is ready, but the data contained within is corrupted. */
 #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 */
 #define V4L2_BUF_FLAG_PREPARED	0x0400	/* Buffer is prepared for queuing */
 /* Cache handling flags */
 #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
-- 
1.7.2.5

Re: [PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Hans Verkuil]

On Monday 30 April 2012 15:34:58 Sakari Ailus wrote:
> Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
> tells the former is valid. The flag is used by no driver currently.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
> Hi all,
> 
> I thought this would be a good time to get rid of the input field in
> v4l2_buffer to avoid writing more useless compat code for it --- the enum
> compat code.
> 
> Comments are welcome. This patch is compile tested on videobuf and
> videobuf2.

I'm all in favor of this. Don't forget to update the documentation as well, 
though!

What would the impact be on applications, though? Any app that currently does 
'reserved = 0' would fail to compile after this change.

Perhaps rather than removing 'input' and changing 'reserved' to 'reserved[2]' 
we should do something like this:

union {
	u32 input;
	u32 reserved2;
};
u32 reserved;

Or perhaps leave out the union and just replace 'input' by 'reserved2'.

Regards,

	Hans


> 
> Update: Fixes according to Laurent's comments. All drivers available on x86
> and arm (N9) have been compile tested. Changes to the previous patch:
> 
> - videobuf2 comment fix (the input field is no longer there).
> - cpia driver still contained a reference to input field and also needed
>   changes to handling of the reserved field.
> - Bring back UNSET. While this is no longer used in videobuf as the result
>   of the removal of the input field, it is quite widely misused outside
>   videobuf, and removing it should be a separate patch.
> 
>  drivers/media/video/cpia2/cpia2_v4l.c     |    3 +--
>  drivers/media/video/v4l2-compat-ioctl32.c |    8 +++-----
>  drivers/media/video/videobuf-core.c       |   16 ----------------
>  drivers/media/video/videobuf2-core.c      |    6 ++----
>  include/linux/videodev2.h                 |    4 +---
>  5 files changed, 7 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/media/video/cpia2/cpia2_v4l.c
> b/drivers/media/video/cpia2/cpia2_v4l.c index 077eb1d..1ed633f 100644
> --- a/drivers/media/video/cpia2/cpia2_v4l.c
> +++ b/drivers/media/video/cpia2/cpia2_v4l.c
> @@ -1289,8 +1289,7 @@ static int cpia2_dqbuf(struct file *file, void *fh,
> struct v4l2_buffer *buf) buf->sequence = cam->buffers[buf->index].seq;
>  	buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
>  	buf->length = cam->frame_size;
> -	buf->input = 0;
> -	buf->reserved = 0;
> +	memset(buf->reserved, 0, sizeof(buf->reserved));
>  	memset(&buf->timecode, 0, sizeof(buf->timecode));
> 
>  	DBG("DQBUF #%d status:%d seq:%d length:%d\n", buf->index,
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> b/drivers/media/video/v4l2-compat-ioctl32.c index 2829d25..a2ddc37 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -387,8 +387,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user get_user(kp->index, &up->index) ||
>  		get_user(kp->type, &up->type) ||
>  		get_user(kp->flags, &up->flags) ||
> -		get_user(kp->memory, &up->memory) ||
> -		get_user(kp->input, &up->input))
> +		get_user(kp->memory, &up->memory)
>  			return -EFAULT;
> 
>  	if (V4L2_TYPE_IS_OUTPUT(kp->type))
> @@ -472,8 +471,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user put_user(kp->index, &up->index) ||
>  		put_user(kp->type, &up->type) ||
>  		put_user(kp->flags, &up->flags) ||
> -		put_user(kp->memory, &up->memory) ||
> -		put_user(kp->input, &up->input))
> +		put_user(kp->memory, &up->memory)
>  			return -EFAULT;
> 
>  	if (put_user(kp->bytesused, &up->bytesused) ||
> @@ -482,7 +480,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user put_user(kp->timestamp.tv_usec,
> &up->timestamp.tv_usec) ||
>  		copy_to_user(&up->timecode, &kp->timecode, sizeof(struct 
v4l2_timecode))
> || put_user(kp->sequence, &up->sequence) ||
> -		put_user(kp->reserved, &up->reserved))
> +		copy_to_user(&kp->reserved, &up->reserved, sizeof(kp-
>reserved))
>  			return -EFAULT;
> 
>  	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
> diff --git a/drivers/media/video/videobuf-core.c
> b/drivers/media/video/videobuf-core.c index ffdf59c..bf7a326 100644
> --- a/drivers/media/video/videobuf-core.c
> +++ b/drivers/media/video/videobuf-core.c
> @@ -359,11 +359,6 @@ static void videobuf_status(struct videobuf_queue *q,
> struct v4l2_buffer *b, break;
>  	}
> 
> -	if (vb->input != UNSET) {
> -		b->flags |= V4L2_BUF_FLAG_INPUT;
> -		b->input  = vb->input;
> -	}
> -
>  	b->field     = vb->field;
>  	b->timestamp = vb->ts;
>  	b->bytesused = vb->size;
> @@ -402,7 +397,6 @@ int __videobuf_mmap_setup(struct videobuf_queue *q,
>  			break;
> 
>  		q->bufs[i]->i      = i;
> -		q->bufs[i]->input  = UNSET;
>  		q->bufs[i]->memory = memory;
>  		q->bufs[i]->bsize  = bsize;
>  		switch (memory) {
> @@ -566,16 +560,6 @@ int videobuf_qbuf(struct videobuf_queue *q, struct
> v4l2_buffer *b) goto done;
>  	}
> 
> -	if (b->flags & V4L2_BUF_FLAG_INPUT) {
> -		if (b->input >= q->inputs) {
> -			dprintk(1, "qbuf: wrong input.\n");
> -			goto done;
> -		}
> -		buf->input = b->input;
> -	} else {
> -		buf->input = UNSET;
> -	}
> -
>  	switch (b->memory) {
>  	case V4L2_MEMORY_MMAP:
>  		if (0 == buf->baddr) {
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c index 3786d88..61a89fb 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -336,10 +336,9 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb,
> struct v4l2_buffer *b) struct vb2_queue *q = vb->vb2_queue;
>  	int ret;
> 
> -	/* Copy back data such as timestamp, flags, input, etc. */
> +	/* Copy back data such as timestamp, flags, etc. */
>  	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
> -	b->input = vb->v4l2_buf.input;
> -	b->reserved = vb->v4l2_buf.reserved;
> +	memcpy(b->reserved, vb->v4l2_buf.reserved, sizeof(b->reserved));
> 
>  	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
>  		ret = __verify_planes_array(vb, b);
> @@ -860,7 +859,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> const struct v4l2_buffer *b,
> 
>  	vb->v4l2_buf.field = b->field;
>  	vb->v4l2_buf.timestamp = b->timestamp;
> -	vb->v4l2_buf.input = b->input;
>  	vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS;
> 
>  	return 0;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 5a09ac3..ae3062d 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -652,8 +652,7 @@ struct v4l2_buffer {
>  		struct v4l2_plane *planes;
>  	} m;
>  	__u32			length;
> -	__u32			input;
> -	__u32			reserved;
> +	__u32			reserved[2];
>  };
> 
>  /*  Flags for 'flags' field */
> @@ -666,7 +665,6 @@ struct v4l2_buffer {
>  /* Buffer is ready, but the data contained within is corrupted. */
>  #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 */
>  #define V4L2_BUF_FLAG_PREPARED	0x0400	/* Buffer is prepared for 
queuing */
>  /* Cache handling flags */
>  #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800

Re: [PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Sakari Ailus]

Hi Hans,

Thanks for your comments.

On Mon, Apr 30, 2012 at 03:48:14PM +0200, Hans Verkuil wrote:
> On Monday 30 April 2012 15:34:58 Sakari Ailus wrote:
> > Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
> > tells the former is valid. The flag is used by no driver currently.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> > Hi all,
> > 
> > I thought this would be a good time to get rid of the input field in
> > v4l2_buffer to avoid writing more useless compat code for it --- the enum
> > compat code.
> > 
> > Comments are welcome. This patch is compile tested on videobuf and
> > videobuf2.
> 
> I'm all in favor of this. Don't forget to update the documentation as well, 
> though!

Good point. I'll go through that next.

> What would the impact be on applications, though? Any app that currently does 
> 'reserved = 0' would fail to compile after this change.

I had a bit of that in drivers and videobuf(2), too.

Is there a known good practice of dealing with this? The reserved fields are
supposed to be set to zero by applications but the reserved fields may even
vanish over time from some structs.

One option is to keep the reserved fields as array even there was just one
of them or if it no longer was there. If so, reserved should have been
reserved[1] in the first place. This would make it easier to deal with
the changing size of the reserved field.

> Perhaps rather than removing 'input' and changing 'reserved' to 'reserved[2]' 
> we should do something like this:
> 
> union {
> 	u32 input;
> 	u32 reserved2;
> };
> u32 reserved;
> 
> Or perhaps leave out the union and just replace 'input' by 'reserved2'.

That sounds like a good option to me, too.

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

Re: [PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Hans Verkuil]

On Monday 30 April 2012 16:06:16 Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks for your comments.
> 
> On Mon, Apr 30, 2012 at 03:48:14PM +0200, Hans Verkuil wrote:
> > On Monday 30 April 2012 15:34:58 Sakari Ailus wrote:
> > > Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT
> > > which tells the former is valid. The flag is used by no driver
> > > currently.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > ---
> > > Hi all,
> > > 
> > > I thought this would be a good time to get rid of the input field in
> > > v4l2_buffer to avoid writing more useless compat code for it --- the
> > > enum compat code.
> > > 
> > > Comments are welcome. This patch is compile tested on videobuf and
> > > videobuf2.
> > 
> > I'm all in favor of this. Don't forget to update the documentation as
> > well, though!
> 
> Good point. I'll go through that next.
> 
> > What would the impact be on applications, though? Any app that currently
> > does 'reserved = 0' would fail to compile after this change.
> 
> I had a bit of that in drivers and videobuf(2), too.
> 
> Is there a known good practice of dealing with this? The reserved fields
> are supposed to be set to zero by applications but the reserved fields may
> even vanish over time from some structs.

We never ran out of reserved fields before (at least to my knowledge), so 
there is no known good practice.

> One option is to keep the reserved fields as array even there was just one
> of them or if it no longer was there. If so, reserved should have been
> reserved[1] in the first place. This would make it easier to deal with
> the changing size of the reserved field.

Definitely. But I think struct v4l2_buffer has been like this for a long time 
(Laurent would know when the input field was added).

Regards,

	Hans

> 
> > Perhaps rather than removing 'input' and changing 'reserved' to
> > 'reserved[2]' we should do something like this:
> > 
> > union {
> > 
> > 	u32 input;
> > 	u32 reserved2;
> > 
> > };
> > u32 reserved;
> > 
> > Or perhaps leave out the union and just replace 'input' by 'reserved2'.
> 
> That sounds like a good option to me, too.
> 
> Kind regards,

Re: [PATCH 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Laurent Pinchart]

On Monday 30 April 2012 16:15:30 Hans Verkuil wrote:
> On Monday 30 April 2012 16:06:16 Sakari Ailus wrote:

[snip]

> > One option is to keep the reserved fields as array even there was just one
> > of them or if it no longer was there. If so, reserved should have been
> > reserved[1] in the first place. This would make it easier to deal with
> > the changing size of the reserved field.
> 
> Definitely. But I think struct v4l2_buffer has been like this for a long
> time (Laurent would know when the input field was added).

That was 8 to 9 years ago. I'm responsible for that terrible idea, so I'd be 
happy to see the input field removed ;-)

-- 
Regards,

Laurent Pinchart

[PATCH v3 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Sakari Ailus]

Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
tells the former is valid. The flag is used by no driver currently.

Also change the documentation accordingly.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Hi,

This is the third version of the v4l2_buffer.input field removal patch.

What has changed since the previous version:

- Rename input as reserved2 instead of combining it to reserved and making
  it an array.
- cpia compile fix.
- Change documentation accordingly.

 Documentation/DocBook/media/v4l/compat.xml      |    6 ++++++
 Documentation/DocBook/media/v4l/io.xml          |   19 +++++--------------
 Documentation/DocBook/media/v4l/vidioc-qbuf.xml |    9 +++------
 drivers/media/video/cpia2/cpia2_v4l.c           |    2 +-
 drivers/media/video/v4l2-compat-ioctl32.c       |   11 +++++------
 drivers/media/video/videobuf-core.c             |   16 ----------------
 drivers/media/video/videobuf2-core.c            |    5 ++---
 include/linux/videodev2.h                       |    3 +--
 8 files changed, 23 insertions(+), 48 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index 87339b2..b939457 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2422,6 +2422,12 @@ details.</para>
 	  &VIDIOC-SUBDEV-G-SELECTION; and
 	  &VIDIOC-SUBDEV-S-SELECTION;.</para>
         </listitem>
+	<listitem>
+	  <para>Replaced <structfield>input</structfield> in
+	  <structname>v4l2_buffer</structname> by
+	  <structfield>reserved2</structfield> and removed
+	  <constant>V4L2_BUF_FLAG_INPUT</constant>.</para>
+	</listitem>
       </orderedlist>
     </section>
 
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index b815929..e4cb063 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -681,14 +681,12 @@ memory, set by the application. See <xref linkend="userp" /> for details.
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>input</structfield></entry>
+	    <entry><structfield>reserved2</structfield></entry>
 	    <entry></entry>
-	    <entry>Some video capture drivers support rapid and
-synchronous video input changes, a function useful for example in
-video surveillance applications. For this purpose applications set the
-<constant>V4L2_BUF_FLAG_INPUT</constant> flag, and this field to the
-number of a video input as in &v4l2-input; field
-<structfield>index</structfield>.</entry>
+	    <entry>A place holder for future extensions and custom
+(driver defined) buffer types
+<constant>V4L2_BUF_TYPE_PRIVATE</constant> and higher. Applications
+should set this to 0.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
@@ -921,13 +919,6 @@ Drivers set or clear this flag when the <constant>VIDIOC_DQBUF</constant>
 ioctl is called.</entry>
 	  </row>
 	  <row>
-	    <entry><constant>V4L2_BUF_FLAG_INPUT</constant></entry>
-	    <entry>0x0200</entry>
-	    <entry>The <structfield>input</structfield> field is valid.
-Applications set or clear this flag before calling the
-<constant>VIDIOC_QBUF</constant> ioctl.</entry>
-	  </row>
-	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_PREPARED</constant></entry>
 	    <entry>0x0400</entry>
 	    <entry>The buffer has been prepared for I/O and can be queued by the
diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
index 9caa49a..77ff5be 100644
--- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
@@ -71,12 +71,9 @@ initialize the <structfield>bytesused</structfield>,
 <structfield>field</structfield> and
 <structfield>timestamp</structfield> fields, see <xref
 linkend="buffer" /> for details.
-Applications must also set <structfield>flags</structfield> to 0. If a driver
-supports capturing from specific video inputs and you want to specify a video
-input, then <structfield>flags</structfield> should be set to
-<constant>V4L2_BUF_FLAG_INPUT</constant> and the field
-<structfield>input</structfield> must be initialized to the desired input.
-The <structfield>reserved</structfield> field must be set to 0. When using
+Applications must also set <structfield>flags</structfield> to 0.
+The <structfield>reserved2</structfield> and
+<structfield>reserved</structfield> fields must be set to 0. When using
 the <link linkend="planar-apis">multi-planar API</link>, the
 <structfield>m.planes</structfield> field must contain a userspace pointer
 to a filled-in array of &v4l2-plane; and the <structfield>length</structfield>
diff --git a/drivers/media/video/cpia2/cpia2_v4l.c b/drivers/media/video/cpia2/cpia2_v4l.c
index 077eb1d..c105612 100644
--- a/drivers/media/video/cpia2/cpia2_v4l.c
+++ b/drivers/media/video/cpia2/cpia2_v4l.c
@@ -1289,7 +1289,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
 	buf->sequence = cam->buffers[buf->index].seq;
 	buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
 	buf->length = cam->frame_size;
-	buf->input = 0;
+	buf->reserved2 = 0;
 	buf->reserved = 0;
 	memset(&buf->timecode, 0, sizeof(buf->timecode));
 
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index 2829d25..6d566b3 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -327,7 +327,7 @@ struct v4l2_buffer32 {
 		compat_caddr_t  planes;
 	} m;
 	__u32			length;
-	__u32			input;
+	__u32			reserved2;
 	__u32			reserved;
 };
 
@@ -387,8 +387,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		get_user(kp->index, &up->index) ||
 		get_user(kp->type, &up->type) ||
 		get_user(kp->flags, &up->flags) ||
-		get_user(kp->memory, &up->memory) ||
-		get_user(kp->input, &up->input))
+		get_user(kp->memory, &up->memory)
 			return -EFAULT;
 
 	if (V4L2_TYPE_IS_OUTPUT(kp->type))
@@ -472,8 +471,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->index, &up->index) ||
 		put_user(kp->type, &up->type) ||
 		put_user(kp->flags, &up->flags) ||
-		put_user(kp->memory, &up->memory) ||
-		put_user(kp->input, &up->input))
+		put_user(kp->memory, &up->memory)
 			return -EFAULT;
 
 	if (put_user(kp->bytesused, &up->bytesused) ||
@@ -482,7 +480,8 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) ||
 		copy_to_user(&up->timecode, &kp->timecode, sizeof(struct v4l2_timecode)) ||
 		put_user(kp->sequence, &up->sequence) ||
-		put_user(kp->reserved, &up->reserved))
+		put_user(kp->reserved2, &up->reserved2) ||
+		put_user(kp->reserved, &up->reserved)
 			return -EFAULT;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index ffdf59c..bf7a326 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -359,11 +359,6 @@ static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b,
 		break;
 	}
 
-	if (vb->input != UNSET) {
-		b->flags |= V4L2_BUF_FLAG_INPUT;
-		b->input  = vb->input;
-	}
-
 	b->field     = vb->field;
 	b->timestamp = vb->ts;
 	b->bytesused = vb->size;
@@ -402,7 +397,6 @@ int __videobuf_mmap_setup(struct videobuf_queue *q,
 			break;
 
 		q->bufs[i]->i      = i;
-		q->bufs[i]->input  = UNSET;
 		q->bufs[i]->memory = memory;
 		q->bufs[i]->bsize  = bsize;
 		switch (memory) {
@@ -566,16 +560,6 @@ int videobuf_qbuf(struct videobuf_queue *q, struct v4l2_buffer *b)
 		goto done;
 	}
 
-	if (b->flags & V4L2_BUF_FLAG_INPUT) {
-		if (b->input >= q->inputs) {
-			dprintk(1, "qbuf: wrong input.\n");
-			goto done;
-		}
-		buf->input = b->input;
-	} else {
-		buf->input = UNSET;
-	}
-
 	switch (b->memory) {
 	case V4L2_MEMORY_MMAP:
 		if (0 == buf->baddr) {
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 3786d88..ccc71f2 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -336,9 +336,9 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 	struct vb2_queue *q = vb->vb2_queue;
 	int ret;
 
-	/* Copy back data such as timestamp, flags, input, etc. */
+	/* Copy back data such as timestamp, flags, etc. */
 	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
-	b->input = vb->v4l2_buf.input;
+	b->reserved2 = vb->v4l2_buf.reserved2;
 	b->reserved = vb->v4l2_buf.reserved;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
@@ -860,7 +860,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
 
 	vb->v4l2_buf.field = b->field;
 	vb->v4l2_buf.timestamp = b->timestamp;
-	vb->v4l2_buf.input = b->input;
 	vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS;
 
 	return 0;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 5a09ac3..fed1d40 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -652,7 +652,7 @@ struct v4l2_buffer {
 		struct v4l2_plane *planes;
 	} m;
 	__u32			length;
-	__u32			input;
+	__u32			reserved2;
 	__u32			reserved;
 };
 
@@ -666,7 +666,6 @@ struct v4l2_buffer {
 /* Buffer is ready, but the data contained within is corrupted. */
 #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 */
 #define V4L2_BUF_FLAG_PREPARED	0x0400	/* Buffer is prepared for queuing */
 /* Cache handling flags */
 #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
-- 
1.7.2.5

Re: [PATCH v3 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Hans Verkuil]

Hi Ezequiel,

I'm just bringing this proposal to your attention as I am wondering how your driver (and
the old easycap driver that your driver will replace) handle the easycap device with
multiple inputs? Is it cycling through all inputs? In that case we might need the input
field.

Regards,

	Hans

On Wed May 2 2012 14:40:03 Sakari Ailus wrote:
> Remove input field in struct v4l2_buffer and flag V4L2_BUF_FLAG_INPUT which
> tells the former is valid. The flag is used by no driver currently.
> 
> Also change the documentation accordingly.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Hi,
> 
> This is the third version of the v4l2_buffer.input field removal patch.
> 
> What has changed since the previous version:
> 
> - Rename input as reserved2 instead of combining it to reserved and making
>   it an array.
> - cpia compile fix.
> - Change documentation accordingly.
> 
>  Documentation/DocBook/media/v4l/compat.xml      |    6 ++++++
>  Documentation/DocBook/media/v4l/io.xml          |   19 +++++--------------
>  Documentation/DocBook/media/v4l/vidioc-qbuf.xml |    9 +++------
>  drivers/media/video/cpia2/cpia2_v4l.c           |    2 +-
>  drivers/media/video/v4l2-compat-ioctl32.c       |   11 +++++------
>  drivers/media/video/videobuf-core.c             |   16 ----------------
>  drivers/media/video/videobuf2-core.c            |    5 ++---
>  include/linux/videodev2.h                       |    3 +--
>  8 files changed, 23 insertions(+), 48 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
> index 87339b2..b939457 100644
> --- a/Documentation/DocBook/media/v4l/compat.xml
> +++ b/Documentation/DocBook/media/v4l/compat.xml
> @@ -2422,6 +2422,12 @@ details.</para>
>  	  &VIDIOC-SUBDEV-G-SELECTION; and
>  	  &VIDIOC-SUBDEV-S-SELECTION;.</para>
>          </listitem>
> +	<listitem>
> +	  <para>Replaced <structfield>input</structfield> in
> +	  <structname>v4l2_buffer</structname> by
> +	  <structfield>reserved2</structfield> and removed
> +	  <constant>V4L2_BUF_FLAG_INPUT</constant>.</para>
> +	</listitem>
>        </orderedlist>
>      </section>
>  
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index b815929..e4cb063 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -681,14 +681,12 @@ memory, set by the application. See <xref linkend="userp" /> for details.
>  	  </row>
>  	  <row>
>  	    <entry>__u32</entry>
> -	    <entry><structfield>input</structfield></entry>
> +	    <entry><structfield>reserved2</structfield></entry>
>  	    <entry></entry>
> -	    <entry>Some video capture drivers support rapid and
> -synchronous video input changes, a function useful for example in
> -video surveillance applications. For this purpose applications set the
> -<constant>V4L2_BUF_FLAG_INPUT</constant> flag, and this field to the
> -number of a video input as in &v4l2-input; field
> -<structfield>index</structfield>.</entry>
> +	    <entry>A place holder for future extensions and custom
> +(driver defined) buffer types
> +<constant>V4L2_BUF_TYPE_PRIVATE</constant> and higher. Applications
> +should set this to 0.</entry>
>  	  </row>
>  	  <row>
>  	    <entry>__u32</entry>
> @@ -921,13 +919,6 @@ Drivers set or clear this flag when the <constant>VIDIOC_DQBUF</constant>
>  ioctl is called.</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>V4L2_BUF_FLAG_INPUT</constant></entry>
> -	    <entry>0x0200</entry>
> -	    <entry>The <structfield>input</structfield> field is valid.
> -Applications set or clear this flag before calling the
> -<constant>VIDIOC_QBUF</constant> ioctl.</entry>
> -	  </row>
> -	  <row>
>  	    <entry><constant>V4L2_BUF_FLAG_PREPARED</constant></entry>
>  	    <entry>0x0400</entry>
>  	    <entry>The buffer has been prepared for I/O and can be queued by the
> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> index 9caa49a..77ff5be 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> @@ -71,12 +71,9 @@ initialize the <structfield>bytesused</structfield>,
>  <structfield>field</structfield> and
>  <structfield>timestamp</structfield> fields, see <xref
>  linkend="buffer" /> for details.
> -Applications must also set <structfield>flags</structfield> to 0. If a driver
> -supports capturing from specific video inputs and you want to specify a video
> -input, then <structfield>flags</structfield> should be set to
> -<constant>V4L2_BUF_FLAG_INPUT</constant> and the field
> -<structfield>input</structfield> must be initialized to the desired input.
> -The <structfield>reserved</structfield> field must be set to 0. When using
> +Applications must also set <structfield>flags</structfield> to 0.
> +The <structfield>reserved2</structfield> and
> +<structfield>reserved</structfield> fields must be set to 0. When using
>  the <link linkend="planar-apis">multi-planar API</link>, the
>  <structfield>m.planes</structfield> field must contain a userspace pointer
>  to a filled-in array of &v4l2-plane; and the <structfield>length</structfield>
> diff --git a/drivers/media/video/cpia2/cpia2_v4l.c b/drivers/media/video/cpia2/cpia2_v4l.c
> index 077eb1d..c105612 100644
> --- a/drivers/media/video/cpia2/cpia2_v4l.c
> +++ b/drivers/media/video/cpia2/cpia2_v4l.c
> @@ -1289,7 +1289,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
>  	buf->sequence = cam->buffers[buf->index].seq;
>  	buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
>  	buf->length = cam->frame_size;
> -	buf->input = 0;
> +	buf->reserved2 = 0;
>  	buf->reserved = 0;
>  	memset(&buf->timecode, 0, sizeof(buf->timecode));
>  
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
> index 2829d25..6d566b3 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -327,7 +327,7 @@ struct v4l2_buffer32 {
>  		compat_caddr_t  planes;
>  	} m;
>  	__u32			length;
> -	__u32			input;
> +	__u32			reserved2;
>  	__u32			reserved;
>  };
>  
> @@ -387,8 +387,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
>  		get_user(kp->index, &up->index) ||
>  		get_user(kp->type, &up->type) ||
>  		get_user(kp->flags, &up->flags) ||
> -		get_user(kp->memory, &up->memory) ||
> -		get_user(kp->input, &up->input))
> +		get_user(kp->memory, &up->memory)
>  			return -EFAULT;
>  
>  	if (V4L2_TYPE_IS_OUTPUT(kp->type))
> @@ -472,8 +471,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
>  		put_user(kp->index, &up->index) ||
>  		put_user(kp->type, &up->type) ||
>  		put_user(kp->flags, &up->flags) ||
> -		put_user(kp->memory, &up->memory) ||
> -		put_user(kp->input, &up->input))
> +		put_user(kp->memory, &up->memory)
>  			return -EFAULT;
>  
>  	if (put_user(kp->bytesused, &up->bytesused) ||
> @@ -482,7 +480,8 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
>  		put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) ||
>  		copy_to_user(&up->timecode, &kp->timecode, sizeof(struct v4l2_timecode)) ||
>  		put_user(kp->sequence, &up->sequence) ||
> -		put_user(kp->reserved, &up->reserved))
> +		put_user(kp->reserved2, &up->reserved2) ||
> +		put_user(kp->reserved, &up->reserved)
>  			return -EFAULT;
>  
>  	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
> diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
> index ffdf59c..bf7a326 100644
> --- a/drivers/media/video/videobuf-core.c
> +++ b/drivers/media/video/videobuf-core.c
> @@ -359,11 +359,6 @@ static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b,
>  		break;
>  	}
>  
> -	if (vb->input != UNSET) {
> -		b->flags |= V4L2_BUF_FLAG_INPUT;
> -		b->input  = vb->input;
> -	}
> -
>  	b->field     = vb->field;
>  	b->timestamp = vb->ts;
>  	b->bytesused = vb->size;
> @@ -402,7 +397,6 @@ int __videobuf_mmap_setup(struct videobuf_queue *q,
>  			break;
>  
>  		q->bufs[i]->i      = i;
> -		q->bufs[i]->input  = UNSET;
>  		q->bufs[i]->memory = memory;
>  		q->bufs[i]->bsize  = bsize;
>  		switch (memory) {
> @@ -566,16 +560,6 @@ int videobuf_qbuf(struct videobuf_queue *q, struct v4l2_buffer *b)
>  		goto done;
>  	}
>  
> -	if (b->flags & V4L2_BUF_FLAG_INPUT) {
> -		if (b->input >= q->inputs) {
> -			dprintk(1, "qbuf: wrong input.\n");
> -			goto done;
> -		}
> -		buf->input = b->input;
> -	} else {
> -		buf->input = UNSET;
> -	}
> -
>  	switch (b->memory) {
>  	case V4L2_MEMORY_MMAP:
>  		if (0 == buf->baddr) {
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index 3786d88..ccc71f2 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -336,9 +336,9 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
>  	struct vb2_queue *q = vb->vb2_queue;
>  	int ret;
>  
> -	/* Copy back data such as timestamp, flags, input, etc. */
> +	/* Copy back data such as timestamp, flags, etc. */
>  	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
> -	b->input = vb->v4l2_buf.input;
> +	b->reserved2 = vb->v4l2_buf.reserved2;
>  	b->reserved = vb->v4l2_buf.reserved;
>  
>  	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
> @@ -860,7 +860,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
>  
>  	vb->v4l2_buf.field = b->field;
>  	vb->v4l2_buf.timestamp = b->timestamp;
> -	vb->v4l2_buf.input = b->input;
>  	vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS;
>  
>  	return 0;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 5a09ac3..fed1d40 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -652,7 +652,7 @@ struct v4l2_buffer {
>  		struct v4l2_plane *planes;
>  	} m;
>  	__u32			length;
> -	__u32			input;
> +	__u32			reserved2;
>  	__u32			reserved;
>  };
>  
> @@ -666,7 +666,6 @@ struct v4l2_buffer {
>  /* Buffer is ready, but the data contained within is corrupted. */
>  #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 */
>  #define V4L2_BUF_FLAG_PREPARED	0x0400	/* Buffer is prepared for queuing */
>  /* Cache handling flags */
>  #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
> 

Re: [PATCH v3 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Ezequiel Garcia]

Hi Hans,

On Mon, May 28, 2012 at 7:27 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Ezequiel,
>
> I'm just bringing this proposal to your attention as I am wondering how your driver (and
> the old easycap driver that your driver will replace) handle the easycap device with
> multiple inputs? Is it cycling through all inputs? In that case we might need the input
> field.
>

I've been delaying the multiple input device handling because:
1) I wanted to understand the simpler case first, and
2) I didn't actually own the device (I've bought it a few days ago).

So, I'm gonna take a look at it and let you all now, as soon
as possible.

Thanks,
Ezequiel.

Re: [PATCH v3 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Ezequiel Garcia]

Hi again,

On Mon, May 28, 2012 at 8:52 AM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>> I'm just bringing this proposal to your attention as I am wondering how your driver (and
>> the old easycap driver that your driver will replace) handle the easycap device with
>> multiple inputs? Is it cycling through all inputs? In that case we might need the input
>> field.

What do you mean by "cycling through all inputs"?

Do you mean registering one video node per video input
and support simultaneous streaming?

In that case, I don't have that in mind and I'm not sure if the hw
supports it.

On the contrary, I was thinking in registering just one video device
and let user select input through ioctl. All that's needed
it to set some stk1160 (and maybe saa711x) registers to route
the selected input.

I may be missing something, but I don't see any relation between
video buffer queue and selected input.
(Perhaps this is OT and we should discuss this in another thread)

Regards,
Ezequiel.

Re: [PATCH v3 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Hans Verkuil]

On Mon May 28 2012 18:29:11 Ezequiel Garcia wrote:
> Hi again,
> 
> On Mon, May 28, 2012 at 8:52 AM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> >> I'm just bringing this proposal to your attention as I am wondering how your driver (and
> >> the old easycap driver that your driver will replace) handle the easycap device with
> >> multiple inputs? Is it cycling through all inputs? In that case we might need the input
> >> field.
> 
> What do you mean by "cycling through all inputs"?
> 
> Do you mean registering one video node per video input
> and support simultaneous streaming?
> 
> In that case, I don't have that in mind and I'm not sure if the hw
> supports it.
> 
> On the contrary, I was thinking in registering just one video device
> and let user select input through ioctl. All that's needed
> it to set some stk1160 (and maybe saa711x) registers to route
> the selected input.
> 
> I may be missing something, but I don't see any relation between
> video buffer queue and selected input.
> (Perhaps this is OT and we should discuss this in another thread)

Well, this particular API was intended to let the hardware switch from one input
to another automatically: e.g. the first frame is from input 1, the second from
input 2, etc. until it has gone through all inputs and goes back to input 1.

This requires hardware support and if the stk1160 can't do that, then you can
forget about all this. I was just wondering about it since the easycap is sold
with surveillance applications in mind:

http://dx.com/p/easycap-4-channel-4-input-usb-2-0-dvr-video-capture-surveillance-dongle-11127?item=5

However, reading through the comments I realize that the software just switches
input every second or so, so this seems to be done by software, not hardware.

In other words, your approach is the right one :-)

Regards,

	Hans

Re: [PATCH v3 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Ezequiel Garcia]

On Mon, May 28, 2012 at 2:10 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> I may be missing something, but I don't see any relation between
>> video buffer queue and selected input.
>> (Perhaps this is OT and we should discuss this in another thread)
>
> Well, this particular API was intended to let the hardware switch from one input
> to another automatically: e.g. the first frame is from input 1, the second from
> input 2, etc. until it has gone through all inputs and goes back to input 1.
>
> This requires hardware support and if the stk1160 can't do that, then you can
> forget about all this.

I did some tests earlier this morning and apparently the input
switching is done
through some stk1160 GPIO port setting.
However, I'll have to figure this out reading easycap staging driver
and trying different values over and over.
Plus, saa7115 driver supports "s_routing" function, but couldn't make
it work: it looked like saa7115 input
setting was fixed and stk1160 handles the input switching.

Also, I'm not sure if saa7115 "s_stream" to enable/disable capture is
also working.
Sounds a bit weird to me.

One last query: I found *no* way to distinguish between one device
(one composite input)
and the other (four composite input). What do you suggest? Just
implement all of them always?

Looks like I'm on the right path anyway :)

Thanks,
Ezequiel.

Re: [PATCH v3 1/1] v4l: drop v4l2_buffer.input and V4L2_BUF_FLAG_INPUT

[Hans Verkuil]

On Mon May 28 2012 19:25:41 Ezequiel Garcia wrote:
> On Mon, May 28, 2012 at 2:10 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >> I may be missing something, but I don't see any relation between
> >> video buffer queue and selected input.
> >> (Perhaps this is OT and we should discuss this in another thread)
> >
> > Well, this particular API was intended to let the hardware switch from one input
> > to another automatically: e.g. the first frame is from input 1, the second from
> > input 2, etc. until it has gone through all inputs and goes back to input 1.
> >
> > This requires hardware support and if the stk1160 can't do that, then you can
> > forget about all this.
> 
> I did some tests earlier this morning and apparently the input
> switching is done
> through some stk1160 GPIO port setting.
> However, I'll have to figure this out reading easycap staging driver
> and trying different values over and over.
> Plus, saa7115 driver supports "s_routing" function, but couldn't make
> it work: it looked like saa7115 input
> setting was fixed and stk1160 handles the input switching.
> 
> Also, I'm not sure if saa7115 "s_stream" to enable/disable capture is
> also working.
> Sounds a bit weird to me.
> 
> One last query: I found *no* way to distinguish between one device
> (one composite input)
> and the other (four composite input). What do you suggest? Just
> implement all of them always?

If you can't tell them apart, then that would be your only option. You
might also add a 'model' module option so users can tell the driver which model
it is.

Another might be (I don't know if that will work) to try all inputs and see
if you can detect a difference in the saa7115: however, I suspect you can't
tell the difference between 'I have only one input' and 'I have four but I
haven't hooked up anything to my inputs'.

I *hate* it when they don't bother to change the USB IDs.
 
> Looks like I'm on the right path anyway :)

Yup!

Regards,

	Hans