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

[David Henningsson]

Behold the third version of the patch.

On 08/19/2011 02:46 PM, Takashi Iwai wrote:
> 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?

Ok, good idea.

>
>> 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.

I've now done that. I don't know if it became simpler though.

>
>> @@ -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;

Ok, works for me.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-alsa-plugins-Pulse-only-underrun-if-no-more-data-has.patch
Type: text/x-patch
Size: 0 bytes
Desc: not available
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20110823/ac5f100b/attachment.patch