[pulseaudio-discuss] Yet another attempt to fight rewinds

[David Henningsson]

A few Atom users have complained about enternal rewinds since they 
upgraded to 0.99.x, see 
https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/825709


So here's just an idea. As a last resort, ratelimit the number of 
rewinds. If there are more than 10 rewinds in 200 ms, go to sleep for 
200 ms. The idea is that during those 200 ms, the client application 
will produce enough packets to fill up the buffer enough. Those packets 
will then be merged into one, due to an earlier rewind patch that is 
already in. The 200 ms sleep might cause a noticable glitch, but 
hopefully we get that one glitch only instead of complete brokenness.


But I don't have any such setup here currently, so maybe any of you 
could check this patch and see if it works as intended, and has real effect?


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
From e1a11d045c898b1875532b9a1497c138968bd2ea Mon Sep 17 00:00:00 2001
From: David Henningsson david.hennings...@canonical.com
Date: Mon, 19 Sep 2011 15:54:58 +0200
Subject: [PATCH] Ratelimit rewinds of sinks

Allow max 10 rewinds in 200 ms in order to try to counteract the
eternal rewind issues still seen on low-end CPUs (e g Atom)

BugLink: http://bugs.launchpad.net/bugs/825709
Signed-off-by: David Henningsson david.hennings...@canonical.com
---
 src/pulsecore/sink.c |8 
 src/pulsecore/sink.h |2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index d97fb7e..2d006a5 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -332,6 +332,7 @@ pa_sink* pa_sink_new(
 s-thread_info.state = s-state;
 s-thread_info.rewind_nbytes = 0;
 s-thread_info.rewind_requested = FALSE;
+PA_INIT_RATELIMIT(s-thread_info.rewind_limit, PA_USEC_PER_MSEC * 200, 10); /* max 10 rewinds in 200 ms */
 s-thread_info.max_rewind = 0;
 s-thread_info.max_request = 0;
 s-thread_info.requested_latency_valid = FALSE;
@@ -932,6 +933,13 @@ void pa_sink_process_rewind(pa_sink *s, size_t nbytes) {
 if (s-monitor_source  PA_SOURCE_IS_LINKED(s-monitor_source-thread_info.state))
 pa_source_process_rewind(s-monitor_source, nbytes);
 }
+
+if (!pa_ratelimit_test(s-thread_info.rewind_limit, PA_LOG_DEBUG)) {
+pa_log_warn(Okay, I'm sick and tired of all this rewinding. I'm going to sleep for %lu ms!,
+s-thread_info.rewind_limit.interval / PA_USEC_PER_MSEC);
+usleep(s-thread_info.rewind_limit.interval);
+pa_log_debug(Waking up.);
+}
 }
 
 /* Called from IO thread context */
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 0642dda..8c998e8 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -47,6 +47,7 @@ typedef struct pa_sink_volume_change pa_sink_volume_change;
 #include pulsecore/queue.h
 #include pulsecore/thread-mq.h
 #include pulsecore/sink-input.h
+#include pulsecore/ratelimit.h
 
 #define PA_MAX_INPUTS_PER_SINK 32
 
@@ -261,6 +262,7 @@ struct pa_sink {
 /* Maximum of what clients requested to rewind in this cycle */
 size_t rewind_nbytes;
 pa_bool_t rewind_requested;
+pa_ratelimit rewind_limit;
 
 /* Both dynamic and fixed latencies will be clamped to this
  * range. */
-- 
1.7.5.4

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Yet another attempt to fight rewinds

[Tanu Kaskinen]

On Mon, 2011-09-19 at 17:49 +0200, David Henningsson wrote:
 +if (!pa_ratelimit_test(s-thread_info.rewind_limit, PA_LOG_DEBUG)) {
 +pa_log_warn(Okay, I'm sick and tired of all this rewinding. I'm 
 going to sleep for %lu ms!,
 +s-thread_info.rewind_limit.interval / PA_USEC_PER_MSEC);
 +usleep(s-thread_info.rewind_limit.interval);
 +pa_log_debug(Waking up.);
 +}

The patch seems otherwise ok to me, but I'd prefer the log prints to
include the sink name. When a system has e.g. 6 sinks, all of which may
be in active use at the same time, I really don't like the sink.c
messages that don't tell which sink is in question... Also, instead of
just waking up, maybe it would be useful to have waking up; not so
sick and tired anymore or something else that clearly connects the
wakeup message to the preceding warning.

-- 
Tanu

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss