uvc: fix cropcap v4l2-compliance failure

PATCH In review Hans Verkuil 2 participants 2 messages Apr 3, 2015 – Apr 4, 2015

Subsystems (2)

Media (V4L/DVB) USB video class

lore

Share & tools

lore

Hans Verkuil

author

2015-04-03 09:54 UTC #0 lore raw

PATCH 1/1 uvc: fix cropcap v4l2-compliance failure

The v4l2-compliance tool expects that if VIDIOC_CROPCAP is defined, then VIDIOC_G_SELECTION for TGT_CROP_BOUNDS/DEFAULT is also defined (or COMPOSE in the case of an output device).

In fact, all that a driver has to do to implement cropcap is to support those two targets since the v4l2 core will implement cropcap and fill in the pixelaspect to 1/1 by default.

Implementing cropcap is only needed if the pixelaspect isn't square.

So implement g_selection instead of cropcap in uvc to fix the v4l2-compliance failure.

Signed-off-by Hans Verkuil <[email protected]>

---

drivers/media/usb/uvc/uvc_v4l2.c +22 ~~-13~~

index 43e953f..7078d3e 100644
@@ -1018,26 +1018,35 @@ static int uvc_ioctl_querymenu(struct file *file, void *fh,
 	return uvc_query_v4l2_menu(chain, qm);
 }
 
-static int uvc_ioctl_cropcap(struct file *file, void *fh,
-			     struct v4l2_cropcap *ccap)
+static int uvc_ioctl_g_selection(struct file *file, void *fh,
+				 struct v4l2_selection *sel)
 {
 	struct uvc_fh *handle = fh;
 	struct uvc_streaming *stream = handle->stream;
 
-	if (ccap->type != stream->type)
+	if (sel->type != stream->type)
 		return -EINVAL;
 
-	ccap->bounds.left = 0;
-	ccap->bounds.top = 0;
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		if (stream->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+			return -EINVAL;
+		break;
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		if (stream->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+	sel->r.left = 0;
+	sel->r.top = 0;
 	mutex_lock(&stream->mutex);
-	ccap->bounds.width = stream->cur_frame->wWidth;
-	ccap->bounds.height = stream->cur_frame->wHeight;
+	sel->r.width = stream->cur_frame->wWidth;
+	sel->r.height = stream->cur_frame->wHeight;
 	mutex_unlock(&stream->mutex);
-
-	ccap->defrect = ccap->bounds;
-
-	ccap->pixelaspect.numerator = 1;
-	ccap->pixelaspect.denominator = 1;
 	return 0;
 }

@@ -1449,7 +1458,7 @@ const struct v4l2_ioctl_ops uvc_ioctl_ops = {
 	.vidioc_s_ext_ctrls = uvc_ioctl_s_ext_ctrls,
 	.vidioc_try_ext_ctrls = uvc_ioctl_try_ext_ctrls,
 	.vidioc_querymenu = uvc_ioctl_querymenu,
-	.vidioc_cropcap = uvc_ioctl_cropcap,
+	.vidioc_g_selection = uvc_ioctl_g_selection,
 	.vidioc_g_parm = uvc_ioctl_g_parm,
 	.vidioc_s_parm = uvc_ioctl_s_parm,
 	.vidioc_enum_framesizes = uvc_ioctl_enum_framesizes,

Laurent Pinchart

maintainer Acked-by on v1 1/1

2015-04-04 12:54 UTC #1 lore raw

Hi Hans,

Thank you for the patch.

Quoted 8 lines · On Friday 03 April 2015 11:53:42 Hans Verkuil wrote:

The v4l2-compliance tool expects that if VIDIOC_CROPCAP is defined, then VIDIOC_G_SELECTION for TGT_CROP_BOUNDS/DEFAULT is also defined (or COMPOSE in the case of an output device).
In fact, all that a driver has to do to implement cropcap is to support those two targets since the v4l2 core will implement cropcap and fill in the pixelaspect to 1/1 by default.

Quoted 6 lines

Implementing cropcap is only needed if the pixelaspect isn't square.
So implement g_selection instead of cropcap in uvc to fix the v4l2-compliance failure.
Signed-off-by Hans Verkuil <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

and applied to my tree with a cosmetic change (please see below).

Quoted 27 lines

---

(quoted diff) +0 -0

b/drivers/media/usb/uvc/uvc_v4l2.c index 43e953f..7078d3e 100644

drivers/media/usb/uvc/uvc_v4l2.c +7 -5

@@ -1018,26 +1018,35 @@ static int uvc_ioctl_querymenu(struct file *file,
 void *fh, return uvc_query_v4l2_menu(chain, qm);
 }
 
-static int uvc_ioctl_cropcap(struct file *file, void *fh,
-			     struct v4l2_cropcap *ccap)
+static int uvc_ioctl_g_selection(struct file *file, void *fh,
+				 struct v4l2_selection *sel)
 {
 	struct uvc_fh *handle = fh;
 	struct uvc_streaming *stream = handle->stream;
 
-	if (ccap->type != stream->type)
+	if (sel->type != stream->type)
 		return -EINVAL;
 
-	ccap->bounds.left = 0;
-	ccap->bounds.top = 0;
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		if (stream->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)

drivers/media/usb/uvc/uvc_v4l2.c static int uvc_ioctl_querymenu(struct file *file, · line 1043

Quoted 10 lines

drivers/media/usb/uvc/uvc_v4l2.c +10 -0

+			return -EINVAL;
+		break;
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		if (stream->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}

I've added a blank line here for improved readability.

drivers/media/usb/uvc/uvc_v4l2.c static int uvc_ioctl_querymenu(struct file *file, void *fh, · line 1040

Quoted 13 lines

drivers/media/usb/uvc/uvc_v4l2.c +4 -7

+	sel->r.left = 0;
+	sel->r.top = 0;
 	mutex_lock(&stream->mutex);
-	ccap->bounds.width = stream->cur_frame->wWidth;
-	ccap->bounds.height = stream->cur_frame->wHeight;
+	sel->r.width = stream->cur_frame->wWidth;
+	sel->r.height = stream->cur_frame->wHeight;
 	mutex_unlock(&stream->mutex);
-
-	ccap->defrect = ccap->bounds;
-
-	ccap->pixelaspect.numerator = 1;
-	ccap->pixelaspect.denominator = 1;

And here too.

Quoted 12 lines

drivers/media/usb/uvc/uvc_v4l2.c +1 -1

 	return 0;
 }

@@ -1449,7 +1458,7 @@ const struct v4l2_ioctl_ops uvc_ioctl_ops = {
 	.vidioc_s_ext_ctrls = uvc_ioctl_s_ext_ctrls,
 	.vidioc_try_ext_ctrls = uvc_ioctl_try_ext_ctrls,
 	.vidioc_querymenu = uvc_ioctl_querymenu,
-	.vidioc_cropcap = uvc_ioctl_cropcap,
+	.vidioc_g_selection = uvc_ioctl_g_selection,
 	.vidioc_g_parm = uvc_ioctl_g_parm,
 	.vidioc_s_parm = uvc_ioctl_s_parm,
 	.vidioc_enum_framesizes = uvc_ioctl_enum_framesizes,

Signature · 3 lines

Regards,

Laurent Pinchart