Re: [PATCH - IOPLUG DRAIN 0/2]

[Takashi Iwai <tiwai@suse.de>]

On Fri, 23 Mar 2018 09:08:43 +0100,
Takashi Iwai wrote:
> 
> On Fri, 23 Mar 2018 09:01:35 +0100,
> Takashi Iwai wrote:
> > 
> > On Fri, 23 Mar 2018 08:43:10 +0100,
> > Wischer, Timo (ADITG/ESB) wrote:
> > > 
> > > > No, again, in non-blocking mode, the drain callback will get never
> > > > called.  It's the responsibility of application to sync with poll()
> > > > instead.
> > > 
> > > Sorry but I do not get it anyway.
> > > 
> > > The user application which is playing some audio has to do the following to drain in nonblocking mode, right?
> > > snd_pcm_poll_descriptors(pfds)
> > > while (snd_pcm_drain() == -EAGAIN) {
> > > poll(pfds)
> > > }
> > > 
> > > 
> > > But in nonblocking mode the drain callback of the IO plugin will never be called with your solution.
> > > Therefore in case of the pulse IO plugin which function should call pa_stream_drain()?
> > > The user application will not do it directly and poll can also not call it.
> > 
> > OK, now I understand your concern.  Yes it's another missing piece,
> > snd_pcm_ioplug_hw_ptr_update() needs to check the current state, and
> > it drops to SETUP state instead of XRUN when it was DRAINING.
> > Then application can simply do poll() and status update until it goes
> > out of DRAINING state.
> > 
> > But still it's outside the plugin, drain callback isn't called there.
> 
> .... and now thinking of this again, the whole story can be folded
> back:
> 
> - The standard drain behavior can be implemented without plugin's own
>   code; it's just a poll and status check.
> 
> - For any special case (or better implementation than poll()), we may
>   leave the whole draining callback action to each plugin; that's the
>   case of PA.

Maybe it's easier to understand by a patch.  Totally untested, but you
get the idea from it.

The check in snd_pcm_ioplug_hw_ptr_update() can be extended to the
XRUN check, too.  But do it in another patch.

And, yeah, this still misses the proper non-blocking mode handling in
pulse plugin.  It's to be fixed there.


Takashi

-- 8< --
--- a/src/pcm/pcm_ioplug.c
+++ b/src/pcm/pcm_ioplug.c
@@ -47,6 +47,11 @@ typedef struct snd_pcm_ioplug_priv {
 	snd_htimestamp_t trigger_tstamp;
 } ioplug_priv_t;
 
+static int snd_pcm_ioplug_drop(snd_pcm_t *pcm);
+static int snd_pcm_ioplug_poll_descriptors_count(snd_pcm_t *pcm);
+static int snd_pcm_ioplug_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int space);
+static int snd_pcm_ioplug_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int nfds, unsigned short *revents);
+
 /* update the hw pointer */
 /* called in lock */
 static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm)
@@ -57,6 +62,7 @@ static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm)
 	hw = io->data->callback->pointer(io->data);
 	if (hw >= 0) {
 		snd_pcm_uframes_t delta;
+		snd_pcm_uframes_t avail;
 
 		if ((snd_pcm_uframes_t)hw >= io->last_hw)
 			delta = hw - io->last_hw;
@@ -67,6 +73,12 @@ static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm)
 			delta = wrap_point + hw - io->last_hw;
 		}
 		snd_pcm_mmap_hw_forward(io->data->pcm, delta);
+		/* stop the stream if all samples are drained */
+		if (io->data->state == SND_PCM_STATE_DRAINING) {
+			avail = snd_pcm_mmap_avail(pcm);
+			if (avail >= pcm->buffer_size)
+				snd_pcm_ioplug_drop(pcm);
+		}
 		io->last_hw = (snd_pcm_uframes_t)hw;
 	} else
 		io->data->state = SNDRV_PCM_STATE_XRUN;
@@ -488,20 +500,66 @@ static int snd_pcm_ioplug_drop(snd_pcm_t *pcm)
 	return 0;
 }
 
+static int ioplug_drain_via_poll(snd_pcm_t *pcm)
+{
+	ioplug_priv_t *io = pcm->private_data;
+	int err;
+
+	/* in non-blocking mode, leave application to poll() by itself */
+	if (io->data->nonblock)
+		return -EAGAIN;
+
+	while (io->data->state == SND_PCM_STATE_DRAINING) {
+		err = snd_pcm_wait_nocheck(pcm, -1);
+		snd_pcm_ioplug_hw_ptr_update(pcm);
+		if (err < 0)
+			break;
+	}
+
+	return 0;
+}
+
 /* need own locking */
 static int snd_pcm_ioplug_drain(snd_pcm_t *pcm)
 {
 	ioplug_priv_t *io = pcm->private_data;
 	int err;
 
-	if (io->data->state == SND_PCM_STATE_OPEN)
+	snd_pcm_lock(pcm);
+	switch (io->data->state) {
+	case SND_PCM_STATE_OPEN:
+	case SND_PCM_STATE_DISCONNECTED:
+	case SND_PCM_STATE_SUSPENDED:
 		return -EBADFD;
+	case SND_PCM_STATE_PREPARED:
+		if (pcm->stream == SND_PCM_STREAM_PLAYBACK) {
+			err = snd_pcm_ioplug_start(pcm);
+			if (err < 0)
+				goto unlock;
+			io->data->state = SND_PCM_STATE_DRAINING;
+		}
+		break;
+	case SND_PCM_STATE_RUNNING:
+		io->data->state = SND_PCM_STATE_DRAINING;
+		break;
+	}
 
-	io->data->state = SND_PCM_STATE_DRAINING;
-	if (io->data->callback->drain)
-		io->data->callback->drain(io->data);
-	snd_pcm_lock(pcm);
-	err = snd_pcm_ioplug_drop(pcm);
+	if (io->data->state == SND_PCM_STATE_DRAINING) {
+		if (io->data->callback->drain) {
+			snd_pcm_unlock(pcm); /* let plugin own locking */
+			err = io->data->callback->drain(io->data);
+			snd_pcm_lock(pcm);
+		} else {
+			err = ioplug_drain_via_poll(pcm);
+		}
+		if (err < 0)
+			goto unlock;
+	}
+
+	if (io->data->state != SND_PCM_STATE_SETUP)
+		err = snd_pcm_ioplug_drop(pcm);
+
+ unlock:
 	snd_pcm_unlock(pcm);
 	return err;
 }