[PATCH v2 2/5] When switching ports, use the port default volume

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

 




On 2015-04-17 22:26, Tanu Kaskinen wrote:
> If two ports have different default volumes, and module-device-restore
> doesn't have an opinion on the port volume, we should change the
> sink/source volume to the new port's default volume when switching
> ports, instead of keeping the volume at whatever it happened to be
> when the old port was active.
>
> This introduces pa_sink/source_port_changed_hook_data, because I
> wanted to call pa_sink/source_set_volume() only once in
> pa_sink/source_set_port(), instead of calling it also in
> module-device-restore's hook callback. The extended hook data allows
> module-device-restore to communicate to the core that the volume
> should be set to a non-default value.

Now that pa_device_port has a default volume, then module-device-restore 
should just set it for all ports at the card_new + PA_HOOK_EARLY 
callback. There is no need for module-device-restore to do anything at 
port_changed anymore; and thus no need to add the 
pa_*_port_changed_hook_data struct either, unless I'm missing something.

There's also the point that now volume and mute are handled differently, 
which I'm not really happy with.

> ---
>   src/modules/dbus/iface-device.c     |  4 ++--
>   src/modules/module-device-restore.c | 32 ++++++++++++++------------------
>   src/pulsecore/sink.c                |  8 +++++++-
>   src/pulsecore/sink.h                | 10 ++++++++++
>   src/pulsecore/source.c              |  8 +++++++-
>   src/pulsecore/source.h              | 10 ++++++++++
>   6 files changed, 50 insertions(+), 22 deletions(-)
>
> diff --git a/src/modules/dbus/iface-device.c b/src/modules/dbus/iface-device.c
> index 2c370a8..911890d 100644
> --- a/src/modules/dbus/iface-device.c
> +++ b/src/modules/dbus/iface-device.c
> @@ -1190,8 +1190,8 @@ static pa_hook_result_t port_changed_cb(void *hook_data, void *call_data, void *
>       DBusMessage *signal_msg = NULL;
>       pa_device_port *new_active_port = NULL;
>
> -    if ((d->type == PA_DEVICE_TYPE_SINK && d->sink != call_data) ||
> -        (d->type == PA_DEVICE_TYPE_SOURCE && d->source != call_data))
> +    if ((d->type == PA_DEVICE_TYPE_SINK && d->sink != ((pa_sink_port_changed_hook_data *) call_data)->sink) ||
> +        (d->type == PA_DEVICE_TYPE_SOURCE && d->source != ((pa_source_port_changed_hook_data *) call_data)->source))
>           return PA_HOOK_OK;
>
>       new_active_port = (d->type == PA_DEVICE_TYPE_SINK) ? d->sink->active_port : d->source->active_port;
> diff --git a/src/modules/module-device-restore.c b/src/modules/module-device-restore.c
> index 8d7b34b..ced2a14 100644
> --- a/src/modules/module-device-restore.c
> +++ b/src/modules/module-device-restore.c
> @@ -798,28 +798,26 @@ static pa_hook_result_t sink_fixate_hook_callback(pa_core *c, pa_sink_new_data *
>       return PA_HOOK_OK;
>   }
>
> -static pa_hook_result_t sink_port_hook_callback(pa_core *c, pa_sink *sink, struct userdata *u) {
> +static pa_hook_result_t sink_port_hook_callback(pa_core *c, pa_sink_port_changed_hook_data *data, struct userdata *u) {
> +    pa_sink *sink;
>       char *name;
>       struct perportentry *e;
>
>       pa_assert(c);
> -    pa_assert(sink);
> +    pa_assert(data);
>       pa_assert(u);
>       pa_assert(u->restore_volume || u->restore_muted);
>
> +    sink = data->sink;
>       name = pa_sprintf_malloc("sink:%s", sink->name);
>
>       if ((e = perportentry_read(u, name, (sink->active_port ? sink->active_port->name : NULL)))) {
>
>           if (u->restore_volume && e->volume_valid) {
> -            pa_cvolume v;
> -
>               pa_log_info("Restoring volume for sink %s.", sink->name);
> -            v = e->volume;
> -            pa_cvolume_remap(&v, &e->channel_map, &sink->channel_map);
> -            pa_sink_set_volume(sink, &v, true, false);
> -
> -            sink->save_volume = true;
> +            data->volume = e->volume;
> +            pa_cvolume_remap(&data->volume, &e->channel_map, &sink->channel_map);
> +            data->save_volume = true;
>           }
>
>           if (u->restore_muted && e->muted_valid) {
> @@ -940,28 +938,26 @@ static pa_hook_result_t source_fixate_hook_callback(pa_core *c, pa_source_new_da
>       return PA_HOOK_OK;
>   }
>
> -static pa_hook_result_t source_port_hook_callback(pa_core *c, pa_source *source, struct userdata *u) {
> +static pa_hook_result_t source_port_hook_callback(pa_core *c, pa_source_port_changed_hook_data *data, struct userdata *u) {
> +    pa_source *source;
>       char *name;
>       struct perportentry *e;
>
>       pa_assert(c);
> -    pa_assert(source);
> +    pa_assert(data);
>       pa_assert(u);
>       pa_assert(u->restore_volume || u->restore_muted);
>
> +    source = data->source;
>       name = pa_sprintf_malloc("source:%s", source->name);
>
>       if ((e = perportentry_read(u, name, (source->active_port ? source->active_port->name : NULL)))) {
>
>           if (u->restore_volume && e->volume_valid) {
> -            pa_cvolume v;
> -
>               pa_log_info("Restoring volume for source %s.", source->name);
> -            v = e->volume;
> -            pa_cvolume_remap(&v, &e->channel_map, &source->channel_map);
> -            pa_source_set_volume(source, &v, true, false);
> -
> -            source->save_volume = true;
> +            data->volume = e->volume;
> +            pa_cvolume_remap(&data->volume, &e->channel_map, &source->channel_map);
> +            data->save_volume = true;
>           }
>
>           if (u->restore_muted && e->muted_valid) {
> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index 3776a5a..652401d 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c
> @@ -3310,6 +3310,7 @@ size_t pa_sink_get_max_request(pa_sink *s) {
>   int pa_sink_set_port(pa_sink *s, const char *name, bool save) {
>       pa_device_port *port;
>       int ret;
> +    pa_sink_port_changed_hook_data hook_data;
>
>       pa_sink_assert_ref(s);
>       pa_assert_ctl_context();
> @@ -3350,7 +3351,12 @@ int pa_sink_set_port(pa_sink *s, const char *name, bool save) {
>
>       pa_sink_set_latency_offset(s, s->active_port->latency_offset);
>
> -    pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_PORT_CHANGED], s);
> +    hook_data.sink = s;
> +    pa_cvolume_set(&hook_data.volume, s->sample_spec.channels, port->default_volume);
> +    hook_data.save_volume = false;
> +    pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_PORT_CHANGED], &hook_data);
> +
> +    pa_sink_set_volume(s, &hook_data.volume, true, hook_data.save_volume);
>
>       return 0;
>   }
> diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
> index cb32782..4150c5a3 100644
> --- a/src/pulsecore/sink.h
> +++ b/src/pulsecore/sink.h
> @@ -337,6 +337,16 @@ typedef enum pa_sink_message {
>       PA_SINK_MESSAGE_MAX
>   } pa_sink_message_t;
>
> +typedef struct {
> +    pa_sink *sink;
> +
> +    /* This will be the new volume for the sink. If the hook callback wants to
> +     * set the sink volume, it should do that by setting these variables
> +     * instead of calling pa_sink_set_volume(). */
> +    pa_cvolume volume;
> +    bool save_volume;
> +} pa_sink_port_changed_hook_data;
> +
>   typedef struct pa_sink_new_data {
>       pa_suspend_cause_t suspend_cause;
>
> diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
> index b06e7fa..17dd9a1 100644
> --- a/src/pulsecore/source.c
> +++ b/src/pulsecore/source.c
> @@ -2589,6 +2589,7 @@ size_t pa_source_get_max_rewind(pa_source *s) {
>   int pa_source_set_port(pa_source *s, const char *name, bool save) {
>       pa_device_port *port;
>       int ret;
> +    pa_source_port_changed_hook_data hook_data;
>
>       pa_source_assert_ref(s);
>       pa_assert_ctl_context();
> @@ -2627,7 +2628,12 @@ int pa_source_set_port(pa_source *s, const char *name, bool save) {
>       s->active_port = port;
>       s->save_port = save;
>
> -    pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SOURCE_PORT_CHANGED], s);
> +    hook_data.source = s;
> +    pa_cvolume_set(&hook_data.volume, s->sample_spec.channels, port->default_volume);
> +    hook_data.save_volume = false;
> +    pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SOURCE_PORT_CHANGED], &hook_data);
> +
> +    pa_source_set_volume(s, &hook_data.volume, true, hook_data.save_volume);
>
>       return 0;
>   }
> diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
> index 7555a9f..b6e9d0e 100644
> --- a/src/pulsecore/source.h
> +++ b/src/pulsecore/source.h
> @@ -274,6 +274,16 @@ typedef enum pa_source_message {
>       PA_SOURCE_MESSAGE_MAX
>   } pa_source_message_t;
>
> +typedef struct {
> +    pa_source *source;
> +
> +    /* This will be the new volume for the source. If the hook callback wants
> +     * to set the source volume, it should do that by setting these variables
> +     * instead of calling pa_source_set_volume(). */
> +    pa_cvolume volume;
> +    bool save_volume;
> +} pa_source_port_changed_hook_data;
> +
>   typedef struct pa_source_new_data {
>       pa_suspend_cause_t suspend_cause;
>
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic


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

  Powered by Linux