[PATCH] core: change configured_default_sink/source type to string

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux