[pulseaudio-discuss] [PATCH 1/2] sink: Allow reconfigure to change the complete sample spec
Arun Raghavan
arun at arunraghavan.net
Sun May 20 06:28:58 UTC 2018
For the passthrough case, we allow the entire sink sample spec to be
changed in reconfigure. This will be needed for high bitrate formats.
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 sample spec. 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/pulsecore/sink-input.c | 29 ++++++++++----
src/pulsecore/sink.c | 70 ++++++++++++++++++++++++++++------
src/pulsecore/sink.h | 8 ++--
src/pulsecore/source-output.c | 21 ++++++++--
src/pulsecore/source.c | 72 +++++++++++++++++++++++++++++------
src/pulsecore/source.h | 8 ++--
6 files changed, 167 insertions(+), 41 deletions(-)
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index ebe5cc957..c84ad2dda 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -416,9 +416,9 @@ int pa_sink_input_new(
/* try to change sink rate. This is done before the FIXATE hook since
module-suspend-on-idle can resume a sink */
- pa_log_info("Trying to change sample rate");
- if (pa_sink_reconfigure(data->sink, &data->sample_spec, pa_sink_input_new_data_is_passthrough(data)) >= 0)
- pa_log_info("Rate changed to %u Hz", data->sink->sample_spec.rate);
+ pa_log_info("Trying to change sample spec");
+ if (pa_sink_reconfigure(data->sink, &data->sample_spec, NULL, pa_sink_input_new_data_is_passthrough(data), false) >= 0)
+ pa_log_info("Spec changed to %u Hz, %d channels", data->sink->sample_spec.rate, data->sink->sample_spec.channels);
}
if (pa_sink_input_new_data_is_passthrough(data) &&
@@ -617,7 +617,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 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, NULL, 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);
@@ -720,9 +720,16 @@ void pa_sink_input_unlink(pa_sink_input *i) {
reset_callbacks(i);
if (i->sink) {
- if (PA_SINK_IS_LINKED(pa_sink_get_state(i->sink)))
+ if (PA_SINK_IS_LINKED(pa_sink_get_state(i->sink))) {
pa_sink_update_status(i->sink);
+ if (pa_sink_input_is_passthrough(i)) {
+ pa_log_debug("Leaving passthrough, trying to restore previous configuration");
+ if (pa_sink_reconfigure(i->sink, NULL, NULL, false, true) >= 0)
+ pa_log_info("Spec changed to %u Hz, %d channels", i->sink->sample_spec.rate, i->sink->sample_spec.channels);
+ }
+ }
+
i->sink = NULL;
}
@@ -1738,6 +1745,12 @@ 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");
+ if (pa_sink_reconfigure(i->sink, NULL, NULL, false, true) >= 0)
+ pa_log_info("Spec changed to %u Hz, %d channels", i->sink->sample_spec.rate, i->sink->sample_spec.channels);
+ }
+
PA_HASHMAP_FOREACH(v, i->volume_factor_sink_items, state)
pa_cvolume_remap(&v->volume, &i->sink->channel_map, &i->channel_map);
@@ -1908,9 +1921,9 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
module-suspend-on-idle resumes destination sink with
SINK_INPUT_MOVE_FINISH hook */
- pa_log_info("Trying to change sample rate");
- if (pa_sink_reconfigure(dest, &i->sample_spec, pa_sink_input_is_passthrough(i)) >= 0)
- pa_log_info("Rate changed to %u Hz", dest->sample_spec.rate);
+ pa_log_info("Trying to change sample spec");
+ if (pa_sink_reconfigure(dest, &i->sample_spec, NULL, pa_sink_input_is_passthrough(i), false) >= 0)
+ pa_log_info("Spec changed to %u Hz, %d channels", dest->sample_spec.rate, dest->sample_spec.channels);
}
if (i->moving)
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index b801b6bcc..625dcae5a 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -265,6 +265,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;
@@ -1440,7 +1442,7 @@ void pa_sink_render_full(pa_sink *s, size_t length, pa_memchunk *result) {
}
/* Called from main thread */
-int 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;
@@ -1450,16 +1452,20 @@ int 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->core->avoid_resampling;
+ pa_channel_map old_map, *new_map;
- /* We currently only try to reconfigure the sample rate */
+ /* We currently only try to reconfigure the sample spec */
- if (pa_sample_spec_equal(spec, &s->sample_spec))
+ 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 -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 -1;
}
@@ -1477,25 +1483,37 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
}
}
- if (PA_UNLIKELY(!pa_sample_spec_valid(spec)))
+ if (PA_UNLIKELY(!restore && !pa_sample_spec_valid(spec)))
return -1;
- desired_spec = s->sample_spec;
-
if (passthrough) {
- /* We have to try to use the sink input rate */
- 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 && (spec->rate >= default_rate || spec->rate >= alternate_rate)) {
/* We just try to set the sink input's sample rate if it's not too low */
+ desired_spec = s->sample_spec;
desired_spec.rate = spec->rate;
} 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;
} 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)
@@ -1520,10 +1538,27 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
pa_log_debug("Suspending sink %s due to changing format.", s->name);
pa_sink_suspend(s, true, PA_SUSPEND_INTERNAL);
- if (s->reconfigure(s, &desired_spec, passthrough) >= 0) {
+ /* 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, &desired_spec, new_map, passthrough) >= 0) {
/* update monitor source as well */
if (s->monitor_source && !passthrough)
- pa_source_reconfigure(s->monitor_source, &desired_spec, false);
+ pa_source_reconfigure(s->monitor_source, &desired_spec, new_map, false, false);
pa_log_info("Changed format successfully");
PA_IDXSET_FOREACH(i, s->inputs, idx) {
@@ -1534,8 +1569,21 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
ret = 0;
}
+ 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 so we don't try to restore it again */
+ pa_sample_spec_init(&s->saved_spec);
+ pa_channel_map_init(&s->saved_map);
+ }
+
return ret;
}
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index abf2d9244..661c04231 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -105,7 +105,9 @@ struct pa_sink {
bool save_volume:1;
bool save_muted: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;
@@ -266,7 +268,7 @@ struct pa_sink {
/* Called whenever device parameters need to be changed. Called from
* main thread. */
- int (*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 */
@@ -441,7 +443,7 @@ unsigned pa_device_init_priority(pa_proplist *p);
/**** May be called by everyone, from main context */
-int 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 17f5da5e1..e265ee505 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -365,7 +365,7 @@ int pa_source_output_new(
module-suspend-on-idle can resume a source */
pa_log_info("Trying to change sample rate");
- if (pa_source_reconfigure(data->source, &data->sample_spec, pa_source_output_new_data_is_passthrough(data)) >= 0)
+ if (pa_source_reconfigure(data->source, &data->sample_spec, NULL, pa_source_output_new_data_is_passthrough(data), false) >= 0)
pa_log_info("Rate changed to %u Hz", data->source->sample_spec.rate);
}
@@ -539,7 +539,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 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, NULL, 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);
@@ -608,9 +608,16 @@ void pa_source_output_unlink(pa_source_output*o) {
reset_callbacks(o);
if (o->source) {
- if (PA_SOURCE_IS_LINKED(pa_source_get_state(o->source)))
+ if (PA_SOURCE_IS_LINKED(pa_source_get_state(o->source))) {
pa_source_update_status(o->source);
+ if (pa_source_output_is_passthrough(o)) {
+ pa_log_debug("Leaving passthrough, trying to restore previous configuration");
+ if (pa_source_reconfigure(o->source, NULL, NULL, false, true) >= 0)
+ pa_log_info("Spec changed to %u Hz, %d channels", o->source->sample_spec.rate, o->source->sample_spec.channels);
+ }
+ }
+
o->source = NULL;
}
@@ -1367,6 +1374,12 @@ 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");
+ if (pa_source_reconfigure(o->source, NULL, NULL, false, true) >= 0)
+ pa_log_info("Spec changed to %u Hz, %d channels", o->source->sample_spec.rate, o->source->sample_spec.channels);
+ }
+
pa_cvolume_remap(&o->volume_factor_source, &o->source->channel_map, &o->channel_map);
o->source = NULL;
@@ -1534,7 +1547,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 rate");
- if (pa_source_reconfigure(dest, &o->sample_spec, pa_source_output_is_passthrough(o)) >= 0)
+ if (pa_source_reconfigure(dest, &o->sample_spec, NULL, pa_source_output_is_passthrough(o), false) >= 0)
pa_log_info("Rate changed to %u Hz", dest->sample_spec.rate);
}
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index ffb8b6ce4..015e625d9 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -252,6 +252,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;
@@ -1023,7 +1025,7 @@ void pa_source_post_direct(pa_source*s, pa_source_output *o, const pa_memchunk *
}
/* Called from main thread */
-int 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) {
int ret;
pa_sample_spec desired_spec;
uint32_t default_rate = s->default_sample_rate;
@@ -1031,16 +1033,20 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
bool default_rate_is_usable = false;
bool alternate_rate_is_usable = false;
bool avoid_resampling = s->core->avoid_resampling;
+ pa_channel_map old_map, *new_map;
- /* We currently only try to reconfigure the sample rate */
+ /* We currently only try to reconfigure the sample spec */
- if (pa_sample_spec_equal(spec, &s->sample_spec))
+ 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 -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 -1;
}
@@ -1058,25 +1064,37 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
}
}
- if (PA_UNLIKELY(!pa_sample_spec_valid(spec)))
+ if (PA_UNLIKELY(!restore && !pa_sample_spec_valid(spec)))
return -1;
- desired_spec = s->sample_spec;
-
if (passthrough) {
- /* We have to try to use the source output rate */
- 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 && (spec->rate >= default_rate || spec->rate >= alternate_rate)) {
/* We just try to set the source output's sample rate if it's not too low */
+ desired_spec = s->sample_spec;
desired_spec.rate = spec->rate;
} 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;
} 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)
@@ -1098,11 +1116,28 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
if (!passthrough && pa_source_used_by(s) > 0)
return -1;
- pa_log_debug("Suspending source %s due to changing the sample rate.", s->name);
+ pa_log_debug("Suspending source %s due to changing the format.", s->name);
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)
- ret = s->reconfigure(s, &desired_spec, passthrough);
+ ret = s->reconfigure(s, &desired_spec, new_map, passthrough);
else {
/* This is a monitor source. */
@@ -1113,7 +1148,7 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
pa_sample_spec old_spec = s->sample_spec;
s->sample_spec = desired_spec;
- ret = pa_sink_reconfigure(s->monitor_of, &desired_spec, false);
+ ret = pa_sink_reconfigure(s->monitor_of, &desired_spec, NULL, false, false);
if (ret < 0) {
/* Changing the sink rate failed, roll back the old rate for
@@ -1144,8 +1179,21 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
pa_log_info("Changed sampling rate successfully");
}
+ 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_source_suspend(s, false, PA_SUSPEND_INTERNAL);
+ if (restore) {
+ /* Reset saved spec and channel map so we don't try to restore it again */
+ pa_sample_spec_init(&s->saved_spec);
+ pa_channel_map_init(&s->saved_map);
+ }
+
return ret;
}
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index b0fa608de..acbc71733 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -106,7 +106,9 @@ struct pa_source {
bool save_volume:1;
bool save_muted: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;
@@ -224,7 +226,7 @@ struct pa_source {
/* Called whenever device parameters need to be changed. Called from
* main thread. */
- int (*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 */
@@ -414,7 +416,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);
-int 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 */
--
2.17.0
More information about the pulseaudio-discuss
mailing list