This allows us to restore the default device properly when a hotpluggable device (e.g. a USB sound card) is set as the default, but unplugged temporarily. Previously we would forget that the unplugged device was ever set as the default, because we had to set configured_default_sink to NULL to avoid having a stale pa_sink pointer, and also because module-default-device-restore couldn't resolve the name of a currently-unplugged device to a pa_sink pointer. BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=89934 --- src/modules/dbus/iface-core.c | 4 +-- src/modules/module-default-device-restore.c | 30 +++++++++---------- src/modules/module-switch-on-connect.c | 8 ++--- src/pulsecore/cli-command.c | 4 +-- src/pulsecore/core.c | 46 ++++++++++++++++++----------- src/pulsecore/core.h | 13 ++++---- src/pulsecore/protocol-native.c | 4 +-- src/pulsecore/sink.c | 5 +--- src/pulsecore/source.c | 5 +--- 9 files changed, 63 insertions(+), 56 deletions(-) diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c index 3f368ab46..7177455bf 100644 --- a/src/modules/dbus/iface-core.c +++ b/src/modules/dbus/iface-core.c @@ -721,7 +721,7 @@ static void handle_set_fallback_sink(DBusConnection *conn, DBusMessage *msg, DBu return; } - pa_core_set_configured_default_sink(c->core, pa_dbusiface_device_get_sink(fallback_sink)); + pa_core_set_configured_default_sink(c->core, pa_dbusiface_device_get_sink(fallback_sink)->name); pa_dbus_send_empty_reply(conn, msg); } @@ -809,7 +809,7 @@ static void handle_set_fallback_source(DBusConnection *conn, DBusMessage *msg, D return; } - pa_core_set_configured_default_source(c->core, pa_dbusiface_device_get_source(fallback_source)); + pa_core_set_configured_default_source(c->core, pa_dbusiface_device_get_source(fallback_source)->name); pa_dbus_send_empty_reply(conn, msg); } diff --git a/src/modules/module-default-device-restore.c b/src/modules/module-default-device-restore.c index 56fee67f8..adf19ff94 100644 --- a/src/modules/module-default-device-restore.c +++ b/src/modules/module-default-device-restore.c @@ -60,7 +60,6 @@ static void load(struct userdata *u) { pa_log_info("Manually configured default sink, not overwriting."); else if ((f = pa_fopen_cloexec(u->sink_filename, "r"))) { char ln[256] = ""; - pa_sink *s; if (fgets(ln, sizeof(ln)-1, f)) pa_strip_nl(ln); @@ -68,11 +67,12 @@ static void load(struct userdata *u) { if (!ln[0]) pa_log_info("No previous default sink setting, ignoring."); - else if ((s = pa_namereg_get(u->core, ln, PA_NAMEREG_SINK))) { - pa_core_set_configured_default_sink(u->core, s); - pa_log_info("Restored default sink '%s'.", ln); - } else - pa_log_info("Saved default sink '%s' not existent, not restoring default sink setting.", ln); + else if (!pa_namereg_is_valid_name(ln)) + pa_log_warn("Invalid sink name: %s", ln); + else { + pa_log_info("Restoring default sink '%s'.", ln); + pa_core_set_configured_default_sink(u->core, ln); + } } else if (errno != ENOENT) pa_log("Failed to load default sink: %s", pa_cstrerror(errno)); @@ -81,7 +81,6 @@ static void load(struct userdata *u) { pa_log_info("Manually configured default source, not overwriting."); else if ((f = pa_fopen_cloexec(u->source_filename, "r"))) { char ln[256] = ""; - pa_source *s; if (fgets(ln, sizeof(ln)-1, f)) pa_strip_nl(ln); @@ -89,14 +88,15 @@ static void load(struct userdata *u) { if (!ln[0]) pa_log_info("No previous default source setting, ignoring."); - else if ((s = pa_namereg_get(u->core, ln, PA_NAMEREG_SOURCE))) { - pa_core_set_configured_default_source(u->core, s); - pa_log_info("Restored default source '%s'.", ln); - } else - pa_log_info("Saved default source '%s' not existent, not restoring default source setting.", ln); + else if (!pa_namereg_is_valid_name(ln)) + pa_log_warn("Invalid source name: %s", ln); + else { + pa_log_info("Restoring default source '%s'.", ln); + pa_core_set_configured_default_source(u->core, ln); + } } else if (errno != ENOENT) - pa_log("Failed to load default sink: %s", pa_cstrerror(errno)); + pa_log("Failed to load default source: %s", pa_cstrerror(errno)); } static void save(struct userdata *u) { @@ -107,7 +107,7 @@ static void save(struct userdata *u) { if (u->sink_filename) { if ((f = pa_fopen_cloexec(u->sink_filename, "w"))) { - fprintf(f, "%s\n", u->core->default_sink ? u->core->default_sink->name : ""); + fprintf(f, "%s\n", u->core->configured_default_sink ? u->core->configured_default_sink : ""); fclose(f); } else pa_log("Failed to save default sink: %s", pa_cstrerror(errno)); @@ -115,7 +115,7 @@ static void save(struct userdata *u) { if (u->source_filename) { if ((f = pa_fopen_cloexec(u->source_filename, "w"))) { - fprintf(f, "%s\n", u->core->default_source ? u->core->default_source->name : ""); + fprintf(f, "%s\n", u->core->configured_default_source ? u->core->configured_default_source : ""); fclose(f); } else pa_log("Failed to save default source: %s", pa_cstrerror(errno)); diff --git a/src/modules/module-switch-on-connect.c b/src/modules/module-switch-on-connect.c index e2da7222f..640024e95 100644 --- a/src/modules/module-switch-on-connect.c +++ b/src/modules/module-switch-on-connect.c @@ -77,7 +77,7 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void* /* No default sink, nothing to move away, just set the new default */ if (!c->default_sink) { - pa_core_set_configured_default_sink(c, sink); + pa_core_set_configured_default_sink(c, sink->name); return PA_HOOK_OK; } @@ -91,7 +91,7 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void* old_default_sink = c->default_sink; /* Actually do the switch to the new sink */ - pa_core_set_configured_default_sink(c, sink); + pa_core_set_configured_default_sink(c, sink->name); /* Now move all old inputs over */ if (pa_idxset_size(old_default_sink->inputs) <= 0) { @@ -143,7 +143,7 @@ static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source, /* No default source, nothing to move away, just set the new default */ if (!c->default_source) { - pa_core_set_configured_default_source(c, source); + pa_core_set_configured_default_source(c, source->name); return PA_HOOK_OK; } @@ -157,7 +157,7 @@ static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source, old_default_source = c->default_source; /* Actually do the switch to the new source */ - pa_core_set_configured_default_source(c, source); + pa_core_set_configured_default_source(c, source->name); /* Now move all old outputs over */ if (pa_idxset_size(old_default_source->outputs) <= 0) { diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c index 5a632be45..01fea475b 100644 --- a/src/pulsecore/cli-command.c +++ b/src/pulsecore/cli-command.c @@ -1030,7 +1030,7 @@ static int pa_cli_command_sink_default(pa_core *c, pa_tokenizer *t, pa_strbuf *b } if ((s = pa_namereg_get(c, n, PA_NAMEREG_SINK))) - pa_core_set_configured_default_sink(c, s); + pa_core_set_configured_default_sink(c, s->name); else pa_strbuf_printf(buf, "Sink %s does not exist.\n", n); @@ -1052,7 +1052,7 @@ static int pa_cli_command_source_default(pa_core *c, pa_tokenizer *t, pa_strbuf } if ((s = pa_namereg_get(c, n, PA_NAMEREG_SOURCE))) - pa_core_set_configured_default_source(c, s); + pa_core_set_configured_default_source(c, s->name); else pa_strbuf_printf(buf, "Source %s does not exist.\n", n); return 0; diff --git a/src/pulsecore/core.c b/src/pulsecore/core.c index 52e51db1a..e01677d5d 100644 --- a/src/pulsecore/core.c +++ b/src/pulsecore/core.c @@ -214,6 +214,8 @@ static void core_free(pa_object *o) { pa_assert(!c->default_source); pa_assert(!c->default_sink); + pa_xfree(c->configured_default_source); + pa_xfree(c->configured_default_sink); pa_silence_cache_done(&c->silence_cache); pa_mempool_unref(c->mempool); @@ -224,38 +226,46 @@ static void core_free(pa_object *o) { pa_xfree(c); } -void pa_core_set_configured_default_sink(pa_core *core, pa_sink *sink) { - pa_sink *old_sink; +void pa_core_set_configured_default_sink(pa_core *core, const char *sink) { + char *old_sink; pa_assert(core); - old_sink = core->configured_default_sink; + old_sink = pa_xstrdup(core->configured_default_sink); - if (sink == old_sink) - return; + if (pa_safe_streq(sink, old_sink)) + goto finish; - core->configured_default_sink = sink; + pa_xfree(core->configured_default_sink); + core->configured_default_sink = pa_xstrdup(sink); pa_log_info("configured_default_sink: %s -> %s", - old_sink ? old_sink->name : "(unset)", sink ? sink->name : "(unset)"); + old_sink ? old_sink : "(unset)", sink ? sink : "(unset)"); pa_core_update_default_sink(core); + +finish: + pa_xfree(old_sink); } -void pa_core_set_configured_default_source(pa_core *core, pa_source *source) { - pa_source *old_source; +void pa_core_set_configured_default_source(pa_core *core, const char *source) { + char *old_source; pa_assert(core); - old_source = core->configured_default_source; + old_source = pa_xstrdup(core->configured_default_source); - if (source == old_source) - return; + if (pa_safe_streq(source, old_source)) + goto finish; - core->configured_default_source = source; + pa_xfree(core->configured_default_source); + core->configured_default_source = pa_xstrdup(source); pa_log_info("configured_default_source: %s -> %s", - old_source ? old_source->name : "(unset)", source ? source->name : "(unset)"); + old_source ? old_source : "(unset)", source ? source : "(unset)"); pa_core_update_default_source(core); + +finish: + pa_xfree(old_source); } /* a < b -> return -1 @@ -275,9 +285,9 @@ static int compare_sinks(pa_sink *a, pa_sink *b) { return 1; /* The configured default sink is preferred over any other sink. */ - if (b == core->configured_default_sink) + if (pa_safe_streq(b->name, core->configured_default_sink)) return -1; - if (a == core->configured_default_sink) + if (pa_safe_streq(a->name, core->configured_default_sink)) return 1; if (a->priority < b->priority) @@ -349,9 +359,9 @@ static int compare_sources(pa_source *a, pa_source *b) { return 1; /* The configured default source is preferred over any other source. */ - if (b == core->configured_default_source) + if (pa_safe_streq(b->name, core->configured_default_source)) return -1; - if (a == core->configured_default_source) + if (pa_safe_streq(a->name, core->configured_default_source)) return 1; /* Monitor sources lose to non-monitor sources. */ diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h index 8af14b047..79a095d24 100644 --- a/src/pulsecore/core.h +++ b/src/pulsecore/core.h @@ -163,9 +163,12 @@ struct pa_core { pa_hashmap *namereg, *shared; /* The default sink/source as configured by the user. If the user hasn't - * explicitly configured anything, these are set to NULL. */ - pa_sink *configured_default_sink; - pa_source *configured_default_source; + * explicitly configured anything, these are set to NULL. These are strings + * instead of sink/source pointers, because that allows us to reference + * devices that don't currently exist. That's useful for remembering that + * a hotplugged USB sink was previously set as the default sink. */ + char *configured_default_sink; + char *configured_default_source; /* The effective default sink/source. If no sink or source is explicitly * configured as the default, we pick the device that ranks highest @@ -236,8 +239,8 @@ enum { pa_core* pa_core_new(pa_mainloop_api *m, bool shared, bool enable_memfd, size_t shm_size); -void pa_core_set_configured_default_sink(pa_core *core, pa_sink *sink); -void pa_core_set_configured_default_source(pa_core *core, pa_source *source); +void pa_core_set_configured_default_sink(pa_core *core, const char *sink); +void pa_core_set_configured_default_source(pa_core *core, const char *source); /* These should be called whenever something changes that may affect the * default sink or source choice. diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c index 266b676de..068e0c00e 100644 --- a/src/pulsecore/protocol-native.c +++ b/src/pulsecore/protocol-native.c @@ -4346,7 +4346,7 @@ static void command_set_default_sink_or_source(pa_pdispatch *pd, uint32_t comman source = pa_namereg_get(c->protocol->core, s, PA_NAMEREG_SOURCE); CHECK_VALIDITY(c->pstream, source, tag, PA_ERR_NOENTITY); - pa_core_set_configured_default_source(c->protocol->core, source); + pa_core_set_configured_default_source(c->protocol->core, source->name); } else { pa_sink *sink; pa_assert(command == PA_COMMAND_SET_DEFAULT_SINK); @@ -4354,7 +4354,7 @@ static void command_set_default_sink_or_source(pa_pdispatch *pd, uint32_t comman sink = pa_namereg_get(c->protocol->core, s, PA_NAMEREG_SINK); CHECK_VALIDITY(c->pstream, sink, tag, PA_ERR_NOENTITY); - pa_core_set_configured_default_sink(c->protocol->core, sink); + pa_core_set_configured_default_sink(c->protocol->core, sink->name); } pa_pstream_send_simple_ack(c->pstream, tag); diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c index 39463bd26..a8b4cd3da 100644 --- a/src/pulsecore/sink.c +++ b/src/pulsecore/sink.c @@ -694,10 +694,7 @@ void pa_sink_unlink(pa_sink* s) { pa_namereg_unregister(s->core, s->name); pa_idxset_remove_by_data(s->core->sinks, s, NULL); - if (s == s->core->configured_default_sink) - pa_core_set_configured_default_sink(s->core, NULL); - else - pa_core_update_default_sink(s->core); + pa_core_update_default_sink(s->core); if (s->card) pa_idxset_remove_by_data(s->card->sinks, s, NULL); diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c index d56a13ddd..b2936c5ff 100644 --- a/src/pulsecore/source.c +++ b/src/pulsecore/source.c @@ -636,10 +636,7 @@ void pa_source_unlink(pa_source *s) { pa_namereg_unregister(s->core, s->name); pa_idxset_remove_by_data(s->core->sources, s, NULL); - if (s == s->core->configured_default_source) - pa_core_set_configured_default_source(s->core, NULL); - else - pa_core_update_default_source(s->core); + pa_core_update_default_source(s->core); if (s->card) pa_idxset_remove_by_data(s->card->sources, s, NULL); -- 2.11.0
