Hello,
I’m investigating an issue where USB Audio does not properly send XRUN
notifications.
The issue can be reproduced with aplay: enable xrun_debug, aplay -D
plughw:0 and CTRL-Z - no XRUN message is seen
Disabling lowlatency_playback via modprobe parameter does make this
issue go away - XRUNs are reported correctly without any changes.
After a lot of tracing the following seems to be happening:
- prepare_playback_urb find avail=48, meaning 48 bytes still to-be-played
- snd_usb_endpoint_next_packet_size decides that 48 is too little and
returns -EAGAIN. Specifically -EAGAIN is returned from next_packet_size
- The return value of prepare_playback_urb is propagated through
prepare_outbound_urb back to snd_usb_queue_pending_output_urbs
- snd_usb_queue_pending_output_urbs receives -EAGAIN from
prepare_outbound_urb
- since err is -EAGAIN the ctx is pushed back to the ready list and
transmission is aborted but notify_xrun is skipped
- no more playback?
It is possible to make XRUNs happen by caling notify_xrun even on
-EAGAIN, diff looks like this:
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 568099467dbb..da64ee0cf60a 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -495,10 +495,11 @@ int snd_usb_queue_pending_output_urbs(struct
snd_usb_endpoint *ep,
break;
if (err < 0) {
/* push back to ready list again for -EAGAIN */
if (err == -EAGAIN) {
push_back_to_ready_list(ep, ctx);
+ notify_xrun(ep);
break;
}
if (!in_stream_lock)
notify_xrun(ep);
This mail was not formatted as proper patch because this seems very
likely incorrect, it undoes an explicit check. What would a correct
solution look like?
There could be a scenario where -EAGAIN from prepare_outbound_urb can
happen without an actual XRUN, but I don’t understand the code enough.
The fact that usb-audio halts playback with a non-zero amount of bytes
that can still be played prevents the XRUN checks inside
`snd_pcm_update_state` from triggering. This means that the driver
always takes responsibility for reporting xrun, correct?
It seems dubious to me that unplayed bytes are kept around - shouldn’t
fragments either be played or dropped? Perhaps the last fragment should
just be padded with some silence and sent anyway, then core sound code
should report xrun anyway.
USB Audio has many parameters. In my particular case:
- lowlatency_playback = true
- channels=2, rate=48000, format=S16_LE, period_bytes=960, periods=3,
implicit_fb=0
- nurbs = 3
This was originally found in a complex application which checks
snd_pcm_state regularly and does not find the SND_PCM_STATE_XRUN state
but audio gaps are seen in a physical line capture.
This was seen in Linux 5.19.139 but also affects 6.6.y and latest
sound/master branch. There are very few relevant changes in this area
that are not already in stable branches.
Hardware is a conexant audio codec, USB VID:PID 0572:1410.
--
Regards,
Leonard
On Tue, 19 Nov 2024 22:54:19 +0100,
Leonard Crestez wrote:
>
> Hello,
>
> I’m investigating an issue where USB Audio does not properly send
> XRUN notifications.
>
> The issue can be reproduced with aplay: enable xrun_debug, aplay -D
> plughw:0 and CTRL-Z - no XRUN message is seen
>
> Disabling lowlatency_playback via modprobe parameter does make this
> issue go away - XRUNs are reported correctly without any changes.
>
>
> After a lot of tracing the following seems to be happening:
>
> - prepare_playback_urb find avail=48, meaning 48 bytes still to-be-played
> - snd_usb_endpoint_next_packet_size decides that 48 is too little and
> returns -EAGAIN. Specifically -EAGAIN is returned from
> next_packet_size
> - The return value of prepare_playback_urb is propagated through
> prepare_outbound_urb back to snd_usb_queue_pending_output_urbs
> - snd_usb_queue_pending_output_urbs receives -EAGAIN from
> prepare_outbound_urb
> - since err is -EAGAIN the ctx is pushed back to the ready list and
> transmission is aborted but notify_xrun is skipped
> - no more playback?
>
> It is possible to make XRUNs happen by caling notify_xrun even on
> -EAGAIN, diff looks like this:
>
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 568099467dbb..da64ee0cf60a 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -495,10 +495,11 @@ int snd_usb_queue_pending_output_urbs(struct
> snd_usb_endpoint *ep,
> break;
> if (err < 0) {
> /* push back to ready list again for -EAGAIN */
> if (err == -EAGAIN) {
> push_back_to_ready_list(ep, ctx);
> + notify_xrun(ep);
> break;
> }
>
> if (!in_stream_lock)
> notify_xrun(ep);
>
>
> This mail was not formatted as proper patch because this seems very
> likely incorrect, it undoes an explicit check. What would a correct
> solution look like?
The -EAGAIN there itself doesn't mean the crucial xrun yet. There may
be still pending URBS to be processed. The real XRUN happens only
when there is no URBs pending, hence nothing will be taken further --
at least for low-latency operation. (In the case of implicit feedback
mode, it can be driven by the feedback from the capture stream, and
the empty URB check might be wrong.)
Could you check the change below? (totally untested)
thanks,
Takashi
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -562,7 +562,10 @@ static void snd_complete_urb(struct urb *urb)
push_back_to_ready_list(ep, ctx);
clear_bit(ctx->index, &ep->active_mask);
snd_usb_queue_pending_output_urbs(ep, false);
- atomic_dec(&ep->submitted_urbs); /* decrement at last */
+ /* decrement at last, and check xrun */
+ if (atomic_dec_and_test(&ep->submitted_urbs) &&
+ !snd_usb_endpoint_implicit_feedback_sink(ep))
+ notify_xrun(ep);
return;
}
On Wed, 20 Nov 2024 08:31:44 +0100,
Takashi Iwai wrote:
>
> On Tue, 19 Nov 2024 22:54:19 +0100,
> Leonard Crestez wrote:
> >
> > Hello,
> >
> > I’m investigating an issue where USB Audio does not properly send
> > XRUN notifications.
> >
> > The issue can be reproduced with aplay: enable xrun_debug, aplay -D
> > plughw:0 and CTRL-Z - no XRUN message is seen
> >
> > Disabling lowlatency_playback via modprobe parameter does make this
> > issue go away - XRUNs are reported correctly without any changes.
> >
> >
> > After a lot of tracing the following seems to be happening:
> >
> > - prepare_playback_urb find avail=48, meaning 48 bytes still to-be-played
> > - snd_usb_endpoint_next_packet_size decides that 48 is too little and
> > returns -EAGAIN. Specifically -EAGAIN is returned from
> > next_packet_size
> > - The return value of prepare_playback_urb is propagated through
> > prepare_outbound_urb back to snd_usb_queue_pending_output_urbs
> > - snd_usb_queue_pending_output_urbs receives -EAGAIN from
> > prepare_outbound_urb
> > - since err is -EAGAIN the ctx is pushed back to the ready list and
> > transmission is aborted but notify_xrun is skipped
> > - no more playback?
> >
> > It is possible to make XRUNs happen by caling notify_xrun even on
> > -EAGAIN, diff looks like this:
> >
> > diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> > index 568099467dbb..da64ee0cf60a 100644
> > --- a/sound/usb/endpoint.c
> > +++ b/sound/usb/endpoint.c
> > @@ -495,10 +495,11 @@ int snd_usb_queue_pending_output_urbs(struct
> > snd_usb_endpoint *ep,
> > break;
> > if (err < 0) {
> > /* push back to ready list again for -EAGAIN */
> > if (err == -EAGAIN) {
> > push_back_to_ready_list(ep, ctx);
> > + notify_xrun(ep);
> > break;
> > }
> >
> > if (!in_stream_lock)
> > notify_xrun(ep);
> >
> >
> > This mail was not formatted as proper patch because this seems very
> > likely incorrect, it undoes an explicit check. What would a correct
> > solution look like?
>
> The -EAGAIN there itself doesn't mean the crucial xrun yet. There may
> be still pending URBS to be processed. The real XRUN happens only
> when there is no URBs pending, hence nothing will be taken further --
> at least for low-latency operation. (In the case of implicit feedback
> mode, it can be driven by the feedback from the capture stream, and
> the empty URB check might be wrong.)
>
> Could you check the change below? (totally untested)
A bit more change would be needed because it can lead to a false xrun
at draining. At stopping, it shouldn't reach to that code path.
The revised patch is below.
Takashi
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -403,10 +403,15 @@ static int prepare_inbound_urb(struct snd_usb_endpoint *ep,
static void notify_xrun(struct snd_usb_endpoint *ep)
{
struct snd_usb_substream *data_subs;
+ struct snd_pcm_substream *psubs;
data_subs = READ_ONCE(ep->data_subs);
- if (data_subs && data_subs->pcm_substream)
- snd_pcm_stop_xrun(data_subs->pcm_substream);
+ if (!data_subs)
+ return;
+ psubs = data_subs->pcm_substream;
+ if (psubs && psubs->runtime &&
+ psubs->runtime->state == SNDRV_PCM_STATE_RUNNING)
+ snd_pcm_stop_xrun(psubs);
}
static struct snd_usb_packet_info *
@@ -562,7 +567,10 @@ static void snd_complete_urb(struct urb *urb)
push_back_to_ready_list(ep, ctx);
clear_bit(ctx->index, &ep->active_mask);
snd_usb_queue_pending_output_urbs(ep, false);
- atomic_dec(&ep->submitted_urbs); /* decrement at last */
+ /* decrement at last, and check xrun */
+ if (atomic_dec_and_test(&ep->submitted_urbs) &&
+ !snd_usb_endpoint_implicit_feedback_sink(ep))
+ notify_xrun(ep);
return;
}
On 11/20/24 10:33, Takashi Iwai wrote:
> On Wed, 20 Nov 2024 08:31:44 +0100,
> Takashi Iwai wrote:
>>
>> On Tue, 19 Nov 2024 22:54:19 +0100,
>> Leonard Crestez wrote:
>>>
>>> Hello,
>>>
>>> I’m investigating an issue where USB Audio does not properly send
>>> XRUN notifications.
>>>
>>> The issue can be reproduced with aplay: enable xrun_debug, aplay -D
>>> plughw:0 and CTRL-Z - no XRUN message is seen
>>>
>>> Disabling lowlatency_playback via modprobe parameter does make this
>>> issue go away - XRUNs are reported correctly without any changes.
>>>
>>>
>>> After a lot of tracing the following seems to be happening:
>>>
>>> - prepare_playback_urb find avail=48, meaning 48 bytes still to-be-played
>>> - snd_usb_endpoint_next_packet_size decides that 48 is too little and
>>> returns -EAGAIN. Specifically -EAGAIN is returned from
>>> next_packet_size
>>> - The return value of prepare_playback_urb is propagated through
>>> prepare_outbound_urb back to snd_usb_queue_pending_output_urbs
>>> - snd_usb_queue_pending_output_urbs receives -EAGAIN from
>>> prepare_outbound_urb
>>> - since err is -EAGAIN the ctx is pushed back to the ready list and
>>> transmission is aborted but notify_xrun is skipped
>>> - no more playback?
>>>
>>> It is possible to make XRUNs happen by caling notify_xrun even on
>>> -EAGAIN, diff looks like this:
>>>
>>> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
>>> index 568099467dbb..da64ee0cf60a 100644
>>> --- a/sound/usb/endpoint.c
>>> +++ b/sound/usb/endpoint.c
>>> @@ -495,10 +495,11 @@ int snd_usb_queue_pending_output_urbs(struct
>>> snd_usb_endpoint *ep,
>>> break;
>>> if (err < 0) {
>>> /* push back to ready list again for -EAGAIN */
>>> if (err == -EAGAIN) {
>>> push_back_to_ready_list(ep, ctx);
>>> + notify_xrun(ep);
>>> break;
>>> }
>>>
>>> if (!in_stream_lock)
>>> notify_xrun(ep);
>>>
>>>
>>> This mail was not formatted as proper patch because this seems very
>>> likely incorrect, it undoes an explicit check. What would a correct
>>> solution look like?
>>
>> The -EAGAIN there itself doesn't mean the crucial xrun yet. There may
>> be still pending URBS to be processed. The real XRUN happens only
>> when there is no URBs pending, hence nothing will be taken further --
>> at least for low-latency operation. (In the case of implicit feedback
>> mode, it can be driven by the feedback from the capture stream, and
>> the empty URB check might be wrong.)
>>
>> Could you check the change below? (totally untested)
>
> A bit more change would be needed because it can lead to a false xrun
> at draining. At stopping, it shouldn't reach to that code path.
> The revised patch is below.
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -403,10 +403,15 @@ static int prepare_inbound_urb(struct snd_usb_endpoint *ep,
> static void notify_xrun(struct snd_usb_endpoint *ep)
> {
> struct snd_usb_substream *data_subs;
> + struct snd_pcm_substream *psubs;
>
> data_subs = READ_ONCE(ep->data_subs);
> - if (data_subs && data_subs->pcm_substream)
> - snd_pcm_stop_xrun(data_subs->pcm_substream);
> + if (!data_subs)
> + return;
> + psubs = data_subs->pcm_substream;
> + if (psubs && psubs->runtime &&
> + psubs->runtime->state == SNDRV_PCM_STATE_RUNNING)
> + snd_pcm_stop_xrun(psubs);
> }
>
> static struct snd_usb_packet_info *
> @@ -562,7 +567,10 @@ static void snd_complete_urb(struct urb *urb)
> push_back_to_ready_list(ep, ctx);
> clear_bit(ctx->index, &ep->active_mask);
> snd_usb_queue_pending_output_urbs(ep, false);
> - atomic_dec(&ep->submitted_urbs); /* decrement at last */
> + /* decrement at last, and check xrun */
> + if (atomic_dec_and_test(&ep->submitted_urbs) &&
> + !snd_usb_endpoint_implicit_feedback_sink(ep))
> + notify_xrun(ep);
> return;
> }
This makes more sense than what I hacked to together and seems to work
well in initial testing.
I'll report back if issues are found.
--
Thanks,
Leonard
© 2016 - 2026 Red Hat, Inc.