At Tue, 12 Feb 2013 15:29:19 +0100, Clemens Ladisch wrote:
snd_pcm_stop(rt->playback.instance,SNDRV_PCM_STATE_XRUN);This requres locking the stream with something like snd_pcm_stream_lock_irq().
OK, but would you mind elaborating a little more why this is needed?
I do not see other drivers doing that,
sound/firewire/{amdtp,isight}.c do. I've never said that all those other drivers would be correct. (And you have to be careful to avoid deadlocks.)
and BTW I see also that some other drivers not calling snd_pcm_stop() at all, e.g. it is commented in sound/usb/endpoint.c::snd_complete_urb().
It's commented because it would crash. That driver isn't in a very good state regarding state changes.
The problem is that you can't kill the urb itself gracefully while in the complete callback. But a patch like below could work a little bit better, although it doesn't notify the XRUN at that moment.
Takashi
--- diff --git a/sound/usb/card.h b/sound/usb/card.h index 8a751b4..d98e7bc 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -118,6 +118,7 @@ struct snd_usb_substream { unsigned int fmt_type; /* USB audio format type (1-3) */
unsigned int running: 1; /* running status */ + unsigned int xrun:1;
unsigned int hwptr_done; /* processed byte position in the buffer */ unsigned int transfer_done; /* processed frames since last period update */ diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 21049b8..e52aee9 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -357,6 +357,9 @@ static void snd_complete_urb(struct urb *urb) ep->chip->shutdown)) /* device disconnected */ goto exit_clear;
+ if (ep->data_subs && ep->data_subs->xrun) + goto exit_clear; + if (usb_pipeout(ep->pipe)) { retire_outbound_urb(ep, ctx); /* can be stopped during retire callback */ @@ -389,7 +392,8 @@ static void snd_complete_urb(struct urb *urb) return;
snd_printk(KERN_ERR "cannot submit urb (err = %d)\n", err); - //snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN); + if (ep->data_subs) + ep->data_subs->xrun = 1;
exit_clear: clear_bit(ctx->index, &ep->active_mask); diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index f94397b..6be6662 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -79,7 +79,7 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream unsigned int hwptr_done;
subs = (struct snd_usb_substream *)substream->runtime->private_data; - if (subs->stream->chip->shutdown) + if (subs->xrun || subs->stream->chip->shutdown) return SNDRV_PCM_POS_XRUN; spin_lock(&subs->lock); hwptr_done = subs->hwptr_done; @@ -726,6 +726,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) subs->transfer_done = 0; subs->last_delay = 0; subs->last_frame_number = 0; + subs->xrun = 0; runtime->delay = 0;
/* for playback, submit the URBs now; otherwise, the first hwptr_done