[pulseaudio-discuss] [PATCH 09/13] loopback: Track the amount of jitter

[Alexander E. Patrakov]

12.12.2015 20:39, Tanu Kaskinen wrote:
> On Sat, 2015-12-12 at 11:52 +0100, Georg Chini wrote:
>> On 08.12.2015 21:47, Tanu Kaskinen wrote:
>>> On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
>>>> ---
>>>>    src/modules/module-loopback.c | 18 +++++++++++++++++-
>>>>    1 file changed, 17 insertions(+), 1 deletion(-)
>>> The commit message should say something about why the jitter is tracked.
>>
>> OK
>>
>>>> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
>>>> index cbd0ac9..b733663 100644
>>>> --- a/src/modules/module-loopback.c
>>>> +++ b/src/modules/module-loopback.c
>>>> @@ -95,6 +95,8 @@ struct userdata {
>>>>
>>>>        pa_usec_t source_latency_sum;
>>>>        pa_usec_t sink_latency_sum;
>>>> +    pa_usec_t next_latency;
>>>> +    double latency_error;
>>>>
>>>>        bool in_pop;
>>>>        bool pop_called;
>>>> @@ -263,15 +265,22 @@ static void adjust_rates(struct userdata *u) {
>>>>                    (double) current_latency / PA_USEC_PER_MSEC,
>>>>                    (double) corrected_latency / PA_USEC_PER_MSEC,
>>>>                    ((double) u->latency_snapshot.sink_latency + current_buffer_latency + u->latency_snapshot.source_latency) / PA_USEC_PER_MSEC);
>>>> -    pa_log_debug("Latency difference: %0.2f ms, rate difference: %i Hz",
>>>> +    pa_log_debug("Latency difference: %0.2f ± %0.2f ms, rate difference: %i Hz",
>>> What does "± %0.2f ms" mean? Is the real latency difference between
>>> those bounds with 100% confidence, or less than 100% confidence?
>>
>> This is just an indication of the current error (with respect to the model),
>> so not 100% confidence.
>
> Can you assign any particular confidence to the numbers? If not, then
> the numbers have very little meaning... They can be useful for seeing
> whether the jitter is higher or lower compared to some other
> measurements, but they say very little about whether the latency
> difference really is between those bounds.
>
> At the very least there should be a comment explaining how the numbers
> should be interpreted.
>
>>>
>>>>                    (double) latency_difference / PA_USEC_PER_MSEC,
>>>> +                (double) 2.5 * u->latency_error * final_latency / PA_USEC_PER_MSEC,
>>> Why is that 2.5 there?
>>>
>>
>> Good question. I think it was copied over from the definition of the
>> deadband in the next
>> patch. The observed errors are however in good agreement with that factor.
>
> What does that mean? What would be different if the factor wasn't
> there? How would it be worse than with the magic factor?

Well, the logic here is still based on the equivalent of the 
n-sigma-rule. We need to figure out whether the latency difference that 
we see can be adequately explained by "the usual noise" in the 
measurements (and thus, according to the current version of the patchset 
does not have to be corrected), or not (and thus has to be corrected as 
quickly as possible). By changing the magic number, one can trade one 
kind of errors for the other. If it is too low, then, quite often, the 
deadband logic will think "oh, we need to correct this latency 
difference", only to find out during the next cycle that the correction 
has to be undone. If it is too high, then more of the latency difference 
(due to the clock drift) will need to accumulate before the controller 
finally decides to correct that.

Future versions of the patchset, if I understand the situation 
correctly, will still contain similar logic, in order to switch between 
the "correct this slowly and carefully" and "correct this as quickly as 
possible" modes.

-- 
Alexander E. Patrakov