sink, source: Fix reconfiguration of monitor sources

We should only reconfigure monitor sources when the corresponding sink
is reconfigured (and when it is, we should unconditionally set it to the
requested spec/map of whatever the sink got reconfigured to).

The previous logic was incorrect on all 3 counts.
This commit is contained in:
Arun Raghavan 2022-01-17 17:12:34 -05:00
parent 2046afe683
commit 9f3e82ac31
2 changed files with 19 additions and 29 deletions

View file

@ -1630,13 +1630,15 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, b
if (s->reconfigure(s, &desired_spec, new_map, passthrough) >= 0) { if (s->reconfigure(s, &desired_spec, new_map, passthrough) >= 0) {
char spec_str[PA_SAMPLE_SPEC_SNPRINT_MAX]; char spec_str[PA_SAMPLE_SPEC_SNPRINT_MAX];
char map_str[PA_CHANNEL_MAP_SNPRINT_MAX];
/* update monitor source as well */ /* update monitor source as well */
if (s->monitor_source && !passthrough) if (s->monitor_source && !passthrough)
pa_source_reconfigure(s->monitor_source, &desired_spec, new_map, false); pa_source_reconfigure(s->monitor_source, &s->sample_spec, &s->channel_map, false);
pa_log_info("Reconfigured successfully to: %s", pa_log_info("Reconfigured successfully to: %s, %s",
pa_sample_spec_snprint(spec_str, sizeof(spec_str), &desired_spec)); pa_sample_spec_snprint(spec_str, sizeof(spec_str), &s->sample_spec),
pa_channel_map_snprint(map_str, sizeof(map_str), &s->channel_map));
} }
PA_IDXSET_FOREACH(i, s->inputs, idx) { PA_IDXSET_FOREACH(i, s->inputs, idx) {

View file

@ -1105,6 +1105,12 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, pa_channel_map *ma
} }
if (s->monitor_of) { if (s->monitor_of) {
/* We only allow monitor source reconfiguration to match its sink */
if (!pa_sample_spec_equal(spec, &s->monitor_of->sample_spec) ||
!pa_channel_map_equal(map, &s->monitor_of->channel_map)) {
pa_log_info("Skipping monitor source reconfigruation to different spec from sink.");
return -1;
}
if (PA_SINK_IS_RUNNING(s->monitor_of->state)) { 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."); pa_log_info("Cannot update sample spec, this is a monitor source and the sink is running.");
return -1; return -1;
@ -1195,32 +1201,14 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, pa_channel_map *ma
if (s->reconfigure) if (s->reconfigure)
ret = s->reconfigure(s, &desired_spec, new_map, passthrough); ret = s->reconfigure(s, &desired_spec, new_map, passthrough);
else { else if (s->monitor_of) {
/* This is a monitor source. */ /* This is a monitor source, just set the desired spec and map */
s->sample_spec = desired_spec;
/* XXX: This code is written with non-passthrough streams in mind. I s->channel_map = *new_map;
* have no idea whether the behaviour with passthrough streams is ret = 0;
* sensible. */ } else {
if (!passthrough) { ret = -1;
pa_sample_spec old_spec = s->sample_spec; goto unsuspend;
s->sample_spec = desired_spec;
ret = pa_sink_reconfigure(s->monitor_of, &desired_spec, new_map, 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;
} }
if (ret >= 0) { if (ret >= 0) {