From cbaf278f1ee7f72c550943765c7e01fe6b30b1ef Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Wed, 19 Sep 2018 18:24:50 +0530 Subject: [PATCH] sink, source: Consolidate passthrough setup in reconfigure This moves over the saving+resetting/restoring of volumes and source suspending/unsuspending while entering/leaving passthrough mode into reconfigure functions. This makes it easier to reason about exactly what behaviour occurs at the time, as well as avoids loss of precision during the remapping of the internal volume values in this case. --- src/pulsecore/sink-input.c | 12 ------ src/pulsecore/sink.c | 74 +++++++++++++++-------------------- src/pulsecore/sink.h | 3 -- src/pulsecore/source-output.c | 12 ------ src/pulsecore/source.c | 42 +++++++++----------- src/pulsecore/source.h | 3 -- 6 files changed, 50 insertions(+), 96 deletions(-) diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c index be05b3e32..958bf76bb 100644 --- a/src/pulsecore/sink-input.c +++ b/src/pulsecore/sink-input.c @@ -799,9 +799,6 @@ void pa_sink_input_unlink(pa_sink_input *i) { i->state = PA_SINK_INPUT_UNLINKED; if (linked && i->sink) { - if (pa_sink_input_is_passthrough(i)) - pa_sink_leave_passthrough(i->sink); - /* We might need to update the sink's volume if we are in flat volume mode. */ if (pa_sink_flat_volume_enabled(i->sink)) pa_sink_set_volume(i->sink, NULL, false, false); @@ -917,9 +914,6 @@ void pa_sink_input_put(pa_sink_input *i) { set_real_ratio(i, &i->volume); } - if (pa_sink_input_is_passthrough(i)) - pa_sink_enter_passthrough(i->sink); - i->thread_info.soft_volume = i->soft_volume; i->thread_info.muted = i->muted; @@ -1896,9 +1890,6 @@ int pa_sink_input_start_move(pa_sink_input *i) { if (i->state == PA_SINK_INPUT_CORKED) pa_assert_se(i->sink->n_corked-- >= 1); - if (pa_sink_input_is_passthrough(i)) - pa_sink_leave_passthrough(i->sink); - if (pa_sink_flat_volume_enabled(i->sink)) /* We might need to update the sink's volume if we are in flat * volume mode. */ @@ -2222,9 +2213,6 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) { update_volume_due_to_moving(i, dest); - if (pa_sink_input_is_passthrough(i)) - pa_sink_enter_passthrough(i->sink); - pa_assert_se(pa_asyncmsgq_send(i->sink->asyncmsgq, PA_MSGOBJECT(i->sink), PA_SINK_MESSAGE_FINISH_MOVE, i, 0, NULL) == 0); /* Reset move variable */ diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c index 7196ba11d..d4011f9e3 100644 --- a/src/pulsecore/sink.c +++ b/src/pulsecore/sink.c @@ -1527,6 +1527,10 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, b /* Save the previous sample spec and channel map, we will try to restore it when leaving passthrough */ s->saved_spec = s->sample_spec; s->saved_map = s->channel_map; + + /* Save the volume, we're going to reset it to NORM while in passthrough */ + s->saved_volume = *pa_sink_get_volume(s, true); + s->saved_save_volume = s->save_volume; } if (restore) { @@ -1612,22 +1616,46 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, b pa_sink_input_update_resampler(i, true); } - if (!pa_channel_map_equal(&old_map, &s->channel_map)) { - /* Remap stored volumes to the new channel map */ + if (!restore && !pa_channel_map_equal(&old_map, &s->channel_map)) { + /* Remap stored volumes to the new channel map if we're not just restoring a previously saved volume */ pa_cvolume_remap(&s->reference_volume, &old_map, &s->channel_map); pa_cvolume_remap(&s->real_volume, &old_map, &s->channel_map); pa_cvolume_remap(&s->soft_volume, &old_map, &s->channel_map); } - pa_sink_suspend(s, false, PA_SUSPEND_INTERNAL); + if (passthrough) { + /* set the volume to NORM */ + pa_cvolume volume; + + pa_cvolume_set(&volume, s->sample_spec.channels, PA_MIN(s->base_volume, PA_VOLUME_NORM)); + pa_sink_set_volume(s, &volume, true, false); + + /* disable the monitor in passthrough mode */ + if (s->monitor_source) { + pa_log_debug("Suspending monitor source %s, because the sink is entering the passthrough mode.", s->monitor_source->name); + pa_source_suspend(s->monitor_source, true, PA_SUSPEND_PASSTHROUGH); + } + } if (restore) { /* Reset saved spec and channel map to bail early if we inadvertently * use them (which is not expected after this) */ pa_sample_spec_init(&s->saved_spec); pa_channel_map_init(&s->saved_map); + + /* Restore sink volume to what it was before we entered passthrough mode */ + pa_sink_set_volume(s, &s->saved_volume, true, s->saved_save_volume); + pa_cvolume_init(&s->saved_volume); + s->saved_save_volume = false; + + if (s->monitor_source) { + pa_log_debug("Resuming monitor source %s, because the sink is leaving the passthrough mode.", s->monitor_source->name); + pa_source_suspend(s->monitor_source, false, PA_SUSPEND_PASSTHROUGH); + } } + pa_sink_suspend(s, false, PA_SUSPEND_INTERNAL); + return ret; } @@ -1781,46 +1809,6 @@ bool pa_sink_is_passthrough(pa_sink *s) { return false; } -/* Called from main context */ -void pa_sink_enter_passthrough(pa_sink *s) { - pa_cvolume volume; - - /* The sink implementation is reconfigured for passthrough in - * pa_sink_reconfigure(). This function sets the PA core objects to - * passthrough mode. */ - - /* disable the monitor in passthrough mode */ - if (s->monitor_source) { - pa_log_debug("Suspending monitor source %s, because the sink is entering the passthrough mode.", s->monitor_source->name); - pa_source_suspend(s->monitor_source, true, PA_SUSPEND_PASSTHROUGH); - } - - /* set the volume to NORM */ - s->saved_volume = *pa_sink_get_volume(s, true); - s->saved_save_volume = s->save_volume; - - pa_cvolume_set(&volume, s->sample_spec.channels, PA_MIN(s->base_volume, PA_VOLUME_NORM)); - pa_sink_set_volume(s, &volume, true, false); - - pa_log_debug("Suspending/Restarting sink %s to enter passthrough mode", s->name); -} - -/* Called from main context */ -void pa_sink_leave_passthrough(pa_sink *s) { - /* Unsuspend monitor */ - if (s->monitor_source) { - pa_log_debug("Resuming monitor source %s, because the sink is leaving the passthrough mode.", s->monitor_source->name); - pa_source_suspend(s->monitor_source, false, PA_SUSPEND_PASSTHROUGH); - } - - /* Restore sink volume to what it was before we entered passthrough mode */ - pa_sink_set_volume(s, &s->saved_volume, true, s->saved_save_volume); - - pa_cvolume_init(&s->saved_volume); - s->saved_save_volume = false; - -} - /* Called from main context. */ static void compute_reference_ratio(pa_sink_input *i) { unsigned c = 0; diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h index c4c0bfd39..8a15415d1 100644 --- a/src/pulsecore/sink.h +++ b/src/pulsecore/sink.h @@ -484,9 +484,6 @@ bool pa_sink_is_filter(pa_sink *s); /* Is the sink in passthrough mode? (that is, is there a passthrough sink input * connected to this sink? */ bool pa_sink_is_passthrough(pa_sink *s); -/* These should be called when a sink enters/leaves passthrough mode */ -void pa_sink_enter_passthrough(pa_sink *s); -void pa_sink_leave_passthrough(pa_sink *s); void pa_sink_set_volume(pa_sink *sink, const pa_cvolume *volume, bool sendmsg, bool save); const pa_cvolume *pa_sink_get_volume(pa_sink *sink, bool force_refresh); diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c index 8b6fd0493..136c0518c 100644 --- a/src/pulsecore/source-output.c +++ b/src/pulsecore/source-output.c @@ -615,9 +615,6 @@ void pa_source_output_unlink(pa_source_output*o) { o->state = PA_SOURCE_OUTPUT_UNLINKED; if (linked && o->source) { - if (pa_source_output_is_passthrough(o)) - pa_source_leave_passthrough(o->source); - /* We might need to update the source's volume if we are in flat volume mode. */ if (pa_source_flat_volume_enabled(o->source)) pa_source_set_volume(o->source, NULL, false, false); @@ -712,9 +709,6 @@ void pa_source_output_put(pa_source_output *o) { set_real_ratio(o, &o->volume); } - if (pa_source_output_is_passthrough(o)) - pa_source_enter_passthrough(o->source); - o->thread_info.soft_volume = o->soft_volume; o->thread_info.muted = o->muted; @@ -1407,9 +1401,6 @@ int pa_source_output_start_move(pa_source_output *o) { if (o->state == PA_SOURCE_OUTPUT_CORKED) pa_assert_se(origin->n_corked-- >= 1); - if (pa_source_output_is_passthrough(o)) - pa_source_leave_passthrough(o->source); - if (pa_source_flat_volume_enabled(o->source)) /* We might need to update the source's volume if we are in flat * volume mode. */ @@ -1646,9 +1637,6 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save update_volume_due_to_moving(o, dest); - if (pa_source_output_is_passthrough(o)) - pa_source_enter_passthrough(o->source); - pa_assert_se(pa_asyncmsgq_send(o->source->asyncmsgq, PA_MSGOBJECT(o->source), PA_SOURCE_MESSAGE_ADD_OUTPUT, o, 0, NULL) == 0); pa_log_debug("Successfully moved source output %i to %s.", o->index, dest->name); diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c index 67f2fb624..c46879828 100644 --- a/src/pulsecore/source.c +++ b/src/pulsecore/source.c @@ -1091,6 +1091,10 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, pa_channel_map *ma /* Save the previous sample spec and channel map, we will try to restore it when leaving passthrough */ s->saved_spec = s->sample_spec; s->saved_map = s->channel_map; + + /* Save the volume, we're going to reset it to NORM while in passthrough */ + s->saved_volume = *pa_source_get_volume(s, true); + s->saved_save_volume = s->save_volume; } if (restore) { @@ -1205,8 +1209,8 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, pa_channel_map *ma } } - if (!pa_channel_map_equal(&old_map, &s->channel_map)) { - /* Remap stored volumes to the new channel map */ + if (!restore && !pa_channel_map_equal(&old_map, &s->channel_map)) { + /* Remap stored volumes to the new channel map if we're not just restoring a previously saved volume */ pa_cvolume_remap(&s->reference_volume, &old_map, &s->channel_map); pa_cvolume_remap(&s->real_volume, &old_map, &s->channel_map); pa_cvolume_remap(&s->soft_volume, &old_map, &s->channel_map); @@ -1214,11 +1218,24 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, pa_channel_map *ma pa_log_info("Reconfigured successfully"); + if (passthrough) { + /* set the volume to NORM */ + pa_cvolume volume; + + pa_cvolume_set(&volume, s->sample_spec.channels, PA_MIN(s->base_volume, PA_VOLUME_NORM)); + pa_source_set_volume(s, &volume, true, false); + } + if (restore) { /* Reset saved spec and channel map to bail early if we inadvertently * use them (which is not expected after this) */ pa_sample_spec_init(&s->saved_spec); pa_channel_map_init(&s->saved_map); + + /* Restore source volume to what it was before we entered passthrough mode */ + pa_source_set_volume(s, &s->saved_volume, true, s->saved_save_volume); + pa_cvolume_init(&s->saved_volume); + s->saved_save_volume = false; } unsuspend: @@ -1332,27 +1349,6 @@ bool pa_source_is_passthrough(pa_source *s) { return (s->monitor_of && pa_sink_is_passthrough(s->monitor_of)); } -/* Called from main context */ -void pa_source_enter_passthrough(pa_source *s) { - pa_cvolume volume; - - /* set the volume to NORM */ - s->saved_volume = *pa_source_get_volume(s, true); - s->saved_save_volume = s->save_volume; - - pa_cvolume_set(&volume, s->sample_spec.channels, PA_MIN(s->base_volume, PA_VOLUME_NORM)); - pa_source_set_volume(s, &volume, true, false); -} - -/* Called from main context */ -void pa_source_leave_passthrough(pa_source *s) { - /* Restore source volume to what it was before we entered passthrough mode */ - pa_source_set_volume(s, &s->saved_volume, true, s->saved_save_volume); - - pa_cvolume_init(&s->saved_volume); - s->saved_save_volume = false; -} - /* Called from main context. */ static void compute_reference_ratio(pa_source_output *o) { unsigned c = 0; diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h index 57ed77bc6..64f534b1b 100644 --- a/src/pulsecore/source.h +++ b/src/pulsecore/source.h @@ -407,9 +407,6 @@ bool pa_source_is_filter(pa_source *s); /* Is the source in passthrough mode? (that is, is this a monitor source for a sink * that has a passthrough sink input connected to it. */ bool pa_source_is_passthrough(pa_source *s); -/* These should be called when a source enters/leaves passthrough mode */ -void pa_source_enter_passthrough(pa_source *s); -void pa_source_leave_passthrough(pa_source *s); void pa_source_set_volume(pa_source *source, const pa_cvolume *volume, bool sendmsg, bool save); const pa_cvolume *pa_source_get_volume(pa_source *source, bool force_refresh);