[pulseaudio-discuss] [PATCH 1/2] Revert "memblockq: remove internal "missing" state variable"

[Tanu Kaskinen]

This reverts commit 74251f07864c63439ea847d7287024ac54578d64.

The reverted commit was not intended to make any behavioral changes, but
it broke at least the case where a client writes more data than the
server has requested.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=99211
---
 src/pulsecore/memblockq.c | 51 ++++++++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/src/pulsecore/memblockq.c b/src/pulsecore/memblockq.c
index b1b4fde..f660ffa 100644
--- a/src/pulsecore/memblockq.c
+++ b/src/pulsecore/memblockq.c
@@ -53,7 +53,7 @@ struct pa_memblockq {
     bool in_prebuf;
     pa_memchunk silence;
     pa_mcalign *mcalign;
-    size_t requested;
+    int64_t missing, requested;
     char *name;
     pa_sample_spec sample_spec;
 };
@@ -251,12 +251,10 @@ static void write_index_changed(pa_memblockq *bq, int64_t old_write_index, bool
 
     delta = bq->write_index - old_write_index;
 
-    if (account) {
-        if (delta > (int64_t)bq->requested)
-            bq->requested = 0;
-        else if (delta > 0)
-            bq->requested -= delta;
-    }
+    if (account)
+        bq->requested -= delta;
+    else
+        bq->missing -= delta;
 
 #ifdef MEMBLOCKQ_DEBUG
      pa_log_debug("[%s] pushed/seeked %lli: requested counter at %lli, account=%i", bq->name, (long long) delta, (long long) bq->requested, account);
@@ -264,14 +262,15 @@ static void write_index_changed(pa_memblockq *bq, int64_t old_write_index, bool
 }
 
 static void read_index_changed(pa_memblockq *bq, int64_t old_read_index) {
-#ifdef MEMBLOCKQ_DEBUG
     int64_t delta;
 
     pa_assert(bq);
 
     delta = bq->read_index - old_read_index;
+    bq->missing += delta;
 
-    pa_log_debug("[%s] popped %lli", bq->name, (long long) delta);
+#ifdef MEMBLOCKQ_DEBUG
+    pa_log_debug("[%s] popped %lli: missing counter at %lli", bq->name, (long long) delta, (long long) bq->missing);
 #endif
 }
 
@@ -830,37 +829,31 @@ size_t pa_memblockq_get_prebuf(pa_memblockq *bq) {
 }
 
 size_t pa_memblockq_pop_missing(pa_memblockq *bq) {
-    int64_t length;
-    size_t missing;
+    size_t l;
 
     pa_assert(bq);
 
-    /* Note that write_index might be before read_index, which means
-     * that we should ask for extra data to catch up. This also means
-     * that we cannot call pa_memblockq_length() as it doesn't return
-     * negative values. */
-
-    length = (bq->write_index - bq->read_index) + bq->requested;
-
-    if (length > (int64_t)bq->tlength)
-        return 0;
-
-    missing = (int64_t)bq->tlength - length;
+#ifdef MEMBLOCKQ_DEBUG
+    pa_log_debug("[%s] pop: %lli", bq->name, (long long) bq->missing);
+#endif
 
-    if (missing == 0)
+    if (bq->missing <= 0)
         return 0;
 
-    if ((missing < bq->minreq) &&
+    if (((size_t) bq->missing < bq->minreq) &&
         !pa_memblockq_prebuf_active(bq))
         return 0;
 
-    bq->requested += missing;
+    l = (size_t) bq->missing;
+
+    bq->requested += bq->missing;
+    bq->missing = 0;
 
 #ifdef MEMBLOCKQ_DEBUG
-    pa_log_debug("[%s] sent %lli: request counter is at %lli", bq->name, (long long) missing, (long long) bq->requested);
+    pa_log_debug("[%s] sent %lli: request counter is at %lli", bq->name, (long long) l, (long long) bq->requested);
 #endif
 
-    return missing;
+    return l;
 }
 
 void pa_memblockq_set_maxlength(pa_memblockq *bq, size_t maxlength) {
@@ -876,11 +869,13 @@ void pa_memblockq_set_maxlength(pa_memblockq *bq, size_t maxlength) {
 }
 
 void pa_memblockq_set_tlength(pa_memblockq *bq, size_t tlength) {
+    size_t old_tlength;
     pa_assert(bq);
 
     if (tlength <= 0 || tlength == (size_t) -1)
         tlength = bq->maxlength;
 
+    old_tlength = bq->tlength;
     bq->tlength = ((tlength+bq->base-1)/bq->base)*bq->base;
 
     if (bq->tlength > bq->maxlength)
@@ -891,6 +886,8 @@ void pa_memblockq_set_tlength(pa_memblockq *bq, size_t tlength) {
 
     if (bq->prebuf > bq->tlength+bq->base-bq->minreq)
         pa_memblockq_set_prebuf(bq, bq->tlength+bq->base-bq->minreq);
+
+    bq->missing += (int64_t) bq->tlength - (int64_t) old_tlength;
 }
 
 void pa_memblockq_set_minreq(pa_memblockq *bq, size_t minreq) {
-- 
2.10.2