From 7ac850d3b7ce803044b58a357b4e27730cf53bc7 Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Tue, 15 Apr 2014 13:56:04 +0300 Subject: [PATCH] sink-input, source-output: Assign to volume from only one place Forcing all volume changes to go through set_volume_direct() makes it easier to check where the stream volume is changed, and it also allows us to have only one place where notifications for changed volume are sent. --- src/pulsecore/sink-input.c | 69 +++++++++++++++++------------------ src/pulsecore/sink-input.h | 7 ++++ src/pulsecore/sink.c | 53 +++++++-------------------- src/pulsecore/source-output.c | 62 ++++++++++++++++--------------- src/pulsecore/source-output.h | 7 ++++ src/pulsecore/source.c | 54 +++++++-------------------- 6 files changed, 108 insertions(+), 144 deletions(-) diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c index 1754a8a1..451ca634 100644 --- a/src/pulsecore/sink-input.c +++ b/src/pulsecore/sink-input.c @@ -1260,7 +1260,7 @@ void pa_sink_input_set_volume(pa_sink_input *i, const pa_cvolume *volume, bool s return; } - i->volume = *volume; + pa_sink_input_set_volume_direct(i, volume); i->save_volume = save; if (pa_sink_flat_volume_enabled(i->sink)) { @@ -1278,13 +1278,6 @@ void pa_sink_input_set_volume(pa_sink_input *i, const pa_cvolume *volume, bool s /* Copy the new soft_volume to the thread_info struct */ pa_assert_se(pa_asyncmsgq_send(i->sink->asyncmsgq, PA_MSGOBJECT(i), PA_SINK_INPUT_MESSAGE_SET_SOFT_VOLUME, NULL, 0, NULL) == 0); } - - /* The volume changed, let's tell people so */ - if (i->volume_changed) - i->volume_changed(i); - - /* The virtual volume changed, let's tell people so */ - pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index); } void pa_sink_input_add_volume_factor(pa_sink_input *i, const char *key, const pa_cvolume *volume_factor) { @@ -1631,7 +1624,6 @@ int pa_sink_input_start_move(pa_sink_input *i) { * then also the origin sink and all streams connected to it need to update * their volume - this function does all that by using recursion. */ static void update_volume_due_to_moving(pa_sink_input *i, pa_sink *dest) { - pa_cvolume old_volume; pa_cvolume new_volume; pa_assert(i); @@ -1681,19 +1673,11 @@ static void update_volume_due_to_moving(pa_sink_input *i, pa_sink *dest) { * always have volume_factor as soft_volume, so no change * should be needed) */ - old_volume = i->volume; - pa_cvolume_reset(&i->volume, i->volume.channels); + pa_cvolume_reset(&new_volume, i->volume.channels); + pa_sink_input_set_volume_direct(i, &new_volume); pa_cvolume_reset(&i->reference_ratio, i->reference_ratio.channels); pa_assert(pa_cvolume_is_norm(&i->real_ratio)); pa_assert(pa_cvolume_equal(&i->soft_volume, &i->volume_factor)); - - /* Notify others about the changed sink input volume. */ - if (!pa_cvolume_equal(&i->volume, &old_volume)) { - if (i->volume_changed) - i->volume_changed(i); - - pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index); - } } /* Additionally, the origin sink volume needs updating: @@ -1725,8 +1709,6 @@ static void update_volume_due_to_moving(pa_sink_input *i, pa_sink *dest) { update_volume_due_to_moving(origin_sink_input, dest); } else { - old_volume = i->volume; - if (pa_sink_flat_volume_enabled(i->sink)) { /* Ok, so this is a regular stream, and flat volume is enabled. The * volume will have to be updated as follows: @@ -1738,9 +1720,10 @@ static void update_volume_due_to_moving(pa_sink_input *i, pa_sink *dest) { * i->soft_volume := i->real_ratio * i->volume_factor * (handled later by pa_sink_set_volume) */ - i->volume = i->sink->reference_volume; - pa_cvolume_remap(&i->volume, &i->sink->channel_map, &i->channel_map); - pa_sw_cvolume_multiply(&i->volume, &i->volume, &i->reference_ratio); + new_volume = i->sink->reference_volume; + pa_cvolume_remap(&new_volume, &i->sink->channel_map, &i->channel_map); + pa_sw_cvolume_multiply(&new_volume, &new_volume, &i->reference_ratio); + pa_sink_input_set_volume_direct(i, &new_volume); } else { /* Ok, so this is a regular stream, and flat volume is disabled. @@ -1751,21 +1734,10 @@ static void update_volume_due_to_moving(pa_sink_input *i, pa_sink *dest) { * i->real_ratio := i->reference_ratio * i->soft_volume := i->real_ratio * i->volume_factor */ - i->volume = i->reference_ratio; + pa_sink_input_set_volume_direct(i, &i->reference_ratio); i->real_ratio = i->reference_ratio; pa_sw_cvolume_multiply(&i->soft_volume, &i->real_ratio, &i->volume_factor); } - - /* Notify others about the changed sink input volume. */ - if (!pa_cvolume_equal(&i->volume, &old_volume)) { - /* XXX: In case i->sink has flat volume enabled, then real_ratio - * and soft_volume are not updated yet. Let's hope that the - * callback implementation doesn't care about those variables... */ - if (i->volume_changed) - i->volume_changed(i); - - pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index); - } } /* If i->sink == dest, then recursion has finished, and we can finally call @@ -2242,3 +2214,28 @@ int pa_sink_input_update_rate(pa_sink_input *i) { return 0; } + +/* Called from the main thread. */ +void pa_sink_input_set_volume_direct(pa_sink_input *i, const pa_cvolume *volume) { + pa_cvolume old_volume; + char old_volume_str[PA_CVOLUME_SNPRINT_VERBOSE_MAX]; + char new_volume_str[PA_CVOLUME_SNPRINT_VERBOSE_MAX]; + + pa_assert(i); + pa_assert(volume); + + old_volume = i->volume; + + if (pa_cvolume_equal(volume, &old_volume)) + return; + + i->volume = *volume; + pa_log_debug("The volume of sink input %u changed from %s to %s.", i->index, + pa_cvolume_snprint_verbose(old_volume_str, sizeof(old_volume_str), &old_volume, &i->channel_map, true), + pa_cvolume_snprint_verbose(new_volume_str, sizeof(new_volume_str), volume, &i->channel_map, true)); + + if (i->volume_changed) + i->volume_changed(i); + + pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index); +} diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h index da337174..2be2c33e 100644 --- a/src/pulsecore/sink-input.h +++ b/src/pulsecore/sink-input.h @@ -417,6 +417,13 @@ bool pa_sink_input_process_underrun(pa_sink_input *i); pa_memchunk* pa_sink_input_get_silence(pa_sink_input *i, pa_memchunk *ret); +/* Called from the main thread, from sink.c only. The normal way to set the + * sink input volume is to call pa_sink_input_set_volume(), but the flat volume + * logic in sink.c needs also a function that doesn't do all the extra stuff + * that pa_sink_input_set_volume() does. This function simply sets i->volume + * and fires change notifications. */ +void pa_sink_input_set_volume_direct(pa_sink_input *i, const pa_cvolume *volume); + #define pa_sink_input_assert_io_context(s) \ pa_assert(pa_thread_mq_get() || !PA_SINK_INPUT_IS_LINKED((s)->state)) diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c index c1cd2db4..7dffedf8 100644 --- a/src/pulsecore/sink.c +++ b/src/pulsecore/sink.c @@ -1835,20 +1835,13 @@ static void update_real_volume(pa_sink *s, const pa_cvolume *new_volume, pa_chan PA_IDXSET_FOREACH(i, s->inputs, idx) { if (i->origin_sink && (i->origin_sink->flags & PA_SINK_SHARE_VOLUME_WITH_MASTER)) { if (pa_sink_flat_volume_enabled(s)) { - pa_cvolume old_volume = i->volume; + pa_cvolume new_input_volume; /* Follow the root sink's real volume. */ - i->volume = *new_volume; - pa_cvolume_remap(&i->volume, channel_map, &i->channel_map); + new_input_volume = *new_volume; + pa_cvolume_remap(&new_input_volume, channel_map, &i->channel_map); + pa_sink_input_set_volume_direct(i, &new_input_volume); compute_reference_ratio(i); - - /* The volume changed, let's tell people so */ - if (!pa_cvolume_equal(&old_volume, &i->volume)) { - if (i->volume_changed) - i->volume_changed(i); - - pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index); - } } update_real_volume(i->origin_sink, new_volume, channel_map); @@ -1903,7 +1896,7 @@ static void propagate_reference_volume(pa_sink *s) { * sink input volumes accordingly */ PA_IDXSET_FOREACH(i, s->inputs, idx) { - pa_cvolume old_volume; + pa_cvolume new_volume; if (i->origin_sink && (i->origin_sink->flags & PA_SINK_SHARE_VOLUME_WITH_MASTER)) { propagate_reference_volume(i->origin_sink); @@ -1914,24 +1907,14 @@ static void propagate_reference_volume(pa_sink *s) { continue; } - old_volume = i->volume; - /* This basically calculates: * * i->volume := s->reference_volume * i->reference_ratio */ - i->volume = s->reference_volume; - pa_cvolume_remap(&i->volume, &s->channel_map, &i->channel_map); - pa_sw_cvolume_multiply(&i->volume, &i->volume, &i->reference_ratio); - - /* The volume changed, let's tell people so */ - if (!pa_cvolume_equal(&old_volume, &i->volume)) { - - if (i->volume_changed) - i->volume_changed(i); - - pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index); - } + new_volume = s->reference_volume; + pa_cvolume_remap(&new_volume, &s->channel_map, &i->channel_map); + pa_sw_cvolume_multiply(&new_volume, &new_volume, &i->reference_ratio); + pa_sink_input_set_volume_direct(i, &new_volume); } } @@ -2126,7 +2109,7 @@ static void propagate_real_volume(pa_sink *s, const pa_cvolume *old_real_volume) if (pa_sink_flat_volume_enabled(s)) { PA_IDXSET_FOREACH(i, s->inputs, idx) { - pa_cvolume old_volume = i->volume; + pa_cvolume new_volume; /* 2. Since the sink's reference and real volumes are equal * now our ratios should be too. */ @@ -2140,18 +2123,10 @@ static void propagate_real_volume(pa_sink *s, const pa_cvolume *old_real_volume) * i->volume = s->reference_volume * i->reference_ratio * * This is identical to propagate_reference_volume() */ - i->volume = s->reference_volume; - pa_cvolume_remap(&i->volume, &s->channel_map, &i->channel_map); - pa_sw_cvolume_multiply(&i->volume, &i->volume, &i->reference_ratio); - - /* Notify if something changed */ - if (!pa_cvolume_equal(&old_volume, &i->volume)) { - - if (i->volume_changed) - i->volume_changed(i); - - pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index); - } + new_volume = s->reference_volume; + pa_cvolume_remap(&new_volume, &s->channel_map, &i->channel_map); + pa_sw_cvolume_multiply(&new_volume, &new_volume, &i->reference_ratio); + pa_sink_input_set_volume_direct(i, &new_volume); if (i->origin_sink && (i->origin_sink->flags & PA_SINK_SHARE_VOLUME_WITH_MASTER)) propagate_real_volume(i->origin_sink, old_real_volume); diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c index 4323bcf8..9709d77e 100644 --- a/src/pulsecore/source-output.c +++ b/src/pulsecore/source-output.c @@ -972,7 +972,7 @@ void pa_source_output_set_volume(pa_source_output *o, const pa_cvolume *volume, return; } - o->volume = *volume; + pa_source_output_set_volume_direct(o, volume); o->save_volume = save; if (pa_source_flat_volume_enabled(o->source)) { @@ -1262,7 +1262,6 @@ int pa_source_output_start_move(pa_source_output *o) { * then also the origin source and all streams connected to it need to update * their volume - this function does all that by using recursion. */ static void update_volume_due_to_moving(pa_source_output *o, pa_source *dest) { - pa_cvolume old_volume; pa_cvolume new_volume; pa_assert(o); @@ -1314,19 +1313,11 @@ static void update_volume_due_to_moving(pa_source_output *o, pa_source *dest) { * always have volume_factor as soft_volume, so no change * should be needed) */ - old_volume = o->volume; - pa_cvolume_reset(&o->volume, o->volume.channels); + pa_cvolume_reset(&new_volume, o->volume.channels); + pa_source_output_set_volume_direct(o, &new_volume); pa_cvolume_reset(&o->reference_ratio, o->reference_ratio.channels); pa_assert(pa_cvolume_is_norm(&o->real_ratio)); pa_assert(pa_cvolume_equal(&o->soft_volume, &o->volume_factor)); - - /* Notify others about the changed source output volume. */ - if (!pa_cvolume_equal(&o->volume, &old_volume)) { - if (o->volume_changed) - o->volume_changed(o); - - pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index); - } } /* Additionally, the origin source volume needs updating: @@ -1358,8 +1349,6 @@ static void update_volume_due_to_moving(pa_source_output *o, pa_source *dest) { update_volume_due_to_moving(destination_source_output, dest); } else { - old_volume = o->volume; - if (pa_source_flat_volume_enabled(o->source)) { /* Ok, so this is a regular stream, and flat volume is enabled. The * volume will have to be updated as follows: @@ -1371,9 +1360,10 @@ static void update_volume_due_to_moving(pa_source_output *o, pa_source *dest) { * o->soft_volume := o->real_ratio * o->volume_factor * (handled later by pa_source_set_volume) */ - o->volume = o->source->reference_volume; - pa_cvolume_remap(&o->volume, &o->source->channel_map, &o->channel_map); - pa_sw_cvolume_multiply(&o->volume, &o->volume, &o->reference_ratio); + new_volume = o->source->reference_volume; + pa_cvolume_remap(&new_volume, &o->source->channel_map, &o->channel_map); + pa_sw_cvolume_multiply(&new_volume, &new_volume, &o->reference_ratio); + pa_source_output_set_volume_direct(o, &new_volume); } else { /* Ok, so this is a regular stream, and flat volume is disabled. @@ -1384,21 +1374,10 @@ static void update_volume_due_to_moving(pa_source_output *o, pa_source *dest) { * o->real_ratio := o->reference_ratio * o->soft_volume := o->real_ratio * o->volume_factor */ - o->volume = o->reference_ratio; + pa_source_output_set_volume_direct(o, &o->reference_ratio); o->real_ratio = o->reference_ratio; pa_sw_cvolume_multiply(&o->soft_volume, &o->real_ratio, &o->volume_factor); } - - /* Notify others about the changed source output volume. */ - if (!pa_cvolume_equal(&o->volume, &old_volume)) { - /* XXX: In case o->source has flat volume enabled, then real_ratio - * and soft_volume are not updated yet. Let's hope that the - * callback implementation doesn't care about those variables... */ - if (o->volume_changed) - o->volume_changed(o); - - pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index); - } } /* If o->source == dest, then recursion has finished, and we can finally call @@ -1692,3 +1671,28 @@ int pa_source_output_update_rate(pa_source_output *o) { return 0; } + +/* Called from the main thread. */ +void pa_source_output_set_volume_direct(pa_source_output *o, const pa_cvolume *volume) { + pa_cvolume old_volume; + char old_volume_str[PA_CVOLUME_SNPRINT_VERBOSE_MAX]; + char new_volume_str[PA_CVOLUME_SNPRINT_VERBOSE_MAX]; + + pa_assert(o); + pa_assert(volume); + + old_volume = o->volume; + + if (pa_cvolume_equal(volume, &old_volume)) + return; + + o->volume = *volume; + pa_log_debug("The volume of source output %u changed from %s to %s.", o->index, + pa_cvolume_snprint_verbose(old_volume_str, sizeof(old_volume_str), &old_volume, &o->channel_map, true), + pa_cvolume_snprint_verbose(new_volume_str, sizeof(new_volume_str), volume, &o->channel_map, true)); + + if (o->volume_changed) + o->volume_changed(o); + + pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index); +} diff --git a/src/pulsecore/source-output.h b/src/pulsecore/source-output.h index 102fb8b8..27d6fd45 100644 --- a/src/pulsecore/source-output.h +++ b/src/pulsecore/source-output.h @@ -353,6 +353,13 @@ int pa_source_output_process_msg(pa_msgobject *mo, int code, void *userdata, int pa_usec_t pa_source_output_set_requested_latency_within_thread(pa_source_output *o, pa_usec_t usec); +/* Called from the main thread, from source.c only. The normal way to set the + * source output volume is to call pa_source_output_set_volume(), but the flat + * volume logic in source.c needs also a function that doesn't do all the extra + * stuff that pa_source_output_set_volume() does. This function simply sets + * o->volume and fires change notifications. */ +void pa_source_output_set_volume_direct(pa_source_output *o, const pa_cvolume *volume); + #define pa_source_output_assert_io_context(s) \ pa_assert(pa_thread_mq_get() || !PA_SOURCE_OUTPUT_IS_LINKED((s)->state)) diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c index c0bc1c99..5c1847e1 100644 --- a/src/pulsecore/source.c +++ b/src/pulsecore/source.c @@ -1429,20 +1429,13 @@ static void update_real_volume(pa_source *s, const pa_cvolume *new_volume, pa_ch PA_IDXSET_FOREACH(o, s->outputs, idx) { if (o->destination_source && (o->destination_source->flags & PA_SOURCE_SHARE_VOLUME_WITH_MASTER)) { if (pa_source_flat_volume_enabled(s)) { - pa_cvolume old_volume = o->volume; + pa_cvolume new_output_volume; /* Follow the root source's real volume. */ - o->volume = *new_volume; - pa_cvolume_remap(&o->volume, channel_map, &o->channel_map); + new_output_volume = *new_volume; + pa_cvolume_remap(&new_output_volume, channel_map, &o->channel_map); + pa_source_output_set_volume_direct(o, &new_output_volume); compute_reference_ratio(o); - - /* The volume changed, let's tell people so */ - if (!pa_cvolume_equal(&old_volume, &o->volume)) { - if (o->volume_changed) - o->volume_changed(o); - - pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index); - } } update_real_volume(o->destination_source, new_volume, channel_map); @@ -1497,7 +1490,7 @@ static void propagate_reference_volume(pa_source *s) { * source output volumes accordingly */ PA_IDXSET_FOREACH(o, s->outputs, idx) { - pa_cvolume old_volume; + pa_cvolume new_volume; if (o->destination_source && (o->destination_source->flags & PA_SOURCE_SHARE_VOLUME_WITH_MASTER)) { propagate_reference_volume(o->destination_source); @@ -1508,24 +1501,14 @@ static void propagate_reference_volume(pa_source *s) { continue; } - old_volume = o->volume; - /* This basically calculates: * * o->volume := o->reference_volume * o->reference_ratio */ - o->volume = s->reference_volume; - pa_cvolume_remap(&o->volume, &s->channel_map, &o->channel_map); - pa_sw_cvolume_multiply(&o->volume, &o->volume, &o->reference_ratio); - - /* The volume changed, let's tell people so */ - if (!pa_cvolume_equal(&old_volume, &o->volume)) { - - if (o->volume_changed) - o->volume_changed(o); - - pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index); - } + new_volume = s->reference_volume; + pa_cvolume_remap(&new_volume, &s->channel_map, &o->channel_map); + pa_sw_cvolume_multiply(&new_volume, &new_volume, &o->reference_ratio); + pa_source_output_set_volume_direct(o, &new_volume); } } @@ -1718,9 +1701,8 @@ static void propagate_real_volume(pa_source *s, const pa_cvolume *old_real_volum } if (pa_source_flat_volume_enabled(s)) { - PA_IDXSET_FOREACH(o, s->outputs, idx) { - pa_cvolume old_volume = o->volume; + pa_cvolume new_volume; /* 2. Since the source's reference and real volumes are equal * now our ratios should be too. */ @@ -1734,18 +1716,10 @@ static void propagate_real_volume(pa_source *s, const pa_cvolume *old_real_volum * o->volume = s->reference_volume * o->reference_ratio * * This is identical to propagate_reference_volume() */ - o->volume = s->reference_volume; - pa_cvolume_remap(&o->volume, &s->channel_map, &o->channel_map); - pa_sw_cvolume_multiply(&o->volume, &o->volume, &o->reference_ratio); - - /* Notify if something changed */ - if (!pa_cvolume_equal(&old_volume, &o->volume)) { - - if (o->volume_changed) - o->volume_changed(o); - - pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index); - } + new_volume = s->reference_volume; + pa_cvolume_remap(&new_volume, &s->channel_map, &o->channel_map); + pa_sw_cvolume_multiply(&new_volume, &new_volume, &o->reference_ratio); + pa_source_output_set_volume_direct(o, &new_volume); if (o->destination_source && (o->destination_source->flags & PA_SOURCE_SHARE_VOLUME_WITH_MASTER)) propagate_real_volume(o->destination_source, old_real_volume); -- 2.39.2