[pulseaudio-discuss] [PATCH 2/3] suspend-on-idle: use earlier (safer) hooks for stream unlink notifications
Tanu Kaskinen
tanuk at iki.fi
Wed Oct 12 14:20:40 UTC 2016
In the "unlink post" hook it's not guaranteed that the stream's old
device exists any more, so let's use the "unlink" hook that is safer.
For example, module-bluetooth-policy does a card profile change in the
source-output "unlink post" hook, which invalidates the source-output's
source pointer.
When the "unlink" hook is fired, the stream is still linked to its
device, which affects the return values of the check_suspend()
functions. The unlinked streams should be ignored by the check_suspend()
functions, so I had to add extra parameters to those functions.
---
src/modules/module-suspend-on-idle.c | 34 +++++++++++++++++-----------------
src/pulsecore/sink.c | 7 +++++--
src/pulsecore/sink.h | 9 ++++++++-
src/pulsecore/source.c | 5 ++++-
src/pulsecore/source.h | 6 +++++-
5 files changed, 39 insertions(+), 22 deletions(-)
diff --git a/src/modules/module-suspend-on-idle.c b/src/modules/module-suspend-on-idle.c
index f7620db..a284f85 100644
--- a/src/modules/module-suspend-on-idle.c
+++ b/src/modules/module-suspend-on-idle.c
@@ -67,13 +67,13 @@ static void timeout_cb(pa_mainloop_api*a, pa_time_event* e, const struct timeval
d->userdata->core->mainloop->time_restart(d->time_event, NULL);
- if (d->sink && pa_sink_check_suspend(d->sink) <= 0 && !(d->sink->suspend_cause & PA_SUSPEND_IDLE)) {
+ if (d->sink && pa_sink_check_suspend(d->sink, NULL, NULL) <= 0 && !(d->sink->suspend_cause & PA_SUSPEND_IDLE)) {
pa_log_info("Sink %s idle for too long, suspending ...", d->sink->name);
pa_sink_suspend(d->sink, true, PA_SUSPEND_IDLE);
pa_core_maybe_vacuum(d->userdata->core);
}
- if (d->source && pa_source_check_suspend(d->source) <= 0 && !(d->source->suspend_cause & PA_SUSPEND_IDLE)) {
+ if (d->source && pa_source_check_suspend(d->source, NULL) <= 0 && !(d->source->suspend_cause & PA_SUSPEND_IDLE)) {
pa_log_info("Source %s idle for too long, suspending ...", d->source->name);
pa_source_suspend(d->source, true, PA_SUSPEND_IDLE);
pa_core_maybe_vacuum(d->userdata->core);
@@ -125,7 +125,7 @@ static pa_hook_result_t sink_input_fixate_hook_cb(pa_core *c, pa_sink_input_new_
if ((d = pa_hashmap_get(u->device_infos, data->sink))) {
resume(d);
- if (pa_sink_check_suspend(d->sink) <= 0)
+ if (pa_sink_check_suspend(d->sink, NULL, NULL) <= 0)
restart(d);
}
@@ -147,12 +147,12 @@ static pa_hook_result_t source_output_fixate_hook_cb(pa_core *c, pa_source_outpu
if (d) {
resume(d);
if (d->source) {
- if (pa_source_check_suspend(d->source) <= 0)
+ if (pa_source_check_suspend(d->source, NULL) <= 0)
restart(d);
} else {
/* The source output is connected to a monitor source. */
pa_assert(d->sink);
- if (pa_sink_check_suspend(d->sink) <= 0)
+ if (pa_sink_check_suspend(d->sink, NULL, NULL) <= 0)
restart(d);
}
}
@@ -168,7 +168,7 @@ static pa_hook_result_t sink_input_unlink_hook_cb(pa_core *c, pa_sink_input *s,
if (!s->sink)
return PA_HOOK_OK;
- if (pa_sink_check_suspend(s->sink) <= 0) {
+ if (pa_sink_check_suspend(s->sink, s, NULL) <= 0) {
struct device_info *d;
if ((d = pa_hashmap_get(u->device_infos, s->sink)))
restart(d);
@@ -188,10 +188,10 @@ static pa_hook_result_t source_output_unlink_hook_cb(pa_core *c, pa_source_outpu
return PA_HOOK_OK;
if (s->source->monitor_of) {
- if (pa_sink_check_suspend(s->source->monitor_of) <= 0)
+ if (pa_sink_check_suspend(s->source->monitor_of, NULL, s) <= 0)
d = pa_hashmap_get(u->device_infos, s->source->monitor_of);
} else {
- if (pa_source_check_suspend(s->source) <= 0)
+ if (pa_source_check_suspend(s->source, s) <= 0)
d = pa_hashmap_get(u->device_infos, s->source);
}
@@ -208,7 +208,7 @@ static pa_hook_result_t sink_input_move_start_hook_cb(pa_core *c, pa_sink_input
pa_sink_input_assert_ref(s);
pa_assert(u);
- if (pa_sink_check_suspend(s->sink) <= 1)
+ if (pa_sink_check_suspend(s->sink, s, NULL) <= 0)
if ((d = pa_hashmap_get(u->device_infos, s->sink)))
restart(d);
@@ -241,10 +241,10 @@ static pa_hook_result_t source_output_move_start_hook_cb(pa_core *c, pa_source_o
pa_assert(u);
if (s->source->monitor_of) {
- if (pa_sink_check_suspend(s->source->monitor_of) <= 1)
+ if (pa_sink_check_suspend(s->source->monitor_of, NULL, s) <= 0)
d = pa_hashmap_get(u->device_infos, s->source->monitor_of);
} else {
- if (pa_source_check_suspend(s->source) <= 1)
+ if (pa_source_check_suspend(s->source, s) <= 0)
d = pa_hashmap_get(u->device_infos, s->source);
}
@@ -354,8 +354,8 @@ static pa_hook_result_t device_new_hook_cb(pa_core *c, pa_object *o, struct user
pa_hashmap_put(u->device_infos, o, d);
- if ((d->sink && pa_sink_check_suspend(d->sink) <= 0) ||
- (d->source && pa_source_check_suspend(d->source) <= 0))
+ if ((d->sink && pa_sink_check_suspend(d->sink, NULL, NULL) <= 0) ||
+ (d->source && pa_source_check_suspend(d->source, NULL) <= 0))
restart(d);
return PA_HOOK_OK;
@@ -398,7 +398,7 @@ static pa_hook_result_t device_state_changed_hook_cb(pa_core *c, pa_object *o, s
pa_sink *s = PA_SINK(o);
pa_sink_state_t state = pa_sink_get_state(s);
- if (pa_sink_check_suspend(s) <= 0)
+ if (pa_sink_check_suspend(s, NULL, NULL) <= 0)
if (PA_SINK_IS_OPENED(state))
restart(d);
@@ -406,7 +406,7 @@ static pa_hook_result_t device_state_changed_hook_cb(pa_core *c, pa_object *o, s
pa_source *s = PA_SOURCE(o);
pa_source_state_t state = pa_source_get_state(s);
- if (pa_source_check_suspend(s) <= 0)
+ if (pa_source_check_suspend(s, NULL) <= 0)
if (PA_SOURCE_IS_OPENED(state))
restart(d);
}
@@ -454,8 +454,8 @@ int pa__init(pa_module*m) {
pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_INPUT_FIXATE], PA_HOOK_NORMAL, (pa_hook_cb_t) sink_input_fixate_hook_cb, u);
pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_FIXATE], PA_HOOK_NORMAL, (pa_hook_cb_t) source_output_fixate_hook_cb, u);
- pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_INPUT_UNLINK_POST], PA_HOOK_NORMAL, (pa_hook_cb_t) sink_input_unlink_hook_cb, u);
- pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_UNLINK_POST], PA_HOOK_NORMAL, (pa_hook_cb_t) source_output_unlink_hook_cb, u);
+ pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_INPUT_UNLINK], PA_HOOK_NORMAL, (pa_hook_cb_t) sink_input_unlink_hook_cb, u);
+ pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_UNLINK], PA_HOOK_NORMAL, (pa_hook_cb_t) source_output_unlink_hook_cb, u);
pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_INPUT_MOVE_START], PA_HOOK_NORMAL, (pa_hook_cb_t) sink_input_move_start_hook_cb, u);
pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_MOVE_START], PA_HOOK_NORMAL, (pa_hook_cb_t) source_output_move_start_hook_cb, u);
pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_INPUT_MOVE_FINISH], PA_HOOK_NORMAL, (pa_hook_cb_t) sink_input_move_finish_hook_cb, u);
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 5c6a9c6..a745f09 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -2381,7 +2381,7 @@ unsigned pa_sink_used_by(pa_sink *s) {
}
/* Called from main thread */
-unsigned pa_sink_check_suspend(pa_sink *s) {
+unsigned pa_sink_check_suspend(pa_sink *s, pa_sink_input *ignore_input, pa_source_output *ignore_output) {
unsigned ret;
pa_sink_input *i;
uint32_t idx;
@@ -2397,6 +2397,9 @@ unsigned pa_sink_check_suspend(pa_sink *s) {
PA_IDXSET_FOREACH(i, s->inputs, idx) {
pa_sink_input_state_t st;
+ if (i == ignore_input)
+ continue;
+
st = pa_sink_input_get_state(i);
/* We do not assert here. It is perfectly valid for a sink input to
@@ -2417,7 +2420,7 @@ unsigned pa_sink_check_suspend(pa_sink *s) {
}
if (s->monitor_source)
- ret += pa_source_check_suspend(s->monitor_source);
+ ret += pa_source_check_suspend(s->monitor_source, ignore_output);
return ret;
}
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index c549869..bc2a200 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -466,7 +466,14 @@ void pa_sink_set_mixer_dirty(pa_sink *s, bool is_dirty);
unsigned pa_sink_linked_by(pa_sink *s); /* Number of connected streams */
unsigned pa_sink_used_by(pa_sink *s); /* Number of connected streams which are not corked */
-unsigned pa_sink_check_suspend(pa_sink *s); /* Returns how many streams are active that don't allow suspensions */
+
+/* Returns how many streams are active that don't allow suspensions. If
+ * "ignore_input" or "ignore_output" is non-NULL, that stream is not included
+ * in the count (the returned count includes the value from
+ * pa_source_check_suspend(), which is called for the monitor source, so that's
+ * why "ignore_output" may be relevant). */
+unsigned pa_sink_check_suspend(pa_sink *s, pa_sink_input *ignore_input, pa_source_output *ignore_output);
+
#define pa_sink_get_state(s) ((s)->state)
/* Moves all inputs away, and stores them in pa_queue */
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index 84f8428..4615702 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -1943,7 +1943,7 @@ unsigned pa_source_used_by(pa_source *s) {
}
/* Called from main thread */
-unsigned pa_source_check_suspend(pa_source *s) {
+unsigned pa_source_check_suspend(pa_source *s, pa_source_output *ignore) {
unsigned ret;
pa_source_output *o;
uint32_t idx;
@@ -1959,6 +1959,9 @@ unsigned pa_source_check_suspend(pa_source *s) {
PA_IDXSET_FOREACH(o, s->outputs, idx) {
pa_source_output_state_t st;
+ if (o == ignore)
+ continue;
+
st = pa_source_output_get_state(o);
/* We do not assert here. It is perfectly valid for a source output to
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index 3a6b5c3..e5b9333 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -400,7 +400,11 @@ 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 */
-unsigned pa_source_check_suspend(pa_source *s); /* Returns how many streams are active that don't allow suspensions */
+
+/* Returns how many streams are active that don't allow suspensions. If
+ * "ignore" is non-NULL, that stream is not included in the count. */
+unsigned pa_source_check_suspend(pa_source *s, pa_source_output *ignore);
+
#define pa_source_get_state(s) ((pa_source_state_t) (s)->state)
/* Moves all inputs away, and stores them in pa_queue */
--
2.9.3
More information about the pulseaudio-discuss
mailing list