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

[Takashi Iwai]

At Wed, 19 Aug 2009 12:50:09 -0400,
Jon Smirl wrote:
> 
> On Wed, Aug 19, 2009 at 11:33 AM, Takashi Iwai<tiwai at suse.de> wrote:
> > At Wed, 19 Aug 2009 11:19:45 -0400,
> > Jon Smirl wrote:
> >>
> >> On Wed, Aug 19, 2009 at 11:12 AM, Takashi Iwai<tiwai at suse.de> wrote:
> >> > At Wed, 19 Aug 2009 11:02:31 -0400,
> >> > Jon Smirl wrote:
> >> >>
> >> >> On Wed, Aug 19, 2009 at 8:14 AM, Takashi Iwai<tiwai at suse.de> wrote:
> >> >> > 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.
> >> >>
> >> >> Brutefir is a server type app, so pulseaudio should be having trouble
> >> >> with this too.
> >> >
> >> > Maybe not.  Otherwise we've got already many bug reports.
> >> >
> >> > The difference is how to wait until all data is out.  PA would likely
> >> > wait using its own timer stuff without sleeping in drain ioctl.
> >>
> >> Can you implement a polled drain by checking if state is RUNNING and
> >> the looking for the transition to STOPPED?
> >
> > Could you elaborate?
> >
> >> Is there anything special about being in state DRAINING?
> >
> > Yes.  The drain needs the following active procedure:
> >
> > 1. app notifies the driver that the stream is drained.
> > 2. app go to sleep (in drain ioctl)
> > 3. the driver marks the PCM state DRAIN
> > 4. when the all data has been sent out, the driver stops the stream,
> >  wakes up the sleeper
> > 5. app returns
> >
> > Without the explicit notification, the driver cannot know whether
> > the stream is supposed to be stopped successfully or just get an
> > XRUN.
> >
> > I guess you think that ioctl(DRAIN) just marks and returns, then
> 
> That would be a function of being in OF_NONBLOCKED state.
> 
> > app does poll() to wait until all data sent out.  This would work,
> > too, after some amount of work.  But, just fixing the existing DRAIN
> > ioctl is far less work in the end...
> 
> Right now drain() is a synchronous API. There is no alternative for
> asynchronously starting a drain and then polling or getting signaled
> when it is finished. If the app is not threaded it will go
> unresponsive while in the drain IOCTL (20 seconds in my case).
> Currently brutefir is not written at a threaded app.

OK, it sounds like a reasonable argument.

Maybe it's worth to check how easy it can be implemented.
Basically the same wait_queue like normal poll is used for drain
checks, thus it wouldn't be too difficult to use poll() for user-space
for the same purpose.  

But, a possible problem is the case of linked PCM streams.  This can 
give far more pains than gains...


thanks,

Takashi