[alsa-devel] SNDRV_PCM_TRIGGER_STOP and audio still queued in the driver

[Takashi Iwai]

At Sat, 15 Aug 2009 23:40:36 -0400,
Jon Smirl wrote:
> 
> On Sat, Aug 15, 2009 at 11:53 AM, Jon Smirl<jonsmirl at gmail.com> wrote:
> > void
> > bfio_synch_stop(void)
> > {
> >    int n;
> >
> >    if (base_handle == NULL) {
> >        return;
> >    }
> >    FOR_IN_AND_OUT {
> >        for (n = 0; n < n_handles[IO]; n++) {
> 
> I added:
> snd_pcm_nonblock(handles[IO][n], 0)
> snd_pcm_drain(handles[IO][n])
> snd_pcm_nonblock(handles[IO][n], SND_PCM_NONBLOCK )
> 
> >            snd_pcm_close(handles[IO][n]);
> >        }
> >    }
> > }
> 
> This is not working correctly.
> snd_pcm_nonblock(handles[IO][n], 0)
> It does not remove O_NONBLOCK for some unknown reason.
> 
> I added printf() to snd_pcm_hw_nonblock()
> The fcntl is not getting an error.
> 	if (fcntl(fd, F_SETFL, flags) < 0) {
> Flags being set are 2 (O_RDWR).
> 
> But when I get over to snd_pcm_pre_drain_init(), I get the -EAGAIN error.
> static int snd_pcm_pre_drain_init(struct snd_pcm_substream *substream,
> int state)
> {
> 	printk("snd_pcm_pre_drain_init\n");
> 	if (substream->f_flags & O_NONBLOCK)
> 		return -EAGAIN;
> 	printk("snd_pcm_pre_drain_init 1\n");
> 	substream->runtime->trigger_master = substream;
> 	return 0;
> }
> So I have to conclude that fcntl(fd, F_SETFL, flags) is not removing
> the O_NONBLOCK flag.

Yeah, you found a long-standing bug :)

Honestly, I think the current designed behavior is just annoying.
An ioctl may be blocked, thus there is no real merit to return -EAGAIN
with DRAIN ioctl.

So, my preferred solution is simply to remove the O_NONBLOCK check
in the code path above.

Another solution would be a patch like below (totally untested)...

Comments?


thanks,

Takashi

---
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index d89c816..5d6b9ad 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1343,8 +1343,6 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream,
 
 static int snd_pcm_pre_drain_init(struct snd_pcm_substream *substream, int state)
 {
-	if (substream->f_flags & O_NONBLOCK)
-		return -EAGAIN;
 	substream->runtime->trigger_master = substream;
 	return 0;
 }
@@ -1404,7 +1402,8 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream);
  * After this call, all streams are supposed to be either SETUP or DRAINING
  * (capture only) state.
  */
-static int snd_pcm_drain(struct snd_pcm_substream *substream)
+static int snd_pcm_drain(struct snd_pcm_substream *substream,
+			 struct file *file)
 {
 	struct snd_card *card;
 	struct snd_pcm_runtime *runtime;
@@ -1412,6 +1411,14 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream)
 	int result = 0;
 	int i, num_drecs;
 	struct drain_rec *drec, drec_tmp, *d;
+	int f_flags;
+
+	if (file)
+		f_flags = file->f_flags;
+	else
+		f_flags = substream->f_flags;
+	if (f_flags & O_NONBLOCK)
+		return -EAGAIN;
 
 	card = substream->pcm->card;
 	runtime = substream->runtime;
@@ -2556,7 +2563,7 @@ static int snd_pcm_common_ioctl1(struct file *file,
 		return snd_pcm_hw_params_old_user(substream, arg);
 #endif
 	case SNDRV_PCM_IOCTL_DRAIN:
-		return snd_pcm_drain(substream);
+		return snd_pcm_drain(substream, file);
 	case SNDRV_PCM_IOCTL_DROP:
 		return snd_pcm_drop(substream);
 	case SNDRV_PCM_IOCTL_PAUSE: