On Tue, 22 Nov 2022 12:16:47 +0100, Carl Hetherington wrote:
Hi Takashi,
Thank you for getting back to me!
On Tue, 22 Nov 2022, Takashi Iwai wrote:
[snip]
Now, snd_usb_endpoint_start() is called on 0x2 and that is fine. Next, snd_usb_endpoint_start() is called on 0x82 and that fails because its state is still STOPPING.
At this point things seem broken.
Does anyone have a hint about where in this sequence things are going wrong, and maybe even why?
The problem is that it's treating XRUNs on the both streams individually. It's correct to recover only the PCM stream when an XRUN is reported to the PCM stream. However, for an XRUN on the capture stream that serves as a sync source, it should stop and recover not only the capture PCM stream but also the playback stream as a sync sink as well.
Below is a possible test fix (totally untested!). This may give XRUNs twice eventually, which is a bit confusing, but it aligns with the actual hardware behavior, at least.
[snip fix]
Makes sense, thank you! Sadly, the fix doesn't seem to work because (I think) the xruns I'm seeing come via a different path (not though notify_xrun()). Mine arrive via this trace:
__snd_pcm_xrun snd_pcm_update_state snd_pcm_update_hw_ptr usb_hcd_giveback_urb snd_pcm_period_elapsed_under_stream_lock snd_pcm_period_elapsed retire_capture_urb snd_complete_urb
I'll see if can apply a similar fix to this case, though to my naive eyes it looks a little trickier as the xrun is found in the snd_pcm code rather than the USB code. Any suggestions most welcome!
OK, then it's a bit different problem, and not so trivial to fix in the kernel side alone, I'm afraid. Basically it's a race between start and stop of two streams. The key point is that, for stopping a (USB) stream, a sync-stop operation is needed, and this can't be performed at the PCM trigger itself (which is an atomic operation). So, the kernel trigger may at most return an error there.
I assume that it's from snd_usb_endpoint_start() and it returning -EPIPE error. If so, we may change the PCM core code to set the PCM state again XRUN in such an error case, so that application may repeat the standard recovery process. Something like below.
Takashi
-- 8< -- --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1424,9 +1424,14 @@ static int snd_pcm_pre_start(struct snd_pcm_substream *substream, static int snd_pcm_do_start(struct snd_pcm_substream *substream, snd_pcm_state_t state) { + int err; + if (substream->runtime->trigger_master != substream) return 0; - return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_START); + err = substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_START); + if (err == -EPIPE) + __snd_pcm_set_state(runtime, SNDRV_PCM_STATE_XRUN); + return err; }
static void snd_pcm_undo_start(struct snd_pcm_substream *substream,