[pulseaudio-commits] Branch 'next' - 3 commits - src/modules

Tanu Kaskinen tanuk at kemper.freedesktop.org
Wed Feb 21 09:40:16 UTC 2018


 src/modules/alsa/alsa-sink.c        |   12 +++-----
 src/modules/alsa/alsa-source.c      |   12 +++-----
 src/modules/alsa/module-alsa-card.c |   49 +++++++++++++++++++++++++++---------
 src/modules/module-solaris.c        |   18 +++++--------
 src/modules/oss/module-oss.c        |   16 +++--------
 5 files changed, 59 insertions(+), 48 deletions(-)

New commits:
commit e984328ef7927300f24a4b00402ef9ba1ff0b4fe
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Mon Feb 19 16:48:21 2018 +0200

    alsa: add a couple of FIXME comments
    
    build_pollfd() isn't likely to fail, but if it does, pa_sink/source_put()
    will crash on an assertion failure. I haven't seen such crash happening,
    this is just something that I noticed while studying the state change
    code.

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 44f006b1..5de52d54 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -1203,6 +1203,9 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
 
                     if (u->sink->thread_info.state == PA_SINK_INIT) {
                         if (build_pollfd(u) < 0)
+                            /* FIXME: This will cause an assertion failure in
+                             * pa_sink_put(), because with the current design
+                             * pa_sink_put() is not allowed to fail. */
                             return -PA_ERR_IO;
                     }
 
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index 608cb0d6..cdafa580 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -1058,6 +1058,9 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
 
                     if (u->source->thread_info.state == PA_SOURCE_INIT) {
                         if (build_pollfd(u) < 0)
+                            /* FIXME: This will cause an assertion failure in
+                             * pa_source_put(), because with the current design
+                             * pa_source_put() is not allowed to fail. */
                             return -PA_ERR_IO;
                     }
 

commit 3f696051a2049b22426a3c583539ade1a04b771a
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Mon Feb 19 16:48:19 2018 +0200

    alsa, solaris, oss: remove unnecessary error handling when suspending
    
    Suspending never fails.

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index a80caab2..44f006b1 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -939,7 +939,7 @@ static int build_pollfd(struct userdata *u) {
 }
 
 /* Called from IO context */
-static int suspend(struct userdata *u) {
+static void suspend(struct userdata *u) {
     pa_assert(u);
     pa_assert(u->pcm_handle);
 
@@ -964,8 +964,6 @@ static int suspend(struct userdata *u) {
     pa_sink_set_max_request_within_thread(u->sink, 0);
 
     pa_log_info("Device suspended...");
-
-    return 0;
 }
 
 /* Called from IO context */
@@ -1192,12 +1190,9 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
             switch ((pa_sink_state_t) PA_PTR_TO_UINT(data)) {
 
                 case PA_SINK_SUSPENDED: {
-                    int r;
-
                     pa_assert(PA_SINK_IS_OPENED(u->sink->thread_info.state));
 
-                    if ((r = suspend(u)) < 0)
-                        return r;
+                    suspend(u);
 
                     break;
                 }
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index 3f87a479..608cb0d6 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -837,7 +837,7 @@ static int build_pollfd(struct userdata *u) {
 }
 
 /* Called from IO context */
-static int suspend(struct userdata *u) {
+static void suspend(struct userdata *u) {
     pa_assert(u);
     pa_assert(u->pcm_handle);
 
@@ -853,8 +853,6 @@ static int suspend(struct userdata *u) {
     }
 
     pa_log_info("Device suspended...");
-
-    return 0;
 }
 
 /* Called from IO context */
@@ -1047,12 +1045,9 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
             switch ((pa_source_state_t) PA_PTR_TO_UINT(data)) {
 
                 case PA_SOURCE_SUSPENDED: {
-                    int r;
-
                     pa_assert(PA_SOURCE_IS_OPENED(u->source->thread_info.state));
 
-                    if ((r = suspend(u)) < 0)
-                        return r;
+                    suspend(u);
 
                     break;
                 }
diff --git a/src/modules/module-solaris.c b/src/modules/module-solaris.c
index 880aa43e..a4960b8b 100644
--- a/src/modules/module-solaris.c
+++ b/src/modules/module-solaris.c
@@ -348,7 +348,7 @@ static int open_audio_device(struct userdata *u, pa_sample_spec *ss) {
     return u->fd;
 }
 
-static int suspend(struct userdata *u) {
+static void suspend(struct userdata *u) {
     pa_assert(u);
     pa_assert(u->fd >= 0);
 
@@ -364,8 +364,6 @@ static int suspend(struct userdata *u) {
     }
 
     pa_log_info("Device suspended.");
-
-    return 0;
 }
 
 static int unsuspend(struct userdata *u) {
@@ -403,10 +401,9 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
 
                     pa_smoother_pause(u->smoother, pa_rtclock_now());
 
-                    if (!u->source || u->source_suspended) {
-                        if (suspend(u) < 0)
-                            return -1;
-                    }
+                    if (!u->source || u->source_suspended)
+                        suspend(u);
+
                     u->sink_suspended = true;
                     break;
 
@@ -457,10 +454,9 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
 
                     pa_assert(PA_SOURCE_IS_OPENED(u->source->thread_info.state));
 
-                    if (!u->sink || u->sink_suspended) {
-                        if (suspend(u) < 0)
-                            return -1;
-                    }
+                    if (!u->sink || u->sink_suspended)
+                        suspend(u);
+
                     u->source_suspended = true;
                     break;
 
diff --git a/src/modules/oss/module-oss.c b/src/modules/oss/module-oss.c
index 93b55ad8..fb978b5e 100644
--- a/src/modules/oss/module-oss.c
+++ b/src/modules/oss/module-oss.c
@@ -485,7 +485,7 @@ static void build_pollfd(struct userdata *u) {
 }
 
 /* Called from IO context */
-static int suspend(struct userdata *u) {
+static void suspend(struct userdata *u) {
     pa_assert(u);
     pa_assert(u->fd >= 0);
 
@@ -530,8 +530,6 @@ static int suspend(struct userdata *u) {
     }
 
     pa_log_info("Device suspended...");
-
-    return 0;
 }
 
 /* Called from IO context */
@@ -670,10 +668,8 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
                 case PA_SINK_SUSPENDED:
                     pa_assert(PA_SINK_IS_OPENED(u->sink->thread_info.state));
 
-                    if (!u->source || u->source_suspended) {
-                        if (suspend(u) < 0)
-                            return -1;
-                    }
+                    if (!u->source || u->source_suspended)
+                        suspend(u);
 
                     do_trigger = true;
 
@@ -753,10 +749,8 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
                 case PA_SOURCE_SUSPENDED:
                     pa_assert(PA_SOURCE_IS_OPENED(u->source->thread_info.state));
 
-                    if (!u->sink || u->sink_suspended) {
-                        if (suspend(u) < 0)
-                            return -1;
-                    }
+                    if (!u->sink || u->sink_suspended)
+                        suspend(u);
 
                     do_trigger = true;
 

commit a821a54f30f67c57aa185b5928cef1239e4017a1
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Tue Feb 20 11:55:14 2018 +0200

    alsa-card: improve the profile availability logic
    
    When a new card shows up (during pulseaudio startup or hotplugged),
    pulseaudio needs to pick the initial profile for the card. Unavailable
    profiles shouldn't be picked, but module-alsa-card sometimes marked
    unavailable profiles as available, causing bad initial profile choices.
    
    This patch changes module-alsa-card so that it marks all profiles
    unavailable whose all output ports or all input ports are unavailable.
    Previously only those profiles were marked as unavailable whose all
    ports were unavailable. For example, if a profile contains one sink and
    one source, and the sink is unavailable and the source is available,
    previously such profile was marked as available, but now it's marked as
    unavailable.
    
    BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=102902

diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c
index b193d40c..385d61d2 100644
--- a/src/modules/alsa/module-alsa-card.c
+++ b/src/modules/alsa/module-alsa-card.c
@@ -442,29 +442,54 @@ static int report_jack_state(snd_mixer_elem_t *melem, unsigned int mask) {
         }
     }
 
-    /* Update profile availabilities. The logic could be improved; for now we
-     * only set obviously unavailable profiles (those that contain only
-     * unavailable ports) to PA_AVAILABLE_NO and all others to
-     * PA_AVAILABLE_UNKNOWN. */
+    /* Update profile availabilities. Ideally we would mark all profiles
+     * unavailable that contain unavailable devices. We can't currently do that
+     * in all cases, because if there are multiple sinks in a profile, and the
+     * profile contains a mix of available and unavailable ports, we don't know
+     * how the ports are distributed between the different sinks. It's possible
+     * that some sinks contain only unavailable ports, in which case we should
+     * mark the profile as unavailable, but it's also possible that all sinks
+     * contain at least one available port, in which case we should mark the
+     * profile as available. Until the data structures are improved so that we
+     * can distinguish between these two cases, we mark the problematic cases
+     * as available (well, "unknown" to be precise, but there's little
+     * practical difference).
+     *
+     * When all output ports are unavailable, we know that all sinks are
+     * unavailable, and therefore the profile is marked unavailable as well.
+     * The same applies to input ports as well, of course.
+     *
+     * If there are no output ports at all, but the profile contains at least
+     * one sink, then the output is considered to be available. */
     PA_HASHMAP_FOREACH(profile, u->card->profiles, state) {
         pa_device_port *port;
         void *state2;
-        pa_available_t available = PA_AVAILABLE_NO;
-
-        /* Don't touch the "off" profile. */
-        if (profile->n_sources == 0 && profile->n_sinks == 0)
-            continue;
+        bool has_input_port = false;
+        bool has_output_port = false;
+        bool found_available_input_port = false;
+        bool found_available_output_port = false;
+        pa_available_t available = PA_AVAILABLE_UNKNOWN;
 
         PA_HASHMAP_FOREACH(port, u->card->ports, state2) {
             if (!pa_hashmap_get(port->profiles, profile->name))
                 continue;
 
-            if (port->available != PA_AVAILABLE_NO) {
-                available = PA_AVAILABLE_UNKNOWN;
-                break;
+            if (port->direction == PA_DIRECTION_INPUT) {
+                has_input_port = true;
+
+                if (port->available != PA_AVAILABLE_NO)
+                    found_available_input_port = true;
+            } else {
+                has_output_port = true;
+
+                if (port->available != PA_AVAILABLE_NO)
+                    found_available_output_port = true;
             }
         }
 
+        if ((has_input_port && !found_available_input_port) || (has_output_port && !found_available_output_port))
+            available = PA_AVAILABLE_NO;
+
         pa_card_profile_set_available(profile, available);
     }
 



More information about the pulseaudio-commits mailing list