From 0d85b18460f4536dc1047fd47efc094bd1e34286 Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Sun, 20 May 2018 08:52:10 +0530 Subject: [PATCH 01/33] sink, source: Allow reconfigure to change the complete sample spec For the passthrough case, we allow the entire sink sample spec to be changed in reconfigure. This will be needed for high bitrate formats. We duplicate this for sources to keep things in sync as well. Relatedly, we also restore the original spec on leaving passthrough mode. We were getting away with not doing so in the past as, while incorrect, not restoring the rate was not disastrous. With the ability to change channel count, not restoring breaks the meaning of profiles entirely. The saving and restoration logic is restricted to sink/source reconfiguration code to allow it to be self-contained and easier to reason about. All this also applies to the channel map. We don't actually explicitly reconfigure the channel map at the moment, but since pa_sink/source_reconfigure() can now change the channel count, it seems to make sense to include the channel map along with that API change for future use. --- src/modules/alsa/alsa-sink.c | 4 +- src/modules/alsa/alsa-source.c | 3 +- src/modules/module-null-sink.c | 9 ++- src/pulsecore/sink-input.c | 20 ++++- src/pulsecore/sink.c | 111 ++++++++++++++++++++------- src/pulsecore/sink.h | 8 +- src/pulsecore/source-output.c | 20 ++++- src/pulsecore/source.c | 135 +++++++++++++++++++++++++-------- src/pulsecore/source.h | 8 +- 9 files changed, 242 insertions(+), 76 deletions(-) diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c index b249df680..b2c142bf3 100644 --- a/src/modules/alsa/alsa-sink.c +++ b/src/modules/alsa/alsa-sink.c @@ -1817,7 +1817,7 @@ static bool sink_set_formats(pa_sink *s, pa_idxset *formats) { return true; } -static void sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, bool passthrough) { +static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough) { struct userdata *u = s->userdata; int i; bool format_supported = false; @@ -1876,6 +1876,8 @@ static void sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, bool passthrou #endif /* Passthrough status change is handled during unsuspend */ + + return 0; } static int process_rewind(struct userdata *u) { diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c index ef8b12c32..24fae97c0 100644 --- a/src/modules/alsa/alsa-source.c +++ b/src/modules/alsa/alsa-source.c @@ -1632,7 +1632,7 @@ static void source_update_requested_latency_cb(pa_source *s) { update_sw_params(u); } -static void source_reconfigure_cb(pa_source *s, pa_sample_spec *spec, bool passthrough) { +static int source_reconfigure_cb(pa_source *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough) { struct userdata *u = s->userdata; int i; bool format_supported = false; @@ -1690,6 +1690,7 @@ static void source_reconfigure_cb(pa_source *s, pa_sample_spec *spec, bool passt pa_smoother_2_set_sample_spec(u->smoother, pa_rtclock_now(), &effective_spec); #endif + return 0; } static void thread_func(void *userdata) { diff --git a/src/modules/module-null-sink.c b/src/modules/module-null-sink.c index 714a81a63..a349bbb20 100644 --- a/src/modules/module-null-sink.c +++ b/src/modules/module-null-sink.c @@ -168,9 +168,16 @@ static void sink_update_requested_latency_cb(pa_sink *s) { sink_recalculate_max_request_and_rewind(s); } -static void sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, bool passthrough) { +static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough) { /* We don't need to do anything */ s->sample_spec = *spec; + + if (map) + s->channel_map = *map; + else + pa_channel_map_init_auto(&s->channel_map, spec->channels, PA_CHANNEL_MAP_DEFAULT); + + return 0; } static bool sink_set_formats_cb(pa_sink *s, pa_idxset *formats) { diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c index 4380087ca..be05b3e32 100644 --- a/src/pulsecore/sink-input.c +++ b/src/pulsecore/sink-input.c @@ -493,7 +493,8 @@ int pa_sink_input_new( module-suspend-on-idle can resume a sink */ pa_log_info("Trying to change sample spec"); - pa_sink_reconfigure(data->sink, &data->sample_spec, pa_sink_input_new_data_is_passthrough(data)); + pa_sink_reconfigure(data->sink, &data->sample_spec, &data->channel_map, pa_sink_input_new_data_is_passthrough(data), + false); } if (pa_sink_input_new_data_is_passthrough(data) && @@ -709,7 +710,7 @@ static void sink_input_set_state(pa_sink_input *i, pa_sink_input_state_t state) !pa_sample_spec_equal(&i->sample_spec, &i->sink->sample_spec)) { /* We were uncorked and the sink was not playing anything -- let's try * to update the sample format and rate to avoid resampling */ - pa_sink_reconfigure(i->sink, &i->sample_spec, pa_sink_input_is_passthrough(i)); + pa_sink_reconfigure(i->sink, &i->sample_spec, &i->channel_map, pa_sink_input_is_passthrough(i), false); } pa_assert_se(pa_asyncmsgq_send(i->sink->asyncmsgq, PA_MSGOBJECT(i), PA_SINK_INPUT_MESSAGE_SET_STATE, PA_UINT_TO_PTR(state), 0, NULL) == 0); @@ -812,9 +813,15 @@ void pa_sink_input_unlink(pa_sink_input *i) { reset_callbacks(i); if (i->sink) { - if (PA_SINK_IS_LINKED(i->sink->state)) + if (PA_SINK_IS_LINKED(i->sink->state)) { pa_sink_update_status(i->sink); + if (pa_sink_input_is_passthrough(i)) { + pa_log_debug("Leaving passthrough, trying to restore previous configuration"); + pa_sink_reconfigure(i->sink, NULL, NULL, false, true); + } + } + i->sink = NULL; } @@ -1901,6 +1908,11 @@ int pa_sink_input_start_move(pa_sink_input *i) { pa_sink_update_status(i->sink); + if (pa_sink_input_is_passthrough(i)) { + pa_log_debug("Leaving passthrough, trying to restore previous configuration"); + pa_sink_reconfigure(i->sink, NULL, NULL, false, true); + } + PA_HASHMAP_FOREACH(v, i->volume_factor_sink_items, state) pa_cvolume_remap(&v->volume, &i->sink->channel_map, &i->channel_map); @@ -2176,7 +2188,7 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) { SINK_INPUT_MOVE_FINISH hook */ pa_log_info("Trying to change sample spec"); - pa_sink_reconfigure(dest, &i->sample_spec, pa_sink_input_is_passthrough(i)); + pa_sink_reconfigure(dest, &i->sample_spec, &i->channel_map, pa_sink_input_is_passthrough(i), false); } if (i->moving) diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c index 0f0dc56fc..7196ba11d 100644 --- a/src/pulsecore/sink.c +++ b/src/pulsecore/sink.c @@ -273,6 +273,8 @@ pa_sink* pa_sink_new( s->sample_spec = data->sample_spec; s->channel_map = data->channel_map; s->default_sample_rate = s->sample_spec.rate; + pa_sample_spec_init(&s->saved_spec); + pa_channel_map_init(&s->saved_map); if (data->alternate_sample_rate_is_set) s->alternate_sample_rate = data->alternate_sample_rate; @@ -1479,7 +1481,8 @@ void pa_sink_render_full(pa_sink *s, size_t length, pa_memchunk *result) { } /* Called from main thread */ -void pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) { +int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough, bool restore) { + int ret = -1; pa_sample_spec desired_spec; uint32_t default_rate = s->default_sample_rate; uint32_t alternate_rate = s->alternate_sample_rate; @@ -1488,55 +1491,69 @@ void pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) { bool default_rate_is_usable = false; bool alternate_rate_is_usable = false; bool avoid_resampling = s->avoid_resampling; + pa_channel_map old_map, *new_map; - if (pa_sample_spec_equal(spec, &s->sample_spec)) - return; + pa_assert(restore || (spec != NULL)); + pa_assert(!restore || (spec == NULL && map == NULL && pa_sample_spec_valid(&s->saved_spec))); + + if (!restore && pa_sample_spec_equal(spec, &s->sample_spec)) + return 0; if (!s->reconfigure) - return; + return -1; - if (PA_UNLIKELY(default_rate == alternate_rate && !passthrough && !avoid_resampling)) { + if (PA_UNLIKELY(default_rate == alternate_rate && !passthrough && !restore && !avoid_resampling)) { pa_log_debug("Default and alternate sample rates are the same, so there is no point in switching."); - return; + return -1; } if (PA_SINK_IS_RUNNING(s->state)) { - pa_log_info("Cannot update sample spec, SINK_IS_RUNNING, will keep using %s and %u Hz", - pa_sample_format_to_string(s->sample_spec.format), s->sample_spec.rate); - return; + pa_log_info("Cannot update sample spec, SINK_IS_RUNNING, will keep using %s, %u ch and %u Hz", + pa_sample_format_to_string(s->sample_spec.format), s->sample_spec.channels, s->sample_spec.rate); + return -1; } if (s->monitor_source) { if (PA_SOURCE_IS_RUNNING(s->monitor_source->state) == true) { pa_log_info("Cannot update sample spec, monitor source is RUNNING"); - return; + return -1; } } - if (PA_UNLIKELY(!pa_sample_spec_valid(spec))) - return; - - desired_spec = s->sample_spec; + if (PA_UNLIKELY(!restore && !pa_sample_spec_valid(spec))) + return -1; if (passthrough) { - /* We have to try to use the sink input format and rate */ - desired_spec.format = spec->format; - desired_spec.rate = spec->rate; + /* 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; + } + + if (restore) { + /* We try to restore the saved spec */ + desired_spec = s->saved_spec; + + } else if (passthrough) { + /* We have to try to use the sink input spec */ + desired_spec = *spec; } else if (avoid_resampling) { /* We just try to set the sink input's sample rate if it's not too low */ + desired_spec = s->sample_spec; if (spec->rate >= default_rate || spec->rate >= alternate_rate) desired_spec.rate = spec->rate; + /* FIXME: don't set this if it's too low */ desired_spec.format = spec->format; } else if (default_rate == spec->rate || alternate_rate == spec->rate) { /* We can directly try to use this rate */ + desired_spec = s->sample_spec; desired_spec.rate = spec->rate; - } - - if (desired_spec.rate != spec->rate) { + } else { /* See if we can pick a rate that results in less resampling effort */ + desired_spec = s->sample_spec; + if (default_rate % 11025 == 0 && spec->rate % 11025 == 0) default_rate_is_usable = true; if (default_rate % 4000 == 0 && spec->rate % 4000 == 0) @@ -1553,28 +1570,65 @@ void pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) { } if (pa_sample_spec_equal(&desired_spec, &s->sample_spec) && passthrough == pa_sink_is_passthrough(s)) - return; + return 0; if (!passthrough && pa_sink_used_by(s) > 0) - return; + return -1; pa_log_debug("Suspending sink %s due to changing format, desired format = %s rate = %u", s->name, pa_sample_format_to_string(desired_spec.format), desired_spec.rate); pa_sink_suspend(s, true, PA_SUSPEND_INTERNAL); - s->reconfigure(s, &desired_spec, passthrough); + /* Keep the old channel map in case it changes */ + old_map = s->channel_map; - /* update monitor source as well */ - if (s->monitor_source && !passthrough) - pa_source_reconfigure(s->monitor_source, &s->sample_spec, false); - pa_log_info("Reconfigured successfully"); + if (restore) { + /* Restore the previous channel map as well */ + new_map = &s->saved_map; + } else if (map) { + /* Set the requested channel map */ + new_map = map; + } else if (desired_spec.channels == s->sample_spec.channels) { + /* No requested channel map, but channel count is unchanged so don't change */ + new_map = &s->channel_map; + } else { + /* No requested channel map, let the device decide */ + new_map = NULL; + } + + if (s->reconfigure(s, &desired_spec, new_map, passthrough) >= 0) { + char spec_str[PA_SAMPLE_SPEC_SNPRINT_MAX]; + + /* update monitor source as well */ + if (s->monitor_source && !passthrough) + pa_source_reconfigure(s->monitor_source, &desired_spec, new_map, false, false); + + pa_log_info("Reconfigured successfully to: %s", + pa_sample_spec_snprint(spec_str, sizeof(spec_str), &desired_spec)); + } PA_IDXSET_FOREACH(i, s->inputs, idx) { if (i->state == PA_SINK_INPUT_CORKED) 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 */ + 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 (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); + } + + return ret; } /* Called from main thread */ @@ -4032,7 +4086,8 @@ void pa_sink_set_reference_volume_direct(pa_sink *s, const pa_cvolume *volume) { s->reference_volume = *volume; pa_log_debug("The reference volume of sink %s changed from %s to %s.", s->name, - pa_cvolume_snprint_verbose(old_volume_str, sizeof(old_volume_str), &old_volume, &s->channel_map, + /* we don't print old volume channel map as it might have changed */ + pa_cvolume_snprint_verbose(old_volume_str, sizeof(old_volume_str), &old_volume, NULL, s->flags & PA_SINK_DECIBEL_VOLUME), pa_cvolume_snprint_verbose(new_volume_str, sizeof(new_volume_str), volume, &s->channel_map, s->flags & PA_SINK_DECIBEL_VOLUME)); diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h index 383edacb5..c4c0bfd39 100644 --- a/src/pulsecore/sink.h +++ b/src/pulsecore/sink.h @@ -107,7 +107,9 @@ struct pa_sink { bool save_muted:1; bool port_changing:1; - /* Saved volume state while we're in passthrough mode */ + /* Saved state while we're in passthrough mode */ + pa_sample_spec saved_spec; + pa_channel_map saved_map; pa_cvolume saved_volume; bool saved_save_volume:1; @@ -268,7 +270,7 @@ struct pa_sink { /* Called whenever device parameters need to be changed. Called from * main thread. */ - void (*reconfigure)(pa_sink *s, pa_sample_spec *spec, bool passthrough); + int (*reconfigure)(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough); /* Contains copies of the above data so that the real-time worker * thread can work without access locking */ @@ -450,7 +452,7 @@ unsigned pa_device_init_priority(pa_proplist *p); /**** May be called by everyone, from main context */ -void pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough); +int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough, bool restore); void pa_sink_set_port_latency_offset(pa_sink *s, int64_t offset); /* The returned value is supposed to be in the time domain of the sound card! */ diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c index 2e2b7a274..8b6fd0493 100644 --- a/src/pulsecore/source-output.c +++ b/src/pulsecore/source-output.c @@ -380,7 +380,8 @@ int pa_source_output_new( module-suspend-on-idle can resume a source */ pa_log_info("Trying to change sample spec"); - pa_source_reconfigure(data->source, &data->sample_spec, pa_source_output_new_data_is_passthrough(data)); + pa_source_reconfigure(data->source, &data->sample_spec, &data->channel_map, + pa_source_output_new_data_is_passthrough(data), false); } if (pa_source_output_new_data_is_passthrough(data) && @@ -559,7 +560,7 @@ static void source_output_set_state(pa_source_output *o, pa_source_output_state_ !pa_sample_spec_equal(&o->sample_spec, &o->source->sample_spec)) { /* We were uncorked and the source was not playing anything -- let's try * to update the sample format and rate to avoid resampling */ - pa_source_reconfigure(o->source, &o->sample_spec, pa_source_output_is_passthrough(o)); + pa_source_reconfigure(o->source, &o->sample_spec, &o->channel_map, pa_source_output_is_passthrough(o), false); } pa_assert_se(pa_asyncmsgq_send(o->source->asyncmsgq, PA_MSGOBJECT(o), PA_SOURCE_OUTPUT_MESSAGE_SET_STATE, PA_UINT_TO_PTR(state), 0, NULL) == 0); @@ -628,9 +629,15 @@ void pa_source_output_unlink(pa_source_output*o) { reset_callbacks(o); if (o->source) { - if (PA_SOURCE_IS_LINKED(o->source->state)) + if (PA_SOURCE_IS_LINKED(o->source->state)) { pa_source_update_status(o->source); + if (pa_source_output_is_passthrough(o)) { + pa_log_debug("Leaving passthrough, trying to restore previous configuration"); + pa_source_reconfigure(o->source, NULL, NULL, false, true); + } + } + o->source = NULL; } @@ -1412,6 +1419,11 @@ int pa_source_output_start_move(pa_source_output *o) { pa_source_update_status(o->source); + if (pa_source_output_is_passthrough(o)) { + pa_log_debug("Leaving passthrough, trying to restore previous configuration"); + pa_source_reconfigure(o->source, NULL, NULL, false, true); + } + pa_cvolume_remap(&o->volume_factor_source, &o->source->channel_map, &o->channel_map); o->source = NULL; @@ -1605,7 +1617,7 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save SOURCE_OUTPUT_MOVE_FINISH hook */ pa_log_info("Trying to change sample spec"); - pa_source_reconfigure(dest, &o->sample_spec, pa_source_output_is_passthrough(o)); + pa_source_reconfigure(dest, &o->sample_spec, &o->channel_map, pa_source_output_is_passthrough(o), false); } if (o->moving) diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c index 99d8dde6e..67f2fb624 100644 --- a/src/pulsecore/source.c +++ b/src/pulsecore/source.c @@ -259,6 +259,8 @@ pa_source* pa_source_new( s->sample_spec = data->sample_spec; s->channel_map = data->channel_map; s->default_sample_rate = s->sample_spec.rate; + pa_sample_spec_init(&s->saved_spec); + pa_channel_map_init(&s->saved_map); if (data->alternate_sample_rate_is_set) s->alternate_sample_rate = data->alternate_sample_rate; @@ -1045,64 +1047,77 @@ void pa_source_post_direct(pa_source*s, pa_source_output *o, const pa_memchunk * } /* Called from main thread */ -void pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough) { - uint32_t idx; - pa_source_output *o; +int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough, bool restore) { + int ret; pa_sample_spec desired_spec; uint32_t default_rate = s->default_sample_rate; uint32_t alternate_rate = s->alternate_sample_rate; bool default_rate_is_usable = false; bool alternate_rate_is_usable = false; bool avoid_resampling = s->avoid_resampling; + pa_channel_map old_map, *new_map; - if (pa_sample_spec_equal(spec, &s->sample_spec)) - return; + pa_assert(restore || (spec != NULL)); + pa_assert(!restore || (spec == NULL && map == NULL && pa_sample_spec_valid(&s->saved_spec))); + + if (!restore && pa_sample_spec_equal(spec, &s->sample_spec)) + return 0; if (!s->reconfigure && !s->monitor_of) - return; + return -1; - if (PA_UNLIKELY(default_rate == alternate_rate && !passthrough && !avoid_resampling)) { + if (PA_UNLIKELY(default_rate == alternate_rate && !passthrough && !restore && !avoid_resampling)) { pa_log_debug("Default and alternate sample rates are the same, so there is no point in switching."); - return; + return -1; } if (PA_SOURCE_IS_RUNNING(s->state)) { - pa_log_info("Cannot update sample spec, SOURCE_IS_RUNNING, will keep using %s and %u Hz", - pa_sample_format_to_string(s->sample_spec.format), s->sample_spec.rate); - return; + pa_log_info("Cannot update sample spec, SOURCE_IS_RUNNING, will keep using %s, %u ch and %u Hz", + pa_sample_format_to_string(s->sample_spec.format), s->sample_spec.channels, s->sample_spec.rate); + return -1; } if (s->monitor_of) { if (PA_SINK_IS_RUNNING(s->monitor_of->state)) { pa_log_info("Cannot update sample spec, this is a monitor source and the sink is running."); - return; + return -1; } } - if (PA_UNLIKELY(!pa_sample_spec_valid(spec))) - return; - - desired_spec = s->sample_spec; + if (PA_UNLIKELY(!restore && !pa_sample_spec_valid(spec))) + return -1; if (passthrough) { - /* We have to try to use the source output format and rate */ - desired_spec.format = spec->format; - desired_spec.rate = spec->rate; + /* 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; + } + + if (restore) { + /* We try to restore the saved spec */ + desired_spec = s->saved_spec; + + } else if (passthrough) { + /* We have to try to use the source output spec */ + desired_spec = *spec; } else if (avoid_resampling) { /* We just try to set the source output's sample rate if it's not too low */ + desired_spec = s->sample_spec; if (spec->rate >= default_rate || spec->rate >= alternate_rate) desired_spec.rate = spec->rate; + /* FIXME: don't set this if it's too low */ desired_spec.format = spec->format; } else if (default_rate == spec->rate || alternate_rate == spec->rate) { /* We can directly try to use this rate */ + desired_spec = s->sample_spec; desired_spec.rate = spec->rate; - } - - if (desired_spec.rate != spec->rate) { + } else { /* See if we can pick a rate that results in less resampling effort */ + desired_spec = s->sample_spec; + if (default_rate % 11025 == 0 && spec->rate % 11025 == 0) default_rate_is_usable = true; if (default_rate % 4000 == 0 && spec->rate % 4000 == 0) @@ -1119,17 +1134,35 @@ void pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough) } if (pa_sample_spec_equal(&desired_spec, &s->sample_spec) && passthrough == pa_source_is_passthrough(s)) - return; + return 0; if (!passthrough && pa_source_used_by(s) > 0) - return; + return -1; pa_log_debug("Suspending source %s due to changing format, desired format = %s rate = %u", s->name, pa_sample_format_to_string(desired_spec.format), desired_spec.rate); + pa_source_suspend(s, true, PA_SUSPEND_INTERNAL); + /* Keep the old channel map in case it changes */ + old_map = s->channel_map; + + if (restore) { + /* Restore the previous channel map as well */ + new_map = &s->saved_map; + } else if (map) { + /* Set the requested channel map */ + new_map = map; + } else if (desired_spec.channels == s->sample_spec.channels) { + /* No requested channel map, but channel count is unchanged so don't change */ + new_map = &s->channel_map; + } else { + /* No requested channel map, let the device decide */ + new_map = NULL; + } + if (s->reconfigure) - s->reconfigure(s, &desired_spec, passthrough); + ret = s->reconfigure(s, &desired_spec, new_map, passthrough); else { /* This is a monitor source. */ @@ -1137,22 +1170,61 @@ void pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough) * have no idea whether the behaviour with passthrough streams is * sensible. */ if (!passthrough) { + pa_sample_spec old_spec = s->sample_spec; s->sample_spec = desired_spec; - pa_sink_reconfigure(s->monitor_of, &desired_spec, false); - s->sample_spec = s->monitor_of->sample_spec; + ret = pa_sink_reconfigure(s->monitor_of, &desired_spec, new_map, false, false); + + if (ret < 0) { + /* Changing the sink rate failed, roll back the old rate for + * the monitor source. Why did we set the source rate before + * calling pa_sink_reconfigure(), you may ask. The reason is + * that pa_sink_reconfigure() tries to update the monitor + * source rate, but we are already in the process of updating + * the monitor source rate, so there's a risk of entering an + * infinite loop. Setting the source rate before calling + * pa_sink_reconfigure() makes the rate == s->sample_spec.rate + * check in the beginning of this function return early, so we + * avoid looping. */ + s->sample_spec = old_spec; + } } else goto unsuspend; } - PA_IDXSET_FOREACH(o, s->outputs, idx) { - if (o->state == PA_SOURCE_OUTPUT_CORKED) - pa_source_output_update_resampler(o); + if (ret >= 0) { + uint32_t idx; + pa_source_output *o; + char spec_str[PA_SAMPLE_SPEC_SNPRINT_MAX]; + + pa_log_info("Changed source format successfully to: %s", + pa_sample_spec_snprint(spec_str, sizeof(spec_str), &desired_spec)); + + PA_IDXSET_FOREACH(o, s->outputs, idx) { + if (o->state == PA_SOURCE_OUTPUT_CORKED) + pa_source_output_update_resampler(o); + } + } + + if (!pa_channel_map_equal(&old_map, &s->channel_map)) { + /* Remap stored volumes to the new channel map */ + 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_log_info("Reconfigured successfully"); + 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); + } + unsuspend: pa_source_suspend(s, false, PA_SUSPEND_INTERNAL); + + return ret; } /* Called from main thread */ @@ -3001,7 +3073,8 @@ void pa_source_set_reference_volume_direct(pa_source *s, const pa_cvolume *volum s->reference_volume = *volume; pa_log_debug("The reference volume of source %s changed from %s to %s.", s->name, - pa_cvolume_snprint_verbose(old_volume_str, sizeof(old_volume_str), &old_volume, &s->channel_map, + /* we don't print old volume channel map as it might have changed */ + pa_cvolume_snprint_verbose(old_volume_str, sizeof(old_volume_str), &old_volume, NULL, s->flags & PA_SOURCE_DECIBEL_VOLUME), pa_cvolume_snprint_verbose(new_volume_str, sizeof(new_volume_str), volume, &s->channel_map, s->flags & PA_SOURCE_DECIBEL_VOLUME)); diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h index aa71ee829..57ed77bc6 100644 --- a/src/pulsecore/source.h +++ b/src/pulsecore/source.h @@ -108,7 +108,9 @@ struct pa_source { bool save_muted:1; bool port_changing:1; - /* Saved volume state while we're in passthrough mode */ + /* Saved state while we're in passthrough mode */ + pa_sample_spec saved_spec; + pa_channel_map saved_map; pa_cvolume saved_volume; bool saved_save_volume:1; @@ -226,7 +228,7 @@ struct pa_source { /* Called whenever device parameters need to be changed. Called from * main thread. */ - void (*reconfigure)(pa_source *s, pa_sample_spec *spec, bool passthrough); + int (*reconfigure)(pa_source *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough); /* Contains copies of the above data so that the real-time worker * thread can work without access locking */ @@ -419,7 +421,7 @@ bool pa_source_update_proplist(pa_source *s, pa_update_mode_t mode, pa_proplist int pa_source_set_port(pa_source *s, const char *name, bool save); -void pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough); +int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough, bool restore); unsigned pa_source_linked_by(pa_source *s); /* Number of connected streams */ unsigned pa_source_used_by(pa_source *s); /* Number of connected streams that are not corked */ From cbaf278f1ee7f72c550943765c7e01fe6b30b1ef Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Wed, 19 Sep 2018 18:24:50 +0530 Subject: [PATCH 02/33] 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); From 01c26546fa4ca4c89d0ca850e2b50a847c3a70f4 Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Mon, 24 Sep 2018 13:51:23 +0530 Subject: [PATCH 03/33] sink, source: Add an avoid-processing mode This generalises the avoid-resampling concept (don't resample for any rate above the default/alternate sample rate) to include channel count and sample format as well. The rationale for this is that users who wish to send out their data untouched by processing in PulseAudio can do so. In addition to this, there are opportunities for certain hardware (such as systems with a DSP connected to a codec) to offload processing to the DSP (providing potential cost savings). Finally, this also enables modules that might be able to perform transformations of (ANY -> sink format), and this allows us to implement such transformations. --- man/pulse-daemon.conf.5.xml.in | 10 +++++++++ src/daemon/daemon-conf.c | 3 +++ src/daemon/daemon-conf.h | 1 + src/daemon/daemon.conf.in | 1 + src/daemon/main.c | 1 + src/modules/alsa/alsa-sink.c | 9 ++++++++ src/modules/alsa/alsa-source.c | 9 ++++++++ src/modules/alsa/module-alsa-card.c | 2 ++ src/modules/module-udev-detect.c | 17 ++++++++++++-- src/pulsecore/core.h | 1 + src/pulsecore/sink.c | 35 ++++++++++++++++++++++++----- src/pulsecore/sink.h | 6 ++++- src/pulsecore/source.c | 34 +++++++++++++++++++++++----- src/pulsecore/source.h | 6 ++++- 14 files changed, 121 insertions(+), 14 deletions(-) diff --git a/man/pulse-daemon.conf.5.xml.in b/man/pulse-daemon.conf.5.xml.in index 52223fb8c..b6e2c9c14 100644 --- a/man/pulse-daemon.conf.5.xml.in +++ b/man/pulse-daemon.conf.5.xml.in @@ -132,6 +132,16 @@ License along with PulseAudio; if not, see . rates.

+ + + +