[pulseaudio-commits] 3 commits - src/modules src/pulsecore

Tanu Kaskinen tanuk at kemper.freedesktop.org
Tue Aug 27 05:36:34 PDT 2013


 src/modules/alsa/alsa-sink.c   |    8 ++--
 src/modules/alsa/alsa-source.c |    8 ++--
 src/pulsecore/sink-input.c     |    4 +-
 src/pulsecore/sink.c           |   24 +++++++-------
 src/pulsecore/sink.h           |    4 +-
 src/pulsecore/source-output.c  |    4 +-
 src/pulsecore/source.c         |   68 +++++++++++++++++++++++++++++++----------
 src/pulsecore/source.h         |    4 +-
 8 files changed, 81 insertions(+), 43 deletions(-)

New commits:
commit 441a5a422cd2ea62aa43e62bbb81369d635e61f9
Author: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
Date:   Fri Aug 23 13:58:55 2013 +0300

    sink, source: Fix error reporting style for rate updates

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index f910d19..6e60646 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -1593,7 +1593,7 @@ static bool sink_set_formats(pa_sink *s, pa_idxset *formats) {
     return true;
 }
 
-static bool sink_update_rate_cb(pa_sink *s, uint32_t rate) {
+static int sink_update_rate_cb(pa_sink *s, uint32_t rate) {
     struct userdata *u = s->userdata;
     int i;
     bool supported = false;
@@ -1609,16 +1609,16 @@ static bool sink_update_rate_cb(pa_sink *s, uint32_t rate) {
 
     if (!supported) {
         pa_log_info("Sink does not support sample rate of %d Hz", rate);
-        return false;
+        return -1;
     }
 
     if (!PA_SINK_IS_OPENED(s->state)) {
         pa_log_info("Updating rate for device %s, new rate is %d",u->device_name, rate);
         u->sink->sample_spec.rate = rate;
-        return true;
+        return 0;
     }
 
-    return false;
+    return -1;
 }
 
 static int process_rewind(struct userdata *u) {
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index faf8965..78125b1 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -1401,7 +1401,7 @@ static void source_update_requested_latency_cb(pa_source *s) {
     update_sw_params(u);
 }
 
-static bool source_update_rate_cb(pa_source *s, uint32_t rate) {
+static int source_update_rate_cb(pa_source *s, uint32_t rate) {
     struct userdata *u = s->userdata;
     int i;
     bool supported = false;
@@ -1417,16 +1417,16 @@ static bool source_update_rate_cb(pa_source *s, uint32_t rate) {
 
     if (!supported) {
         pa_log_info("Source does not support sample rate of %d Hz", rate);
-        return false;
+        return -1;
     }
 
     if (!PA_SOURCE_IS_OPENED(s->state)) {
         pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, rate);
         u->source->sample_spec.rate = rate;
-        return true;
+        return 0;
     }
 
-    return false;
+    return -1;
 }
 
 static void thread_func(void *userdata) {
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index a275715..b5e8a0b 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -426,7 +426,7 @@ int pa_sink_input_new(
            module-suspend-on-idle can resume a sink */
 
         pa_log_info("Trying to change sample rate");
-        if (pa_sink_update_rate(data->sink, data->sample_spec.rate, pa_sink_input_new_data_is_passthrough(data)) == true)
+        if (pa_sink_update_rate(data->sink, data->sample_spec.rate, pa_sink_input_new_data_is_passthrough(data)) >= 0)
             pa_log_info("Rate changed to %u Hz", data->sink->sample_spec.rate);
     }
 
@@ -1829,7 +1829,7 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
            SINK_INPUT_MOVE_FINISH hook */
 
         pa_log_info("Trying to change sample rate");
-        if (pa_sink_update_rate(dest, i->sample_spec.rate, pa_sink_input_is_passthrough(i)) == true)
+        if (pa_sink_update_rate(dest, i->sample_spec.rate, pa_sink_input_is_passthrough(i)) >= 0)
             pa_log_info("Rate changed to %u Hz", dest->sample_spec.rate);
     }
 
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index bc8a3fd..de8c82e 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -1377,8 +1377,8 @@ void pa_sink_render_full(pa_sink *s, size_t length, pa_memchunk *result) {
 }
 
 /* Called from main thread */
-bool pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
-    bool ret = false;
+int pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
+    int ret = -1;
     uint32_t desired_rate = rate;
     uint32_t default_rate = s->default_sample_rate;
     uint32_t alternate_rate = s->alternate_sample_rate;
@@ -1387,32 +1387,32 @@ bool pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
     bool use_alternate = false;
 
     if (rate == s->sample_spec.rate)
-        return true;
+        return 0;
 
     if (!s->update_rate)
-        return false;
+        return -1;
 
     if (PA_UNLIKELY(default_rate == alternate_rate && !passthrough)) {
         pa_log_debug("Default and alternate sample rates are the same.");
-        return false;
+        return -1;
     }
 
     if (PA_SINK_IS_RUNNING(s->state)) {
         pa_log_info("Cannot update rate, SINK_IS_RUNNING, will keep using %u Hz",
                     s->sample_spec.rate);
-        return false;
+        return -1;
     }
 
     if (s->monitor_source) {
         if (PA_SOURCE_IS_RUNNING(s->monitor_source->state) == true) {
             pa_log_info("Cannot update rate, monitor source is RUNNING");
-            return false;
+            return -1;
         }
     }
 
     if (PA_UNLIKELY (desired_rate < 8000 ||
                      desired_rate > PA_RATE_MAX))
-        return false;
+        return -1;
 
     if (!passthrough) {
         pa_assert((default_rate % 4000 == 0) || (default_rate % 11025 == 0));
@@ -1436,15 +1436,15 @@ bool pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
     }
 
     if (desired_rate == s->sample_spec.rate)
-        return false;
+        return -1;
 
     if (!passthrough && pa_sink_used_by(s) > 0)
-        return false;
+        return -1;
 
     pa_log_debug("Suspending sink %s due to changing the sample rate.", s->name);
     pa_sink_suspend(s, true, PA_SUSPEND_INTERNAL);
 
-    if (s->update_rate(s, desired_rate) == true) {
+    if (s->update_rate(s, desired_rate) >= 0) {
         /* update monitor source as well */
         if (s->monitor_source && !passthrough)
             pa_source_update_rate(s->monitor_source, desired_rate, false);
@@ -1455,7 +1455,7 @@ bool pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
                 pa_sink_input_update_rate(i);
         }
 
-        ret = true;
+        ret = 0;
     }
 
     pa_sink_suspend(s, false, PA_SUSPEND_INTERNAL);
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 54056a7..332e2b1 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -230,7 +230,7 @@ struct pa_sink {
 
     /* Called whenever the sampling frequency shall be changed. Called from
      * main thread. */
-    bool (*update_rate)(pa_sink *s, uint32_t rate);
+    int (*update_rate)(pa_sink *s, uint32_t rate);
 
     /* Contains copies of the above data so that the real-time worker
      * thread can work without access locking */
@@ -411,7 +411,7 @@ unsigned pa_device_init_priority(pa_proplist *p);
 
 /**** May be called by everyone, from main context */
 
-bool pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough);
+int pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough);
 void pa_sink_set_latency_offset(pa_sink *s, int64_t offset);
 
 /* The returned value is supposed to be in the time domain of the sound card! */
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 5a48935..b158f14 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -366,7 +366,7 @@ int pa_source_output_new(
            module-suspend-on-idle can resume a source */
 
         pa_log_info("Trying to change sample rate");
-        if (pa_source_update_rate(data->source, data->sample_spec.rate, pa_source_output_new_data_is_passthrough(data)) == true)
+        if (pa_source_update_rate(data->source, data->sample_spec.rate, pa_source_output_new_data_is_passthrough(data)) >= 0)
             pa_log_info("Rate changed to %u Hz", data->source->sample_spec.rate);
     }
 
@@ -1449,7 +1449,7 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save
            SOURCE_OUTPUT_MOVE_FINISH hook */
 
         pa_log_info("Trying to change sample rate");
-        if (pa_source_update_rate(dest, o->sample_spec.rate, pa_source_output_is_passthrough(o)) == true)
+        if (pa_source_update_rate(dest, o->sample_spec.rate, pa_source_output_is_passthrough(o)) >= 0)
             pa_log_info("Rate changed to %u Hz", dest->sample_spec.rate);
     }
 
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index 5e59a40..f695655 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -967,40 +967,40 @@ void pa_source_post_direct(pa_source*s, pa_source_output *o, const pa_memchunk *
 }
 
 /* Called from main thread */
-bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
-    bool ret = false;
+int pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
+    int ret;
     uint32_t desired_rate = rate;
     uint32_t default_rate = s->default_sample_rate;
     uint32_t alternate_rate = s->alternate_sample_rate;
     bool use_alternate = false;
 
     if (rate == s->sample_spec.rate)
-        return true;
+        return 0;
 
     if (!s->update_rate && !s->monitor_of)
-        return false;
+        return -1;
 
     if (PA_UNLIKELY(default_rate == alternate_rate && !passthrough)) {
         pa_log_debug("Default and alternate sample rates are the same.");
-        return false;
+        return -1;
     }
 
     if (PA_SOURCE_IS_RUNNING(s->state)) {
         pa_log_info("Cannot update rate, SOURCE_IS_RUNNING, will keep using %u Hz",
                     s->sample_spec.rate);
-        return false;
+        return -1;
     }
 
     if (s->monitor_of) {
         if (PA_SINK_IS_RUNNING(s->monitor_of->state)) {
             pa_log_info("Cannot update rate, this is a monitor source and the sink is running.");
-            return false;
+            return -1;
         }
     }
 
     if (PA_UNLIKELY (desired_rate < 8000 ||
                      desired_rate > PA_RATE_MAX))
-        return false;
+        return -1;
 
     if (!passthrough) {
         pa_assert((default_rate % 4000 == 0) || (default_rate % 11025 == 0));
@@ -1024,10 +1024,10 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
     }
 
     if (desired_rate == s->sample_spec.rate)
-        return false;
+        return -1;
 
     if (!passthrough && pa_source_used_by(s) > 0)
-        return false;
+        return -1;
 
     pa_log_debug("Suspending source %s due to changing the sample rate.", s->name);
     pa_source_suspend(s, true, PA_SUSPEND_INTERNAL);
@@ -1046,7 +1046,7 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
             s->sample_spec.rate = desired_rate;
             ret = pa_sink_update_rate(s->monitor_of, desired_rate, false);
 
-            if (!ret) {
+            if (ret < 0) {
                 /* Changing the sink rate failed, roll back the old rate for
                  * the monitor source. Why did we set the source rate before
                  * calling pa_sink_update_rate(), you may ask. The reason is
@@ -1060,10 +1060,10 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
                 s->sample_spec.rate = old_rate;
             }
         } else
-            ret = false;
+            ret = -1;
     }
 
-    if (ret) {
+    if (ret >= 0) {
         uint32_t idx;
         pa_source_output *o;
 
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index 6bdd472..b20d4c0 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -187,7 +187,7 @@ struct pa_source {
 
     /* Called whenever the sampling frequency shall be changed. Called from
      * main thread. */
-    bool (*update_rate)(pa_source *s, uint32_t rate);
+    int (*update_rate)(pa_source *s, uint32_t rate);
 
     /* Contains copies of the above data so that the real-time worker
      * thread can work without access locking */
@@ -382,7 +382,7 @@ bool pa_source_update_proplist(pa_source *s, pa_update_mode_t mode, pa_proplist
 int pa_source_set_port(pa_source *s, const char *name, bool save);
 void pa_source_set_mixer_dirty(pa_source *s, bool is_dirty);
 
-bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough);
+int pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough);
 
 unsigned pa_source_linked_by(pa_source *s); /* Number of connected streams */
 unsigned pa_source_used_by(pa_source *s); /* Number of connected streams that are not corked */

commit a32c5e435452f7e7d1f8f595eb6e494b7c0aaee7
Author: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
Date:   Fri Aug 9 09:39:49 2013 +0300

    source: When updating a monitor source's rate, update the sink rate too
    
    If the sink rate is not updated, then the monitor source will appear
    to have a different rate than the sink, but in reality there's never
    any resampling done when moving data from the sink to the monitor
    source, so it's a lie that the monitor source has a different rate.
    The result of lying is that clients that capture from the monitor
    source will have streams that run too fast or slow.

diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index c692511..5e59a40 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -991,6 +991,13 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
         return false;
     }
 
+    if (s->monitor_of) {
+        if (PA_SINK_IS_RUNNING(s->monitor_of->state)) {
+            pa_log_info("Cannot update rate, this is a monitor source and the sink is running.");
+            return false;
+        }
+    }
+
     if (PA_UNLIKELY (desired_rate < 8000 ||
                      desired_rate > PA_RATE_MAX))
         return false;
@@ -1029,8 +1036,31 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
         ret = s->update_rate(s, desired_rate);
     else {
         /* This is a monitor source. */
-        s->sample_spec.rate = desired_rate;
-        ret = true;
+
+        /* XXX: This code is written with non-passthrough streams in mind. I
+         * have no idea whether the behaviour with passthrough streams is
+         * sensible. */
+        if (!passthrough) {
+            uint32_t old_rate = s->sample_spec.rate;
+
+            s->sample_spec.rate = desired_rate;
+            ret = pa_sink_update_rate(s->monitor_of, desired_rate, false);
+
+            if (!ret) {
+                /* Changing the sink rate failed, roll back the old rate for
+                 * the monitor source. Why did we set the source rate before
+                 * calling pa_sink_update_rate(), you may ask. The reason is
+                 * that pa_sink_update_rate() tries to update the monitor
+                 * source rate, but we are already in the process of updating
+                 * the monitor source rate, so there's a risk of entering an
+                 * infinite loop. Setting the source rate before calling
+                 * pa_sink_update_rate() makes the rate == s->sample_spec.rate
+                 * check in the beginning of this function return early, so we
+                 * avoid looping. */
+                s->sample_spec.rate = old_rate;
+            }
+        } else
+            ret = false;
     }
 
     if (ret) {

commit 2c14306507ce5049fb2bc883acea4f0d8c879876
Author: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
Date:   Fri Aug 9 07:45:26 2013 +0300

    source: Fix monitor source rate changing
    
    When a sink changes its sample rate, also the monitor source rate
    needs to be changed. In order to determine whether a source supports
    rate changing, the code checks if the update_rate() callback is set,
    but monitor sources don't have that callback set, so the old code
    always failed to change the monitor source rate.
    
    This patch fixes the monitor source rate changing by handling monitor
    sources as a special case in pa_source_update_rate(): if the source is
    a monitor source, then the update_rate() callback is not required.
    
    BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=66424

diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index 5cb6b73..c692511 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -972,14 +972,12 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
     uint32_t desired_rate = rate;
     uint32_t default_rate = s->default_sample_rate;
     uint32_t alternate_rate = s->alternate_sample_rate;
-    uint32_t idx;
-    pa_source_output *o;
     bool use_alternate = false;
 
     if (rate == s->sample_spec.rate)
         return true;
 
-    if (!s->update_rate)
+    if (!s->update_rate && !s->monitor_of)
         return false;
 
     if (PA_UNLIKELY(default_rate == alternate_rate && !passthrough)) {
@@ -1027,14 +1025,24 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
     pa_log_debug("Suspending source %s due to changing the sample rate.", s->name);
     pa_source_suspend(s, true, PA_SUSPEND_INTERNAL);
 
-    if (s->update_rate(s, desired_rate) == true) {
-        pa_log_info("Changed sampling rate successfully ");
+    if (s->update_rate)
+        ret = s->update_rate(s, desired_rate);
+    else {
+        /* This is a monitor source. */
+        s->sample_spec.rate = desired_rate;
+        ret = true;
+    }
+
+    if (ret) {
+        uint32_t idx;
+        pa_source_output *o;
 
         PA_IDXSET_FOREACH(o, s->outputs, idx) {
             if (o->state == PA_SOURCE_OUTPUT_CORKED)
                 pa_source_output_update_rate(o);
         }
-        ret = true;
+
+        pa_log_info("Changed sampling rate successfully");
     }
 
     pa_source_suspend(s, false, PA_SUSPEND_INTERNAL);



More information about the pulseaudio-commits mailing list