[pulseaudio-discuss] [RFC] [PATCH] alsa-sink: Reduce unnecessarily large max rewind

[arun at accosted.net]

From: Arun Raghavan <git at arunraghavan.net>

This makes us not report more than the hardware buffer size being used
as the max rewind that sink inputs need to support. The rationale is
that we won't ever rewind more than the length of the buffer that we are
currently using, so we don't need to keep the extra data around. On
large HDA buffers, this should save some memory for each sink input.

However, I do notice a problem -- when there is a filter sink attached,
the first stream after a resume from suspend playse fine, but the second
stream has a lag that seems to be in the order of the buffer size. I'm
not sure where the problem lies.

I'll try to take a look at what's going wrong, but if anyone has any
ideas, I'm all ears.
---
 src/modules/alsa/alsa-sink.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index c5a72c3..94aeff0 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -956,12 +956,14 @@ static int suspend(struct userdata *u) {
 }
 
 /* Called from IO context */
-static int update_sw_params(struct userdata *u) {
+static int update_sw_params(struct userdata *u, bool rewind) {
     snd_pcm_uframes_t avail_min;
+    size_t before;
     int err;
 
     pa_assert(u);
 
+    before = u->hwbuf_unused;
     /* Use the full buffer if no one asked us for anything specific */
     u->hwbuf_unused = 0;
 
@@ -1006,9 +1008,21 @@ static int update_sw_params(struct userdata *u) {
         return err;
     }
 
+    if (rewind) {
+        /* Let's check whether we now use only a smaller part of the
+           buffer then before. If so, we need to make sure that subsequent
+           rewinds are relative to the new maximum fill level and not to the
+           current fill level. Thus, let's do a full rewind once, to clear
+           things up. */
+        if (u->hwbuf_unused > before) {
+            pa_log_debug("Requesting rewind due to latency change.");
+            pa_sink_request_rewind(u->sink, (size_t) -1);
+        }
+    }
+
     pa_sink_set_max_request_within_thread(u->sink, u->hwbuf_size - u->hwbuf_unused);
-     if (pa_alsa_pcm_is_hw(u->pcm_handle))
-         pa_sink_set_max_rewind_within_thread(u->sink, u->hwbuf_size);
+    if (pa_alsa_pcm_is_hw(u->pcm_handle))
+        pa_sink_set_max_rewind_within_thread(u->sink, u->hwbuf_size - u->hwbuf_unused);
     else {
         pa_log_info("Disabling rewind_within_thread for device %s", u->device_name);
         pa_sink_set_max_rewind_within_thread(u->sink, 0);
@@ -1109,7 +1123,7 @@ static int unsuspend(struct userdata *u) {
         goto fail;
     }
 
-    if (update_sw_params(u) < 0)
+    if (update_sw_params(u, false) < 0)
         goto fail;
 
     if (build_pollfd(u) < 0)
@@ -1505,7 +1519,6 @@ static int sink_set_port_cb(pa_sink *s, pa_device_port *p) {
 
 static void sink_update_requested_latency_cb(pa_sink *s) {
     struct userdata *u = s->userdata;
-    size_t before;
     pa_assert(u);
     pa_assert(u->use_tsched); /* only when timer scheduling is used
                                * we can dynamically adjust the
@@ -1514,19 +1527,7 @@ static void sink_update_requested_latency_cb(pa_sink *s) {
     if (!u->pcm_handle)
         return;
 
-    before = u->hwbuf_unused;
-    update_sw_params(u);
-
-    /* Let's check whether we now use only a smaller part of the
-    buffer then before. If so, we need to make sure that subsequent
-    rewinds are relative to the new maximum fill level and not to the
-    current fill level. Thus, let's do a full rewind once, to clear
-    things up. */
-
-    if (u->hwbuf_unused > before) {
-        pa_log_debug("Requesting rewind due to latency change.");
-        pa_sink_request_rewind(s, (size_t) -1);
-    }
+    update_sw_params(u, true);
 }
 
 static pa_idxset* sink_get_formats(pa_sink *s) {
@@ -2360,7 +2361,7 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca
 
     reserve_update(u);
 
-    if (update_sw_params(u) < 0)
+    if (update_sw_params(u, false) < 0)
         goto fail;
 
     if (u->ucm_context) {
-- 
2.5.0