[REVIEW PATCH 03/11] vb2: if bytesused is 0, then fill with output buffer length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Hans Verkuil <hans.verkuil@xxxxxxxxx>

The application should really always fill in bytesused for output
buffers, unfortunately the vb2 framework never checked for that.

So for single planar formats replace a bytesused of 0 by the length
of the buffer, and for multiplanar format do the same if bytesused is
0 for ALL planes.

This seems to be what the user really intended if v4l2_buffer was
just memset to 0.

I'm afraid that just checking for this and returning an error would
break too many applications. Quite a few drivers never check for bytesused
at all and just use the buffer length instead.

Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
 drivers/media/v4l2-core/videobuf2-core.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 1a09442..83e78e9 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1145,19 +1145,35 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 			memset(v4l2_planes[plane].reserved, 0,
 			       sizeof(v4l2_planes[plane].reserved));
 			v4l2_planes[plane].data_offset = 0;
+			v4l2_planes[plane].bytesused = 0;
 		}
 
 		/* Fill in driver-provided information for OUTPUT types */
 		if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+			bool bytesused_is_used;
+
+			/* Check if bytesused == 0 for all planes */
+			for (plane = 0; plane < vb->num_planes; ++plane)
+				if (b->m.planes[plane].bytesused)
+					break;
+			bytesused_is_used = plane < vb->num_planes;
+
 			/*
 			 * Will have to go up to b->length when API starts
 			 * accepting variable number of planes.
+			 *
+			 * If bytesused_is_used is false, then fall back to the
+			 * full buffer size. In that case userspace clearly
+			 * never bothered to set it and it's a safe assumption
+			 * that they really meant to use the full plane sizes.
 			 */
 			for (plane = 0; plane < vb->num_planes; ++plane) {
-				v4l2_planes[plane].bytesused =
-					b->m.planes[plane].bytesused;
-				v4l2_planes[plane].data_offset =
-					b->m.planes[plane].data_offset;
+				struct v4l2_plane *pdst = &v4l2_planes[plane];
+				struct v4l2_plane *psrc = &b->m.planes[plane];
+
+				pdst->bytesused = bytesused_is_used ?
+					psrc->bytesused : psrc->length;
+				pdst->data_offset = psrc->data_offset;
 			}
 		}
 
@@ -1183,9 +1199,15 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 		 * so fill in relevant v4l2_buffer struct fields instead.
 		 * In videobuf we use our internal V4l2_planes struct for
 		 * single-planar buffers as well, for simplicity.
+		 *
+		 * If bytesused == 0, then fall back to the full buffer size
+		 * as that's a sensible default.
 		 */
 		if (V4L2_TYPE_IS_OUTPUT(b->type))
-			v4l2_planes[0].bytesused = b->bytesused;
+			v4l2_planes[0].bytesused =
+				b->bytesused ? b->bytesused : b->length;
+		else
+			v4l2_planes[0].bytesused = 0;
 		/* Single-planar buffers never use data_offset */
 		v4l2_planes[0].data_offset = 0;
 
-- 
1.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux