git: 755685dd665e - main - snd_uaudio: Don't let an idle stream reprogram a shared UAC2 clock
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 24 Jul 2026 11:20:40 UTC
The branch main has been updated by christos:
URL: https://cgit.FreeBSD.org/src/commit/?id=755685dd665ef209912c59da6a7d0e7f2c9f464b
commit 755685dd665ef209912c59da6a7d0e7f2c9f464b
Author: giacomo <delleceste@gmail.com>
AuthorDate: 2026-07-15 11:34:34 +0000
Commit: Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2026-07-24 11:20:08 +0000
snd_uaudio: Don't let an idle stream reprogram a shared UAC2 clock
Some UAC2 devices expose a single Clock Source entity that is shared
between their playback and capture interfaces (it appears in both the
output and input clock bitmaps). On such a device uaudio(4) programs
the sample rate for both directions when a stream starts. If playback
runs at a 44.1 kHz-family rate while the idle capture channel is left
at its 48 kHz-family default, the capture
SET_CUR(UA20_CS_SAM_FREQ_CONTROL) is issued after the playback one and
overwrites the rate on the shared clock. The device then runs at
~48 kHz while the playback stream carries 44.1 kHz data. Consuming
samples faster than they arrive, the device repeatedly runs out of
data, loses sync with the playback stream, and re-locks onto it
(audible dropouts, front-panel play/idle flicker). The 48 kHz family
is unaffected because both directions then agree on the rate.
Fix it in three parts:
- Add a shared-clock guard: before issuing SET_CUR to a clock id, if
that clock is shared between playback and capture and the other
direction is already streaming at a different rate, skip it. The
first active stream owns the clock; a later one follows it.
- When the recording channel is auto-started only as a source of jitter
information for asynchronous playback, align its nominal rate to the
playback rate before starting it, so it neither reprograms the shared
clock to a conflicting rate nor produces mismatched frame sizes.
- Always submit the explicit-feedback SYNC transfer so
dev.pcm.%d.feedback_rate stays live as a diagnostic even when a
capture stream is present.
Reproduced on an OKTO RESEARCH DAC8 STEREO (0x152a:0x88c5), whose
vestigial capture interface never streams; the same device plays the
44.1 kHz family correctly under Linux's snd-usb-audio.
As a side effect, this patch also fixes the sample rate bug mentioned in
the BUGS section of sound(4)'s man page, where a device needs to have
the same sample rate set for both playback and recording in order to
work properly.
PR: 295933
Assisted-By: Claude Opus 4.8 (claude-opus-4-8)
Signed-off-by: giacomo <delleceste@gmail.com>
MFC after: 2 weeks
Reviewed by: christos
Pull-Request: https://github.com/freebsd/freebsd-src/pull/2323
---
sys/dev/sound/usb/uaudio.c | 103 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 95 insertions(+), 8 deletions(-)
diff --git a/sys/dev/sound/usb/uaudio.c b/sys/dev/sound/usb/uaudio.c
index 4cdd20315258..c2de07abf0a5 100644
--- a/sys/dev/sound/usb/uaudio.c
+++ b/sys/dev/sound/usb/uaudio.c
@@ -1358,6 +1358,46 @@ uaudio_max_buffer_size(struct uaudio_chan *ch, uint8_t alt)
return (buf_size);
}
+static bool
+uaudio20_clock_is_shared(struct uaudio_softc *sc, unsigned int x)
+{
+ /*
+ * A clock entity that feeds both an OUTPUT (playback) and an
+ * INPUT (capture) terminal shows up in both bitmaps: it is a
+ * sample clock shared between the two directions.
+ */
+ return ((sc->sc_mixer_clocks.bit_output[x / 8] & (1 << (x % 8))) != 0 &&
+ (sc->sc_mixer_clocks.bit_input[x / 8] & (1 << (x % 8))) != 0);
+}
+
+static uint32_t
+uaudio_dir_running_rate(struct uaudio_chan *chans)
+{
+ unsigned int i;
+
+ for (i = 0; i != UAUDIO_MAX_CHILD; i++) {
+ struct uaudio_chan *ch = &chans[i];
+
+ if (ch->running != 0 && ch->cur_alt < ch->num_alt)
+ return (ch->usb_alt[ch->cur_alt].sample_rate);
+ }
+ return (0); /* nothing streaming in this direction */
+}
+
+static bool
+uaudio_chan_match_rate(struct uaudio_chan *ch, uint32_t rate, uint8_t *p_alt)
+{
+ uint8_t x;
+
+ for (x = 0; x != ch->num_alt; x++) {
+ if (ch->usb_alt[x].sample_rate == rate) {
+ *p_alt = x;
+ return (true);
+ }
+ }
+ return (false);
+}
+
static void
uaudio_configure_msg_sub(struct uaudio_softc *sc,
struct uaudio_chan *chan, int dir)
@@ -1455,6 +1495,35 @@ uaudio_configure_msg_sub(struct uaudio_softc *sc,
}
}
+ /*
+ * Shared-clock guard. If this clock entity is
+ * shared between the playback and capture paths,
+ * and the OTHER direction is already streaming at
+ * a different rate, do not reprogram the clock --
+ * the rate that is already locked wins. This
+ * stops an idle or secondary stream from yanking
+ * the shared clock out from under an active
+ * stream and dropping USB stream lock. The first
+ * active stream owns the clock; a later one
+ * follows it (see uaudio_chan_start(), which
+ * re-aligns the jitter-info record stream to the
+ * playback rate before it is started).
+ */
+ if (uaudio20_clock_is_shared(sc, x)) {
+ uint32_t other = (dir == PCMDIR_PLAY) ?
+ uaudio_dir_running_rate(sc->sc_rec_chan) :
+ uaudio_dir_running_rate(sc->sc_play_chan);
+
+ if (other != 0 &&
+ other != chan_alt->sample_rate) {
+ DPRINTF("shared clock ID=%u busy at "
+ "%u Hz; not reprogramming to "
+ "%u Hz\n", x, other,
+ chan_alt->sample_rate);
+ continue;
+ }
+ }
+
if (uaudio20_set_speed(sc->sc_udev,
sc->sc_mixer_iface_no, x, chan_alt->sample_rate)) {
/*
@@ -2334,13 +2403,10 @@ uaudio_chan_play_sync_callback(struct usb_xfer *xfer, usb_error_t error)
case USB_ST_SETUP:
/*
- * Check if the recording stream can be used as a
- * source of jitter information to save some
- * isochronous bandwidth:
+ * Submit the transfer even when the recording stream
+ * provides the jitter information, so that the feedback
+ * rate keeps being sampled for diagnostic purposes.
*/
- if (ch->priv_sc->sc_rec_chan[i].num_alt != 0 &&
- uaudio_debug == 0)
- break;
usbd_xfer_set_frames(xfer, 1);
usbd_xfer_set_frame_len(xfer, 0, usbd_xfer_max_framelen(xfer));
usbd_transfer_submit(xfer);
@@ -2884,12 +2950,33 @@ uaudio_chan_start(struct uaudio_chan *ch)
if (uaudio_chan_need_both(
&sc->sc_play_chan[i],
&sc->sc_rec_chan[i])) {
+ struct uaudio_chan *ch_play = &sc->sc_play_chan[i];
+ struct uaudio_chan *ch_rec = &sc->sc_rec_chan[i];
+ uint8_t rec_alt;
+
+ /*
+ * The recording channel is only being started as a
+ * source of jitter information for the playback
+ * stream. Align its nominal rate with the
+ * playback rate so that (a) it does not reprogram
+ * a sample clock shared with the playback path to
+ * a conflicting rate, and (b) its expected frame
+ * sizes match what the device actually produces,
+ * keeping the derived jitter information valid.
+ * The USB explore lock is held here, which also
+ * serializes against uaudio_chan_set_param_speed().
+ */
+ if (uaudio_chan_match_rate(ch_rec,
+ ch_play->usb_alt[ch_play->set_alt].sample_rate,
+ &rec_alt))
+ ch_rec->set_alt = rec_alt;
+
/*
* Start both endpoints because of need for
* jitter information:
*/
- uaudio_chan_reconfigure(&sc->sc_rec_chan[i], CHAN_OP_START);
- uaudio_chan_reconfigure(&sc->sc_play_chan[i], CHAN_OP_START);
+ uaudio_chan_reconfigure(ch_rec, CHAN_OP_START);
+ uaudio_chan_reconfigure(ch_play, CHAN_OP_START);
} else {
uaudio_chan_reconfigure(ch, CHAN_OP_START);
}