[pulseaudio-discuss] [RFC PATCH] alsa-sink: Reduce hardware pointer update syscalls

David Henningsson david.henningsson at canonical.com
Thu Sep 18 00:13:20 PDT 2014


Calling snd_pcm_avail/delay causes a syscall to the kernel, which
communicates with the audio hardware, and can therefore be expensive
on some cards.

By only updating this value after a sleep and after unusual events,
we can reduce calls to update the hardware pointer.
In particular, if a write goes well, we will now assume that the
buffer has been filled up, instead of re-asking the hardware that
this is actually the case.

Signed-off-by: David Henningsson <david.henningsson at canonical.com>
---
 src/modules/alsa/alsa-sink.c | 92 +++++++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 30 deletions(-)

Hi,

This patch is not really finished, but I'd like you to test it and see
if you have any performance improvements, before I go ahead and try to make
it more robust.
When I tested it, I didn't see much performance gain, at least not consistently.
But maybe this is because the sound cards I tested with have quick hw pointer updates.

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 9e9b863..5330c78 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -155,6 +155,11 @@ struct userdata {
     pa_reserve_monitor_wrapper *monitor;
     pa_hook_slot *monitor_slot;
 
+    snd_pcm_status_t *pcm_status;
+    snd_pcm_sframes_t pcm_status_delay;
+    snd_pcm_sframes_t pcm_status_avail;
+    bool pcm_status_valid;
+
     /* ucm context */
     pa_alsa_ucm_mapping_context *ucm_context;
 };
@@ -440,6 +445,7 @@ static int try_recover(struct userdata *u, const char *call, int err) {
     if (err == -ESTRPIPE)
         pa_log_debug("%s: System suspended!", call);
 
+    u->pcm_status_valid = false;
     if ((err = snd_pcm_recover(u->pcm_handle, err, 1)) < 0) {
         pa_log("%s: %s", call, pa_alsa_strerror(err));
         return -1;
@@ -450,6 +456,29 @@ static int try_recover(struct userdata *u, const char *call, int err) {
     return 0;
 }
 
+static bool ensure_pcm_status(struct userdata *u) {
+    int err;
+
+    if (u->pcm_status_valid)
+        return true;
+#ifdef DEBUG_TIMING
+    pa_log_debug("Calling snd_pcm_status (hwsync)");
+#endif
+    if (PA_UNLIKELY((err = pa_alsa_safe_delay(u->pcm_handle, u->pcm_status, &u->pcm_status_delay, u->hwbuf_size, &u->sink->sample_spec, false)) < 0)) {
+        pa_log_warn("Failed to retrieve current pcm status: %s", pa_alsa_strerror(err));
+        if (try_recover(u, "snd_pcm_status", err) < 0)
+            return false;
+        if ((err = pa_alsa_safe_delay(u->pcm_handle, u->pcm_status, &u->pcm_status_delay, u->hwbuf_size, &u->sink->sample_spec, false)) < 0) {
+            pa_log_warn("Failed to re-retrieve current pcm status: %s", pa_alsa_strerror(err));
+            return false;
+        }
+    }
+
+    u->pcm_status_avail = snd_pcm_status_get_avail(u->pcm_status); /* FIXME: This should be checked for weird values, just like delay */
+    u->pcm_status_valid = true;
+    return true;
+}
+
 static size_t check_left_to_play(struct userdata *u, size_t n_bytes, bool on_timeout) {
     size_t left_to_play;
     bool underrun = false;
@@ -521,23 +550,16 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
         hw_sleep_time(u, &max_sleep_usec, &process_usec);
 
     for (;;) {
-        snd_pcm_sframes_t n;
         size_t n_bytes;
         int r;
         bool after_avail = true;
 
         /* First we determine how many samples are missing to fill the
          * buffer up to 100% */
+        if (PA_UNLIKELY(!ensure_pcm_status(u)))
+            return -1;
 
-        if (PA_UNLIKELY((n = pa_alsa_safe_avail(u->pcm_handle, u->hwbuf_size, &u->sink->sample_spec)) < 0)) {
-
-            if ((r = try_recover(u, "snd_pcm_avail", (int) n)) == 0)
-                continue;
-
-            return r;
-        }
-
-        n_bytes = (size_t) n * u->frame_size;
+        n_bytes = (size_t) u->pcm_status_avail * u->frame_size;
 
 #ifdef DEBUG_TIMING
         pa_log_debug("avail: %lu", (unsigned long) n_bytes);
@@ -609,6 +631,7 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
 
             if (PA_UNLIKELY((err = pa_alsa_safe_mmap_begin(u->pcm_handle, &areas, &offset, &frames, u->hwbuf_size, &u->sink->sample_spec)) < 0)) {
 
+                u->pcm_status_valid = false;
                 if (!after_avail && err == -EAGAIN)
                     break;
 
@@ -647,6 +670,7 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
             pa_memblock_unref_fixed(chunk.memblock);
 
             if (PA_UNLIKELY((sframes = snd_pcm_mmap_commit(u->pcm_handle, offset, frames)) < 0)) {
+                u->pcm_status_valid = false;
 
                 if (!after_avail && (int) sframes == -EAGAIN)
                     break;
@@ -661,6 +685,8 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
 
             u->write_count += written;
             u->since_start += written;
+            u->pcm_status_avail -= sframes;
+            u->pcm_status_delay += sframes;
 
 #ifdef DEBUG_TIMING
             pa_log_debug("Wrote %lu bytes (of possible %lu bytes)", (unsigned long) written, (unsigned long) n_bytes);
@@ -706,20 +732,14 @@ static int unix_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
         hw_sleep_time(u, &max_sleep_usec, &process_usec);
 
     for (;;) {
-        snd_pcm_sframes_t n;
         size_t n_bytes;
         int r;
         bool after_avail = true;
 
-        if (PA_UNLIKELY((n = pa_alsa_safe_avail(u->pcm_handle, u->hwbuf_size, &u->sink->sample_spec)) < 0)) {
+        if (PA_UNLIKELY(!ensure_pcm_status(u)))
+            return -1;
 
-            if ((r = try_recover(u, "snd_pcm_avail", (int) n)) == 0)
-                continue;
-
-            return r;
-        }
-
-        n_bytes = (size_t) n * u->frame_size;
+        n_bytes = (size_t) u->pcm_status_avail * u->frame_size;
 
 #ifdef DEBUG_TIMING
         pa_log_debug("avail: %lu", (unsigned long) n_bytes);
@@ -789,6 +809,7 @@ static int unix_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
 
             if (PA_UNLIKELY(frames < 0)) {
 
+                u->pcm_status_valid = false;
                 if (!after_avail && (int) frames == -EAGAIN)
                     break;
 
@@ -815,6 +836,8 @@ static int unix_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
 
             work_done = true;
 
+            u->pcm_status_avail -= frames;
+            u->pcm_status_delay += frames;
             u->write_count += written;
             u->since_start += written;
 
@@ -848,26 +871,19 @@ static int unix_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
 }
 
 static void update_smoother(struct userdata *u) {
-    snd_pcm_sframes_t delay = 0;
     int64_t position;
-    int err;
     pa_usec_t now1 = 0, now2;
-    snd_pcm_status_t *status;
     snd_htimestamp_t htstamp = { 0, 0 };
 
-    snd_pcm_status_alloca(&status);
-
     pa_assert(u);
     pa_assert(u->pcm_handle);
-
     /* Let's update the time smoother */
-
-    if (PA_UNLIKELY((err = pa_alsa_safe_delay(u->pcm_handle, status, &delay, u->hwbuf_size, &u->sink->sample_spec, false)) < 0)) {
-        pa_log_warn("Failed to query DSP status data: %s", pa_alsa_strerror(err));
+    if (!ensure_pcm_status(u)) {
+        pa_log_warn("Failed to query DSP status data");
         return;
     }
 
-    snd_pcm_status_get_htstamp(status, &htstamp);
+    snd_pcm_status_get_htstamp(u->pcm_status, &htstamp);
     now1 = pa_timespec_load(&htstamp);
 
     /* Hmm, if the timestamp is 0, then it wasn't set and we take the current time */
@@ -879,7 +895,7 @@ static void update_smoother(struct userdata *u) {
         if (u->last_smoother_update + u->smoother_interval > now1)
             return;
 
-    position = (int64_t) u->write_count - ((int64_t) delay * (int64_t) u->frame_size);
+    position = (int64_t) u->write_count - ((int64_t) u->pcm_status_delay * (int64_t) u->frame_size);
 
     if (PA_UNLIKELY(position < 0))
         position = 0;
@@ -935,6 +951,7 @@ static int suspend(struct userdata *u) {
 
     /* Let's suspend -- we don't call snd_pcm_drain() here since that might
      * take awfully long with our long buffer sizes today. */
+    u->pcm_status_valid = false;
     snd_pcm_close(u->pcm_handle);
     u->pcm_handle = NULL;
 
@@ -1002,6 +1019,7 @@ static int update_sw_params(struct userdata *u) {
 
     pa_log_debug("setting avail_min=%lu", (unsigned long) avail_min);
 
+    u->pcm_status_valid = false;
     if ((err = pa_alsa_set_sw_params(u->pcm_handle, avail_min, !u->use_tsched)) < 0) {
         pa_log("Failed to set software parameters: %s", pa_alsa_strerror(err));
         return err;
@@ -1073,6 +1091,7 @@ static int unsuspend(struct userdata *u) {
         pa_snprintf(device_name, len, "%s,AES0=6", u->device_name);
     }
 
+    u->pcm_status_valid = false;
     if ((err = snd_pcm_open(&u->pcm_handle, device_name ? device_name : u->device_name, SND_PCM_STREAM_PLAYBACK,
                             SND_PCM_NONBLOCK|
                             SND_PCM_NO_AUTO_RESAMPLE|
@@ -1136,6 +1155,7 @@ static int unsuspend(struct userdata *u) {
 
 fail:
     if (u->pcm_handle) {
+        u->pcm_status_valid = false;
         snd_pcm_close(u->pcm_handle);
         u->pcm_handle = NULL;
     }
@@ -1653,6 +1673,7 @@ static int process_rewind(struct userdata *u) {
 
         in_frames = (snd_pcm_sframes_t) (rewind_nbytes / u->frame_size);
         pa_log_debug("before: %lu", (unsigned long) in_frames);
+        u->pcm_status_valid = false;
         if ((out_frames = snd_pcm_rewind(u->pcm_handle, (snd_pcm_uframes_t) in_frames)) < 0) {
             pa_log("snd_pcm_rewind() failed: %s", pa_alsa_strerror((int) out_frames));
             if (try_recover(u, "process_rewind", out_frames) < 0)
@@ -1727,6 +1748,7 @@ static void thread_func(void *userdata) {
 
                 if (u->first) {
                     pa_log_info("Starting playback.");
+                    u->pcm_status_valid = false;
                     snd_pcm_start(u->pcm_handle);
 
                     pa_smoother_resume(u->smoother, pa_rtclock_now(), true);
@@ -1811,6 +1833,7 @@ static void thread_func(void *userdata) {
                     (double) (real_sleep - rtpoll_sleep) / PA_USEC_PER_MSEC,
                     (double) (u->tsched_watermark_usec) / PA_USEC_PER_MSEC);
         }
+        u->pcm_status_valid = false;
 
         if (u->sink->flags & PA_SINK_DEFERRED_VOLUME)
             pa_sink_volume_change_apply(u->sink, NULL);
@@ -1827,11 +1850,14 @@ static void thread_func(void *userdata) {
             pollfd = pa_rtpoll_item_get_pollfd(u->alsa_rtpoll_item, &n);
 
             if ((err = snd_pcm_poll_descriptors_revents(u->pcm_handle, pollfd, n, &revents)) < 0) {
+                u->pcm_status_valid = false;
                 pa_log("snd_pcm_poll_descriptors_revents() failed: %s", pa_alsa_strerror(err));
                 goto fail;
             }
 
             if (revents & ~POLLOUT) {
+                u->pcm_status_valid = false;
+
                 if (pa_alsa_recover_from_poll(u->pcm_handle, revents) < 0)
                     goto fail;
 
@@ -2141,6 +2167,9 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca
     if (reserve_monitor_init(u, dev_id) < 0)
         goto fail;
 
+    if (snd_pcm_status_malloc(&u->pcm_status) < 0)
+        goto fail;
+
     b = use_mmap;
     d = use_tsched;
 
@@ -2465,6 +2494,9 @@ static void userdata_free(struct userdata *u) {
         snd_pcm_close(u->pcm_handle);
     }
 
+    if (u->pcm_status)
+        snd_pcm_status_free(u->pcm_status);
+
     if (u->mixer_fdl)
         pa_alsa_fdlist_free(u->mixer_fdl);
 
-- 
1.9.1



More information about the pulseaudio-discuss mailing list