[pulseaudio-discuss] [PATCH] core: Drop empty gaps in the memblockq when playing data from it.

[Antti-Ville Jansson]

It's possible that the memblockq of a sink input is rewound to a negative read
index if the sink input is moved between sinks shortly after its creation. When
this happens, pa_memblockq_peek() returns a memchunk whose 'memblock' field is
NULL and whose 'length' field indicates the length of the gap caused by the
negative read index. This will trigger an assert in play-memblockq.c.

If the memblockq had a silence memchunk, pa_memblockq_peek() would return
silence for the duration of the gap and the assert would be avoided. However,
this approach would prevent the sink input from being drained and is thus not
possible. Instead, we handle the aforementioned situation by dropping the gap
indicated by the 'length' field of the memchunk and by peeking the actual data
that comes after the gap.

This scenario seems to be quite rare in everyday use, but it causes a severe
bug in the handheld world. The assert can be triggered e.g. by loading two null
sinks, playing a sample from the cache to one of them and then moving the
created sink input between the two sinks. The rewinds done by the null sinks
seem to be quite long (I don't know if this is normal behaviour or something
fishy in module-null-sink).

See also:

    6bd34156b130c07b130de10111a12ef6dab18b52
    virtual-sink: Fix a crash when moving the sink to a new master right after setup.

    https://tango.0pointer.de/pipermail/pulseaudio-discuss/2011-February/009105.html
---
 src/pulsecore/play-memblockq.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/pulsecore/play-memblockq.c b/src/pulsecore/play-memblockq.c
index f075a5b..66e47ea 100644
--- a/src/pulsecore/play-memblockq.c
+++ b/src/pulsecore/play-memblockq.c
@@ -135,11 +135,14 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk
         return -1;
     }
 
-    /* FIXME: u->memblockq doesn't have a silence memchunk set, so
-     * pa_memblockq_peek() will return 0 without returning any memblock if the
-     * read index points to a hole. If the memblockq is rewound beyond index 0,
-     * then there will be a hole. */
-    pa_assert(chunk->memblock);
+    /* If there's no memblock, there's going to be data in the memblockq after
+     * a gap with length chunk->length. Drop the the gap and peek the actual
+     * data. There should always be some data coming - hence the assert. The
+     * gap will occur if the memblockq is rewound beyond index 0.*/
+    if (!chunk->memblock) {
+        pa_memblockq_drop(u->memblockq, chunk->length);
+        pa_assert_se(pa_memblockq_peek(u->memblockq, chunk) >= 0);
+    }
 
     chunk->length = PA_MIN(chunk->length, nbytes);
     pa_memblockq_drop(u->memblockq, chunk->length);
-- 
1.7.0.4