[alsa-devel] [PATCH] alsa-plugins: Pulse: only underrun if no more data has been written

[Takashi Iwai]

At Fri, 19 Aug 2011 11:49:25 +0200,
David Henningsson wrote:
> 
> >From 8098c77a447a80d2860588387d18b3aa6d23b6bb Mon Sep 17 00:00:00 2001
> From: David Henningsson <david.henningsson at canonical.com>
> Date: Fri, 19 Aug 2011 11:47:07 +0200
> Subject: [PATCH] alsa-plugins: Pulse: only underrun if no more data has been
>  written
> 
> If more data has already been written after the underrun, the underrun
> will automatically end and therefore we should not report it or
> restart the stream.
> 
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>  configure.in      |    7 +++++++
>  pulse/pcm_pulse.c |   28 ++++++++++++++++++++++++++--
>  2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.in b/configure.in
> index ccf59ba..f6886b1 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -31,8 +31,14 @@ AC_ARG_ENABLE([pulseaudio],
>  
>  if test "x$enable_pulseaudio" != "xno"; then
>    PKG_CHECK_MODULES(pulseaudio, [libpulse >= 0.9.11], [HAVE_PULSE=yes], [HAVE_PULSE=no])
> +  PKG_CHECK_MODULES(pulseaudio_099, [libpulse >= 0.99.1], 
> +    [HAVE_PULSE_UNDERRUN_INDEX=yes], [HAVE_PULSE_UNDERRUN_INDEX=no])

Hm, can we use PA_CHECK_VERSION() or such simply in the code?

> diff --git a/pulse/pcm_pulse.c b/pulse/pcm_pulse.c
> index d6c6792..61f1c0c 100644
> --- a/pulse/pcm_pulse.c
> +++ b/pulse/pcm_pulse.c
> @@ -26,6 +26,10 @@
>  #include <alsa/asoundlib.h>
>  #include <alsa/pcm_external.h>
>  
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
>  #include "pulse.h"
>  
>  typedef struct snd_pcm_pulse {
> @@ -39,9 +43,10 @@ typedef struct snd_pcm_pulse {
>  	size_t last_size;
>  	size_t ptr;
>  	int underrun;
> -	int handle_underrun;
> +	int handle_underrun; /* can be 0=never, 1=always or 2,3=only if more data has not been written */

I think it'd be simpler to introduce one more boolean variable.
Otherwise the logic looks too complex than needed.

> @@ -594,7 +600,12 @@ static void stream_underrun_cb(pa_stream * p, void *userdata)
>  	if (!pcm->p)
>  		return;
>  
> -	pcm->underrun = 1;
> +	if (pcm->handle_underrun == 1
> +#ifdef HAVE_PULSE_UNDERRUN_INDEX
> +		|| pcm->written <= pa_stream_get_underflow_index(p)
> +#endif
> +	)
> +		pcm->underrun = 1;

A nicer way is to define a macro such as

#ifdef HAVE_PULSE_UNDERRUN_INDEX
static inline int handle_underrun_detect(snd_pcm_pulse_t *pcm, pa_stream *p)
{
	return !pcm->handle_underrun_detect ||
		pcm->written <= pa_stream_get_underflow_index(p);
}
#else
#define handle_underrun_detect(pcm, p)	1
#endif

Then

	if (pcm->handle_underrun && handle_underrun_detect(pcm, p))
		pcm->underrun = 1;


thanks,

Takashi