[pulseaudio-commits] Branch 'next' - 3 commits - src/modules src/pulsecore
Tanu Kaskinen
tanuk at kemper.freedesktop.org
Thu Feb 22 10:04:57 UTC 2018
src/modules/alsa/alsa-sink.c | 2
src/modules/alsa/alsa-source.c | 2
src/modules/echo-cancel/module-echo-cancel.c | 4
src/modules/macosx/module-coreaudio-device.c | 4
src/modules/module-combine-sink.c | 7 +
src/modules/module-equalizer-sink.c | 2
src/modules/module-ladspa-sink.c | 2
src/modules/module-remap-sink.c | 2
src/modules/module-remap-source.c | 2
src/modules/module-tunnel.c | 14 ++
src/modules/module-virtual-sink.c | 2
src/modules/module-virtual-source.c | 4
src/modules/module-virtual-surround-sink.c | 2
src/pulsecore/sink.c | 125 ++++++++++++++------------
src/pulsecore/sink.h | 12 ++
src/pulsecore/source.c | 128 ++++++++++++++++-----------
src/pulsecore/source.h | 12 ++
17 files changed, 202 insertions(+), 124 deletions(-)
New commits:
commit b59fd184abc2a56fbd27d0c62204ca0019f7d895
Author: Tanu Kaskinen <tanuk at iki.fi>
Date: Thu Feb 22 10:06:59 2018 +0200
sink: don't sync monitor suspend state when unlinking
When the sink is unlinked, there's no need to update the monitor suspend
state. In fact, trying to do that causes an assertion failure, because
pa_source_sync_suspend() wasn't written to handle the case where the
sink is unlinked.
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 0dfc91c8..f19e8b09 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -481,7 +481,7 @@ static int sink_set_state(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t
}
finish:
- if ((suspending || resuming || suspend_cause_changed) && s->monitor_source)
+ if ((suspending || resuming || suspend_cause_changed) && s->monitor_source && state != PA_SINK_UNLINKED)
pa_source_sync_suspend(s->monitor_source);
return ret;
commit ee9d26adb561016ac8d6ec2fedb742121859f5a8
Author: Tanu Kaskinen <tanuk at iki.fi>
Date: Mon Feb 19 16:48:23 2018 +0200
pass pa_suspend_cause_t to set_state() callbacks
The suspend cause isn't yet used by any of the callbacks. The alsa sink
and source will use it to sync the mixer when the SESSION suspend cause
is removed. Currently the syncing is done in pa_sink/source_suspend(),
and I want to change that, because pa_sink/source_suspend() shouldn't
have any alsa specific code.
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 5de52d54..5c8ccf31 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -1230,7 +1230,7 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
}
/* Called from main context */
-static int sink_set_state_cb(pa_sink *s, pa_sink_state_t new_state) {
+static int sink_set_state_cb(pa_sink *s, pa_sink_state_t new_state, pa_suspend_cause_t new_suspend_cause) {
pa_sink_state_t old_state;
struct userdata *u;
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index cdafa580..fec6c4e0 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -1085,7 +1085,7 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
}
/* Called from main context */
-static int source_set_state_cb(pa_source *s, pa_source_state_t new_state) {
+static int source_set_state_cb(pa_source *s, pa_source_state_t new_state, pa_suspend_cause_t new_suspend_cause) {
pa_source_state_t old_state;
struct userdata *u;
diff --git a/src/modules/echo-cancel/module-echo-cancel.c b/src/modules/echo-cancel/module-echo-cancel.c
index 0702f2fd..8e416563 100644
--- a/src/modules/echo-cancel/module-echo-cancel.c
+++ b/src/modules/echo-cancel/module-echo-cancel.c
@@ -477,7 +477,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
}
/* Called from main context */
-static int source_set_state_cb(pa_source *s, pa_source_state_t state) {
+static int source_set_state_cb(pa_source *s, pa_source_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_source_assert_ref(s);
@@ -502,7 +502,7 @@ static int source_set_state_cb(pa_source *s, pa_source_state_t state) {
}
/* Called from main context */
-static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
diff --git a/src/modules/macosx/module-coreaudio-device.c b/src/modules/macosx/module-coreaudio-device.c
index 73e332a3..f9ef7c5a 100644
--- a/src/modules/macosx/module-coreaudio-device.c
+++ b/src/modules/macosx/module-coreaudio-device.c
@@ -353,7 +353,7 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
return pa_source_process_msg(o, code, data, offset, chunk);;
}
-static int ca_sink_set_state(pa_sink *s, pa_sink_state_t state) {
+static int ca_sink_set_state(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
coreaudio_sink *sink = s->userdata;
switch (state) {
@@ -511,7 +511,7 @@ static int ca_device_create_sink(pa_module *m, AudioBuffer *buf, int channel_idx
return 0;
}
-static int ca_source_set_state(pa_source *s, pa_source_state_t state) {
+static int ca_source_set_state(pa_source *s, pa_source_state_t state, pa_suspend_cause_t suspend_cause) {
coreaudio_source *source = s->userdata;
switch (state) {
diff --git a/src/modules/module-combine-sink.c b/src/modules/module-combine-sink.c
index baaac44d..7a80028a 100644
--- a/src/modules/module-combine-sink.c
+++ b/src/modules/module-combine-sink.c
@@ -680,12 +680,17 @@ static void unsuspend(struct userdata *u) {
}
/* Called from main context */
-static int sink_set_state(pa_sink *sink, pa_sink_state_t state) {
+static int sink_set_state(pa_sink *sink, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(sink);
pa_assert_se(u = sink->userdata);
+ /* It may be that only the suspend cause is changing, in which
+ * case there's nothing to do. */
+ if (state == u->sink->state)
+ return 0;
+
/* Please note that in contrast to the ALSA modules we call
* suspend/unsuspend from main context here! */
diff --git a/src/modules/module-equalizer-sink.c b/src/modules/module-equalizer-sink.c
index d3551153..bcc8dafe 100644
--- a/src/modules/module-equalizer-sink.c
+++ b/src/modules/module-equalizer-sink.c
@@ -285,7 +285,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
}
/* Called from main context */
-static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
diff --git a/src/modules/module-ladspa-sink.c b/src/modules/module-ladspa-sink.c
index 323a989a..4d5cd68f 100644
--- a/src/modules/module-ladspa-sink.c
+++ b/src/modules/module-ladspa-sink.c
@@ -392,7 +392,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
}
/* Called from main context */
-static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
diff --git a/src/modules/module-remap-sink.c b/src/modules/module-remap-sink.c
index 1599e160..f063576f 100644
--- a/src/modules/module-remap-sink.c
+++ b/src/modules/module-remap-sink.c
@@ -112,7 +112,7 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
}
/* Called from main context */
-static int sink_set_state(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
diff --git a/src/modules/module-remap-source.c b/src/modules/module-remap-source.c
index 109c913c..88eccc22 100644
--- a/src/modules/module-remap-source.c
+++ b/src/modules/module-remap-source.c
@@ -108,7 +108,7 @@ static int source_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t
}
/* Called from main context */
-static int source_set_state_cb(pa_source *s, pa_source_state_t state) {
+static int source_set_state_cb(pa_source *s, pa_source_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_source_assert_ref(s);
diff --git a/src/modules/module-tunnel.c b/src/modules/module-tunnel.c
index e54a8242..1db79ef6 100644
--- a/src/modules/module-tunnel.c
+++ b/src/modules/module-tunnel.c
@@ -568,11 +568,16 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
}
/* Called from main context */
-static int sink_set_state(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
u = s->userdata;
+ /* It may be that only the suspend cause is changing, in which
+ * case there's nothing to do. */
+ if (state == s->state)
+ return 0;
+
switch ((pa_sink_state_t) state) {
case PA_SINK_SUSPENDED:
@@ -665,11 +670,16 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
}
/* Called from main context */
-static int source_set_state(pa_source *s, pa_source_state_t state) {
+static int source_set_state(pa_source *s, pa_source_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_source_assert_ref(s);
u = s->userdata;
+ /* It may be that only the suspend cause is changing, in which
+ * case there's nothing to do. */
+ if (state == s->state)
+ return 0;
+
switch ((pa_source_state_t) state) {
case PA_SOURCE_SUSPENDED:
diff --git a/src/modules/module-virtual-sink.c b/src/modules/module-virtual-sink.c
index d477fb21..5fa4ce4d 100644
--- a/src/modules/module-virtual-sink.c
+++ b/src/modules/module-virtual-sink.c
@@ -124,7 +124,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
}
/* Called from main context */
-static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
diff --git a/src/modules/module-virtual-source.c b/src/modules/module-virtual-source.c
index df0f4cb3..c002ae84 100644
--- a/src/modules/module-virtual-source.c
+++ b/src/modules/module-virtual-source.c
@@ -111,7 +111,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
}
/* Called from main context */
-static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
@@ -194,7 +194,7 @@ static int source_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t
}
/* Called from main context */
-static int source_set_state_cb(pa_source *s, pa_source_state_t state) {
+static int source_set_state_cb(pa_source *s, pa_source_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_source_assert_ref(s);
diff --git a/src/modules/module-virtual-surround-sink.c b/src/modules/module-virtual-surround-sink.c
index 143b5cac..876d618e 100644
--- a/src/modules/module-virtual-surround-sink.c
+++ b/src/modules/module-virtual-surround-sink.c
@@ -152,7 +152,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
}
/* Called from main context */
-static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
+static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
struct userdata *u;
pa_sink_assert_ref(s);
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 50e17c17..0dfc91c8 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -427,8 +427,8 @@ static int sink_set_state(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t
* cause, or it might just add unnecessary complexity, given that the
* current approach of not setting any suspend cause works well enough. */
- if (s->set_state && state_changed) {
- ret = s->set_state(s, state);
+ if (s->set_state) {
+ ret = s->set_state(s, state, suspend_cause);
/* set_state() is allowed to fail only when resuming. */
pa_assert(ret >= 0 || resuming);
}
@@ -439,7 +439,7 @@ static int sink_set_state(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t
pa_assert(resuming);
if (s->set_state)
- s->set_state(s, PA_SINK_SUSPENDED);
+ s->set_state(s, PA_SINK_SUSPENDED, 0);
}
if (suspend_cause_changed) {
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 3fb23012..b7e21f9f 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -126,8 +126,16 @@ struct pa_sink {
/* Called when the main loop requests a state change. Called from
* main loop context. If returns -1 the state change will be
- * inhibited */
- int (*set_state)(pa_sink *s, pa_sink_state_t state); /* may be NULL */
+ * inhibited. This will also be called even if only the suspend cause
+ * changes.
+ *
+ * s->state and s->suspend_cause haven't been updated yet when this is
+ * called, so the callback can get the old state through those variables.
+ *
+ * If set_state() is successful, the IO thread will be notified with the
+ * SET_STATE message. The message handler is allowed to fail, in which
+ * case the old state is restored, and set_state() is called again. */
+ int (*set_state)(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause); /* may be NULL */
/* Sink drivers that support hardware volume may set this
* callback. This is called when the current volume needs to be
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index c46dc2b1..7ea75ff0 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -381,8 +381,8 @@ static int source_set_state(pa_source *s, pa_source_state_t state, pa_suspend_ca
* cause, or it might just add unnecessary complexity, given that the
* current approach of not setting any suspend cause works well enough. */
- if (s->set_state && state_changed) {
- ret = s->set_state(s, state);
+ if (s->set_state) {
+ ret = s->set_state(s, state, suspend_cause);
/* set_state() is allowed to fail only when resuming. */
pa_assert(ret >= 0 || resuming);
}
@@ -393,7 +393,7 @@ static int source_set_state(pa_source *s, pa_source_state_t state, pa_suspend_ca
pa_assert(resuming);
if (s->set_state)
- s->set_state(s, PA_SOURCE_SUSPENDED);
+ s->set_state(s, PA_SOURCE_SUSPENDED, 0);
}
if (suspend_cause_changed) {
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index 75ce241f..ea314725 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -127,8 +127,16 @@ struct pa_source {
/* Called when the main loop requests a state change. Called from
* main loop context. If returns -1 the state change will be
- * inhibited */
- int (*set_state)(pa_source*source, pa_source_state_t state); /* may be NULL */
+ * inhibited. This will also be called even if only the suspend cause
+ * changes.
+ *
+ * s->state and s->suspend_cause haven't been updated yet when this is
+ * called, so the callback can get the old state through those variables.
+ *
+ * If set_state() is successful, the IO thread will be notified with the
+ * SET_STATE message. The message handler is allowed to fail, in which
+ * case the old state is restored, and set_state() is called again. */
+ int (*set_state)(pa_source *source, pa_source_state_t state, pa_suspend_cause_t suspend_cause); /* may be NULL */
/* Called when the volume is queried. Called from main loop
* context. If this is NULL a PA_SOURCE_MESSAGE_GET_VOLUME message
commit d43b9b4b58af37bdfeb46fc91ab85ea63f723904
Author: Tanu Kaskinen <tanuk at iki.fi>
Date: Mon Feb 19 16:48:22 2018 +0200
sink, source: redo state changing code
This adds a pa_suspend_cause_t parameter to the sink/source_set_state()
functions, and moves part of the work that pa_sink/source_suspend() does
to sink/source_set_state(). The reason for this code shuffling is that I
plan to make all suspend cause changes available to modules through the
state change callbacks. This is the first step towards that.
Additionally, pa_source_sync_suspend() is changed to also update the
suspend cause of the monitor source when the suspend cause of the
monitored sink changes. That probably doesn't have much effect on
anything, but I think it makes sense to mirror the sink suspend cause in
the monitor source.
pa_source_sync_suspend() has also a bug fix: previously it was probably
possible that a sink might get suspended while in the passthrough mode.
When the sink then resumed (while still in the passthrough mode),
pa_source_sync_suspend() would resume also the monitor source, even
though the monitor source should be kept suspended when the sink is in
the passthrough mode. Now the monitor source won't be resumed in this
situation.
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 7f5c37f4..50e17c17 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -397,45 +397,76 @@ pa_sink* pa_sink_new(
}
/* Called from main context */
-static int sink_set_state(pa_sink *s, pa_sink_state_t state) {
- int ret;
- bool suspend_change;
- pa_sink_state_t original_state;
+static int sink_set_state(pa_sink *s, pa_sink_state_t state, pa_suspend_cause_t suspend_cause) {
+ int ret = 0;
+ bool state_changed;
+ bool suspend_cause_changed;
+ bool suspending;
+ bool resuming;
pa_assert(s);
pa_assert_ctl_context();
- if (s->state == state)
+ state_changed = state != s->state;
+ suspend_cause_changed = suspend_cause != s->suspend_cause;
+
+ if (!state_changed && !suspend_cause_changed)
return 0;
- original_state = s->state;
+ suspending = PA_SINK_IS_OPENED(s->state) && state == PA_SINK_SUSPENDED;
+ resuming = s->state == PA_SINK_SUSPENDED && PA_SINK_IS_OPENED(state);
- suspend_change =
- (original_state == PA_SINK_SUSPENDED && PA_SINK_IS_OPENED(state)) ||
- (PA_SINK_IS_OPENED(original_state) && state == PA_SINK_SUSPENDED);
+ /* If we are resuming, suspend_cause must be 0. */
+ pa_assert(!resuming || !suspend_cause);
- if (s->set_state)
- if ((ret = s->set_state(s, state)) < 0)
- return ret;
+ /* Here's something to think about: what to do with the suspend cause if
+ * resuming the sink fails? The old suspend cause will be incorrect, so we
+ * can't use that. On the other hand, if we set no suspend cause (as is the
+ * case currently), then it looks strange to have a sink suspended without
+ * any cause. It might be a good idea to add a new "resume failed" suspend
+ * cause, or it might just add unnecessary complexity, given that the
+ * current approach of not setting any suspend cause works well enough. */
- if (s->asyncmsgq)
+ if (s->set_state && state_changed) {
+ ret = s->set_state(s, state);
+ /* set_state() is allowed to fail only when resuming. */
+ pa_assert(ret >= 0 || resuming);
+ }
+
+ if (ret >= 0 && s->asyncmsgq && state_changed)
if ((ret = pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s), PA_SINK_MESSAGE_SET_STATE, PA_UINT_TO_PTR(state), 0, NULL)) < 0) {
+ /* SET_STATE is allowed to fail only when resuming. */
+ pa_assert(resuming);
if (s->set_state)
- s->set_state(s, original_state);
-
- return ret;
+ s->set_state(s, PA_SINK_SUSPENDED);
}
- pa_log_debug("%s: state: %s -> %s", s->name, pa_sink_state_to_string(s->state), pa_sink_state_to_string(state));
- s->state = state;
+ if (suspend_cause_changed) {
+ char old_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
+ char new_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
- if (state != PA_SINK_UNLINKED) { /* if we enter UNLINKED state pa_sink_unlink() will fire the appropriate events */
- pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_STATE_CHANGED], s);
- pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SINK | PA_SUBSCRIPTION_EVENT_CHANGE, s->index);
+ pa_log_debug("%s: suspend_cause: %s -> %s", s->name, pa_suspend_cause_to_string(s->suspend_cause, old_cause_buf),
+ pa_suspend_cause_to_string(suspend_cause, new_cause_buf));
+ s->suspend_cause = suspend_cause;
}
- if (suspend_change) {
+ if (ret < 0)
+ goto finish;
+
+ if (state_changed) {
+ pa_log_debug("%s: state: %s -> %s", s->name, pa_sink_state_to_string(s->state), pa_sink_state_to_string(state));
+ s->state = state;
+
+ /* If we enter UNLINKED state, then we don't send change notifications.
+ * pa_sink_unlink() will send unlink notifications instead. */
+ if (state != PA_SINK_UNLINKED) {
+ pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_STATE_CHANGED], s);
+ pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SINK | PA_SUBSCRIPTION_EVENT_CHANGE, s->index);
+ }
+ }
+
+ if (suspending || resuming) {
pa_sink_input *i;
uint32_t idx;
@@ -447,12 +478,13 @@ static int sink_set_state(pa_sink *s, pa_sink_state_t state) {
pa_sink_input_kill(i);
else if (i->suspend)
i->suspend(i, state == PA_SINK_SUSPENDED);
-
- if (s->monitor_source)
- pa_source_sync_suspend(s->monitor_source);
}
- return 0;
+finish:
+ if ((suspending || resuming || suspend_cause_changed) && s->monitor_source)
+ pa_source_sync_suspend(s->monitor_source);
+
+ return ret;
}
void pa_sink_set_get_volume_callback(pa_sink *s, pa_sink_cb_t cb) {
@@ -655,9 +687,9 @@ void pa_sink_put(pa_sink* s) {
pa_assert(s->monitor_source->thread_info.max_latency == s->thread_info.max_latency);
if (s->suspend_cause)
- pa_assert_se(sink_set_state(s, PA_SINK_SUSPENDED) == 0);
+ pa_assert_se(sink_set_state(s, PA_SINK_SUSPENDED, s->suspend_cause) == 0);
else
- pa_assert_se(sink_set_state(s, PA_SINK_IDLE) == 0);
+ pa_assert_se(sink_set_state(s, PA_SINK_IDLE, 0) == 0);
pa_source_put(s->monitor_source);
@@ -707,7 +739,7 @@ void pa_sink_unlink(pa_sink* s) {
}
if (linked)
- sink_set_state(s, PA_SINK_UNLINKED);
+ sink_set_state(s, PA_SINK_UNLINKED, 0);
else
s->state = PA_SINK_UNLINKED;
@@ -830,7 +862,7 @@ int pa_sink_update_status(pa_sink*s) {
if (s->state == PA_SINK_SUSPENDED)
return 0;
- return sink_set_state(s, pa_sink_used_by(s) ? PA_SINK_RUNNING : PA_SINK_IDLE);
+ return sink_set_state(s, pa_sink_used_by(s) ? PA_SINK_RUNNING : PA_SINK_IDLE, 0);
}
/* Called from any context - must be threadsafe */
@@ -840,31 +872,19 @@ void pa_sink_set_mixer_dirty(pa_sink *s, bool is_dirty) {
/* Called from main context */
int pa_sink_suspend(pa_sink *s, bool suspend, pa_suspend_cause_t cause) {
- pa_suspend_cause_t old_cause;
- char old_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
- char new_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
+ pa_suspend_cause_t merged_cause;
pa_sink_assert_ref(s);
pa_assert_ctl_context();
pa_assert(PA_SINK_IS_LINKED(s->state));
pa_assert(cause != 0);
- old_cause = s->suspend_cause;
-
- if (suspend) {
- s->suspend_cause |= cause;
- s->monitor_source->suspend_cause |= cause;
- } else {
- s->suspend_cause &= ~cause;
- s->monitor_source->suspend_cause &= ~cause;
- }
-
- if (s->suspend_cause != old_cause) {
- pa_log_debug("%s: suspend_cause: %s -> %s", s->name, pa_suspend_cause_to_string(old_cause, old_cause_buf),
- pa_suspend_cause_to_string(s->suspend_cause, new_cause_buf));
- }
+ if (suspend)
+ merged_cause = s->suspend_cause | cause;
+ else
+ merged_cause = s->suspend_cause & ~cause;
- if (!(s->suspend_cause & PA_SUSPEND_SESSION) && (pa_atomic_load(&s->mixer_dirty) != 0)) {
+ if (!(merged_cause & PA_SUSPEND_SESSION) && (pa_atomic_load(&s->mixer_dirty) != 0)) {
/* This might look racy but isn't: If somebody sets mixer_dirty exactly here,
it'll be handled just fine. */
pa_sink_set_mixer_dirty(s, false);
@@ -885,13 +905,10 @@ int pa_sink_suspend(pa_sink *s, bool suspend, pa_suspend_cause_t cause) {
}
}
- if ((pa_sink_get_state(s) == PA_SINK_SUSPENDED) == !!s->suspend_cause)
- return 0;
-
- if (s->suspend_cause)
- return sink_set_state(s, PA_SINK_SUSPENDED);
+ if (merged_cause)
+ return sink_set_state(s, PA_SINK_SUSPENDED, merged_cause);
else
- return sink_set_state(s, pa_sink_used_by(s) ? PA_SINK_RUNNING : PA_SINK_IDLE);
+ return sink_set_state(s, pa_sink_used_by(s) ? PA_SINK_RUNNING : PA_SINK_IDLE, 0);
}
/* Called from main context */
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index 2a6b1f11..c46dc2b1 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -351,45 +351,76 @@ pa_source* pa_source_new(
}
/* Called from main context */
-static int source_set_state(pa_source *s, pa_source_state_t state) {
- int ret;
- bool suspend_change;
- pa_source_state_t original_state;
+static int source_set_state(pa_source *s, pa_source_state_t state, pa_suspend_cause_t suspend_cause) {
+ int ret = 0;
+ bool state_changed;
+ bool suspend_cause_changed;
+ bool suspending;
+ bool resuming;
pa_assert(s);
pa_assert_ctl_context();
- if (s->state == state)
+ state_changed = state != s->state;
+ suspend_cause_changed = suspend_cause != s->suspend_cause;
+
+ if (!state_changed && !suspend_cause_changed)
return 0;
- original_state = s->state;
+ suspending = PA_SOURCE_IS_OPENED(s->state) && state == PA_SOURCE_SUSPENDED;
+ resuming = s->state == PA_SOURCE_SUSPENDED && PA_SOURCE_IS_OPENED(state);
- suspend_change =
- (original_state == PA_SOURCE_SUSPENDED && PA_SOURCE_IS_OPENED(state)) ||
- (PA_SOURCE_IS_OPENED(original_state) && state == PA_SOURCE_SUSPENDED);
+ /* If we are resuming, suspend_cause must be 0. */
+ pa_assert(!resuming || !suspend_cause);
- if (s->set_state)
- if ((ret = s->set_state(s, state)) < 0)
- return ret;
+ /* Here's something to think about: what to do with the suspend cause if
+ * resuming the source fails? The old suspend cause will be incorrect, so we
+ * can't use that. On the other hand, if we set no suspend cause (as is the
+ * case currently), then it looks strange to have a source suspended without
+ * any cause. It might be a good idea to add a new "resume failed" suspend
+ * cause, or it might just add unnecessary complexity, given that the
+ * current approach of not setting any suspend cause works well enough. */
- if (s->asyncmsgq)
+ if (s->set_state && state_changed) {
+ ret = s->set_state(s, state);
+ /* set_state() is allowed to fail only when resuming. */
+ pa_assert(ret >= 0 || resuming);
+ }
+
+ if (ret >= 0 && s->asyncmsgq && state_changed)
if ((ret = pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s), PA_SOURCE_MESSAGE_SET_STATE, PA_UINT_TO_PTR(state), 0, NULL)) < 0) {
+ /* SET_STATE is allowed to fail only when resuming. */
+ pa_assert(resuming);
if (s->set_state)
- s->set_state(s, original_state);
-
- return ret;
+ s->set_state(s, PA_SOURCE_SUSPENDED);
}
- pa_log_debug("%s: state: %s -> %s", s->name, pa_source_state_to_string(s->state), pa_source_state_to_string(state));
- s->state = state;
+ if (suspend_cause_changed) {
+ char old_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
+ char new_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
- if (state != PA_SOURCE_UNLINKED) { /* if we enter UNLINKED state pa_source_unlink() will fire the appropriate events */
- pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SOURCE_STATE_CHANGED], s);
- pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SOURCE | PA_SUBSCRIPTION_EVENT_CHANGE, s->index);
+ pa_log_debug("%s: suspend_cause: %s -> %s", s->name, pa_suspend_cause_to_string(s->suspend_cause, old_cause_buf),
+ pa_suspend_cause_to_string(suspend_cause, new_cause_buf));
+ s->suspend_cause = suspend_cause;
}
- if (suspend_change) {
+ if (ret < 0)
+ goto finish;
+
+ if (state_changed) {
+ pa_log_debug("%s: state: %s -> %s", s->name, pa_source_state_to_string(s->state), pa_source_state_to_string(state));
+ s->state = state;
+
+ /* If we enter UNLINKED state, then we don't send change notifications.
+ * pa_source_unlink() will send unlink notifications instead. */
+ if (state == PA_SOURCE_UNLINKED) {
+ pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SOURCE_STATE_CHANGED], s);
+ pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SOURCE | PA_SUBSCRIPTION_EVENT_CHANGE, s->index);
+ }
+ }
+
+ if (suspending || resuming) {
pa_source_output *o;
uint32_t idx;
@@ -403,7 +434,8 @@ static int source_set_state(pa_source *s, pa_source_state_t state) {
o->suspend(o, state == PA_SOURCE_SUSPENDED);
}
- return 0;
+finish:
+ return ret;
}
void pa_source_set_get_volume_callback(pa_source *s, pa_source_cb_t cb) {
@@ -600,9 +632,9 @@ void pa_source_put(pa_source *s) {
pa_assert(!(s->flags & PA_SOURCE_DYNAMIC_LATENCY) == !(s->thread_info.fixed_latency == 0));
if (s->suspend_cause)
- pa_assert_se(source_set_state(s, PA_SOURCE_SUSPENDED) == 0);
+ pa_assert_se(source_set_state(s, PA_SOURCE_SUSPENDED, s->suspend_cause) == 0);
else
- pa_assert_se(source_set_state(s, PA_SOURCE_IDLE) == 0);
+ pa_assert_se(source_set_state(s, PA_SOURCE_IDLE, 0) == 0);
pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SOURCE | PA_SUBSCRIPTION_EVENT_NEW, s->index);
pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SOURCE_PUT], s);
@@ -649,7 +681,7 @@ void pa_source_unlink(pa_source *s) {
}
if (linked)
- source_set_state(s, PA_SOURCE_UNLINKED);
+ source_set_state(s, PA_SOURCE_UNLINKED, 0);
else
s->state = PA_SOURCE_UNLINKED;
@@ -751,7 +783,7 @@ int pa_source_update_status(pa_source*s) {
if (s->state == PA_SOURCE_SUSPENDED)
return 0;
- return source_set_state(s, pa_source_used_by(s) ? PA_SOURCE_RUNNING : PA_SOURCE_IDLE);
+ return source_set_state(s, pa_source_used_by(s) ? PA_SOURCE_RUNNING : PA_SOURCE_IDLE, 0);
}
/* Called from any context - must be threadsafe */
@@ -761,9 +793,7 @@ void pa_source_set_mixer_dirty(pa_source *s, bool is_dirty) {
/* Called from main context */
int pa_source_suspend(pa_source *s, bool suspend, pa_suspend_cause_t cause) {
- pa_suspend_cause_t old_cause;
- char old_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
- char new_cause_buf[PA_SUSPEND_CAUSE_TO_STRING_BUF_SIZE];
+ pa_suspend_cause_t merged_cause;
pa_source_assert_ref(s);
pa_assert_ctl_context();
@@ -773,19 +803,12 @@ int pa_source_suspend(pa_source *s, bool suspend, pa_suspend_cause_t cause) {
if (s->monitor_of && cause != PA_SUSPEND_PASSTHROUGH)
return -PA_ERR_NOTSUPPORTED;
- old_cause = s->suspend_cause;
-
if (suspend)
- s->suspend_cause |= cause;
+ merged_cause = s->suspend_cause | cause;
else
- s->suspend_cause &= ~cause;
-
- if (s->suspend_cause != old_cause) {
- pa_log_debug("%s: suspend_cause: %s -> %s", s->name, pa_suspend_cause_to_string(old_cause, old_cause_buf),
- pa_suspend_cause_to_string(s->suspend_cause, new_cause_buf));
- }
+ merged_cause = s->suspend_cause & ~cause;
- if (!(s->suspend_cause & PA_SUSPEND_SESSION) && (pa_atomic_load(&s->mixer_dirty) != 0)) {
+ if (!(merged_cause & PA_SUSPEND_SESSION) && (pa_atomic_load(&s->mixer_dirty) != 0)) {
/* This might look racy but isn't: If somebody sets mixer_dirty exactly here,
it'll be handled just fine. */
pa_source_set_mixer_dirty(s, false);
@@ -806,18 +829,16 @@ int pa_source_suspend(pa_source *s, bool suspend, pa_suspend_cause_t cause) {
}
}
- if ((pa_source_get_state(s) == PA_SOURCE_SUSPENDED) == !!s->suspend_cause)
- return 0;
-
- if (s->suspend_cause)
- return source_set_state(s, PA_SOURCE_SUSPENDED);
+ if (merged_cause)
+ return source_set_state(s, PA_SOURCE_SUSPENDED, merged_cause);
else
- return source_set_state(s, pa_source_used_by(s) ? PA_SOURCE_RUNNING : PA_SOURCE_IDLE);
+ return source_set_state(s, pa_source_used_by(s) ? PA_SOURCE_RUNNING : PA_SOURCE_IDLE, 0);
}
/* Called from main context */
int pa_source_sync_suspend(pa_source *s) {
pa_sink_state_t state;
+ pa_suspend_cause_t suspend_cause;
pa_source_assert_ref(s);
pa_assert_ctl_context();
@@ -825,13 +846,22 @@ int pa_source_sync_suspend(pa_source *s) {
pa_assert(s->monitor_of);
state = pa_sink_get_state(s->monitor_of);
+ suspend_cause = s->monitor_of->suspend_cause;
+
+ /* The monitor source usually has the same state and suspend cause as the
+ * sink, the only exception is when the monitor source is suspended due to
+ * the sink being in the passthrough mode. If the monitor currently has the
+ * PASSTHROUGH suspend cause, then we have to keep the monitor suspended
+ * even if the sink is running. */
+ if (s->suspend_cause & PA_SUSPEND_PASSTHROUGH)
+ suspend_cause |= PA_SUSPEND_PASSTHROUGH;
- if (state == PA_SINK_SUSPENDED)
- return source_set_state(s, PA_SOURCE_SUSPENDED);
+ if (state == PA_SINK_SUSPENDED || suspend_cause)
+ return source_set_state(s, PA_SOURCE_SUSPENDED, suspend_cause);
pa_assert(PA_SINK_IS_OPENED(state));
- return source_set_state(s, pa_source_used_by(s) ? PA_SOURCE_RUNNING : PA_SOURCE_IDLE);
+ return source_set_state(s, pa_source_used_by(s) ? PA_SOURCE_RUNNING : PA_SOURCE_IDLE, 0);
}
/* Called from main context */
More information about the pulseaudio-commits
mailing list