Hi Pierre,

sorry for the long delay.

This seems to make a lot of sense, but a question - does this impose a requirement on a certain version of alsa-lib and/or the linux kernel?

// David


2012-07-30 02:46, Pierre-Louis Bossart skrev:
Refactor code to fetch avail, delay and timestamp values
in a single call to snd_pcm_status().
The information reported is exactly the same as before,
however it is extracted in a more atomic manner to
improve timer-based scheduling.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
  src/modules/alsa/alsa-sink.c   |   12 ++++--------
  src/modules/alsa/alsa-source.c |   12 ++++--------
  src/modules/alsa/alsa-util.c   |   17 ++++++++++++-----
  src/modules/alsa/alsa-util.h   |    2 +-
  4 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 544399e..e4baa1f 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -825,6 +825,7 @@ static void update_smoother(struct userdata *u) {
      int err;
      pa_usec_t now1 = 0, now2;
      snd_pcm_status_t *status;
+    snd_htimestamp_t htstamp = { 0, 0 };
snd_pcm_status_alloca(&status); @@ -833,18 +834,13 @@ static void update_smoother(struct userdata *u) { /* Let's update the time smoother */ - if (PA_UNLIKELY((err = pa_alsa_safe_delay(u->pcm_handle, &delay, u->hwbuf_size, &u->sink->sample_spec, FALSE)) < 0)) {
+    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));
          return;
      }
- if (PA_UNLIKELY((err = snd_pcm_status(u->pcm_handle, status)) < 0))
-        pa_log_warn("Failed to get timestamp: %s", pa_alsa_strerror(err));
-    else {
-        snd_htimestamp_t htstamp = { 0, 0 };
-        snd_pcm_status_get_htstamp(status, &htstamp);
-        now1 = pa_timespec_load(&htstamp);
-    }
+    snd_pcm_status_get_htstamp(status, &htstamp);
+    now1 = pa_timespec_load(&htstamp);
/* Hmm, if the timestamp is 0, then it wasn't set and we take the current time */
      if (now1 <= 0)
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index 94d4a09..17566e7 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -767,6 +767,7 @@ static void update_smoother(struct userdata *u) {
      int err;
      pa_usec_t now1 = 0, now2;
      snd_pcm_status_t *status;
+    snd_htimestamp_t htstamp = { 0, 0 };
snd_pcm_status_alloca(&status); @@ -775,18 +776,13 @@ static void update_smoother(struct userdata *u) { /* Let's update the time smoother */ - if (PA_UNLIKELY((err = pa_alsa_safe_delay(u->pcm_handle, &delay, u->hwbuf_size, &u->source->sample_spec, TRUE)) < 0)) {
+    if (PA_UNLIKELY((err = pa_alsa_safe_delay(u->pcm_handle, status, &delay, u->hwbuf_size, 
&u->source->sample_spec, TRUE)) < 0)) {
          pa_log_warn("Failed to get delay: %s", pa_alsa_strerror(err));
          return;
      }
- if (PA_UNLIKELY((err = snd_pcm_status(u->pcm_handle, status)) < 0))
-        pa_log_warn("Failed to get timestamp: %s", pa_alsa_strerror(err));
-    else {
-        snd_htimestamp_t htstamp = { 0, 0 };
-        snd_pcm_status_get_htstamp(status, &htstamp);
-        now1 = pa_timespec_load(&htstamp);
-    }
+    snd_pcm_status_get_htstamp(status, &htstamp);
+    now1 = pa_timespec_load(&htstamp);
/* Hmm, if the timestamp is 0, then it wasn't set and we take the current time */
      if (now1 <= 0)
diff --git a/src/modules/alsa/alsa-util.c b/src/modules/alsa/alsa-util.c
index bb4e307..4a29a9a 100644
--- a/src/modules/alsa/alsa-util.c
+++ b/src/modules/alsa/alsa-util.c
@@ -1141,10 +1141,11 @@ snd_pcm_sframes_t pa_alsa_safe_avail(snd_pcm_t *pcm, 
size_t hwbuf_size, const pa
      return n;
  }
-int pa_alsa_safe_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delay, size_t hwbuf_size, const pa_sample_spec *ss, pa_bool_t capture) {
+int pa_alsa_safe_delay(snd_pcm_t *pcm, snd_pcm_status_t *status, 
snd_pcm_sframes_t *delay, size_t hwbuf_size, const pa_sample_spec *ss,
+                       pa_bool_t capture) {
      ssize_t k;
      size_t abs_k;
-    int r;
+    int err;
      snd_pcm_sframes_t avail = 0;
pa_assert(pcm);
@@ -1154,10 +1155,16 @@ int pa_alsa_safe_delay(snd_pcm_t *pcm, 
snd_pcm_sframes_t *delay, size_t hwbuf_si
/* Some ALSA driver expose weird bugs, let's inform the user about
       * what is going on. We're going to get both the avail and delay values so
-     * that we can compare and check them for capture */
+     * that we can compare and check them for capture.
+     * This is done with snd_pcm_status() which provides
+     * avail, delay and timestamp values in a single kernel call to improve
+     * timer-based scheduling */
- if ((r = snd_pcm_avail_delay(pcm, &avail, delay)) < 0)
-        return r;
+    if ((err = snd_pcm_status(pcm, status)) < 0)
+        return err;
+
+    avail = snd_pcm_status_get_avail(status);
+    *delay = snd_pcm_status_get_delay(status);
k = (ssize_t) *delay * (ssize_t) pa_frame_size(ss); diff --git a/src/modules/alsa/alsa-util.h b/src/modules/alsa/alsa-util.h
index a4beed2..236a329 100644
--- a/src/modules/alsa/alsa-util.h
+++ b/src/modules/alsa/alsa-util.h
@@ -125,7 +125,7 @@ int pa_alsa_recover_from_poll(snd_pcm_t *pcm, int revents);
  pa_rtpoll_item* pa_alsa_build_pollfd(snd_pcm_t *pcm, pa_rtpoll *rtpoll);
snd_pcm_sframes_t pa_alsa_safe_avail(snd_pcm_t *pcm, size_t hwbuf_size, const pa_sample_spec *ss);
-int pa_alsa_safe_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delay, size_t 
hwbuf_size, const pa_sample_spec *ss, pa_bool_t capture);
+int pa_alsa_safe_delay(snd_pcm_t *pcm, snd_pcm_status_t *status, 
snd_pcm_sframes_t *delay, size_t hwbuf_size, const pa_sample_spec *ss, 
pa_bool_t capture);
  int pa_alsa_safe_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t 
**areas, snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames, size_t 
hwbuf_size, const pa_sample_spec *ss);
char *pa_alsa_get_driver_name(int card);


--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to