From c3393d27a5b18bca3e052d54a734d5cfb9e0aea0 Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Wed, 12 Oct 2016 17:20:40 +0300 Subject: [PATCH] suspend-on-idle: use earlier (safer) hooks for stream unlink notifications 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 f7620db04..a284f858e 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 5c6a9c630..a745f09b6 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 c549869c1..bc2a20030 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 84f8428eb..4615702bf 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 3a6b5c344..e5b933345 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 */