[pulseaudio-discuss] [PATCH] loopback: underrun_protection argument

[Georg Chini]

On 14.02.2018 15:25, Raman Shyshniou wrote:
> This patch adds a underrun_protection argument to
> control underrun protection algorithm. Disabling
> protection will keep loopback latency regardless
> of underruns.

Again I do not understand the motivation of the patch.
In what situations are you seeing so many underruns and
still want to keep the original configured latency value?
Audio will be very bad in that case.

> ---
>   src/modules/module-loopback.c | 31 +++++++++++++++++++++++++------
>   1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> index aecac0a..c452e1a 100644
> --- a/src/modules/module-loopback.c
> +++ b/src/modules/module-loopback.c
> @@ -45,6 +45,7 @@ PA_MODULE_USAGE(
>           "sink=<sink to connect to> "
>           "adjust_time=<how often to readjust rates in s> "
>           "latency_msec=<latency in ms> "
> +        "underrun_protection=<boolean> "
>           "format=<sample format> "
>           "rate=<sample rate> "
>           "channels=<number of channels> "
> @@ -90,6 +91,7 @@ struct userdata {
>       /* Values from command line configuration */
>       pa_usec_t latency;
>       pa_usec_t adjust_time;
> +    bool underrun_protection;
>   
>       /* Latency boundaries and current values */
>       pa_usec_t min_source_latency;
> @@ -158,6 +160,7 @@ static const char* const valid_modargs[] = {
>       "sink",
>       "adjust_time",
>       "latency_msec",
> +    "underrun_protection",
>       "format",
>       "rate",
>       "channels",
> @@ -325,11 +328,20 @@ static void adjust_rates(struct userdata *u) {
>   
>       /* If we are seeing underruns then the latency is too small */
>       if (u->underrun_counter > 2) {
> -        u->underrun_latency_limit = PA_MAX(u->latency, u->minimum_latency) + 5 * PA_USEC_PER_MSEC;
> -        u->underrun_latency_limit = PA_CLIP_SUB((int64_t)u->underrun_latency_limit, u->sink_latency_offset + u->source_latency_offset);
> -        update_minimum_latency(u, u->sink_input->sink, false);
> -        pa_log_warn("Too many underruns, increasing latency to %0.2f ms", (double)u->minimum_latency / PA_USEC_PER_MSEC);
> -        u->underrun_counter = 0;
> +        int64_t target_latency;
> +
> +        target_latency = PA_MAX(u->latency, u->minimum_latency);
> +
> +        if (u->underrun_protection)
> +            target_latency += 5 * PA_USEC_PER_MSEC;
> +
> +        u->underrun_latency_limit = PA_CLIP_SUB(target_latency, u->sink_latency_offset + u->source_latency_offset);
> +
> +        if (u->minimum_latency < u->underrun_latency_limit) {

The above comparison does not make sense to me. The minimum latency
should always be smaller than (or equal to) the underrun_latency_limit.
Otherwise we would not be seeing underruns. There are two ways, the
minimum latency is calculated:
1) Theoretically. Anything below that value will definitely cause underruns.
The value is directly derived from the minimum sink/source latency.
2) Practically: If we are seeing underruns at some latency, we know the
theoretical value is too small and update it to the empirically found value.

> +            update_minimum_latency(u, u->sink_input->sink, false);
> +            pa_log_warn("Too many underruns, increasing latency to %0.2f ms", (double)u->minimum_latency / PA_USEC_PER_MSEC);
> +            u->underrun_counter = 0;
> +        }
>       }
>   
>       /* Allow one underrun per hour */
> @@ -347,7 +359,7 @@ static void adjust_rates(struct userdata *u) {
>       }
>       u->adjust_time_stamp = now;
>   
> -    /* Rates and latencies*/
> +    /* Rates and latencies */
>       old_rate = u->sink_input->sample_spec.rate;
>       base_rate = u->source_output->sample_spec.rate;
>   
> @@ -1251,6 +1263,7 @@ int pa__init(pa_module *m) {
>       pa_source *source = NULL;
>       pa_source_output_new_data source_output_data;
>       bool source_dont_move;
> +    bool underrun_protection = true;
>       uint32_t latency_msec;
>       pa_sample_spec ss;
>       pa_channel_map map;
> @@ -1336,10 +1349,16 @@ int pa__init(pa_module *m) {
>           goto fail;
>       }
>   
> +    if (pa_modargs_get_value_boolean(ma, "underrun_protection", &underrun_protection) < 0) {
> +        pa_log("Failed to parse underrun_protection value.");
> +        goto fail;
> +    }
> +
>       m->userdata = u = pa_xnew0(struct userdata, 1);
>       u->core = m->core;
>       u->module = m;
>       u->latency = (pa_usec_t) latency_msec * PA_USEC_PER_MSEC;
> +    u->underrun_protection = underrun_protection;
>       u->output_thread_info.pop_called = false;
>       u->output_thread_info.pop_adjust = false;
>       u->output_thread_info.push_called = false;