Hi everybody,
I've been working on code refactoring to make the UVC driver more robust when
used with non-compliant UVC devices (we all know there are many of them).
Before you rejoice, this has nothing to do with the Logitech firmware bug.
Sorry.
Anyway, the attached patch (against SVN head) modifies video control handling
to handle failed GET_MIN/GET_MAX/GET_DEF requests more gracefully. Failed
requests will now generate a one-time warning message instead of the
usual "Failed to query..." error, which should be more user-friendly. The
driver will also recover automatically from failed GET_MIN/GET_MAX requests
when the device is half-broken without requiring the MINMAX quirk (fully
broken devices still need the quirk).
The patch is required to implement JPEG quality control which I will soon work
on. As it's quite intrusive, I'd like to make sure it doesn't break anything.
I'd appreciate if you could test it, especially if you got one of the
following cameras:
ALi M5606 (Clevo M540SR) (0402:5606)
Creative Live! Optia (041e:4057)
Microsoft Lifecam NX-6000 (045e:00f8)
Microsoft Lifecam VX-7000 (045e:0723)
Apple Built-In iSight (05ac:8501)
Silicon Motion SM371 (090c:b371)
Acer OEM Webcam (5986:0100)
Packard Bell OEM Webcam (5986:0101)
Acer Crystal Eye webcam (5986:0102)
Compaq Presario B1200 (5986:0104)
Acer Travelmate 7720 (5986:0105)
Medion Akoya Mini E1210 (5986:0141)
Acer OrbiCam (5986:0200)
Fujitsu Amilo SI2636 (5986:0202)
Advent 4211 (5986:0203)
??? (5986:0300)
Clevo M570TU (5986:0303)
Testing the patch means making sure the webcam is still detected properly by
the driver (it should be as I haven't changed anything there) and is still
able to stream video (use any software you like, pick one that you know is
working).
Of course nothing should break, but bugs happen. That's not completely true,
as I don't have enough information about the two Microsoft cameras to make
sure they will still work. I'm thus especially interesting in hearing from
you if you got a supported Microsoft webcam.
Cameras in the above list should print a few extra messages in the kernel log
when starting a video stream. Please include them in your report.
Enough said, I wish you all happy testing :-)
Best regards,
Laurent Pinchart
Index: uvc_video.c
===================================================================
--- uvc_video.c (revision 260)
+++ uvc_video.c (working copy)
@@ -38,15 +38,22 @@
{
__u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
unsigned int pipe;
- int ret;
pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
: usb_sndctrlpipe(dev->udev, 0);
type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
- ret = usb_control_msg(dev->udev, pipe, query, type, cs << 8,
+ return usb_control_msg(dev->udev, pipe, query, type, cs << 8,
unit << 8 | intfnum, data, size, timeout);
+}
+int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
+ __u8 intfnum, __u8 cs, void *data, __u16 size)
+{
+ int ret;
+
+ ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
+ UVC_CTRL_CONTROL_TIMEOUT);
if (ret != size) {
uvc_printk(KERN_ERR, "Failed to query (%u) UVC control %u "
"(unit %u) : %d (exp. %u).\n", query, cs, unit, ret,
@@ -57,13 +64,6 @@
return 0;
}
-int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
- __u8 intfnum, __u8 cs, void *data, __u16 size)
-{
- return __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
- UVC_CTRL_CONTROL_TIMEOUT);
-}
-
static void uvc_fixup_buffer_size(struct uvc_video_device *video,
struct uvc_streaming_control *ctrl)
{
@@ -104,8 +104,37 @@
ret = __uvc_query_ctrl(video->dev, query, 0, video->streaming->intfnum,
probe ? VS_PROBE_CONTROL : VS_COMMIT_CONTROL, data, size,
UVC_CTRL_STREAMING_TIMEOUT);
- if (ret < 0)
+
+ if ((query == GET_MIN || query == GET_MAX) && ret == 2) {
+ /* Some cameras, mostly based on Bison Electronics chipsets,
+ * answer a GET_MIN or GET_MAX request with the wCompQuality
+ * field only.
+ */
+ uvc_warn_once(video->dev, UVC_WARN_MINMAX, "UVC non "
+ "compliance - GET_MIN/MAX(PROBE) incorrectly "
+ "supported. Enabling workaround.\n");
+ memset(ctrl, 0, sizeof ctrl);
+ ctrl->wCompQuality = le16_to_cpup((__le16 *)data);
+ uvc_printk(KERN_INFO, "wCompQuality(%u) : %u\n", query, ctrl->wCompQuality);
+ ret = 0;
goto out;
+ } else if (query == GET_DEF && probe == 1) {
+ /* Many cameras don't support the GET_DEF request on their
+ * video probe control. Warn once and return, the caller will
+ * fall back to GET_CUR.
+ */
+ uvc_warn_once(video->dev, UVC_WARN_PROBE_DEF, "UVC non "
+ "compliance - GET_DEF(PROBE) not supported. "
+ "Enabling workaround.\n");
+ ret = -EIO;
+ goto out;
+ } else if (ret != size) {
+ uvc_printk(KERN_ERR, "Failed to query (%u) UVC %s control : "
+ "%d (exp. %u).\n", query, probe ? "probe" : "commit",
+ ret, size);
+ ret = -EIO;
+ goto out;
+ }
ctrl->bmHint = le16_to_cpup((__le16 *)&data[0]);
ctrl->bFormatIndex = data[2];
@@ -140,13 +169,14 @@
* Try to get the value from the format and frame descriptor.
*/
uvc_fixup_buffer_size(video, ctrl);
+ ret = 0;
out:
kfree(data);
return ret;
}
-int uvc_set_video_ctrl(struct uvc_video_device *video,
+static int uvc_set_video_ctrl(struct uvc_video_device *video,
struct uvc_streaming_control *ctrl, int probe)
{
__u8 *data;
@@ -188,6 +218,12 @@
video->streaming->intfnum,
probe ? VS_PROBE_CONTROL : VS_COMMIT_CONTROL, data, size,
UVC_CTRL_STREAMING_TIMEOUT);
+ if (ret != size) {
+ uvc_printk(KERN_ERR, "Failed to set UVC %s control : "
+ "%d (exp. %u).\n", probe ? "probe" : "commit",
+ ret, size);
+ ret = -EIO;
+ }
kfree(data);
return ret;
@@ -254,6 +290,12 @@
return ret;
}
+int uvc_commit_video(struct uvc_video_device *video,
+ struct uvc_streaming_control *probe)
+{
+ return uvc_set_video_ctrl(video, probe, 0);
+}
+
/* ------------------------------------------------------------------------
* Video codecs
*/
Index: uvc_v4l2.c
===================================================================
--- uvc_v4l2.c (revision 260)
+++ uvc_v4l2.c (working copy)
@@ -256,7 +256,7 @@
if (ret < 0)
return ret;
- if ((ret = uvc_set_video_ctrl(video, &probe, 0)) < 0)
+ if ((ret = uvc_commit_video(video, &probe)) < 0)
return ret;
memcpy(&video->streaming->ctrl, &probe, sizeof probe);
@@ -320,7 +320,7 @@
return ret;
/* Commit the new settings. */
- if ((ret = uvc_set_video_ctrl(video, &probe, 0)) < 0)
+ if ((ret = uvc_commit_video(video, &probe)) < 0)
return ret;
memcpy(&video->streaming->ctrl, &probe, sizeof probe);
Index: uvc_driver.c
===================================================================
--- uvc_driver.c (revision 260)
+++ uvc_driver.c (working copy)
@@ -1718,42 +1718,6 @@
* though they are compliant.
*/
static struct usb_device_id uvc_ids[] = {
- /* ALi M5606 (Clevo M540SR) */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x0402,
- .idProduct = 0x5606,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Creative Live! Optia */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x041e,
- .idProduct = 0x4057,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Microsoft Lifecam NX-6000 */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x045e,
- .idProduct = 0x00f8,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Microsoft Lifecam VX-7000 */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x045e,
- .idProduct = 0x0723,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
/* Logitech Quickcam Fusion */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -1802,16 +1766,6 @@
.bInterfaceClass = USB_CLASS_VENDOR_SPEC,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0 },
- /* Apple Built-In iSight */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x05ac,
- .idProduct = 0x8501,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX
- | UVC_QUIRK_BUILTIN_ISIGHT },
/* Genesys Logic USB 2.0 PC Camera */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -1821,15 +1775,6 @@
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = UVC_QUIRK_STREAM_NO_FID },
- /* Silicon Motion SM371 */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x090c,
- .idProduct = 0xb371,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
/* MT6227 */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -1914,105 +1859,6 @@
.bInterfaceProtocol = 0,
.driver_info = UVC_QUIRK_PROBE_MINMAX
| UVC_QUIRK_IGNORE_SELECTOR_UNIT},
- /* Acer OEM Webcam - Unknown vendor */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0100,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Packard Bell OEM Webcam - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0101,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Acer Crystal Eye webcam - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0102,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Compaq Presario B1200 - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0104,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Acer Travelmate 7720 - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0105,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Medion Akoya Mini E1210 - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0141,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Acer OrbiCam - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0200,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Fujitsu Amilo SI2636 - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0202,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Advent 4211 - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0203,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0300,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
- /* Clevo M570TU - Bison Electronics */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
- | USB_DEVICE_ID_MATCH_INT_INFO,
- .idVendor = 0x5986,
- .idProduct = 0x0303,
- .bInterfaceClass = USB_CLASS_VIDEO,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_PROBE_MINMAX },
/* Generic USB Video Class */
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, 0) },
{}
Index: uvcvideo.h
===================================================================
--- uvcvideo.h (revision 260)
+++ uvcvideo.h (working copy)
@@ -618,6 +618,7 @@
struct uvc_device {
struct usb_device *udev;
struct usb_interface *intf;
+ unsigned long warnings;
__u32 quirks;
int intfnum;
char name[32];
@@ -680,6 +681,9 @@
#define UVC_TRACE_SUSPEND (1 << 8)
#define UVC_TRACE_STATUS (1 << 9)
+#define UVC_WARN_MINMAX 0
+#define UVC_WARN_PROBE_DEF 1
+
extern unsigned int uvc_trace_param;
#define uvc_trace(flag, msg...) \
@@ -688,6 +692,12 @@
printk(KERN_DEBUG "uvcvideo: " msg); \
} while (0)
+#define uvc_warn_once(dev, warn, msg...) \
+ do { \
+ if (!test_and_set_bit(warn, &dev->warnings)) \
+ printk(KERN_INFO "uvcvideo: " msg); \
+ } while (0)
+
#define uvc_printk(level, msg...) \
printk(level "uvcvideo: " msg)
@@ -741,10 +751,10 @@
extern int uvc_video_enable(struct uvc_video_device *video, int enable);
extern int uvc_probe_video(struct uvc_video_device *video,
struct uvc_streaming_control *probe);
+extern int uvc_commit_video(struct uvc_video_device *video,
+ struct uvc_streaming_control *ctrl);
extern int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
__u8 intfnum, __u8 cs, void *data, __u16 size);
-extern int uvc_set_video_ctrl(struct uvc_video_device *video,
- struct uvc_streaming_control *ctrl, int probe);
/* Status */
extern int uvc_status_init(struct uvc_device *dev);
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
