| Message ID | CAEsQvcvF7LnO8PxyyCxuRCx=7jNeSCvFAd-+dE0g_rd1rOxxdw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] Behringer UFX1604 / UFX1204: get rid of unneeded implicit feedback and pops and clicks while on 96000hz | expand |
On Sat, 10 Apr 2021 03:36:14 +0200, Geraldo Nascimento wrote: > > More complete patch disabling unneeded implicit feedback and setting > clock selector to default clock on rate change for UFX1604 > > After re-reading https://bugzilla.kernel.org/show_bug.cgi?id=199327 it > is even more clear to me that implicit feedback for the > UFX1604/UFX1204 needs to be disabled. > > This is a more complete patch that disables that and for the UFX1604 > only sets the clock selector to its pin 1 default clock synced to the > USB SOF upon rate change. This is needed because apparently the > endpoints are hardwired to the clock selector and after we change the > rate on the main USB SOF synced clock the clock selector is left in a > halfway state in regards to the sampling rate. > > That's why the pops and clicks aren't evident at stock 48000Hz, become > slightly audible at 44100Hz and detestable at 96000Hz. Seems the clock > selector needs a nudge or it will screw up the sync. > > Unfortunately I don't have access to the lsusb -v of the UFX1204 soI'm > waiting for someone to share it here in the list or in the bugzilla > thread. This patch needs some more love from the community. > > --- > > This one has been bugging me for quite a while. I went deep hard in > the guts of ALSA to try to solve it, and it turned out to be a minor > thing apparently. The problem is old, and every UFX1604 Linux user can > attest that it's impossible to use 96000hz in DUPLEX mode without > annoying pops and clicks on the capture stream. > > The fix is simple: after we alter the CLOCK_SOURCE to match our sample > rate, let's tell the CLOCK_SELECTOR we want CLOCK_SOURCE 212 (synced > to USB SOF) on pin 1. Solves the problem for me, no more pops and > clicks while on 96000hz. > > --- > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com> Thanks for the patch. But we'd like to avoid the setup with a magic ID number. Judging from what it achieves, does the change like below give the similar effect? We might need to apply it conditionally, so this is just meant for testing. Takashi --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -324,6 +324,8 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, ret = __uac_clock_find_source(chip, fmt, selector->baCSourceID[ret - 1], visited, validate); + if (ret > 0) + uac_clock_selector_set_val(chip, entity_id, cur); if (!validate || ret > 0 || !chip->autoclock) return ret;
OK, Dr. Iwai, only briefly tested but at least on my setup, JACK starts at 96000hz and I am able to record my voice in Ardour 6, with no pops and clicks detected. I'll record a full acapella just to be sure, but these 96000Hz pops and clicks I used to experience - with that implicit feedback hack on by the way - tended to be evident within seconds, not intermittent and very annoying. Now I turned the implicit feedback hack off for both the UFX1604 and the UFX1204 on my tree and with your two lines of code I have crystal clear sound. I took the liberty to comment your clever hack inside clock.c and the pops / clicks problem reappears on 96000Hz. PulseAudio users (see https://bugzilla.kernel.org/show_bug.cgi?id=199327 ) will generally run at 44100Hz and won't be affected so much by noise because the clock selector will be "tied" to the standard 48000Hz of these mixer desks DAC, in that invalid state but close enough to the main clock rate. Henceforth the general confusion where people complain the forced implicit feedback made PulseAudio break for their UFX1604 / UFX1204, but they do not experience noise. This approach should solve both problems by disabling implicit feedback on the synchronous endpoints but making sure the sound is crystal clear even at 96000Hz. Thank you, Geraldo Em Sáb, 10 de abr de 2021 05:59, Takashi Iwai <tiwai@suse.de> escreveu: > On Sat, 10 Apr 2021 03:36:14 +0200, > Geraldo Nascimento wrote: > > > > More complete patch disabling unneeded implicit feedback and setting > > clock selector to default clock on rate change for UFX1604 > > > > After re-reading https://bugzilla.kernel.org/show_bug.cgi?id=199327 it > > is even more clear to me that implicit feedback for the > > UFX1604/UFX1204 needs to be disabled. > > > > This is a more complete patch that disables that and for the UFX1604 > > only sets the clock selector to its pin 1 default clock synced to the > > USB SOF upon rate change. This is needed because apparently the > > endpoints are hardwired to the clock selector and after we change the > > rate on the main USB SOF synced clock the clock selector is left in a > > halfway state in regards to the sampling rate. > > > > That's why the pops and clicks aren't evident at stock 48000Hz, become > > slightly audible at 44100Hz and detestable at 96000Hz. Seems the clock > > selector needs a nudge or it will screw up the sync. > > > > Unfortunately I don't have access to the lsusb -v of the UFX1204 soI'm > > waiting for someone to share it here in the list or in the bugzilla > > thread. This patch needs some more love from the community. > > > > --- > > > > This one has been bugging me for quite a while. I went deep hard in > > the guts of ALSA to try to solve it, and it turned out to be a minor > > thing apparently. The problem is old, and every UFX1604 Linux user can > > attest that it's impossible to use 96000hz in DUPLEX mode without > > annoying pops and clicks on the capture stream. > > > > The fix is simple: after we alter the CLOCK_SOURCE to match our sample > > rate, let's tell the CLOCK_SELECTOR we want CLOCK_SOURCE 212 (synced > > to USB SOF) on pin 1. Solves the problem for me, no more pops and > > clicks while on 96000hz. > > > > --- > > > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com> > > Thanks for the patch. > > But we'd like to avoid the setup with a magic ID number. > Judging from what it achieves, does the change like below give the > similar effect? > > We might need to apply it conditionally, so this is just meant for > testing. > > > Takashi > > --- a/sound/usb/clock.c > +++ b/sound/usb/clock.c > @@ -324,6 +324,8 @@ static int __uac_clock_find_source(struct > snd_usb_audio *chip, > ret = __uac_clock_find_source(chip, fmt, > selector->baCSourceID[ret - > 1], > visited, validate); > + if (ret > 0) > + uac_clock_selector_set_val(chip, entity_id, cur); > if (!validate || ret > 0 || !chip->autoclock) > return ret; > >
On Sat, 10 Apr 2021 22:27:15 +0200, Geraldo Nascimento wrote: > > OK, Dr. Iwai, only briefly tested but at least on my setup, JACK starts at > 96000hz and I am able to record my voice in Ardour 6, with no pops and clicks > detected. > > I'll record a full acapella just to be sure, but these 96000Hz pops and clicks > I used to experience - with that implicit feedback hack on by the way - tended > to be evident within seconds, not intermittent and very annoying. > > Now I turned the implicit feedback hack off for both the UFX1604 and the > UFX1204 on my tree and with your two lines of code I have crystal clear sound. > > I took the liberty to comment your clever hack inside clock.c and the pops / > clicks problem reappears on 96000Hz. PulseAudio users (see > https://bugzilla.kernel.org/show_bug.cgi?id=199327 ) will generally run at > 44100Hz and won't be affected so much by noise because the clock selector will > be "tied" to the standard 48000Hz of these mixer desks DAC, in that invalid > state but close enough to the main clock rate. > > Henceforth the general confusion where people complain the forced implicit > feedback made PulseAudio break for their UFX1604 / UFX1204, but they do not > experience noise. This approach should solve both problems by disabling > implicit feedback on the synchronous endpoints but making sure the sound is > crystal clear even at 96000Hz. Is the PA breakage still seen in the very latest kernel? There have been a few issues due to the changes but they should have been already covered. Apart from that, the choice of implicit feedback was taken at the commit 5e35dc0338d8 in a few years ago for UFX1024. I don't remember the details, but judging from the lsusb output, the devices don't look like a typical device that requires the implicit feedback (that has usually ASYNC for both directions). So, as long as it's confirmed that the proper clock selector setup fixes the problem, we can drop the implicit feedback quirks. However, it's better to split the patches; one for the fix for the missing clock selector setup, another is the drop of the implciit fb quirk for Behringer devices. In anyway, let's see whether the fix works for others. Then we can go forward and apply the fixes to the upstream. thanks, Takashi > > Thank you, > Geraldo > > Em Sáb, 10 de abr de 2021 05:59, Takashi Iwai <tiwai@suse.de> escreveu: > > On Sat, 10 Apr 2021 03:36:14 +0200, > Geraldo Nascimento wrote: > > > > More complete patch disabling unneeded implicit feedback and setting > > clock selector to default clock on rate change for UFX1604 > > > > After re-reading https://bugzilla.kernel.org/show_bug.cgi?id=199327 it > > is even more clear to me that implicit feedback for the > > UFX1604/UFX1204 needs to be disabled. > > > > This is a more complete patch that disables that and for the UFX1604 > > only sets the clock selector to its pin 1 default clock synced to the > > USB SOF upon rate change. This is needed because apparently the > > endpoints are hardwired to the clock selector and after we change the > > rate on the main USB SOF synced clock the clock selector is left in a > > halfway state in regards to the sampling rate. > > > > That's why the pops and clicks aren't evident at stock 48000Hz, become > > slightly audible at 44100Hz and detestable at 96000Hz. Seems the clock > > selector needs a nudge or it will screw up the sync. > > > > Unfortunately I don't have access to the lsusb -v of the UFX1204 soI'm > > waiting for someone to share it here in the list or in the bugzilla > > thread. This patch needs some more love from the community. > > > > --- > > > > This one has been bugging me for quite a while. I went deep hard in > > the guts of ALSA to try to solve it, and it turned out to be a minor > > thing apparently. The problem is old, and every UFX1604 Linux user can > > attest that it's impossible to use 96000hz in DUPLEX mode without > > annoying pops and clicks on the capture stream. > > > > The fix is simple: after we alter the CLOCK_SOURCE to match our sample > > rate, let's tell the CLOCK_SELECTOR we want CLOCK_SOURCE 212 (synced > > to USB SOF) on pin 1. Solves the problem for me, no more pops and > > clicks while on 96000hz. > > > > --- > > > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com> > > Thanks for the patch. > > But we'd like to avoid the setup with a magic ID number. > Judging from what it achieves, does the change like below give the > similar effect? > > We might need to apply it conditionally, so this is just meant for > testing. > > Takashi > > --- a/sound/usb/clock.c > +++ b/sound/usb/clock.c > @@ -324,6 +324,8 @@ static int __uac_clock_find_source(struct > snd_usb_audio *chip, > ret = __uac_clock_find_source(chip, fmt, > selector->baCSourceID[ret - > 1], > visited, validate); > + if (ret > 0) > + uac_clock_selector_set_val(chip, entity_id, cur); > if (!validate || ret > 0 || !chip->autoclock) > return ret; > >
Sorry, Dr. Iwai, the truth is that I refuse to install PulseAudio in my production systems, so I can't comment. To be sure you'd have to ask in bugzilla #199327 Always a pleasure to work and learn with you, splitting the commits seems like the ideal choice. I do hope the fixes work out for others as I put a lot of time in tracking the problem. Let's hope for the best. Thank you, Geraldo Em Seg, 12 de abr de 2021 07:14, Takashi Iwai <tiwai@suse.de> escreveu: > On Sat, 10 Apr 2021 22:27:15 +0200, > Geraldo Nascimento wrote: > > > > OK, Dr. Iwai, only briefly tested but at least on my setup, JACK starts > at > > 96000hz and I am able to record my voice in Ardour 6, with no pops and > clicks > > detected. > > > > I'll record a full acapella just to be sure, but these 96000Hz pops and > clicks > > I used to experience - with that implicit feedback hack on by the way - > tended > > to be evident within seconds, not intermittent and very annoying. > > > > Now I turned the implicit feedback hack off for both the UFX1604 and the > > UFX1204 on my tree and with your two lines of code I have crystal clear > sound. > > > > I took the liberty to comment your clever hack inside clock.c and the > pops / > > clicks problem reappears on 96000Hz. PulseAudio users (see > > https://bugzilla.kernel.org/show_bug.cgi?id=199327 ) will generally run > at > > 44100Hz and won't be affected so much by noise because the clock > selector will > > be "tied" to the standard 48000Hz of these mixer desks DAC, in that > invalid > > state but close enough to the main clock rate. > > > > Henceforth the general confusion where people complain the forced > implicit > > feedback made PulseAudio break for their UFX1604 / UFX1204, but they do > not > > experience noise. This approach should solve both problems by disabling > > implicit feedback on the synchronous endpoints but making sure the sound > is > > crystal clear even at 96000Hz. > > Is the PA breakage still seen in the very latest kernel? There have > been a few issues due to the changes but they should have been already > covered. > > Apart from that, the choice of implicit feedback was taken at the > commit 5e35dc0338d8 in a few years ago for UFX1024. I don't remember > the details, but judging from the lsusb output, the devices don't look > like a typical device that requires the implicit feedback (that has > usually ASYNC for both directions). So, as long as it's confirmed > that the proper clock selector setup fixes the problem, we can drop > the implicit feedback quirks. > > However, it's better to split the patches; one for the fix for the > missing clock selector setup, another is the drop of the implciit fb > quirk for Behringer devices. > > In anyway, let's see whether the fix works for others. Then we can go > forward and apply the fixes to the upstream. > > > thanks, > > Takashi > > > > > Thank you, > > Geraldo > > > > Em Sáb, 10 de abr de 2021 05:59, Takashi Iwai <tiwai@suse.de> escreveu: > > > > On Sat, 10 Apr 2021 03:36:14 +0200, > > Geraldo Nascimento wrote: > > > > > > More complete patch disabling unneeded implicit feedback and > setting > > > clock selector to default clock on rate change for UFX1604 > > > > > > After re-reading > https://bugzilla.kernel.org/show_bug.cgi?id=199327 it > > > is even more clear to me that implicit feedback for the > > > UFX1604/UFX1204 needs to be disabled. > > > > > > This is a more complete patch that disables that and for the > UFX1604 > > > only sets the clock selector to its pin 1 default clock synced to > the > > > USB SOF upon rate change. This is needed because apparently the > > > endpoints are hardwired to the clock selector and after we change > the > > > rate on the main USB SOF synced clock the clock selector is left > in a > > > halfway state in regards to the sampling rate. > > > > > > That's why the pops and clicks aren't evident at stock 48000Hz, > become > > > slightly audible at 44100Hz and detestable at 96000Hz. Seems the > clock > > > selector needs a nudge or it will screw up the sync. > > > > > > Unfortunately I don't have access to the lsusb -v of the UFX1204 > soI'm > > > waiting for someone to share it here in the list or in the bugzilla > > > thread. This patch needs some more love from the community. > > > > > > --- > > > > > > This one has been bugging me for quite a while. I went deep hard in > > > the guts of ALSA to try to solve it, and it turned out to be a > minor > > > thing apparently. The problem is old, and every UFX1604 Linux user > can > > > attest that it's impossible to use 96000hz in DUPLEX mode without > > > annoying pops and clicks on the capture stream. > > > > > > The fix is simple: after we alter the CLOCK_SOURCE to match our > sample > > > rate, let's tell the CLOCK_SELECTOR we want CLOCK_SOURCE 212 > (synced > > > to USB SOF) on pin 1. Solves the problem for me, no more pops and > > > clicks while on 96000hz. > > > > > > --- > > > > > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com> > > > > Thanks for the patch. > > > > But we'd like to avoid the setup with a magic ID number. > > Judging from what it achieves, does the change like below give the > > similar effect? > > > > We might need to apply it conditionally, so this is just meant for > > testing. > > > > Takashi > > > > --- a/sound/usb/clock.c > > +++ b/sound/usb/clock.c > > @@ -324,6 +324,8 @@ static int __uac_clock_find_source(struct > > snd_usb_audio *chip, > > ret = __uac_clock_find_source(chip, fmt, > > > selector->baCSourceID[ret - > > 1], > > visited, validate); > > + if (ret > 0) > > + uac_clock_selector_set_val(chip, entity_id, > cur); > > if (!validate || ret > 0 || !chip->autoclock) > > return ret; > > > > >
--- implicit.c 2021-04-04 20:51:57.226754632 -0300 +++ implicit.c 2021-04-09 21:55:19.593994214 -0300 @@ -50,8 +50,6 @@ static const struct snd_usb_implicit_fb_ /* Fixed EP */ /* FIXME: check the availability of generic matching */ - IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer UFX1604 */ - IMPLICIT_FB_FIXED_DEV(0x1397, 0x0002, 0x81, 1), /* Behringer UFX1204 */ IMPLICIT_FB_FIXED_DEV(0x2466, 0x8010, 0x81, 2), /* Fractal Audio Axe-Fx III */ IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0001, 0x81, 2), /* Solid State Logic SSL2 */ IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0002, 0x81, 2), /* Solid State Logic SSL2+ */ --- clock.c 2021-04-09 22:01:08.234001611 -0300 +++ clock.c 2021-04-09 22:01:53.640669243 -0300 @@ -610,6 +610,14 @@ int snd_usb_set_sample_rate_v2v3(struct if (err < 0) return err; + if (chip->usb_id == USB_ID(0x1397, 0x0001)) { /* Behringer UFX1604 */ + printk(KERN_WARNING "Setting clock selector for UFX1604"); + err = uac_clock_selector_set_val(chip, 211, 1); + if (err < 0) + return err; + } + + return get_sample_rate_v2v3(chip, fmt->iface, fmt->altsetting, clock); }
More complete patch disabling unneeded implicit feedback and setting clock selector to default clock on rate change for UFX1604 After re-reading https://bugzilla.kernel.org/show_bug.cgi?id=199327 it is even more clear to me that implicit feedback for the UFX1604/UFX1204 needs to be disabled. This is a more complete patch that disables that and for the UFX1604 only sets the clock selector to its pin 1 default clock synced to the USB SOF upon rate change. This is needed because apparently the endpoints are hardwired to the clock selector and after we change the rate on the main USB SOF synced clock the clock selector is left in a halfway state in regards to the sampling rate. That's why the pops and clicks aren't evident at stock 48000Hz, become slightly audible at 44100Hz and detestable at 96000Hz. Seems the clock selector needs a nudge or it will screw up the sync. Unfortunately I don't have access to the lsusb -v of the UFX1204 soI'm waiting for someone to share it here in the list or in the bugzilla thread. This patch needs some more love from the community. --- This one has been bugging me for quite a while. I went deep hard in the guts of ALSA to try to solve it, and it turned out to be a minor thing apparently. The problem is old, and every UFX1604 Linux user can attest that it's impossible to use 96000hz in DUPLEX mode without annoying pops and clicks on the capture stream. The fix is simple: after we alter the CLOCK_SOURCE to match our sample rate, let's tell the CLOCK_SELECTOR we want CLOCK_SOURCE 212 (synced to USB SOF) on pin 1. Solves the problem for me, no more pops and clicks while on 96000hz. --- Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>