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
