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

[Raman Shishniou]

On 02/14/2018 07:20 PM, Georg Chini wrote:
> 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.
> 

All situations where where latency is more important than data integrity.
Voice over IP (telephony) for example, receiving audio data using network
by UDP/RTP. Any data loss leads to underruns in loopback module. Current
implementation will increase overall latency by 5 msec every 3 underruns. 

>> ---
>>   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;
> 
> 
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss