At Tue, 23 Aug 2011 15:57:05 +0200, David Henningsson wrote:
On 2011-08-23 15:40, Takashi Iwai wrote:
At Tue, 23 Aug 2011 13:56:58 +0200, David Henningsson wrote:
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 Henningssondavid.henningsson@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 Henningssondavid.henningsson@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.
Hm, indeed.
I think we can start off from more simple. With the new PA, the underrun detection should work better. So why we do we need yet another mode to disable it? It should be enough just to have a boolean for underrun_detect yes/no.
So, the patch would be like below.
What do you think?
Sure. For me, I'd go with the new functionality without ever configuring anything else. So I'm happy to exchange "always underrun" for the new underrun detection or even to remove the configuration functionality entirely.
The configuration is still needed for older PA, so let's keep it for now. I committed the previous patch now to git tree.
Thanks!
Takashi