Message ID | CAE5BBpTEw2pTzDhLxaQiGwAvnH_q6aChLkuDXxEQrZFvS6wVyQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | Fix mic sound on Jieli webcam | expand |
On Tue, 15 Dec 2020 10:44:45 +0100, Marco Giunta wrote: > > Hi, > recently I've bought a usb webcam with integrated mic: > > Jieli Technology USB PHY 2.0 (1224:2a25) > > The video part works well, but the mic sound is speedups, "like > minions" (cit.). When I connect the camera, these are the dmesg > messages: > > kernel: usb 1-8: current rate 0 is different from the runtime rate 8000 > kernel: usb 1-8: current rate 0 is different from the runtime rate 16000 > kernel: usb 1-8: current rate 0 is different from the runtime rate 44100 > kernel: usb 1-8: current rate 0 is different from the runtime rate 48000 > kernel: usb 1-8: Warning! Unlikely big volume range (=4096), cval->res > is probably wrong. > kernel: usb 1-8: [3] FU [Mic Capture Volume] ch = 1, val = 0/4096/1 > kernel: usbcore: registered new interface driver snd-usb-audio > > and after a while, dmesg log is filled, every 5 seconds, with: > > kernel: retire_capture_urb: 84 callbacks suppressed > kernel: retire_capture_urb: 1714 callbacks suppressed > > A guy reports on ArchLinux bug website the same problem > (https://bugs.archlinux.org/task/68141?opened=12995&status%5B0%5D=) > and provides a patch to fix the sound issue. I've applied the patch on > kernel 5.9.13 (Fedora 33 x86_64) and now the mic works, no more > minions voice effect. Now dmesg messages are only: > > kernel: usb 1-8: Warning! Unlikely big volume range (=4096), cval->res > is probably wrong. > kernel: usb 1-8: [3] FU [Mic Capture Volume] ch = 1, val = 0/4096/1 > kernel: usbcore: registered new interface driver snd-usb-audio > > the retire_capture_urb messages are gone. > > All credits for the patch go to him but I don't know how to contact > that guy nor I don't know if he has already contacted you, so my > question is if you could review his patch and finally apply upstream. > > If you need other information or you need a tester, I'm here. Thanks for the patch. The still remaining warnings are about the mixer, and your patch doesn't touch about it. You may apply the similar change in volume_control_quirks() like other webcams. And now about the patch: > --- a/sound/usb/format.c 2020-10-01 18:36:35.000000000 +0300 > +++ b/sound/usb/format.c 2020-10-04 02:10:21.678685952 +0300 > @@ -217,6 +217,21 @@ > (chip->usb_id == USB_ID(0x041e, 0x4064) || > chip->usb_id == USB_ID(0x041e, 0x4068))) > rate = 8000; > + > + // hack for "Jieli Technology USB PHY 2.0" webcam > + if (chip->usb_id == USB_ID(0x1224, 0x2a25)) { > + switch (rate) { > + case 8000: > + fp->datainterval += 4; > + break; > + case 16000: > + fp->datainterval += 3; > + break; > + default: > + fp->datainterval += 1; > + break; > + } > + } Modifying datainterval at this point doesn't look intuitive. What's the original datainterval value for those altsettings? The value is retrieved in snd_usb_parse_datainterval() in helper.c, and if any, the correction there would be more sensible. > fp->rate_table[fp->nr_rates] = rate; > if (!fp->rate_min || rate < fp->rate_min) > --- a/sound/usb/endpoint.c 2020-10-01 18:36:35.000000000 +0300 > +++ b/sound/usb/endpoint.c 2020-10-04 02:09:09.471978982 +0300 > @@ -882,6 +882,8 @@ > if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL) { > packs_per_ms = 8 >> ep->datainterval; > max_packs_per_urb = MAX_PACKS_HS; > + if (!packs_per_ms) > + packs_per_ms = 1; This rather indicates that the datainterval is somehow wrong. > } else { > packs_per_ms = 1; > max_packs_per_urb = MAX_PACKS; > --- a/sound/usb/quirks.c 2020-10-01 18:36:35.000000000 +0300 > +++ b/sound/usb/quirks.c 2020-10-04 02:14:04.532196519 +0300 > @@ -1516,6 +1516,7 @@ > case USB_ID(0x1901, 0x0191): /* GE B850V3 CP2114 audio interface */ > case USB_ID(0x21b4, 0x0081): /* AudioQuest DragonFly */ > case USB_ID(0x2912, 0x30c8): /* Audioengine D1 */ > + case USB_ID(0x1224, 0x2a25): /* Jieli Technology USB PHY 2.0 */ > return true; > } This looks fine. thanks, Takashi
On Thu, 17 Dec 2020 12:49:53 +0100, Marco Giunta wrote: > > Hi, > thank you for your reply. As I told you, I'm not the guy who wrote the > patch, but I try to continue his work. > > > What's the original datainterval value for those altsettings? > Where I can read the original datainterval value ? Is it an USB > property (so I can search in udev) ? Or can I debug in some way the > snd_usb_audio module load to get the values ? bInterval in the descriptor should show the value. At best, give the output of lsusb -v for the device. thanks, Takashi > > Thanks, > Marco > > > On Thu, Dec 17, 2020 at 10:50 AM Takashi Iwai <tiwai@suse.de> wrote: > > > > On Tue, 15 Dec 2020 10:44:45 +0100, > > Marco Giunta wrote: > > > > > > Hi, > > > recently I've bought a usb webcam with integrated mic: > > > > > > Jieli Technology USB PHY 2.0 (1224:2a25) > > > > > > The video part works well, but the mic sound is speedups, "like > > > minions" (cit.). When I connect the camera, these are the dmesg > > > messages: > > > > > > kernel: usb 1-8: current rate 0 is different from the runtime rate 8000 > > > kernel: usb 1-8: current rate 0 is different from the runtime rate 16000 > > > kernel: usb 1-8: current rate 0 is different from the runtime rate 44100 > > > kernel: usb 1-8: current rate 0 is different from the runtime rate 48000 > > > kernel: usb 1-8: Warning! Unlikely big volume range (=4096), cval->res > > > is probably wrong. > > > kernel: usb 1-8: [3] FU [Mic Capture Volume] ch = 1, val = 0/4096/1 > > > kernel: usbcore: registered new interface driver snd-usb-audio > > > > > > and after a while, dmesg log is filled, every 5 seconds, with: > > > > > > kernel: retire_capture_urb: 84 callbacks suppressed > > > kernel: retire_capture_urb: 1714 callbacks suppressed > > > > > > A guy reports on ArchLinux bug website the same problem > > > (https://bugs.archlinux.org/task/68141?opened=12995&status%5B0%5D=) > > > and provides a patch to fix the sound issue. I've applied the patch on > > > kernel 5.9.13 (Fedora 33 x86_64) and now the mic works, no more > > > minions voice effect. Now dmesg messages are only: > > > > > > kernel: usb 1-8: Warning! Unlikely big volume range (=4096), cval->res > > > is probably wrong. > > > kernel: usb 1-8: [3] FU [Mic Capture Volume] ch = 1, val = 0/4096/1 > > > kernel: usbcore: registered new interface driver snd-usb-audio > > > > > > the retire_capture_urb messages are gone. > > > > > > All credits for the patch go to him but I don't know how to contact > > > that guy nor I don't know if he has already contacted you, so my > > > question is if you could review his patch and finally apply upstream. > > > > > > If you need other information or you need a tester, I'm here. > > > > Thanks for the patch. The still remaining warnings are about the > > mixer, and your patch doesn't touch about it. You may apply the > > similar change in volume_control_quirks() like other webcams. > > > > And now about the patch: > > > > > --- a/sound/usb/format.c 2020-10-01 18:36:35.000000000 +0300 > > > +++ b/sound/usb/format.c 2020-10-04 02:10:21.678685952 +0300 > > > @@ -217,6 +217,21 @@ > > > (chip->usb_id == USB_ID(0x041e, 0x4064) || > > > chip->usb_id == USB_ID(0x041e, 0x4068))) > > > rate = 8000; > > > + > > > + // hack for "Jieli Technology USB PHY 2.0" webcam > > > + if (chip->usb_id == USB_ID(0x1224, 0x2a25)) { > > > + switch (rate) { > > > + case 8000: > > > + fp->datainterval += 4; > > > + break; > > > + case 16000: > > > + fp->datainterval += 3; > > > + break; > > > + default: > > > + fp->datainterval += 1; > > > + break; > > > + } > > > + } > > > > Modifying datainterval at this point doesn't look intuitive. > > What's the original datainterval value for those altsettings? > > The value is retrieved in snd_usb_parse_datainterval() in helper.c, > > and if any, the correction there would be more sensible. > > > > > > > fp->rate_table[fp->nr_rates] = rate; > > > if (!fp->rate_min || rate < fp->rate_min) > > > --- a/sound/usb/endpoint.c 2020-10-01 18:36:35.000000000 +0300 > > > +++ b/sound/usb/endpoint.c 2020-10-04 02:09:09.471978982 +0300 > > > @@ -882,6 +882,8 @@ > > > if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL) { > > > packs_per_ms = 8 >> ep->datainterval; > > > max_packs_per_urb = MAX_PACKS_HS; > > > + if (!packs_per_ms) > > > + packs_per_ms = 1; > > > > This rather indicates that the datainterval is somehow wrong. > > > > > } else { > > > packs_per_ms = 1; > > > max_packs_per_urb = MAX_PACKS; > > > --- a/sound/usb/quirks.c 2020-10-01 18:36:35.000000000 +0300 > > > +++ b/sound/usb/quirks.c 2020-10-04 02:14:04.532196519 +0300 > > > @@ -1516,6 +1516,7 @@ > > > case USB_ID(0x1901, 0x0191): /* GE B850V3 CP2114 audio interface */ > > > case USB_ID(0x21b4, 0x0081): /* AudioQuest DragonFly */ > > > case USB_ID(0x2912, 0x30c8): /* Audioengine D1 */ > > > + case USB_ID(0x1224, 0x2a25): /* Jieli Technology USB PHY 2.0 */ > > > return true; > > > } > > > > This looks fine. > > > > > > thanks, > > > > Takashi >
On Thu, 17 Dec 2020 14:24:58 +0100, Marco Giunta wrote: > > On Thu, Dec 17, 2020 at 1:17 PM Takashi Iwai <tiwai@suse.de> wrote: > > > > bInterval in the descriptor should show the value. > > At best, give the output of lsusb -v for the device. > > Here my lsusb: Thanks. > Interface Association: > bLength 8 > bDescriptorType 11 > bFirstInterface 2 > bInterfaceCount 2 > bFunctionClass 1 Audio > bFunctionSubClass 2 Streaming > bFunctionProtocol 0 > iFunction 5 USB Microphone > Interface Descriptor: (snip) > AudioStreaming Interface Descriptor: > bLength 11 > bDescriptorType 36 > bDescriptorSubtype 2 (FORMAT_TYPE) > bFormatType 1 (FORMAT_TYPE_I) > bNrChannels 1 > bSubframeSize 2 > bBitResolution 16 > bSamFreqType 1 Discrete > tSamFreq[ 0] 8000 > Endpoint Descriptor: > bLength 9 > bDescriptorType 5 > bEndpointAddress 0x82 EP 2 IN > bmAttributes 1 > Transfer Type Isochronous > Synch Type None > Usage Type Data > wMaxPacketSize 0x0100 1x 256 bytes > bInterval 4 It's 4, and the same is set for all sample rates (8000, 16000, 44100, 48000). If you don't tweak the datainterval, which error do you get? The actual error message should appear before "... xx callbacks suppressed" line. BTW, the errors at the current sample rate checks could be better avoided by treating the return value 0 specially, something like below. Takashi -- 8< -- From: Takashi Iwai <tiwai@suse.de> Subject: [PATCH] ALSA: usb-audio: Disable sample read check if firmware doesn't give back Some buggy firmware don't give the current sample rate but leaves zero. Handle this case more gracefully without warning but just skip the current rate verification from the next time. Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/usb/clock.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/usb/clock.c b/sound/usb/clock.c index e940dcee792b..31051f2be46d 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -534,6 +534,12 @@ static int set_sample_rate_v1(struct snd_usb_audio *chip, } crate = data[0] | (data[1] << 8) | (data[2] << 16); + if (!crate) { + dev_info(&dev->dev, "failed to read current rate; disabling the check\n"); + chip->sample_rate_read_error = 3; /* three strikes, see above */ + return 0; + } + if (crate != rate) { dev_warn(&dev->dev, "current rate %d is different from the runtime rate %d\n", crate, rate); // runtime->rate = crate;
On Thu, Dec 17, 2020 at 3:12 PM Takashi Iwai <tiwai@suse.de> wrote: > > Thanks. > > > Interface Association: > > bLength 8 > > bDescriptorType 11 > > bFirstInterface 2 > > bInterfaceCount 2 > > bFunctionClass 1 Audio > > bFunctionSubClass 2 Streaming > > bFunctionProtocol 0 > > iFunction 5 USB Microphone > > Interface Descriptor: > (snip) > > AudioStreaming Interface Descriptor: > > bLength 11 > > bDescriptorType 36 > > bDescriptorSubtype 2 (FORMAT_TYPE) > > bFormatType 1 (FORMAT_TYPE_I) > > bNrChannels 1 > > bSubframeSize 2 > > bBitResolution 16 > > bSamFreqType 1 Discrete > > tSamFreq[ 0] 8000 > > Endpoint Descriptor: > > bLength 9 > > bDescriptorType 5 > > bEndpointAddress 0x82 EP 2 IN > > bmAttributes 1 > > Transfer Type Isochronous > > Synch Type None > > Usage Type Data > > wMaxPacketSize 0x0100 1x 256 bytes > > bInterval 4 > > It's 4, and the same is set for all sample rates (8000, 16000, 44100, > 48000). > > If you don't tweak the datainterval, which error do you get? > The actual error message should appear before "... xx callbacks > suppressed" line. Without the tweak, only error messages are: kernel: usb 1-8: current rate 0 is different from the runtime rate 8000 kernel: usb 1-8: current rate 0 is different from the runtime rate 16000 kernel: usb 1-8: current rate 0 is different from the runtime rate 44100 kernel: usb 1-8: current rate 0 is different from the runtime rate 48000 kernel: usb 1-8: Warning! Unlikely big volume range (=4096), cval->res is probably wrong. kernel: usb 1-8: [3] FU [Mic Capture Volume] ch = 1, val = 0/4096/1 but recording from mic at any rate (8000, 16000, 44100, 48000) results in an incomprehensible sound, like Minion voice. With your patch 'ALSA: usb-audio: Disable sample read check if firmware doesn't give back' error messages have gone away, but result is the same: Minion voice. Thanks, Marco
On Fri, 18 Dec 2020 10:26:11 +0100, Marco Giunta wrote: > > On Thu, Dec 17, 2020 at 3:12 PM Takashi Iwai <tiwai@suse.de> wrote: > > > > Thanks. > > > > > Interface Association: > > > bLength 8 > > > bDescriptorType 11 > > > bFirstInterface 2 > > > bInterfaceCount 2 > > > bFunctionClass 1 Audio > > > bFunctionSubClass 2 Streaming > > > bFunctionProtocol 0 > > > iFunction 5 USB Microphone > > > Interface Descriptor: > > (snip) > > > AudioStreaming Interface Descriptor: > > > bLength 11 > > > bDescriptorType 36 > > > bDescriptorSubtype 2 (FORMAT_TYPE) > > > bFormatType 1 (FORMAT_TYPE_I) > > > bNrChannels 1 > > > bSubframeSize 2 > > > bBitResolution 16 > > > bSamFreqType 1 Discrete > > > tSamFreq[ 0] 8000 > > > Endpoint Descriptor: > > > bLength 9 > > > bDescriptorType 5 > > > bEndpointAddress 0x82 EP 2 IN > > > bmAttributes 1 > > > Transfer Type Isochronous > > > Synch Type None > > > Usage Type Data > > > wMaxPacketSize 0x0100 1x 256 bytes > > > bInterval 4 > > > > It's 4, and the same is set for all sample rates (8000, 16000, 44100, > > 48000). > > > > If you don't tweak the datainterval, which error do you get? > > The actual error message should appear before "... xx callbacks > > suppressed" line. > > Without the tweak, only error messages are: > > kernel: usb 1-8: current rate 0 is different from the runtime rate 8000 > kernel: usb 1-8: current rate 0 is different from the runtime rate 16000 > kernel: usb 1-8: current rate 0 is different from the runtime rate 44100 > kernel: usb 1-8: current rate 0 is different from the runtime rate 48000 > kernel: usb 1-8: Warning! Unlikely big volume range (=4096), cval->res > is probably wrong. > kernel: usb 1-8: [3] FU [Mic Capture Volume] ch = 1, val = 0/4096/1 > > but recording from mic at any rate (8000, 16000, 44100, 48000) results > in an incomprehensible sound, like Minion voice. > > With your patch 'ALSA: usb-audio: Disable sample read check if > firmware doesn't give back' error messages have gone away, but result > is the same: Minion voice. OK, the patch for suppressing the rate errors was submitted and will be merged to upstream later. Now let's hunt Minions. Just as a blind shot, could you try the following? Takashi --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -788,7 +788,7 @@ static int usb_audio_probe(struct usb_interface *intf, if (!chip->ctrl_intf) chip->ctrl_intf = alts; - chip->txfr_quirk = 0; + chip->txfr_quirk = 1; err = 1; /* continue */ if (quirk && quirk->ifnum != QUIRK_NO_INTERFACE) { /* need some special handlings */
On Fri, Dec 18, 2020 at 4:31 PM Takashi Iwai <tiwai@suse.de> wrote: > > OK, the patch for suppressing the rate errors was submitted and will > be merged to upstream later. Now let's hunt Minions. > > Just as a blind shot, could you try the following? > nope, Minion voice is still there ;-P I've tried to record at any rate (8k, 16k, 44,1k and 48k), but no luck. Moreover, there are a lot of new error messages: kernel: usb 1-6.2: 3:3: usb_set_interface failed (-19) kernel: usb 1-6.2: 3:3: usb_set_interface failed (-19) kernel: usb 1-6.2: 3:3: usb_set_interface failed (-19) kernel: usb 1-6.2: 3:3: usb_set_interface failed (-19) pulseaudio[2818]: Failed to find a working profile. pulseaudio[2818]: Failed to load module "module-alsa-card" (argument: "device_id="1" name="usb-Jieli_Technology_USB_PHY_2.0-02" card_name="alsa_card.usb-Jieli_Technology_USB_PHY_2.0-02" namereg_fail=false tsched=yes fixed_latency_range=no ignore_dB=no deferred_volume=yes use_ucm=yes avoid_resampling=no card_properties="module-udev-detect.discovered=1""): initialization failed. pulseaudio[2818]: Failed to get card object. pulseaudio[2818]: Failed to find a working profile. pulseaudio[2818]: Failed to load module "module-alsa-card" (argument: "device_id="1" name="usb-Jieli_Technology_USB_PHY_2.0-02" card_name="alsa_card.usb-Jieli_Technology_USB_PHY_2.0-02" namereg_fail=false tsched=yes fixed_latency_range=no ignore_dB=no deferred_volume=yes use_ucm=yes avoid_resampling=no card_properties="module-udev-detect.discovered=1""): initialization failed. kernel: usb 1-6.2: cannot submit urb (err = -19) kernel: usb 1-6.2: cannot submit urb 0, error -19: no device Thanks, Marco
--- a/sound/usb/format.c 2020-10-01 18:36:35.000000000 +0300 +++ b/sound/usb/format.c 2020-10-04 02:10:21.678685952 +0300 @@ -217,6 +217,21 @@ (chip->usb_id == USB_ID(0x041e, 0x4064) || chip->usb_id == USB_ID(0x041e, 0x4068))) rate = 8000; + + // hack for "Jieli Technology USB PHY 2.0" webcam + if (chip->usb_id == USB_ID(0x1224, 0x2a25)) { + switch (rate) { + case 8000: + fp->datainterval += 4; + break; + case 16000: + fp->datainterval += 3; + break; + default: + fp->datainterval += 1; + break; + } + } fp->rate_table[fp->nr_rates] = rate; if (!fp->rate_min || rate < fp->rate_min) --- a/sound/usb/endpoint.c 2020-10-01 18:36:35.000000000 +0300 +++ b/sound/usb/endpoint.c 2020-10-04 02:09:09.471978982 +0300 @@ -882,6 +882,8 @@ if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL) { packs_per_ms = 8 >> ep->datainterval; max_packs_per_urb = MAX_PACKS_HS; + if (!packs_per_ms) + packs_per_ms = 1; } else { packs_per_ms = 1; max_packs_per_urb = MAX_PACKS; --- a/sound/usb/quirks.c 2020-10-01 18:36:35.000000000 +0300 +++ b/sound/usb/quirks.c 2020-10-04 02:14:04.532196519 +0300 @@ -1516,6 +1516,7 @@ case USB_ID(0x1901, 0x0191): /* GE B850V3 CP2114 audio interface */ case USB_ID(0x21b4, 0x0081): /* AudioQuest DragonFly */ case USB_ID(0x2912, 0x30c8): /* Audioengine D1 */ + case USB_ID(0x1224, 0x2a25): /* Jieli Technology USB PHY 2.0 */ return true; }