At Thu, 20 Aug 2009 16:52:35 +0200,I wrote:> > At Wed, 19 Aug 2009 20:02:23 +0200,> I wrote:> > > > At Wed, 19 Aug 2009 12:50:09 -0400,> > Jon Smirl wrote:> > > > > > On Wed, Aug 19, 2009 at 11:33 AM, Takashi Iwai<tiwai@xxxxxxx> 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@xxxxxxx> 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@xxxxxxx> 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@xxxxxxxxx> 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. > > The patch is below. It's easier than I thought initially.> It seems working fine with my small test case.> > A remaining question is whether ioctl(DRAIN) should give -EAGAIN> in non-blocking mode although it successfully changed the PCM state> to DRAIN. I kept the return value just for compatibility reason,> but returning zero appears more reasonable in this case.> > Anyway, the patch is in sound-unstable GIT tree right now. I'll move> it to the main sound GIT tree later if no one objects.
The patch is merged into sound git tree now.
Takashi_______________________________________________Alsa-devel mailing listAlsa-devel@xxxxxxxxxxxxxxxxxxxx://mailman.alsa-project.org/mailman/listinfo/alsa-devel
