src/modules/alsa/alsa-sink.c   |   12 +++++-------
 src/modules/alsa/alsa-source.c |   12 +++++-------
 src/modules/module-null-sink.c |    4 ++--
 src/modules/module-pipe-sink.c |    6 +++---
 src/modules/module-solaris.c   |   18 +++++++-----------
 src/modules/oss/module-oss.c   |   16 +++++-----------
 6 files changed, 27 insertions(+), 41 deletions(-)

New commits:
commit 7f09164ed7979210adcdb7674b9d6217fd44ed66
Author: Tanu Kaskinen <ta...@iki.fi>
Date:   Wed Feb 21 11:54:41 2018 +0200

    null-sink, pipe-sink: some state variable cleanups
    
    pa_sink_get_state() is supposed to be used from the main thread. In this
    case it doesn't really matter, because the SET_STATE handler is executed
    while the main thread is waiting, but since the state is available also
    in thread_info, let's use that. All other modules use thread_info.state
    too, so at least this change improves consistency.
    
    Also, we can use the PA_SINK_IS_OPENED macro to simplify the code a bit.

diff --git a/src/modules/module-null-sink.c b/src/modules/module-null-sink.c
index 25b0f309..3ace082d 100644
--- a/src/modules/module-null-sink.c
+++ b/src/modules/module-null-sink.c
@@ -91,8 +91,8 @@ static int sink_process_msg(
     switch (code) {
         case PA_SINK_MESSAGE_SET_STATE:
 
-            if (pa_sink_get_state(u->sink) == PA_SINK_SUSPENDED || 
pa_sink_get_state(u->sink) == PA_SINK_INIT) {
-                if (PA_PTR_TO_UINT(data) == PA_SINK_RUNNING || 
PA_PTR_TO_UINT(data) == PA_SINK_IDLE)
+            if (u->sink->thread_info.state == PA_SINK_SUSPENDED || 
u->sink->thread_info.state == PA_SINK_INIT) {
+                if (PA_SINK_IS_OPENED(PA_PTR_TO_UINT(data)))
                     u->timestamp = pa_rtclock_now();
             }
 
diff --git a/src/modules/module-pipe-sink.c b/src/modules/module-pipe-sink.c
index a2074c1f..995785e1 100644
--- a/src/modules/module-pipe-sink.c
+++ b/src/modules/module-pipe-sink.c
@@ -111,10 +111,10 @@ static int sink_process_msg(pa_msgobject *o, int code, 
void *data, int64_t offse
 
     switch (code) {
         case PA_SINK_MESSAGE_SET_STATE:
-            if (pa_sink_get_state(u->sink) == PA_SINK_SUSPENDED || 
pa_sink_get_state(u->sink) == PA_SINK_INIT) {
-                if (PA_PTR_TO_UINT(data) == PA_SINK_RUNNING || 
PA_PTR_TO_UINT(data) == PA_SINK_IDLE)
+            if (u->sink->thread_info.state == PA_SINK_SUSPENDED || 
u->sink->thread_info.state == PA_SINK_INIT) {
+                if (PA_SINK_IS_OPENED(PA_PTR_TO_UINT(data)))
                     u->timestamp = pa_rtclock_now();
-            } else if (pa_sink_get_state(u->sink) == PA_SINK_RUNNING || 
pa_sink_get_state(u->sink) == PA_SINK_IDLE) {
+            } else if (u->sink->thread_info.state == PA_SINK_RUNNING || 
u->sink->thread_info.state == PA_SINK_IDLE) {
                 if (PA_PTR_TO_UINT(data) == PA_SINK_SUSPENDED) {
                     /* Clear potential FIFO error flag */
                     u->fifo_error = false;

commit 2dff0d6a6a4df2aab6f36212b705489d5af42835
Author: Tanu Kaskinen <ta...@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 fe0a21a5..309d726e 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 b11a03f8..adaa42cb 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 7f201b1fd419b91a226d23ee1e216661ae082dcf
Author: Tanu Kaskinen <ta...@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 5e817289..fe0a21a5 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 79d3364a..b11a03f8 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;
 

_______________________________________________
pulseaudio-commits mailing list
pulseaudio-commits@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-commits

Reply via email to