[RFCv2,1/4] videobuf2-core: move plane verification out of __fill_v4l2/vb_buffer.

Commit Message

Hans Verkuil Sept. 28, 2012, 9:30 a.m. UTC

From: Hans Verkuil <[e-mail]>

The plane verification should be done before actually queuing or
dequeuing buffers, so move it out of __fill_v4l2_buffer and __fill_vb_buffer
and call it as a separate step.

This also makes it possible to change the return type of __fill_v4l2/vb_buffer
to void.

The dqbuf case took some special care: before removing a buffer from the
done_list you have to verify that the receiving v4l2_buffer has enough room
for all the planes. The number of planes can differ between buffers due to the
fact that buffers for other formats can be prepared using VIDIOC_PREPARE_BUF.

Signed-off-by: Hans Verkuil <[e-mail]>
---
 drivers/media/v4l2-core/videobuf2-core.c |   78 +++++++++++++++---------------
 1 file changed, 38 insertions(+), 40 deletions(-)

Patch

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index e6a26b4..5a132b2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -276,6 +276,9 @@  static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
  */
 static int __verify_planes_array(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
+	if (!V4L2_TYPE_IS_MULTIPLANAR(b->type))
+		return 0;
+
 	/* Is memory for copying plane information present? */
 	if (NULL == b->m.planes) {
 		dprintk(1, "Multi-planar buffer passed but "
@@ -331,10 +334,9 @@  static bool __buffers_in_use(struct vb2_queue *q)
  * __fill_v4l2_buffer() - fill in a struct v4l2_buffer with information to be
  * returned to userspace
  */
-static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static void __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, etc. */
 	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
@@ -342,13 +344,9 @@  static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 	b->reserved = vb->v4l2_buf.reserved;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
-		ret = __verify_planes_array(vb, b);
-		if (ret)
-			return ret;
-
 		/*
 		 * Fill in plane-related data if userspace provided an array
-		 * for it. The memory and size is verified above.
+		 * for it. The caller has already verified memory and size.
 		 */
 		memcpy(b->m.planes, vb->v4l2_planes,
 			b->length * sizeof(struct v4l2_plane));
@@ -391,8 +389,6 @@  static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 
 	if (__buffer_in_use(q, vb))
 		b->flags |= V4L2_BUF_FLAG_MAPPED;
-
-	return 0;
 }
 
 /**
@@ -411,6 +407,7 @@  static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
 {
 	struct vb2_buffer *vb;
+	int ret;
 
 	if (b->type != q->type) {
 		dprintk(1, "querybuf: wrong buffer type\n");
@@ -422,8 +419,10 @@  int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
 		return -EINVAL;
 	}
 	vb = q->bufs[b->index];
-
-	return __fill_v4l2_buffer(vb, b);
+	ret = __verify_planes_array(vb, b);
+	if (!ret)
+		__fill_v4l2_buffer(vb, b);
+	return ret;
 }
 EXPORT_SYMBOL(vb2_querybuf);
 
@@ -813,24 +812,16 @@  void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 EXPORT_SYMBOL_GPL(vb2_buffer_done);
 
 /**
- * __fill_vb2_buffer() - fill a vb2_buffer with information provided in
- * a v4l2_buffer by the userspace
+ * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a
+ * v4l2_buffer by the userspace. The caller has already verified that struct
+ * v4l2_buffer has a valid number of planes.
  */
-static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
+static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
 				struct v4l2_plane *v4l2_planes)
 {
 	unsigned int plane;
-	int ret;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
-		/*
-		 * Verify that the userspace gave us a valid array for
-		 * plane information.
-		 */
-		ret = __verify_planes_array(vb, b);
-		if (ret)
-			return ret;
-
 		/* Fill in driver-provided information for OUTPUT types */
 		if (V4L2_TYPE_IS_OUTPUT(b->type)) {
 			/*
@@ -872,8 +863,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.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS;
-
-	return 0;
 }
 
 /**
@@ -888,10 +877,8 @@  static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	int ret;
 	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
 
-	/* Verify and copy relevant information provided by the userspace */
-	ret = __fill_vb2_buffer(vb, b, planes);
-	if (ret)
-		return ret;
+	/* Copy relevant information provided by the userspace */
+	__fill_vb2_buffer(vb, b, planes);
 
 	for (plane = 0; plane < vb->num_planes; ++plane) {
 		/* Skip the plane if already verified */
@@ -966,7 +953,8 @@  err:
  */
 static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
-	return __fill_vb2_buffer(vb, b, vb->v4l2_planes);
+	__fill_vb2_buffer(vb, b, vb->v4l2_planes);
+	return 0;
 }
 
 /**
@@ -1059,7 +1047,9 @@  int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
 		dprintk(1, "%s(): invalid buffer state %d\n", __func__, vb->state);
 		return -EINVAL;
 	}
-
+	ret = __verify_planes_array(vb, b);
+	if (ret < 0)
+		return ret;
 	ret = __buf_prepare(vb, b);
 	if (ret < 0)
 		return ret;
@@ -1147,6 +1137,9 @@  int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 		ret = -EINVAL;
 		goto unlock;
 	}
+	ret = __verify_planes_array(vb, b);
+	if (ret)
+		goto unlock;
 
 	switch (vb->state) {
 	case VB2_BUF_STATE_DEQUEUED:
@@ -1243,8 +1236,10 @@  static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 		 * the locks or return an error if one occurred.
 		 */
 		call_qop(q, wait_finish, q);
-		if (ret)
+		if (ret) {
+			dprintk(1, "Sleep was interrupted\n");
 			return ret;
+		}
 	}
 	return 0;
 }
@@ -1255,7 +1250,7 @@  static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
  * Will sleep if required for nonblocking == false.
  */
 static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
-				int nonblocking)
+				struct v4l2_buffer *b, int nonblocking)
 {
 	unsigned long flags;
 	int ret;
@@ -1273,10 +1268,16 @@  static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
 	 */
 	spin_lock_irqsave(&q->done_lock, flags);
 	*vb = list_first_entry(&q->done_list, struct vb2_buffer, done_entry);
-	list_del(&(*vb)->done_entry);
+	/*
+	 * Only remove the buffer from done_list if v4l2_buffer can handle all
+	 * the planes.
+	 */
+	ret = __verify_planes_array(*vb, b);
+	if (!ret)
+		list_del(&(*vb)->done_entry);
 	spin_unlock_irqrestore(&q->done_lock, flags);
 
-	return 0;
+	return ret;
 }
 
 /**
@@ -1335,12 +1336,9 @@  int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
 		dprintk(1, "dqbuf: invalid buffer type\n");
 		return -EINVAL;
 	}
-
-	ret = __vb2_get_done_vb(q, &vb, nonblocking);
-	if (ret < 0) {
-		dprintk(1, "dqbuf: error getting next done buffer\n");
+	ret = __vb2_get_done_vb(q, &vb, b, nonblocking);
+	if (ret < 0)
 		return ret;
-	}
 
 	ret = call_qop(q, buf_finish, vb);
 	if (ret) {