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