Re: uaudio: fix logitech c310 integrated mic
On Fri, Jun 29, 2018 at 09:05:56AM +0200, Remco wrote: > > > If you don't find any better solution, I'd suggest using a device ID > > > check rather than adding a quirk. Because such quirk cannot be generic. > > > In that case you have an off-by-one, but another device might have a > > > different problem. > > > > That was in the case we'd encounter other devices where the descriptor > > is bogus and has the same issue, but i agree this is unlikely. > > > > More likely for Logitech USB webcams I'm afraid. > My dad's Logitech C500 showed the same issue several years back. > (I'll try to find some time to borrow the device and test your patch) > There's also a report that suggests a C200 may be affected as well. > (https://marc.info/?l=openbsd-misc&m=127952275021290&w=2) Aaah, very interesting data point. Thanks for the pointer.. maybe a 'generic fix' would be to ignore size checks for all logitech webcams, or to print a warning but not error out in this case. Is there some kind of 'usbdevs -v/lsusb -v' database somewhere ? Landry
Re: uaudio: fix logitech c310 integrated mic
If you don't find any better solution, I'd suggest using a device ID check rather than adding a quirk. Because such quirk cannot be generic. In that case you have an off-by-one, but another device might have a different problem. That was in the case we'd encounter other devices where the descriptor is bogus and has the same issue, but i agree this is unlikely. More likely for Logitech USB webcams I'm afraid. My dad's Logitech C500 showed the same issue several years back. (I'll try to find some time to borrow the device and test your patch) There's also a report that suggests a C200 may be affected as well. (https://marc.info/?l=openbsd-misc&m=127952275021290&w=2)
Re: uaudio: fix logitech c310 integrated mic
On Mon, Jun 25, 2018 at 11:59:46AM +0200, Martin Pieuchot wrote: > On 24/06/18(Sun) 14:15, Landry Breuil wrote: > > Hi, > > > > the logitech c310 is supported by uvideo, but its uaudio fails to > > attach properly and fallbacks to ugen. > > > > from that point, i dunno if my analysis is wrong or right, nor how to > > properly handle it, so here's my take: > > - add a UAUDIO_FLAG_BAD_ADC_LEN quirk > > - set it for the c310 vendor/product > > - if the device matches this quirk, aclen++ > > > > feedback or better ideas welcome :) > > Did you look at how other OSes deal with that problem? Maybe you'll > find a more generic hack. I've looked on bxr.su but it seems netbsd and freebsd don't have the c310 in their usbdevs, and https://wiki.freebsd.org/WebcamCompat mentions it needing this strange 'usbconfig -d ugenX.Y do_request' hack. afaict, netbsd uses more or less our logic (cf http://bxr.su/NetBSD/sys/dev/usb/uaudio.c#1914, iirc our uaudio comes from it) freebsd has a 5000+ lines uaudio.c in http://bxr.su/FreeBSD/sys/dev/sound/usb/uaudio.c#955 and i didnt see support for such specific case. > If you don't find any better solution, I'd suggest using a device ID > check rather than adding a quirk. Because such quirk cannot be generic. > In that case you have an off-by-one, but another device might have a > different problem. That was in the case we'd encounter other devices where the descriptor is bogus and has the same issue, but i agree this is unlikely. So checking for USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310, maybe the device revision, and eventually checking if the advertised size is 38 ? Landry
Re: uaudio: fix logitech c310 integrated mic
On 24/06/18(Sun) 14:15, Landry Breuil wrote:
> Hi,
>
> the logitech c310 is supported by uvideo, but its uaudio fails to
> attach properly and fallbacks to ugen.
>
> uaudio0 at uhub0 port 1 configuration 1 interface 2 "Logitech Webcam C310"
> rev 2.00/0.12 addr 2
> uaudio_identify_ac: AC interface is 2
> uaudio_identify_ac: found AC header, vers=100, len=38
> uaudio0: audio descriptors make no sense, error=4
> ugen0 at uhub0 port 1 configuration 1 "Logitech Webcam C310" rev 2.00/0.12
> addr 2
>
> after a bit of poking, and looking at lsusb -v output i figured out that
> the audio descriptor was lying on its size, saying it was 38:
>
> AudioControl Interface Descriptor:
> bLength 9
> bDescriptorSubtype 1 (HEADER)
> bcdADC 1.00
> wTotalLength 38
>
> but the sum of the sub-descriptor lengths is in fact 9+12+9+9=39..
>
> AudioControl Interface Descriptor:
> bLength 9
> bDescriptorSubtype 1 (HEADER)
> AudioControl Interface Descriptor:
> bLength12
> bDescriptorSubtype 2 (INPUT_TERMINAL)
> wTerminalType 0x0201 Microphone
> AudioControl Interface Descriptor:
> bLength 9
> bDescriptorSubtype 3 (OUTPUT_TERMINAL)
> wTerminalType 0x0101 USB Streaming
> AudioControl Interface Descriptor:
> bLength 9
> bDescriptorSubtype 6 (FEATURE_UNIT)
>
> if i 'fix' aclen to be 39 in sys/dev/usb/uaudio.c, line 1806 then the
> device attaches and works fine:
> uaudio0 at uhub1 port 2 configuration 1 interface 2 "Logitech Webcam C310"
> rev 2.00/0.12 addr 5
>
> from that point, i dunno if my analysis is wrong or right, nor how to
> properly handle it, so here's my take:
> - add a UAUDIO_FLAG_BAD_ADC_LEN quirk
> - set it for the c310 vendor/product
> - if the device matches this quirk, aclen++
>
> feedback or better ideas welcome :)
Did you look at how other OSes deal with that problem? Maybe you'll
find a more generic hack.
If you don't find any better solution, I'd suggest using a device ID
check rather than adding a quirk. Because such quirk cannot be generic.
In that case you have an off-by-one, but another device might have a
different problem.
> Index: uaudio.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.128
> diff -u -r1.128 uaudio.c
> --- uaudio.c 30 Dec 2017 23:08:29 - 1.128
> +++ uaudio.c 24 Jun 2018 11:10:53 -
> @@ -62,10 +62,11 @@
> #include
>
> /* #define UAUDIO_DEBUG */
> +#define UAUDIO_DEBUG
> #ifdef UAUDIO_DEBUG
> #define DPRINTF(x) do { if (uaudiodebug) printf x; } while (0)
> #define DPRINTFN(n,x)do { if (uaudiodebug>(n)) printf x; } while (0)
> -int uaudiodebug = 0;
> +int uaudiodebug = 5;
> #else
> #define DPRINTF(x)
> #define DPRINTFN(n,x)
> @@ -172,6 +173,7 @@
> #define UAUDIO_FLAG_VENDOR_CLASS 0x0010 /* claims vendor class but
> works */
> #define UAUDIO_FLAG_DEPENDENT 0x0020 /* play and record params must
> equal */
> #define UAUDIO_FLAG_EMU0202 0x0040
> +#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor
> size */
>
> struct uaudio_devs {
> struct usb_devno uv_dev;
> @@ -222,6 +224,8 @@
> UAUDIO_FLAG_BAD_AUDIO },
> { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
> UAUDIO_FLAG_BAD_AUDIO },
> + { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
> + UAUDIO_FLAG_BAD_ADC_LEN },
> { { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
> UAUDIO_FLAG_NO_FRAC }
> };
> @@ -1806,6 +1810,9 @@
> aclen = UGETW(acdp->wTotalLength);
> if (offs + aclen > size)
> return (USBD_INVAL);
> +
> + if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
> + aclen++;
>
> if (!(sc->sc_quirks & UAUDIO_FLAG_BAD_ADC) &&
>UGETW(acdp->bcdADC) != UAUDIO_VERSION)
uaudio: fix logitech c310 integrated mic
Hi,
the logitech c310 is supported by uvideo, but its uaudio fails to
attach properly and fallbacks to ugen.
uaudio0 at uhub0 port 1 configuration 1 interface 2 "Logitech Webcam C310" rev
2.00/0.12 addr 2
uaudio_identify_ac: AC interface is 2
uaudio_identify_ac: found AC header, vers=100, len=38
uaudio0: audio descriptors make no sense, error=4
ugen0 at uhub0 port 1 configuration 1 "Logitech Webcam C310" rev 2.00/0.12 addr
2
after a bit of poking, and looking at lsusb -v output i figured out that
the audio descriptor was lying on its size, saying it was 38:
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 1 (HEADER)
bcdADC 1.00
wTotalLength 38
but the sum of the sub-descriptor lengths is in fact 9+12+9+9=39..
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 1 (HEADER)
AudioControl Interface Descriptor:
bLength12
bDescriptorSubtype 2 (INPUT_TERMINAL)
wTerminalType 0x0201 Microphone
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 3 (OUTPUT_TERMINAL)
wTerminalType 0x0101 USB Streaming
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 6 (FEATURE_UNIT)
if i 'fix' aclen to be 39 in sys/dev/usb/uaudio.c, line 1806 then the
device attaches and works fine:
uaudio0 at uhub1 port 2 configuration 1 interface 2 "Logitech Webcam C310" rev
2.00/0.12 addr 5
from that point, i dunno if my analysis is wrong or right, nor how to
properly handle it, so here's my take:
- add a UAUDIO_FLAG_BAD_ADC_LEN quirk
- set it for the c310 vendor/product
- if the device matches this quirk, aclen++
feedback or better ideas welcome :)
Landry
Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.128
diff -u -r1.128 uaudio.c
--- uaudio.c30 Dec 2017 23:08:29 - 1.128
+++ uaudio.c24 Jun 2018 11:10:53 -
@@ -62,10 +62,11 @@
#include
/* #define UAUDIO_DEBUG */
+#define UAUDIO_DEBUG
#ifdef UAUDIO_DEBUG
#define DPRINTF(x) do { if (uaudiodebug) printf x; } while (0)
#define DPRINTFN(n,x) do { if (uaudiodebug>(n)) printf x; } while (0)
-intuaudiodebug = 0;
+intuaudiodebug = 5;
#else
#define DPRINTF(x)
#define DPRINTFN(n,x)
@@ -172,6 +173,7 @@
#define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but
works */
#define UAUDIO_FLAG_DEPENDENT 0x0020 /* play and record params must equal */
#define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor
size */
struct uaudio_devs {
struct usb_devno uv_dev;
@@ -222,6 +224,8 @@
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
+ { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
+ UAUDIO_FLAG_BAD_ADC_LEN },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
UAUDIO_FLAG_NO_FRAC }
};
@@ -1806,6 +1810,9 @@
aclen = UGETW(acdp->wTotalLength);
if (offs + aclen > size)
return (USBD_INVAL);
+
+ if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
+ aclen++;
if (!(sc->sc_quirks & UAUDIO_FLAG_BAD_ADC) &&
UGETW(acdp->bcdADC) != UAUDIO_VERSION)
