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

[Georg Chini]

On 12.12.2015 17:04, Alexander E. Patrakov wrote:
> 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?
>

I think I explained it in one of the other mails. With the factor 2.5 it 
matches the observation,
meaning that when it says plus/minus 150 usec then it is highly likely 
that the next measurement
will fit into that interval. I cannot give you a level of confidence 
however.

> 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.
>
No, the strength of the P-controller is only dependent on the distance 
to the target
latency in the latest version. The average error is only used to modify 
one parameter
of the drift controller.