[pulseaudio-discuss] [RFC PATCH] First draft implementation of better drain reporting

David Henningsson david.henningsson at canonical.com
Fri Mar 1 07:05:44 PST 2013


First of all, this is an unfinished patch. At this point, it doesn't add any
new "device underrun" events, only responds to drains more timely. It's quite
tricky to get right (or I'm just dumb :-D ).

In short, the alsa-sink object first starts to keep track of how much has been
played back (since last fill up). The sink is informed about this value, which it
uses for two purposes:
 1) If a stream is currently underrunning, it returns how much underrun the stream
    actually is, so that alsa-sink can wake up at exactly this point in time.
 2) If all of a stream has actually been played back, it calls into the stream's
    new verify_underrun callback, and the stream's render data is emptied.

Things remaining are:
 - First, I'm not sure the calculations are entirely correct. I think I shouldn't need
   to add process_usec to the wakeup, but if I don't I wake up a few samples before the
   stream has finished playback.
 - This is implemented in mmap_write but should also be in unix_write
 - A new device underrun callback for clients (requires protocol extensions)
 - protocol-native's verify_underrun_cb contains copy-pasted code from pop_cb
 - remove debug prints and maybe some other cleanup.

Signed-off-by: David Henningsson <david.henningsson at canonical.com>
---
 src/modules/alsa/alsa-sink.c    |   41 +++++++++++++++++++++++++++++++--------
 src/pulsecore/protocol-native.c |   28 ++++++++++++++++++++++++++
 src/pulsecore/sink-input.c      |   33 +++++++++++++++++++++++++++++++
 src/pulsecore/sink-input.h      |    6 ++++++
 src/pulsecore/sink.c            |   25 ++++++++++++++++++++++++
 src/pulsecore/sink.h            |    2 ++
 6 files changed, 127 insertions(+), 8 deletions(-)

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 69d006e..59be22f 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -146,6 +146,8 @@ struct userdata {
     uint64_t since_start;
     pa_usec_t smoother_interval;
     pa_usec_t last_smoother_update;
+    int64_t since_fill;
+    int64_t expected_avail;
 
     pa_idxset *formats;
 
@@ -508,7 +510,7 @@ static size_t check_left_to_play(struct userdata *u, size_t n_bytes, pa_bool_t o
 static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, pa_bool_t polled, pa_bool_t on_timeout) {
     pa_bool_t work_done = FALSE;
     pa_usec_t max_sleep_usec = 0, process_usec = 0;
-    size_t left_to_play;
+    size_t left_to_play, input_underrun;
     unsigned j = 0;
 
     pa_assert(u);
@@ -535,11 +537,14 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, pa_bool_t polle
         }
 
         n_bytes = (size_t) n * u->frame_size;
+        u->since_fill += n_bytes - u->expected_avail;
 
 #ifdef DEBUG_TIMING
-        pa_log_debug("avail: %lu", (unsigned long) n_bytes);
+        pa_log_debug("avail: %lu, since fill: %ld, advance: %lu", (unsigned long) n_bytes,
+                     (long) u->since_fill, (unsigned long) n_bytes - u->expected_avail);
 #endif
 
+        u->expected_avail = n_bytes;
         left_to_play = check_left_to_play(u, n_bytes, on_timeout);
         on_timeout = FALSE;
 
@@ -600,6 +605,7 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, pa_bool_t polle
             const snd_pcm_channel_area_t *areas;
             snd_pcm_uframes_t offset, frames;
             snd_pcm_sframes_t sframes;
+            size_t written;
 
             frames = (snd_pcm_uframes_t) (n_bytes / u->frame_size);
 /*             pa_log_debug("%lu frames to write", (unsigned long) frames); */
@@ -635,12 +641,14 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, pa_bool_t polle
 
             p = (uint8_t*) areas[0].addr + (offset * u->frame_size);
 
-            chunk.memblock = pa_memblock_new_fixed(u->core->mempool, p, frames * u->frame_size, TRUE);
+            written = frames * u->frame_size;
+            chunk.memblock = pa_memblock_new_fixed(u->core->mempool, p, written, TRUE);
             chunk.length = pa_memblock_get_length(chunk.memblock);
             chunk.index = 0;
 
             pa_sink_render_into_full(u->sink, &chunk);
             pa_memblock_unref_fixed(chunk.memblock);
+            u->since_fill = 0;
 
             if (PA_UNLIKELY((sframes = snd_pcm_mmap_commit(u->pcm_handle, offset, frames)) < 0)) {
 
@@ -655,21 +663,29 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, pa_bool_t polle
 
             work_done = TRUE;
 
-            u->write_count += frames * u->frame_size;
-            u->since_start += frames * u->frame_size;
+            u->write_count += written;
+            u->since_start += written;
+            u->expected_avail -= written;
 
 #ifdef DEBUG_TIMING
-            pa_log_debug("Wrote %lu bytes (of possible %lu bytes)", (unsigned long) (frames * u->frame_size), (unsigned long) n_bytes);
+            pa_log_debug("Wrote %lu bytes (of possible %lu bytes)", (unsigned long) written, (unsigned long) n_bytes);
 #endif
 
-            if ((size_t) frames * u->frame_size >= n_bytes)
+            if (written >= n_bytes)
                 break;
 
-            n_bytes -= (size_t) frames * u->frame_size;
+            n_bytes -= written;
         }
     }
 
+    input_underrun = pa_sink_find_underrun(u->sink, u->since_fill, left_to_play);
+
     if (u->use_tsched) {
+        pa_usec_t underrun_sleep = 0;
+        if (input_underrun > 0)
+             /* Wakeup to notify that stream is drained */
+            underrun_sleep = pa_bytes_to_usec(left_to_play - input_underrun, &u->sink->sample_spec);
+
         *sleep_usec = pa_bytes_to_usec(left_to_play, &u->sink->sample_spec);
         process_usec = pa_bytes_to_usec(u->tsched_watermark, &u->sink->sample_spec);
 
@@ -677,6 +693,11 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, pa_bool_t polle
             *sleep_usec -= process_usec;
         else
             *sleep_usec = 0;
+        if (underrun_sleep > 0) {
+            underrun_sleep += process_usec; /* Add some margin. I haven't figured out why we need this :-( */
+            *sleep_usec = PA_MIN(*sleep_usec, underrun_sleep);
+        }
+
     } else
         *sleep_usec = 0;
 
@@ -1099,6 +1120,8 @@ static int unsuspend(struct userdata *u) {
     pa_smoother_reset(u->smoother, pa_rtclock_now(), TRUE);
     u->smoother_interval = SMOOTHER_MIN_INTERVAL;
     u->last_smoother_update = 0;
+    u->since_fill = 0;
+    u->expected_avail = u->hwbuf_size;
 
     u->first = TRUE;
     u->since_start = 0;
@@ -1663,6 +1686,8 @@ static int process_rewind(struct userdata *u) {
             pa_log_info("Tried rewind, but was apparently not possible.");
         else {
             u->write_count -= rewind_nbytes;
+            u->since_fill -= rewind_nbytes;
+            u->expected_avail += rewind_nbytes;
             pa_log_debug("Rewound %lu bytes.", (unsigned long) rewind_nbytes);
             pa_sink_process_rewind(u->sink, rewind_nbytes);
 
diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index a4ee920..0a39f2e 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -231,6 +231,7 @@ enum {
     CONNECTION_MESSAGE_REVOKE
 };
 
+static pa_bool_t sink_input_verify_underrun_cb(pa_sink_input *i);
 static int sink_input_pop_cb(pa_sink_input *i, size_t length, pa_memchunk *chunk);
 static void sink_input_kill_cb(pa_sink_input *i);
 static void sink_input_suspend_cb(pa_sink_input *i, pa_bool_t suspend);
@@ -1171,6 +1172,7 @@ static playback_stream* playback_stream_new(
 
     s->sink_input->parent.process_msg = sink_input_process_msg;
     s->sink_input->pop = sink_input_pop_cb;
+    s->sink_input->verify_underrun = sink_input_verify_underrun_cb;
     s->sink_input->process_rewind = sink_input_process_rewind_cb;
     s->sink_input->update_max_rewind = sink_input_update_max_rewind_cb;
     s->sink_input->update_max_request = sink_input_update_max_request_cb;
@@ -1574,6 +1576,32 @@ static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int
 }
 
 /* Called from thread context */
+static pa_bool_t sink_input_verify_underrun_cb(pa_sink_input *i) {
+    playback_stream *s;
+
+    pa_sink_input_assert_ref(i);
+    s = PLAYBACK_STREAM(i->userdata);
+    playback_stream_assert_ref(s);
+
+    if (pa_memblockq_is_readable(s->memblockq))
+        return FALSE;
+
+    pa_log_debug("%s of '%s'", s->drain_request ? "Finished playback" : "Real underrun", pa_strnull(pa_proplist_gets(i->proplist, PA_PROP_MEDIA_NAME)));
+
+   if (s->drain_request) {
+        s->drain_request = FALSE;
+        pa_log("IMPLEMENTOR: Sending PLAYBACK_STREAM_MESSAGE_DRAIN_ACK");
+        pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(s), PLAYBACK_STREAM_MESSAGE_DRAIN_ACK, PA_UINT_TO_PTR(s->drain_tag), 0, NULL, NULL);
+    } else if (!s->is_underrun)
+        pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(s), PLAYBACK_STREAM_MESSAGE_UNDERFLOW, NULL, pa_memblockq_get_read_index(s->memblockq), NULL, NULL);
+
+    s->is_underrun = TRUE;
+/*    playback_stream_request_bytes(s); */
+    return TRUE;
+}
+
+
+/* Called from thread context */
 static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk) {
     playback_stream *s;
 
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 519f47e..7968abd 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -826,6 +826,18 @@ pa_usec_t pa_sink_input_get_latency(pa_sink_input *i, pa_usec_t *sink_latency) {
     return r[0];
 }
 
+/* Returns length of underrun, in sink bytes. Called from thread context */
+uint64_t pa_sink_input_get_underrun_for_sink(pa_sink_input *i)
+{
+    uint64_t u = i->thread_info.underrun_for;
+    if (u == (uint64_t) -1)
+        return 0;
+    if (!i->thread_info.resampler)
+        return u;
+    return pa_resampler_result(i->thread_info.resampler, u);
+}
+
+
 /* Called from thread context */
 void pa_sink_input_peek(pa_sink_input *i, size_t slength /* in sink frames */, pa_memchunk *chunk, pa_cvolume *volume) {
     pa_bool_t do_volume_adj_here, need_volume_factor_sink;
@@ -1020,6 +1032,27 @@ void pa_sink_input_drop(pa_sink_input *i, size_t nbytes /* in sink sample spec *
 }
 
 /* Called from thread context */
+pa_bool_t pa_sink_input_verify_real_underrun(pa_sink_input *i, size_t nbytes /* in the sink's sample spec */) {
+    pa_sink_input_assert_ref(i);
+    pa_sink_input_assert_io_context(i);
+
+    if (nbytes < pa_memblockq_get_maxrewind(i->thread_info.render_memblockq))
+        return FALSE;
+    if (pa_memblockq_is_readable(i->thread_info.render_memblockq))
+        return FALSE;
+
+    if (!i->verify_underrun)
+        return FALSE;
+    if (i->verify_underrun(i)) {
+        /* All valid data has been played back, so we can empty this queue. */
+        pa_memblockq_silence(i->thread_info.render_memblockq);
+        return TRUE;
+    }
+    return FALSE;
+}
+
+
+/* Called from thread context */
 void pa_sink_input_process_rewind(pa_sink_input *i, size_t nbytes /* in sink sample spec */) {
     size_t lbq;
     pa_bool_t called = FALSE;
diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h
index 8bd5912..097a9fb 100644
--- a/src/pulsecore/sink-input.h
+++ b/src/pulsecore/sink-input.h
@@ -135,6 +135,9 @@ struct pa_sink_input {
      * the full block. */
     int (*pop) (pa_sink_input *i, size_t request_nbytes, pa_memchunk *chunk); /* may NOT be NULL */
 
+    /* TODO: Write something here */
+    pa_bool_t (*verify_underrun) (pa_sink_input *i);
+
     /* Rewind the queue by the specified number of bytes. Called just
      * before peek() if it is called at all. Only called if the sink
      * input driver ever plans to call
@@ -407,6 +410,9 @@ int pa_sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int64_t
 pa_usec_t pa_sink_input_set_requested_latency_within_thread(pa_sink_input *i, pa_usec_t usec);
 
 pa_bool_t pa_sink_input_safe_to_remove(pa_sink_input *i);
+uint64_t pa_sink_input_get_underrun_for_sink(pa_sink_input *i);
+pa_bool_t pa_sink_input_verify_real_underrun(pa_sink_input *i, size_t nbytes /* in the sink's sample spec */);
+
 
 pa_memchunk* pa_sink_input_get_silence(pa_sink_input *i, pa_memchunk *ret);
 
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 6ebe956..977b555 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -915,6 +915,31 @@ void pa_sink_move_all_fail(pa_queue *q) {
     pa_queue_free(q, NULL);
 }
 
+ /* Called from IO thread context */
+size_t pa_sink_find_underrun(pa_sink *s, size_t since_fill, size_t left_to_play) {
+    pa_sink_input *i;
+    void *state = NULL;
+    size_t result = 0;
+
+    pa_sink_assert_ref(s);
+    pa_sink_assert_io_context(s);
+/*    pa_log("PLAYBACK: %d bytes, left to play %d, total %d", (int) since_fill, (int) left_to_play, (int) (since_fill + left_to_play)); */
+    PA_HASHMAP_FOREACH(i, s->thread_info.inputs, state) {
+        size_t uf = pa_sink_input_get_underrun_for_sink(i);
+        if (uf == 0)
+            continue;
+        uf += since_fill;
+        if (pa_sink_input_verify_real_underrun(i, uf))
+            continue;
+        if (uf < left_to_play && uf > result)
+            result = uf;
+    }
+
+    if (result > 0)
+        pa_log_debug("Found underrun %d bytes ago", (int) result);
+    return result;
+}
+
 /* Called from IO thread context */
 void pa_sink_process_rewind(pa_sink *s, size_t nbytes) {
     pa_sink_input *i;
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index fcda5ef..6510405 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -492,6 +492,8 @@ void pa_sink_update_volume_and_mute(pa_sink *s);
 
 pa_bool_t pa_sink_volume_change_apply(pa_sink *s, pa_usec_t *usec_to_next);
 
+size_t pa_sink_find_underrun(pa_sink *s, size_t since_fill, size_t left_to_play);
+
 /*** To be called exclusively by sink input drivers, from IO context */
 
 void pa_sink_request_rewind(pa_sink*s, size_t nbytes);
-- 
1.7.9.5



More information about the pulseaudio-discuss mailing list