[alsa-devel] PCM states and snd_pcm_drain()
Hi,
Can snd_pcm_drain() be safely called on a non-running stream - particularly one in SND_PCM_STATE_XRUN? I can demonstrate lock-ups (on rather old Alsa versions, though) when _drain is called on a stream in Xrun but it does seem to vary between hardwares.
However, the docs at:
http://www.alsa-project.org/alsa-doc/alsa-lib/pcm.html
suggest that it should be fine.
In our case it seems safer (for widest compatibility) to always check for SND_PCM_STATE_RUNNING, and call snd_pcm_drop() if the stream is not running.
Regards
Alan
At Sun, 20 May 2012 18:25:30 +0100, Alan Horstmann wrote:
Hi,
Can snd_pcm_drain() be safely called on a non-running stream - particularly one in SND_PCM_STATE_XRUN? I can demonstrate lock-ups (on rather old Alsa versions, though) when _drain is called on a stream in Xrun but it does seem to vary between hardwares.
Hm, that's odd. At least there should be no lock up in the core code. Are you running the recent kernel?
However, the docs at:
http://www.alsa-project.org/alsa-doc/alsa-lib/pcm.html
suggest that it should be fine.
The snd_pcm_drop() calls simply ignore streams other than RUNNING, PREPARE and PAUSED. So, it's basically safe.
OTOH, it'd be better to drop XRUN streams to SETUP after this call, and there should be sane state checks at the beginning. I committed the patch below to my tree now.
Takashi
--- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: pcm - Add proper state checks to snd_pcm_drain()
The handling for some PCM states is missing for snd_pcm_drain(). At least, XRUN streams should be simply dropped to SETUP, and a few initial invalid states should be rejected.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_native.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 3fe99e6..53b5ada 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1360,7 +1360,14 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream,
static int snd_pcm_pre_drain_init(struct snd_pcm_substream *substream, int state) { - substream->runtime->trigger_master = substream; + struct snd_pcm_runtime *runtime = substream->runtime; + switch (runtime->status->state) { + case SNDRV_PCM_STATE_OPEN: + case SNDRV_PCM_STATE_DISCONNECTED: + case SNDRV_PCM_STATE_SUSPENDED: + return -EBADFD; + } + runtime->trigger_master = substream; return 0; }
@@ -1379,6 +1386,9 @@ static int snd_pcm_do_drain_init(struct snd_pcm_substream *substream, int state) case SNDRV_PCM_STATE_RUNNING: runtime->status->state = SNDRV_PCM_STATE_DRAINING; break; + case SNDRV_PCM_STATE_XRUN: + runtime->status->state = SNDRV_PCM_STATE_SETUP; + break; default: break; }
On Monday 21 May 2012 11:10, you wrote:
At Sun, 20 May 2012 18:25:30 +0100,
Alan Horstmann wrote:
Can snd_pcm_drain() be safely called on a non-running stream - particularly one in SND_PCM_STATE_XRUN? I can demonstrate lock-ups (on rather old Alsa versions, though) when _drain is called on a stream in Xrun but it does seem to vary between hardwares.
Hm, that's odd. At least there should be no lock up in the core code. Are you running the recent kernel?
Oh certainly not - sorry, I hadn't intended this post to be interpreted as a bug report, since I'm evaluating issues that arise (with Portaudio, again) on Alsa versions back to 1.0.12! This showed up on the older ones. I haven't precisely tracked it down since it is easy (and necessary) to work around.
However, the docs at:
http://www.alsa-project.org/alsa-doc/alsa-lib/pcm.html
suggest that it should be fine.
The snd_pcm_drop() calls simply ignore streams other than RUNNING, PREPARE and PAUSED. So, it's basically safe.
Thanks, that is what I had wanted to check - it is expected to be handled OK.
OTOH, it'd be better to drop XRUN streams to SETUP after this call, and there should be sane state checks at the beginning. I committed the patch below to my tree now.
Just one comment below
From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: pcm - Add proper state checks to snd_pcm_drain()
The handling for some PCM states is missing for snd_pcm_drain(). At least, XRUN streams should be simply dropped to SETUP, and a few initial invalid states should be rejected.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/pcm_native.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 3fe99e6..53b5ada 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1360,7 +1360,14 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream,
static int snd_pcm_pre_drain_init(struct snd_pcm_substream *substream, int state) {
- substream->runtime->trigger_master = substream;
- struct snd_pcm_runtime *runtime = substream->runtime;
- switch (runtime->status->state) {
- case SNDRV_PCM_STATE_OPEN:
- case SNDRV_PCM_STATE_DISCONNECTED:
- case SNDRV_PCM_STATE_SUSPENDED:
return -EBADFD;
I noticed the check for SNDRV_PCM_STATE_OPEN is already in the main snd_pcm_drain() function further down; is it better to add the others there?
Thanks for your assistance.
Alan
At Tue, 22 May 2012 09:22:56 +0100, Alan Horstmann wrote:
On Monday 21 May 2012 11:10, you wrote:
At Sun, 20 May 2012 18:25:30 +0100,
Alan Horstmann wrote:
Can snd_pcm_drain() be safely called on a non-running stream - particularly one in SND_PCM_STATE_XRUN? I can demonstrate lock-ups (on rather old Alsa versions, though) when _drain is called on a stream in Xrun but it does seem to vary between hardwares.
Hm, that's odd. At least there should be no lock up in the core code. Are you running the recent kernel?
Oh certainly not - sorry, I hadn't intended this post to be interpreted as a bug report, since I'm evaluating issues that arise (with Portaudio, again) on Alsa versions back to 1.0.12! This showed up on the older ones. I haven't precisely tracked it down since it is easy (and necessary) to work around.
I see.
However, the docs at:
http://www.alsa-project.org/alsa-doc/alsa-lib/pcm.html
suggest that it should be fine.
The snd_pcm_drop() calls simply ignore streams other than RUNNING, PREPARE and PAUSED. So, it's basically safe.
Thanks, that is what I had wanted to check - it is expected to be handled OK.
OTOH, it'd be better to drop XRUN streams to SETUP after this call, and there should be sane state checks at the beginning. I committed the patch below to my tree now.
Just one comment below
From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: pcm - Add proper state checks to snd_pcm_drain()
The handling for some PCM states is missing for snd_pcm_drain(). At least, XRUN streams should be simply dropped to SETUP, and a few initial invalid states should be rejected.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/pcm_native.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 3fe99e6..53b5ada 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1360,7 +1360,14 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream,
static int snd_pcm_pre_drain_init(struct snd_pcm_substream *substream, int state) {
- substream->runtime->trigger_master = substream;
- struct snd_pcm_runtime *runtime = substream->runtime;
- switch (runtime->status->state) {
- case SNDRV_PCM_STATE_OPEN:
- case SNDRV_PCM_STATE_DISCONNECTED:
- case SNDRV_PCM_STATE_SUSPENDED:
return -EBADFD;I noticed the check for SNDRV_PCM_STATE_OPEN is already in the main snd_pcm_drain() function further down; is it better to add the others there?
The difference is that snd_pcm_pre_drain_init() is applied to each linked stream while the check in snd_pcm_drain() is only for the given stream. So, checking in snd_pcm_pre_drain_init() is more strict.
Takashi
participants (2)
-
Alan Horstmann -
Takashi Iwai