Re: [PATCH - JACK plugin 4/4] jack: Support snd_pcm_drain()

From: "Wischer, Timo (ADITG/ESB)" <twischer@de.adit-jv.com>
To: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.de>,
	ALSA development <alsa-devel@alsa-project.org>
Subject: Re: [PATCH - JACK plugin 4/4] jack: Support snd_pcm_drain()
Date: Fri, 16 Mar 2018 15:52:31 +0000	[thread overview]
Message-ID: <B0FB33DC1499054591F62C0EF1E013D7684FDF04@HI2EXCH01.adit-jv.com> (raw)
In-Reply-To: <07334145-d1f1-54b4-84d0-eacbea0a3987@perex.cz>

Hi Jaroslav,

> But it changes the PCM API behaviour, so some apps might have trouble
> with it, because it might cause the unexpected task blocking. The
> correct behaviour is return with -EAGAIN

I have checked all IO plugins (pulse, oss, a52, jack) only a52 could return -EAGAIN.
oss and pulse do always block and it seems that they are not updating the file descriptor in case of draining.
Therefore snd_pcm_wait() would block infinitely.

I think an application developer would not expect that he has to call snd_pcm_drop() after snd_pcm_drain().
Therefore snd_pcm_drop has to be called internally.
(On a real hardware it will be handled in the kernel)

Therefore I think snd_pcm_drop() has to be called from snd_pcm_wait() before returning.
But snd_pcm_wait callback is not available in the IO plug API.
Therefore I think we have to call snd_pcm_drop() from snd_pcm_ioplug_poll_revents().
But this looks a little bit ugly for me.
Do you have another idea?

One approach could be the following
snd_pcm_ioplug_poll_revents()
{
	...
	if (draining)
		snd_pcm_drop()
}

snd_pcm_ioplug_drain()
{
	int err = 0
	if (io->data->callback->drain)
		err = io->data->callback->drain(io->data);
	if (err != -EAGAIN) {
		snd_pcm_lock(pcm);
		err = snd_pcm_ioplug_drop(pcm);
		snd_pcm_unlock(pcm);
	}
	return err;
}

But I am not aware of if this would brake any of the existing IO plugins.
I would prefer to avoid these changes.

Best regards

Timo Wischer

Advanced Driver Information Technology GmbH
Engineering Software Base (ADITG/ESB)
Robert-Bosch-Str. 200
31139 Hildesheim
Germany

Tel. +49 5121 49 6938
Fax +49 5121 49 6999
twischer@de.adit-jv.com

ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car Multimedia GmbH and DENSO Corporation
Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
Geschäftsführung: Wilhelm Grabow, Ken Yaguchi