Discussion:
[pulseaudio-discuss] [PATCH v2] alsa-sink/source, sink,
Sangchul Lee
2018-07-27 15:38:07 UTC
Permalink
Sample format(e.g. 16 bit, 24 bit) was not considered even if the
avoid-resampling option is set or the passthrough mode is used.
This patch checks both sample format and rate of a stream to
determine whether to avoid resampling in case of the option is set.
In other word, it is possble to use the stream's original sample
format and rate without resampling as long as these are supported
by the device.

pa_sink_input_update_rate() and pa_source_output_update_rate() are
renamed to pa_sink_input_update_resampler() and pa_source_output
_update_resampler() respectively.

Signed-off-by: Sangchul Lee <***@samsung.com>
---
src/modules/alsa/alsa-sink.c | 101 ++++++++++++++++++++++++++++++++---------
src/modules/alsa/alsa-source.c | 98 ++++++++++++++++++++++++++++++---------
src/pulsecore/sink-input.c | 19 ++++----
src/pulsecore/sink-input.h | 2 +-
src/pulsecore/sink.c | 26 ++++++-----
src/pulsecore/source-output.c | 22 +++++----
src/pulsecore/source-output.h | 2 +-
src/pulsecore/source.c | 24 +++++-----
8 files changed, 206 insertions(+), 88 deletions(-)

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 200d12b..60a4ed2 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -113,11 +113,19 @@ struct userdata {

pa_sample_format_t *supported_formats;
unsigned int *supported_rates;
+ struct {
+ size_t fragment_size;
+ size_t nfrags;
+ size_t tsched_size;
+ size_t tsched_watermark;
+ size_t rewind_safeguard;
+ } initial_info;

size_t
frame_size,
fragment_size,
hwbuf_size,
+ tsched_size,
tsched_watermark,
tsched_watermark_ref,
hwbuf_unused,
@@ -1081,12 +1089,34 @@ static void reset_watermark(struct userdata *u, size_t tsched_watermark, pa_samp
(double) u->tsched_watermark_usec / PA_USEC_PER_MSEC);
}

+/* Called from IO Context on unsuspend */
+static void update_size(struct userdata *u, pa_sample_spec *ss) {
+ pa_assert(u);
+ pa_assert(ss);
+
+ u->frame_size = pa_frame_size(ss);
+ u->frames_per_block = pa_mempool_block_size_max(u->core->mempool) / u->frame_size;
+
+ /* use initial values including module arguments */
+ u->fragment_size = u->initial_info.fragment_size;
+ u->hwbuf_size = u->initial_info.nfrags * u->fragment_size;
+ u->tsched_size = u->initial_info.tsched_size;
+ u->tsched_watermark = u->initial_info.tsched_watermark;
+ u->rewind_safeguard = u->initial_info.rewind_safeguard;
+
+ u->tsched_watermark_ref = u->tsched_watermark;
+
+ pa_log_info("Updated frame_size %zu, frames_per_block %lu, fragment_size %zu, hwbuf_size %zu, tsched(size %zu, watermark %zu), rewind_safeguard %zu",
+ u->frame_size, (unsigned long) u->frames_per_block, u->fragment_size, u->hwbuf_size, u->tsched_size, u->tsched_watermark, u->rewind_safeguard);
+}
+
/* Called from IO context */
static int unsuspend(struct userdata *u) {
pa_sample_spec ss;
int err;
bool b, d;
- snd_pcm_uframes_t period_size, buffer_size;
+ snd_pcm_uframes_t period_frames, buffer_frames;
+ snd_pcm_uframes_t tsched_frames = 0;
char *device_name = NULL;

pa_assert(u);
@@ -1111,13 +1141,18 @@ static int unsuspend(struct userdata *u) {
goto fail;
}

+ if (pa_frame_size(&u->sink->sample_spec) != u->frame_size) {
+ update_size(u, &u->sink->sample_spec);
+ tsched_frames = u->tsched_size / u->frame_size;
+ }
+
ss = u->sink->sample_spec;
- period_size = u->fragment_size / u->frame_size;
- buffer_size = u->hwbuf_size / u->frame_size;
+ period_frames = u->fragment_size / u->frame_size;
+ buffer_frames = u->hwbuf_size / u->frame_size;
b = u->use_mmap;
d = u->use_tsched;

- if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_size, &buffer_size, 0, &b, &d, true)) < 0) {
+ if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_frames, &buffer_frames, tsched_frames, &b, &d, true)) < 0) {
pa_log("Failed to set hardware parameters: %s", pa_alsa_strerror(err));
goto fail;
}
@@ -1132,11 +1167,17 @@ static int unsuspend(struct userdata *u) {
goto fail;
}

- if (period_size*u->frame_size != u->fragment_size ||
- buffer_size*u->frame_size != u->hwbuf_size) {
- pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %lu/%lu, New %lu/%lu)",
- (unsigned long) u->hwbuf_size, (unsigned long) u->fragment_size,
- (unsigned long) (buffer_size*u->frame_size), (unsigned long) (period_size*u->frame_size));
+ if (tsched_frames) {
+ u->fragment_size = (size_t)(period_frames * u->frame_size);
+ u->hwbuf_size = (size_t)(buffer_frames * u->frame_size);
+ pa_proplist_setf(u->sink->proplist, PA_PROP_DEVICE_BUFFERING_BUFFER_SIZE, "%zu", u->hwbuf_size);
+ pa_proplist_setf(u->sink->proplist, PA_PROP_DEVICE_BUFFERING_FRAGMENT_SIZE, "%zu", u->fragment_size);
+
+ } else if (period_frames * u->frame_size != u->fragment_size ||
+ buffer_frames * u->frame_size != u->hwbuf_size) {
+ pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %zu/%zu, New %lu/%lu)",
+ u->hwbuf_size, u->fragment_size,
+ (unsigned long) buffer_frames * u->frame_size, (unsigned long) period_frames * u->frame_size);
goto fail;
}

@@ -1674,33 +1715,43 @@ static bool sink_set_formats(pa_sink *s, pa_idxset *formats) {
static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
struct userdata *u = s->userdata;
int i;
- bool supported = false;
-
- /* FIXME: we only update rate for now */
+ bool format_supported = false;
+ bool rate_supported = false;

pa_assert(u);

+ if (PA_SINK_IS_OPENED(s->state)) {
+ pa_log_warn("Sink is opened, skip it");
+ return -1;
+ }
+
+ for (i = 0; u->supported_formats[i] != PA_SAMPLE_MAX; i++) {
+ if (u->supported_formats[i] == spec->format) {
+ u->sink->sample_spec.format = spec->format;
+ format_supported = true;
+ pa_log_info("Sink supports sample format of %s", pa_sample_format_to_string(spec->format));
+ break;
+ }
+ }
+
for (i = 0; u->supported_rates[i]; i++) {
if (u->supported_rates[i] == spec->rate) {
- supported = true;
+ u->sink->sample_spec.rate = spec->rate;
+ rate_supported = true;
+ pa_log_info("Sink supports sample rate of %d Hz", spec->rate);
break;
}
}

- if (!supported) {
- pa_log_info("Sink does not support sample rate of %d Hz", spec->rate);
+ if (!format_supported && !rate_supported) {
+ pa_log_warn("Sink does not support both sample format of %s and rate of %d Hz",
+ pa_sample_format_to_string(spec->format), spec->rate);
return -1;
}

- if (!PA_SINK_IS_OPENED(s->state)) {
- pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, spec->rate);
- u->sink->sample_spec.rate = spec->rate;
- return 0;
- }
-
/* Passthrough status change is handled during unsuspend */

- return -1;
+ return 0;
}

static int process_rewind(struct userdata *u) {
@@ -2212,6 +2263,12 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca
u->module = m;
u->use_mmap = use_mmap;
u->use_tsched = use_tsched;
+ u->tsched_size = tsched_size;
+ u->initial_info.nfrags = (size_t) nfrags;
+ u->initial_info.fragment_size = (size_t) frag_size;
+ u->initial_info.tsched_size = (size_t) tsched_size;
+ u->initial_info.tsched_watermark = (size_t) tsched_watermark;
+ u->initial_info.rewind_safeguard = (size_t) rewind_safeguard;
u->deferred_volume = deferred_volume;
u->fixed_latency_range = fixed_latency_range;
u->first = true;
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index cba86ca..92b2125 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -101,11 +101,18 @@ struct userdata {

pa_sample_format_t *supported_formats;
unsigned int *supported_rates;
+ struct {
+ size_t fragment_size;
+ size_t nfrags;
+ size_t tsched_size;
+ size_t tsched_watermark;
+ } initial_info;

size_t
frame_size,
fragment_size,
hwbuf_size,
+ tsched_size,
tsched_watermark,
tsched_watermark_ref,
hwbuf_unused,
@@ -947,12 +954,33 @@ static void reset_watermark(struct userdata *u, size_t tsched_watermark, pa_samp
(double) u->tsched_watermark_usec / PA_USEC_PER_MSEC);
}

+/* Called from IO Context on unsuspend */
+static void update_size(struct userdata *u, pa_sample_spec *ss) {
+ pa_assert(u);
+ pa_assert(ss);
+
+ u->frame_size = pa_frame_size(ss);
+ u->frames_per_block = pa_mempool_block_size_max(u->core->mempool) / u->frame_size;
+
+ /* use initial values including module arguments */
+ u->fragment_size = u->initial_info.fragment_size;
+ u->hwbuf_size = u->initial_info.nfrags * u->fragment_size;
+ u->tsched_size = u->initial_info.tsched_size;
+ u->tsched_watermark = u->initial_info.tsched_watermark;
+
+ u->tsched_watermark_ref = u->tsched_watermark;
+
+ pa_log_info("Updated frame_size %zu, frames_per_block %lu, fragment_size %zu, hwbuf_size %zu, tsched(size %zu, watermark %zu)",
+ u->frame_size, (unsigned long) u->frames_per_block, u->fragment_size, u->hwbuf_size, u->tsched_size, u->tsched_watermark);
+}
+
/* Called from IO context */
static int unsuspend(struct userdata *u) {
pa_sample_spec ss;
int err;
bool b, d;
- snd_pcm_uframes_t period_size, buffer_size;
+ snd_pcm_uframes_t period_frames, buffer_frames;
+ snd_pcm_uframes_t tsched_frames = 0;

pa_assert(u);
pa_assert(!u->pcm_handle);
@@ -968,13 +996,18 @@ static int unsuspend(struct userdata *u) {
goto fail;
}

+ if (pa_frame_size(&u->source->sample_spec) != u->frame_size) {
+ update_size(u, &u->source->sample_spec);
+ tsched_frames = u->tsched_size / u->frame_size;
+ }
+
ss = u->source->sample_spec;
- period_size = u->fragment_size / u->frame_size;
- buffer_size = u->hwbuf_size / u->frame_size;
+ period_frames = u->fragment_size / u->frame_size;
+ buffer_frames = u->hwbuf_size / u->frame_size;
b = u->use_mmap;
d = u->use_tsched;

- if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_size, &buffer_size, 0, &b, &d, true)) < 0) {
+ if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_frames, &buffer_frames, tsched_frames, &b, &d, true)) < 0) {
pa_log("Failed to set hardware parameters: %s", pa_alsa_strerror(err));
goto fail;
}
@@ -989,11 +1022,17 @@ static int unsuspend(struct userdata *u) {
goto fail;
}

- if (period_size*u->frame_size != u->fragment_size ||
- buffer_size*u->frame_size != u->hwbuf_size) {
- pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %lu/%lu, New %lu/%lu)",
- (unsigned long) u->hwbuf_size, (unsigned long) u->fragment_size,
- (unsigned long) (buffer_size*u->frame_size), (unsigned long) (period_size*u->frame_size));
+ if (tsched_frames) {
+ u->fragment_size = (size_t)(period_frames * u->frame_size);
+ u->hwbuf_size = (size_t)(buffer_frames * u->frame_size);
+ pa_proplist_setf(u->source->proplist, PA_PROP_DEVICE_BUFFERING_BUFFER_SIZE, "%zu", u->hwbuf_size);
+ pa_proplist_setf(u->source->proplist, PA_PROP_DEVICE_BUFFERING_FRAGMENT_SIZE, "%zu", u->fragment_size);
+
+ } else if (period_frames * u->frame_size != u->fragment_size ||
+ buffer_frames * u->frame_size != u->hwbuf_size) {
+ pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %zu/%zu, New %lu/%lu)",
+ u->hwbuf_size, u->fragment_size,
+ (unsigned long) buffer_frames * u->frame_size, (unsigned long) period_frames * u->frame_size);
goto fail;
}

@@ -1470,31 +1509,41 @@ static void source_update_requested_latency_cb(pa_source *s) {
static int source_reconfigure_cb(pa_source *s, pa_sample_spec *spec, bool passthrough) {
struct userdata *u = s->userdata;
int i;
- bool supported = false;
-
- /* FIXME: we only update rate for now */
+ bool format_supported = false;
+ bool rate_supported = false;

pa_assert(u);

+ if (PA_SOURCE_IS_OPENED(s->state)) {
+ pa_log_warn("Source is opened, skip it");
+ return -1;
+ }
+
+ for (i = 0; u->supported_formats[i] != PA_SAMPLE_MAX; i++) {
+ if (u->supported_formats[i] == spec->format) {
+ u->source->sample_spec.format = spec->format;
+ format_supported = true;
+ pa_log_info("Source supports sample format of %s", pa_sample_format_to_string(spec->format));
+ break;
+ }
+ }
+
for (i = 0; u->supported_rates[i]; i++) {
if (u->supported_rates[i] == spec->rate) {
- supported = true;
+ u->source->sample_spec.rate = spec->rate;
+ rate_supported = true;
+ pa_log_info("Source supports sample rate of %d Hz", spec->rate);
break;
}
}

- if (!supported) {
- pa_log_info("Source does not support sample rate of %d Hz", spec->rate);
+ if (!format_supported && !rate_supported) {
+ pa_log_warn("Source does not support both sample format of %s and rate of %d Hz",
+ pa_sample_format_to_string(spec->format), spec->rate);
return -1;
}

- if (!PA_SOURCE_IS_OPENED(s->state)) {
- pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, spec->rate);
- u->source->sample_spec.rate = spec->rate;
- return 0;
- }
-
- return -1;
+ return 0;
}

static void thread_func(void *userdata) {
@@ -1899,6 +1948,11 @@ pa_source *pa_alsa_source_new(pa_module *m, pa_modargs *ma, const char*driver, p
u->module = m;
u->use_mmap = use_mmap;
u->use_tsched = use_tsched;
+ u->tsched_size = tsched_size;
+ u->initial_info.nfrags = (size_t) nfrags;
+ u->initial_info.fragment_size = (size_t) frag_size;
+ u->initial_info.tsched_size = (size_t) tsched_size;
+ u->initial_info.tsched_watermark = (size_t) tsched_watermark;
u->deferred_volume = deferred_volume;
u->fixed_latency_range = fixed_latency_range;
u->first = true;
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 312ec4a..81b4456 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -417,10 +417,10 @@ int pa_sink_input_new(

if (!(data->flags & PA_SINK_INPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&data->sample_spec, &data->sink->sample_spec)) {
- /* try to change sink rate. This is done before the FIXATE hook since
+ /* try to change sink format and 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");
+ pa_log_info("Trying to change sample spec");
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);
}
@@ -616,7 +616,7 @@ static void sink_input_set_state(pa_sink_input *i, pa_sink_input_state_t state)
if (i->state == PA_SINK_INPUT_CORKED && state == PA_SINK_INPUT_RUNNING && pa_sink_used_by(i->sink) == 0 &&
!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 */
+ * to update the sample format and rate to avoid resampling */
pa_sink_reconfigure(i->sink, &i->sample_spec, pa_sink_input_is_passthrough(i));
}

@@ -1901,13 +1901,14 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {

if (!(i->flags & PA_SINK_INPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&i->sample_spec, &dest->sample_spec)) {
- /* try to change dest sink rate if possible without glitches.
+ /* try to change dest sink format and rate if possible without glitches.
module-suspend-on-idle resumes destination sink with
SINK_INPUT_MOVE_FINISH hook */

- pa_log_info("Trying to change sample rate");
+ pa_log_info("Trying to change sample spec");
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("format, rate are updated to %s, %u Hz",
+ pa_sample_format_to_string(dest->sample_spec.format), dest->sample_spec.rate);
}

if (i->moving)
@@ -1925,7 +1926,7 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
if (i->state == PA_SINK_INPUT_CORKED)
i->sink->n_corked++;

- pa_sink_input_update_rate(i);
+ pa_sink_input_update_resampler(i);

pa_sink_update_status(dest);

@@ -2264,8 +2265,8 @@ finish:

/* Called from main context */
/* Updates the sink input's resampler with whatever the current sink requires
- * -- useful when the underlying sink's rate might have changed */
-int pa_sink_input_update_rate(pa_sink_input *i) {
+ * -- useful when the underlying sink's sample spec might have changed */
+int pa_sink_input_update_resampler(pa_sink_input *i) {
pa_resampler *new_resampler;
char *memblockq_name;

diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h
index 9e90291..6bf406c 100644
--- a/src/pulsecore/sink-input.h
+++ b/src/pulsecore/sink-input.h
@@ -353,7 +353,7 @@ void pa_sink_input_request_rewind(pa_sink_input *i, size_t nbytes, bool rewrite,
void pa_sink_input_cork(pa_sink_input *i, bool b);

int pa_sink_input_set_rate(pa_sink_input *i, uint32_t rate);
-int pa_sink_input_update_rate(pa_sink_input *i);
+int pa_sink_input_update_resampler(pa_sink_input *i);

/* This returns the sink's fields converted into out sample type */
size_t pa_sink_input_get_max_rewind(pa_sink_input *i);
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 566367f..b2ebc2f 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -1448,8 +1448,6 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
bool alternate_rate_is_usable = false;
bool avoid_resampling = s->avoid_resampling;

- /* We currently only try to reconfigure the sample rate */
-
if (pa_sample_spec_equal(spec, &s->sample_spec))
return 0;

@@ -1462,14 +1460,14 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
}

if (PA_SINK_IS_RUNNING(s->state)) {
- pa_log_info("Cannot update rate, SINK_IS_RUNNING, will keep using %u Hz",
- s->sample_spec.rate);
+ 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 -1;
}

if (s->monitor_source) {
if (PA_SOURCE_IS_RUNNING(s->monitor_source->state) == true) {
- pa_log_info("Cannot update rate, monitor source is RUNNING");
+ pa_log_info("Cannot update sample spec, monitor source is RUNNING");
return -1;
}
}
@@ -1480,12 +1478,15 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
desired_spec = s->sample_spec;

if (passthrough) {
- /* We have to try to use the sink input rate */
+ /* We have to try to use the sink input format and rate */
+ desired_spec.format = spec->format;
desired_spec.rate = spec->rate;

- } else if (avoid_resampling && (spec->rate >= default_rate || spec->rate >= alternate_rate)) {
+ } else if (avoid_resampling) {
/* We just try to set the sink input's sample rate if it's not too low */
- desired_spec.rate = spec->rate;
+ if (spec->rate >= default_rate || spec->rate >= alternate_rate)
+ desired_spec.rate = spec->rate;
+ desired_spec.format = spec->format;

} else if (default_rate == spec->rate || alternate_rate == spec->rate) {
/* We can directly try to use this rate */
@@ -1514,18 +1515,19 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
if (!passthrough && pa_sink_used_by(s) > 0)
return -1;

- pa_log_debug("Suspending sink %s due to changing format, desired rate = %u", s->name, desired_spec.rate);
+ 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);

if (s->reconfigure(s, &desired_spec, passthrough) >= 0) {
/* update monitor source as well */
if (s->monitor_source && !passthrough)
- pa_source_reconfigure(s->monitor_source, &desired_spec, false);
- pa_log_info("Changed format successfully");
+ pa_source_reconfigure(s->monitor_source, &s->sample_spec, false);
+ pa_log_info("Reconfigured successfully");

PA_IDXSET_FOREACH(i, s->inputs, idx) {
if (i->state == PA_SINK_INPUT_CORKED)
- pa_sink_input_update_rate(i);
+ pa_sink_input_update_resampler(i);
}

ret = 0;
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 955a2ac..6e56b92 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -365,12 +365,13 @@ int pa_source_output_new(

if (!(data->flags & PA_SOURCE_OUTPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&data->sample_spec, &data->source->sample_spec)) {
- /* try to change source rate. This is done before the FIXATE hook since
+ /* try to change source format and rate. This is done before the FIXATE hook since
module-suspend-on-idle can resume a source */

- pa_log_info("Trying to change sample rate");
+ pa_log_info("Trying to change sample spec");
if (pa_source_reconfigure(data->source, &data->sample_spec, pa_source_output_new_data_is_passthrough(data)) >= 0)
- pa_log_info("Rate changed to %u Hz", data->source->sample_spec.rate);
+ pa_log_info("format, rate are updated to %s, %u Hz",
+ pa_sample_format_to_string(data->source->sample_spec.format), data->source->sample_spec.rate);
}

if (pa_source_output_new_data_is_passthrough(data) &&
@@ -542,7 +543,7 @@ static void source_output_set_state(pa_source_output *o, pa_source_output_state_
if (o->state == PA_SOURCE_OUTPUT_CORKED && state == PA_SOURCE_OUTPUT_RUNNING && pa_source_used_by(o->source) == 0 &&
!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 */
+ * to update the sample format and rate to avoid resampling */
pa_source_reconfigure(o->source, &o->sample_spec, pa_source_output_is_passthrough(o));
}

@@ -1533,13 +1534,14 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save

if (!(o->flags & PA_SOURCE_OUTPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&o->sample_spec, &dest->sample_spec)) {
- /* try to change dest source rate if possible without glitches.
+ /* try to change dest source format and rate if possible without glitches.
module-suspend-on-idle resumes destination source with
SOURCE_OUTPUT_MOVE_FINISH hook */

- pa_log_info("Trying to change sample rate");
+ pa_log_info("Trying to change sample spec");
if (pa_source_reconfigure(dest, &o->sample_spec, pa_source_output_is_passthrough(o)) >= 0)
- pa_log_info("Rate changed to %u Hz", dest->sample_spec.rate);
+ pa_log_info("format, rate are updated to %s, %u Hz",
+ pa_sample_format_to_string(dest->sample_spec.format), dest->sample_spec.rate);
}

if (o->moving)
@@ -1554,7 +1556,7 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save
if (o->state == PA_SOURCE_OUTPUT_CORKED)
o->source->n_corked++;

- pa_source_output_update_rate(o);
+ pa_source_output_update_resampler(o);

pa_source_update_status(dest);

@@ -1729,8 +1731,8 @@ finish:

/* Called from main context */
/* Updates the source output's resampler with whatever the current source
- * requires -- useful when the underlying source's rate might have changed */
-int pa_source_output_update_rate(pa_source_output *o) {
+ * requires -- useful when the underlying source's sample spec might have changed */
+int pa_source_output_update_resampler(pa_source_output *o) {
pa_resampler *new_resampler;
char *memblockq_name;

diff --git a/src/pulsecore/source-output.h b/src/pulsecore/source-output.h
index 661b64b..9cbc286 100644
--- a/src/pulsecore/source-output.h
+++ b/src/pulsecore/source-output.h
@@ -307,7 +307,7 @@ pa_usec_t pa_source_output_set_requested_latency(pa_source_output *o, pa_usec_t
void pa_source_output_cork(pa_source_output *o, bool b);

int pa_source_output_set_rate(pa_source_output *o, uint32_t rate);
-int pa_source_output_update_rate(pa_source_output *o);
+int pa_source_output_update_resampler(pa_source_output *o);

size_t pa_source_output_get_max_rewind(pa_source_output *o);

diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index b501733..b012516 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -1029,8 +1029,6 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
bool alternate_rate_is_usable = false;
bool avoid_resampling = s->avoid_resampling;

- /* We currently only try to reconfigure the sample rate */
-
if (pa_sample_spec_equal(spec, &s->sample_spec))
return 0;

@@ -1043,14 +1041,14 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
}

if (PA_SOURCE_IS_RUNNING(s->state)) {
- pa_log_info("Cannot update rate, SOURCE_IS_RUNNING, will keep using %u Hz",
- s->sample_spec.rate);
+ 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 -1;
}

if (s->monitor_of) {
if (PA_SINK_IS_RUNNING(s->monitor_of->state)) {
- pa_log_info("Cannot update rate, 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;
}
}
@@ -1061,12 +1059,15 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
desired_spec = s->sample_spec;

if (passthrough) {
- /* We have to try to use the source output rate */
+ /* We have to try to use the source output format and rate */
+ desired_spec.format = spec->format;
desired_spec.rate = spec->rate;

- } else if (avoid_resampling && (spec->rate >= default_rate || spec->rate >= alternate_rate)) {
+ } else if (avoid_resampling) {
/* We just try to set the source output's sample rate if it's not too low */
- desired_spec.rate = spec->rate;
+ if (spec->rate >= default_rate || spec->rate >= alternate_rate)
+ desired_spec.rate = spec->rate;
+ desired_spec.format = spec->format;

} else if (default_rate == spec->rate || alternate_rate == spec->rate) {
/* We can directly try to use this rate */
@@ -1095,7 +1096,8 @@ 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 to %u", s->name, desired_spec.rate);
+ 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);

if (s->reconfigure)
@@ -1135,10 +1137,10 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)

PA_IDXSET_FOREACH(o, s->outputs, idx) {
if (o->state == PA_SOURCE_OUTPUT_CORKED)
- pa_source_output_update_rate(o);
+ pa_source_output_update_resampler(o);
}

- pa_log_info("Changed sampling rate successfully");
+ pa_log_info("Reconfigured successfully");
}

pa_source_suspend(s, false, PA_SUSPEND_INTERNAL);
--
2.7.4
Tanu Kaskinen
2018-08-13 14:09:24 UTC
Permalink
Post by Sangchul Lee
Sample format(e.g. 16 bit, 24 bit) was not considered even if the
avoid-resampling option is set or the passthrough mode is used.
This patch checks both sample format and rate of a stream to
determine whether to avoid resampling in case of the option is set.
In other word, it is possble to use the stream's original sample
format and rate without resampling as long as these are supported
by the device.
pa_sink_input_update_rate() and pa_source_output_update_rate() are
renamed to pa_sink_input_update_resampler() and pa_source_output
_update_resampler() respectively.
---
src/modules/alsa/alsa-sink.c | 101 ++++++++++++++++++++++++++++++++---------
src/modules/alsa/alsa-source.c | 98 ++++++++++++++++++++++++++++++---------
src/pulsecore/sink-input.c | 19 ++++----
src/pulsecore/sink-input.h | 2 +-
src/pulsecore/sink.c | 26 ++++++-----
src/pulsecore/source-output.c | 22 +++++----
src/pulsecore/source-output.h | 2 +-
src/pulsecore/source.c | 24 +++++-----
8 files changed, 206 insertions(+), 88 deletions(-)
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 200d12b..60a4ed2 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -113,11 +113,19 @@ struct userdata {
pa_sample_format_t *supported_formats;
unsigned int *supported_rates;
+ struct {
+ size_t fragment_size;
+ size_t nfrags;
+ size_t tsched_size;
+ size_t tsched_watermark;
+ size_t rewind_safeguard;
+ } initial_info;
size_t
frame_size,
fragment_size,
hwbuf_size,
+ tsched_size,
tsched_watermark,
tsched_watermark_ref,
hwbuf_unused,
@@ -1081,12 +1089,34 @@ static void reset_watermark(struct userdata *u, size_t tsched_watermark, pa_samp
(double) u->tsched_watermark_usec / PA_USEC_PER_MSEC);
}
+/* Called from IO Context on unsuspend */
+static void update_size(struct userdata *u, pa_sample_spec *ss) {
+ pa_assert(u);
+ pa_assert(ss);
+
+ u->frame_size = pa_frame_size(ss);
+ u->frames_per_block = pa_mempool_block_size_max(u->core->mempool) / u->frame_size;
+
+ /* use initial values including module arguments */
+ u->fragment_size = u->initial_info.fragment_size;
+ u->hwbuf_size = u->initial_info.nfrags * u->fragment_size;
+ u->tsched_size = u->initial_info.tsched_size;
+ u->tsched_watermark = u->initial_info.tsched_watermark;
+ u->rewind_safeguard = u->initial_info.rewind_safeguard;
+
+ u->tsched_watermark_ref = u->tsched_watermark;
+
+ pa_log_info("Updated frame_size %zu, frames_per_block %lu, fragment_size %zu, hwbuf_size %zu, tsched(size %zu, watermark %zu), rewind_safeguard %zu",
+ u->frame_size, (unsigned long) u->frames_per_block, u->fragment_size, u->hwbuf_size, u->tsched_size, u->tsched_watermark, u->rewind_safeguard);
+}
+
/* Called from IO context */
static int unsuspend(struct userdata *u) {
pa_sample_spec ss;
int err;
bool b, d;
- snd_pcm_uframes_t period_size, buffer_size;
+ snd_pcm_uframes_t period_frames, buffer_frames;
+ snd_pcm_uframes_t tsched_frames = 0;
char *device_name = NULL;
pa_assert(u);
@@ -1111,13 +1141,18 @@ static int unsuspend(struct userdata *u) {
goto fail;
}
+ if (pa_frame_size(&u->sink->sample_spec) != u->frame_size) {
+ update_size(u, &u->sink->sample_spec);
+ tsched_frames = u->tsched_size / u->frame_size;
+ }
+
ss = u->sink->sample_spec;
- period_size = u->fragment_size / u->frame_size;
- buffer_size = u->hwbuf_size / u->frame_size;
+ period_frames = u->fragment_size / u->frame_size;
+ buffer_frames = u->hwbuf_size / u->frame_size;
b = u->use_mmap;
d = u->use_tsched;
- if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_size, &buffer_size, 0, &b, &d, true)) < 0) {
+ if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_frames, &buffer_frames, tsched_frames, &b, &d, true)) < 0) {
pa_log("Failed to set hardware parameters: %s", pa_alsa_strerror(err));
goto fail;
}
@@ -1132,11 +1167,17 @@ static int unsuspend(struct userdata *u) {
goto fail;
}
- if (period_size*u->frame_size != u->fragment_size ||
- buffer_size*u->frame_size != u->hwbuf_size) {
- pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %lu/%lu, New %lu/%lu)",
- (unsigned long) u->hwbuf_size, (unsigned long) u->fragment_size,
- (unsigned long) (buffer_size*u->frame_size), (unsigned long) (period_size*u->frame_size));
+ if (tsched_frames) {
I think a separate "frame_size_changed" flag should be used here,
because now it looks like u->fragment_size and u->hwbuf_size are
updated only if tsched is enabled (I got confused by this initially).

(Now when I read my previous review, I see that using tsched_frames was
my idea. It was a bad idea nevertheless.)
Post by Sangchul Lee
+ u->fragment_size = (size_t)(period_frames * u->frame_size);
+ u->hwbuf_size = (size_t)(buffer_frames * u->frame_size);
+ pa_proplist_setf(u->sink->proplist, PA_PROP_DEVICE_BUFFERING_BUFFER_SIZE, "%zu", u->hwbuf_size);
+ pa_proplist_setf(u->sink->proplist, PA_PROP_DEVICE_BUFFERING_FRAGMENT_SIZE, "%zu", u->fragment_size);
+
+ } else if (period_frames * u->frame_size != u->fragment_size ||
+ buffer_frames * u->frame_size != u->hwbuf_size) {
+ pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %zu/%zu, New %lu/%lu)",
+ u->hwbuf_size, u->fragment_size,
+ (unsigned long) buffer_frames * u->frame_size, (unsigned long) period_frames * u->frame_size);
goto fail;
}
@@ -1674,33 +1715,43 @@ static bool sink_set_formats(pa_sink *s, pa_idxset *formats) {
static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
struct userdata *u = s->userdata;
int i;
- bool supported = false;
-
- /* FIXME: we only update rate for now */
+ bool format_supported = false;
+ bool rate_supported = false;
pa_assert(u);
+ if (PA_SINK_IS_OPENED(s->state)) {
+ pa_log_warn("Sink is opened, skip it");
+ return -1;
+ }
The sink should never be opened when reconfiguring. You can remove this
or change it into an assertion.
Post by Sangchul Lee
+
+ for (i = 0; u->supported_formats[i] != PA_SAMPLE_MAX; i++) {
+ if (u->supported_formats[i] == spec->format) {
+ u->sink->sample_spec.format = spec->format;
+ format_supported = true;
+ pa_log_info("Sink supports sample format of %s", pa_sample_format_to_string(spec->format));
I think it's more interesting that the format is changed than that the
format is supported. I suggest you add pa_sink_set_format() function
that logs the following message:

pa_log_info("%s: format: %s -> %s", sink->name, old_format, new_format);

and pa_sink_set_format() should also send a change notification with
pa_subscription_post(). Both of these things should be done only if the
format actually changes.

If the requested format is not supported, I think we should switch to
the default format. Imagine a device that supports 8-bit and 16-bit
audio. First a 8-bit stream plays, and the device switches to that, and
then a 24-bit stream plays. If we don't switch to the default format,
the device will keep using the 8-bit format, which is not nice.

In my previous review I said that if the user has requested a specific
format with the "format" modarg, we should never switch the format, but
now I think that's not a good idea, since it could prevent passthrough
and avoid-resampling from working. If the "format" modarg is given, it
should just be used as the default format.
Post by Sangchul Lee
+ break;
+ }
+ }
+
for (i = 0; u->supported_rates[i]; i++) {
if (u->supported_rates[i] == spec->rate) {
- supported = true;
+ u->sink->sample_spec.rate = spec->rate;
+ rate_supported = true;
+ pa_log_info("Sink supports sample rate of %d Hz", spec->rate);
Same comments as above.
Post by Sangchul Lee
break;
}
}
- if (!supported) {
- pa_log_info("Sink does not support sample rate of %d Hz", spec->rate);
+ if (!format_supported && !rate_supported) {
+ pa_log_warn("Sink does not support both sample format of %s and rate of %d Hz",
+ pa_sample_format_to_string(spec->format), spec->rate);
The log level should not be warning, because it's perfectly normal that
the hardware doesn't support all rates and formats.
Post by Sangchul Lee
return -1;
}
- if (!PA_SINK_IS_OPENED(s->state)) {
- pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, spec->rate);
- u->sink->sample_spec.rate = spec->rate;
- return 0;
- }
-
/* Passthrough status change is handled during unsuspend */
- return -1;
+ return 0;
}
static int process_rewind(struct userdata *u) {
@@ -2212,6 +2263,12 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca
u->module = m;
u->use_mmap = use_mmap;
u->use_tsched = use_tsched;
+ u->tsched_size = tsched_size;
+ u->initial_info.nfrags = (size_t) nfrags;
+ u->initial_info.fragment_size = (size_t) frag_size;
+ u->initial_info.tsched_size = (size_t) tsched_size;
+ u->initial_info.tsched_watermark = (size_t) tsched_watermark;
+ u->initial_info.rewind_safeguard = (size_t) rewind_safeguard;
u->deferred_volume = deferred_volume;
u->fixed_latency_range = fixed_latency_range;
u->first = true;
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index cba86ca..92b2125 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -101,11 +101,18 @@ struct userdata {
pa_sample_format_t *supported_formats;
unsigned int *supported_rates;
+ struct {
+ size_t fragment_size;
+ size_t nfrags;
+ size_t tsched_size;
+ size_t tsched_watermark;
+ } initial_info;
size_t
frame_size,
fragment_size,
hwbuf_size,
+ tsched_size,
tsched_watermark,
tsched_watermark_ref,
hwbuf_unused,
@@ -947,12 +954,33 @@ static void reset_watermark(struct userdata *u, size_t tsched_watermark, pa_samp
(double) u->tsched_watermark_usec / PA_USEC_PER_MSEC);
}
+/* Called from IO Context on unsuspend */
+static void update_size(struct userdata *u, pa_sample_spec *ss) {
+ pa_assert(u);
+ pa_assert(ss);
+
+ u->frame_size = pa_frame_size(ss);
+ u->frames_per_block = pa_mempool_block_size_max(u->core->mempool) / u->frame_size;
+
+ /* use initial values including module arguments */
+ u->fragment_size = u->initial_info.fragment_size;
+ u->hwbuf_size = u->initial_info.nfrags * u->fragment_size;
+ u->tsched_size = u->initial_info.tsched_size;
+ u->tsched_watermark = u->initial_info.tsched_watermark;
+
+ u->tsched_watermark_ref = u->tsched_watermark;
+
+ pa_log_info("Updated frame_size %zu, frames_per_block %lu, fragment_size %zu, hwbuf_size %zu, tsched(size %zu, watermark %zu)",
+ u->frame_size, (unsigned long) u->frames_per_block, u->fragment_size, u->hwbuf_size, u->tsched_size, u->tsched_watermark);
+}
+
/* Called from IO context */
static int unsuspend(struct userdata *u) {
pa_sample_spec ss;
int err;
bool b, d;
- snd_pcm_uframes_t period_size, buffer_size;
+ snd_pcm_uframes_t period_frames, buffer_frames;
+ snd_pcm_uframes_t tsched_frames = 0;
pa_assert(u);
pa_assert(!u->pcm_handle);
@@ -968,13 +996,18 @@ static int unsuspend(struct userdata *u) {
goto fail;
}
+ if (pa_frame_size(&u->source->sample_spec) != u->frame_size) {
+ update_size(u, &u->source->sample_spec);
+ tsched_frames = u->tsched_size / u->frame_size;
+ }
+
ss = u->source->sample_spec;
- period_size = u->fragment_size / u->frame_size;
- buffer_size = u->hwbuf_size / u->frame_size;
+ period_frames = u->fragment_size / u->frame_size;
+ buffer_frames = u->hwbuf_size / u->frame_size;
b = u->use_mmap;
d = u->use_tsched;
- if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_size, &buffer_size, 0, &b, &d, true)) < 0) {
+ if ((err = pa_alsa_set_hw_params(u->pcm_handle, &ss, &period_frames, &buffer_frames, tsched_frames, &b, &d, true)) < 0) {
pa_log("Failed to set hardware parameters: %s", pa_alsa_strerror(err));
goto fail;
}
@@ -989,11 +1022,17 @@ static int unsuspend(struct userdata *u) {
goto fail;
}
- if (period_size*u->frame_size != u->fragment_size ||
- buffer_size*u->frame_size != u->hwbuf_size) {
- pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %lu/%lu, New %lu/%lu)",
- (unsigned long) u->hwbuf_size, (unsigned long) u->fragment_size,
- (unsigned long) (buffer_size*u->frame_size), (unsigned long) (period_size*u->frame_size));
+ if (tsched_frames) {
+ u->fragment_size = (size_t)(period_frames * u->frame_size);
+ u->hwbuf_size = (size_t)(buffer_frames * u->frame_size);
+ pa_proplist_setf(u->source->proplist, PA_PROP_DEVICE_BUFFERING_BUFFER_SIZE, "%zu", u->hwbuf_size);
+ pa_proplist_setf(u->source->proplist, PA_PROP_DEVICE_BUFFERING_FRAGMENT_SIZE, "%zu", u->fragment_size);
+
+ } else if (period_frames * u->frame_size != u->fragment_size ||
+ buffer_frames * u->frame_size != u->hwbuf_size) {
+ pa_log_warn("Resume failed, couldn't restore original fragment settings. (Old: %zu/%zu, New %lu/%lu)",
+ u->hwbuf_size, u->fragment_size,
+ (unsigned long) buffer_frames * u->frame_size, (unsigned long) period_frames * u->frame_size);
goto fail;
}
@@ -1470,31 +1509,41 @@ static void source_update_requested_latency_cb(pa_source *s) {
static int source_reconfigure_cb(pa_source *s, pa_sample_spec *spec, bool passthrough) {
struct userdata *u = s->userdata;
int i;
- bool supported = false;
-
- /* FIXME: we only update rate for now */
+ bool format_supported = false;
+ bool rate_supported = false;
pa_assert(u);
+ if (PA_SOURCE_IS_OPENED(s->state)) {
+ pa_log_warn("Source is opened, skip it");
+ return -1;
+ }
+
+ for (i = 0; u->supported_formats[i] != PA_SAMPLE_MAX; i++) {
+ if (u->supported_formats[i] == spec->format) {
+ u->source->sample_spec.format = spec->format;
+ format_supported = true;
+ pa_log_info("Source supports sample format of %s", pa_sample_format_to_string(spec->format));
+ break;
+ }
+ }
+
for (i = 0; u->supported_rates[i]; i++) {
if (u->supported_rates[i] == spec->rate) {
- supported = true;
+ u->source->sample_spec.rate = spec->rate;
+ rate_supported = true;
+ pa_log_info("Source supports sample rate of %d Hz", spec->rate);
break;
}
}
- if (!supported) {
- pa_log_info("Source does not support sample rate of %d Hz", spec->rate);
+ if (!format_supported && !rate_supported) {
+ pa_log_warn("Source does not support both sample format of %s and rate of %d Hz",
+ pa_sample_format_to_string(spec->format), spec->rate);
return -1;
}
- if (!PA_SOURCE_IS_OPENED(s->state)) {
- pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, spec->rate);
- u->source->sample_spec.rate = spec->rate;
- return 0;
- }
-
- return -1;
+ return 0;
}
static void thread_func(void *userdata) {
@@ -1899,6 +1948,11 @@ pa_source *pa_alsa_source_new(pa_module *m, pa_modargs *ma, const char*driver, p
u->module = m;
u->use_mmap = use_mmap;
u->use_tsched = use_tsched;
+ u->tsched_size = tsched_size;
+ u->initial_info.nfrags = (size_t) nfrags;
+ u->initial_info.fragment_size = (size_t) frag_size;
+ u->initial_info.tsched_size = (size_t) tsched_size;
+ u->initial_info.tsched_watermark = (size_t) tsched_watermark;
u->deferred_volume = deferred_volume;
u->fixed_latency_range = fixed_latency_range;
u->first = true;
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 312ec4a..81b4456 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -417,10 +417,10 @@ int pa_sink_input_new(
if (!(data->flags & PA_SINK_INPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&data->sample_spec, &data->sink->sample_spec)) {
- /* try to change sink rate. This is done before the FIXATE hook since
+ /* try to change sink format and 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");
+ pa_log_info("Trying to change sample spec");
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);
The log message needs updating, because it may be that only the format
was changed. Or actually the message can be removed if you add the
pa_sink_set_format() and pa_sink_set_rate() functions that do the
logging as I suggested above.
Post by Sangchul Lee
}
@@ -616,7 +616,7 @@ static void sink_input_set_state(pa_sink_input *i, pa_sink_input_state_t state)
if (i->state == PA_SINK_INPUT_CORKED && state == PA_SINK_INPUT_RUNNING && pa_sink_used_by(i->sink) == 0 &&
!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 */
+ * to update the sample format and rate to avoid resampling */
pa_sink_reconfigure(i->sink, &i->sample_spec, pa_sink_input_is_passthrough(i));
}
@@ -1901,13 +1901,14 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
if (!(i->flags & PA_SINK_INPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&i->sample_spec, &dest->sample_spec)) {
- /* try to change dest sink rate if possible without glitches.
+ /* try to change dest sink format and rate if possible without glitches.
module-suspend-on-idle resumes destination sink with
SINK_INPUT_MOVE_FINISH hook */
- pa_log_info("Trying to change sample rate");
+ pa_log_info("Trying to change sample spec");
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("format, rate are updated to %s, %u Hz",
+ pa_sample_format_to_string(dest->sample_spec.format), dest->sample_spec.rate);
This message can also be removed if you add the pa_sink_set_format()
and pa_sink_set_rate() functions.
Post by Sangchul Lee
}
if (i->moving)
@@ -1925,7 +1926,7 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
if (i->state == PA_SINK_INPUT_CORKED)
i->sink->n_corked++;
- pa_sink_input_update_rate(i);
+ pa_sink_input_update_resampler(i);
pa_sink_update_status(dest);
/* Called from main context */
/* Updates the sink input's resampler with whatever the current sink requires
- * -- useful when the underlying sink's rate might have changed */
-int pa_sink_input_update_rate(pa_sink_input *i) {
+ * -- useful when the underlying sink's sample spec might have changed */
+int pa_sink_input_update_resampler(pa_sink_input *i) {
pa_resampler *new_resampler;
char *memblockq_name;
diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h
index 9e90291..6bf406c 100644
--- a/src/pulsecore/sink-input.h
+++ b/src/pulsecore/sink-input.h
@@ -353,7 +353,7 @@ void pa_sink_input_request_rewind(pa_sink_input *i, size_t nbytes, bool rewrite,
void pa_sink_input_cork(pa_sink_input *i, bool b);
int pa_sink_input_set_rate(pa_sink_input *i, uint32_t rate);
-int pa_sink_input_update_rate(pa_sink_input *i);
+int pa_sink_input_update_resampler(pa_sink_input *i);
/* This returns the sink's fields converted into out sample type */
size_t pa_sink_input_get_max_rewind(pa_sink_input *i);
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 566367f..b2ebc2f 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -1448,8 +1448,6 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
bool alternate_rate_is_usable = false;
bool avoid_resampling = s->avoid_resampling;
- /* We currently only try to reconfigure the sample rate */
-
if (pa_sample_spec_equal(spec, &s->sample_spec))
return 0;
@@ -1462,14 +1460,14 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
}
if (PA_SINK_IS_RUNNING(s->state)) {
- pa_log_info("Cannot update rate, SINK_IS_RUNNING, will keep using %u Hz",
- s->sample_spec.rate);
+ 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 -1;
}
if (s->monitor_source) {
if (PA_SOURCE_IS_RUNNING(s->monitor_source->state) == true) {
- pa_log_info("Cannot update rate, monitor source is RUNNING");
+ pa_log_info("Cannot update sample spec, monitor source is RUNNING");
return -1;
}
}
@@ -1480,12 +1478,15 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
desired_spec = s->sample_spec;
if (passthrough) {
- /* We have to try to use the sink input rate */
+ /* We have to try to use the sink input format and rate */
+ desired_spec.format = spec->format;
desired_spec.rate = spec->rate;
- } else if (avoid_resampling && (spec->rate >= default_rate || spec->rate >= alternate_rate)) {
+ } else if (avoid_resampling) {
/* We just try to set the sink input's sample rate if it's not too low */
- desired_spec.rate = spec->rate;
+ if (spec->rate >= default_rate || spec->rate >= alternate_rate)
+ desired_spec.rate = spec->rate;
If the rate is too low and desired_spec is not updated, then we should
apply the logic in the "else" section (where we pick either
default_rate or alternate_rate). With your current code this doesn't
happen.

To fix this, let's replace

} else {

with

}

if (desired_spec.rate != spec->rate) {
Post by Sangchul Lee
+ desired_spec.format = spec->format;
} else if (default_rate == spec->rate || alternate_rate == spec->rate) {
/* We can directly try to use this rate */
@@ -1514,18 +1515,19 @@ int pa_sink_reconfigure(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
if (!passthrough && pa_sink_used_by(s) > 0)
return -1;
- pa_log_debug("Suspending sink %s due to changing format, desired rate = %u", s->name, desired_spec.rate);
+ 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);
if (s->reconfigure(s, &desired_spec, passthrough) >= 0) {
/* update monitor source as well */
if (s->monitor_source && !passthrough)
- pa_source_reconfigure(s->monitor_source, &desired_spec, false);
- pa_log_info("Changed format successfully");
+ pa_source_reconfigure(s->monitor_source, &s->sample_spec, false);
+ pa_log_info("Reconfigured successfully");
PA_IDXSET_FOREACH(i, s->inputs, idx) {
if (i->state == PA_SINK_INPUT_CORKED)
- pa_sink_input_update_rate(i);
+ pa_sink_input_update_resampler(i);
}
ret = 0;
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 955a2ac..6e56b92 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -365,12 +365,13 @@ int pa_source_output_new(
if (!(data->flags & PA_SOURCE_OUTPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&data->sample_spec, &data->source->sample_spec)) {
- /* try to change source rate. This is done before the FIXATE hook since
+ /* try to change source format and rate. This is done before the FIXATE hook since
module-suspend-on-idle can resume a source */
- pa_log_info("Trying to change sample rate");
+ pa_log_info("Trying to change sample spec");
if (pa_source_reconfigure(data->source, &data->sample_spec, pa_source_output_new_data_is_passthrough(data)) >= 0)
- pa_log_info("Rate changed to %u Hz", data->source->sample_spec.rate);
+ pa_log_info("format, rate are updated to %s, %u Hz",
+ pa_sample_format_to_string(data->source->sample_spec.format), data->source->sample_spec.rate);
}
if (pa_source_output_new_data_is_passthrough(data) &&
@@ -542,7 +543,7 @@ static void source_output_set_state(pa_source_output *o, pa_source_output_state_
if (o->state == PA_SOURCE_OUTPUT_CORKED && state == PA_SOURCE_OUTPUT_RUNNING && pa_source_used_by(o->source) == 0 &&
!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 */
+ * to update the sample format and rate to avoid resampling */
pa_source_reconfigure(o->source, &o->sample_spec, pa_source_output_is_passthrough(o));
}
@@ -1533,13 +1534,14 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save
if (!(o->flags & PA_SOURCE_OUTPUT_VARIABLE_RATE) &&
!pa_sample_spec_equal(&o->sample_spec, &dest->sample_spec)) {
- /* try to change dest source rate if possible without glitches.
+ /* try to change dest source format and rate if possible without glitches.
module-suspend-on-idle resumes destination source with
SOURCE_OUTPUT_MOVE_FINISH hook */
- pa_log_info("Trying to change sample rate");
+ pa_log_info("Trying to change sample spec");
if (pa_source_reconfigure(dest, &o->sample_spec, pa_source_output_is_passthrough(o)) >= 0)
- pa_log_info("Rate changed to %u Hz", dest->sample_spec.rate);
+ pa_log_info("format, rate are updated to %s, %u Hz",
+ pa_sample_format_to_string(dest->sample_spec.format), dest->sample_spec.rate);
}
if (o->moving)
@@ -1554,7 +1556,7 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save
if (o->state == PA_SOURCE_OUTPUT_CORKED)
o->source->n_corked++;
- pa_source_output_update_rate(o);
+ pa_source_output_update_resampler(o);
pa_source_update_status(dest);
/* Called from main context */
/* Updates the source output's resampler with whatever the current source
- * requires -- useful when the underlying source's rate might have changed */
-int pa_source_output_update_rate(pa_source_output *o) {
+ * requires -- useful when the underlying source's sample spec might have changed */
+int pa_source_output_update_resampler(pa_source_output *o) {
pa_resampler *new_resampler;
char *memblockq_name;
diff --git a/src/pulsecore/source-output.h b/src/pulsecore/source-output.h
index 661b64b..9cbc286 100644
--- a/src/pulsecore/source-output.h
+++ b/src/pulsecore/source-output.h
@@ -307,7 +307,7 @@ pa_usec_t pa_source_output_set_requested_latency(pa_source_output *o, pa_usec_t
void pa_source_output_cork(pa_source_output *o, bool b);
int pa_source_output_set_rate(pa_source_output *o, uint32_t rate);
-int pa_source_output_update_rate(pa_source_output *o);
+int pa_source_output_update_resampler(pa_source_output *o);
size_t pa_source_output_get_max_rewind(pa_source_output *o);
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index b501733..b012516 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -1029,8 +1029,6 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
bool alternate_rate_is_usable = false;
bool avoid_resampling = s->avoid_resampling;
- /* We currently only try to reconfigure the sample rate */
-
if (pa_sample_spec_equal(spec, &s->sample_spec))
return 0;
@@ -1043,14 +1041,14 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
}
if (PA_SOURCE_IS_RUNNING(s->state)) {
- pa_log_info("Cannot update rate, SOURCE_IS_RUNNING, will keep using %u Hz",
- s->sample_spec.rate);
+ 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 -1;
}
if (s->monitor_of) {
if (PA_SINK_IS_RUNNING(s->monitor_of->state)) {
- pa_log_info("Cannot update rate, 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;
}
}
@@ -1061,12 +1059,15 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
desired_spec = s->sample_spec;
if (passthrough) {
- /* We have to try to use the source output rate */
+ /* We have to try to use the source output format and rate */
+ desired_spec.format = spec->format;
desired_spec.rate = spec->rate;
- } else if (avoid_resampling && (spec->rate >= default_rate || spec->rate >= alternate_rate)) {
+ } else if (avoid_resampling) {
/* We just try to set the source output's sample rate if it's not too low */
- desired_spec.rate = spec->rate;
+ if (spec->rate >= default_rate || spec->rate >= alternate_rate)
+ desired_spec.rate = spec->rate;
+ desired_spec.format = spec->format;
} else if (default_rate == spec->rate || alternate_rate == spec->rate) {
/* We can directly try to use this rate */
@@ -1095,7 +1096,8 @@ 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 to %u", s->name, desired_spec.rate);
+ 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);
if (s->reconfigure)
@@ -1135,10 +1137,10 @@ int pa_source_reconfigure(pa_source *s, pa_sample_spec *spec, bool passthrough)
PA_IDXSET_FOREACH(o, s->outputs, idx) {
if (o->state == PA_SOURCE_OUTPUT_CORKED)
- pa_source_output_update_rate(o);
+ pa_source_output_update_resampler(o);
}
- pa_log_info("Changed sampling rate successfully");
+ pa_log_info("Reconfigured successfully");
}
pa_source_suspend(s, false, PA_SUSPEND_INTERNAL);
--
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk
Loading...