[pulseaudio-discuss] [PATCH] core: change configured_default_sink/source type to string
Arun Raghavan
arun at arunraghavan.net
Sat Jul 15 05:45:38 UTC 2017
On Fri, 30 Jun 2017, at 02:39 AM, Tanu Kaskinen wrote:
> 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)");
This is okay for now, but we should probably make this more concise in
the future with something like pa_str_safe() or something which returns
the string or a default value on null.
Looks good, otherwise.
-- Arun
More information about the pulseaudio-discuss
mailing list