On 08.12.2015 23:04, Tanu Kaskinen wrote:
> On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
>> With USB or Bluetooth sources, the controller exhibited random
>> deviations of new_rate around the correct value, due to latency jitter.
>> Use the already-known latency error due to jitter, and assume that there
>> is no latency difference if the difference is below that error margin.
>> ---
>> src/modules/module-loopback.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
>> index b733663..6b48fc6 100644
>> --- a/src/modules/module-loopback.c
>> +++ b/src/modules/module-loopback.c
>> @@ -183,12 +183,12 @@ static void teardown(struct userdata *u) {
>> /* rate controller
>> * - maximum deviation from base rate is less than 1%
>> * - can create audible artifacts by changing the rate too quickly
>> - * - exhibits hunting with USB or Bluetooth sources
>> + * - deadband to handle error of latency measurement
>> */
>> static uint32_t rate_controller(
>> uint32_t base_rate,
>> pa_usec_t adjust_time,
>> - int32_t latency_difference_usec) {
>> + int32_t latency_difference_usec, pa_usec_t latency_error_usec) {
>>
>> uint32_t new_rate;
>> double min_cycles;
>> @@ -198,6 +198,10 @@ static uint32_t rate_controller(
>> min_cycles = (double)abs(latency_difference_usec) / adjust_time / 0.0095 + 1;
>> new_rate = base_rate * (1.0 + (double)latency_difference_usec / min_cycles / adjust_time);
>>
>> + /* Adjust as good as physics allows (with some safety margin) */
>> + if (abs(latency_difference_usec) <= 2.5 * latency_error_usec + adjust_time / 2 / base_rate + 100)
>> + new_rate = base_rate;
> There's that magic number 2.5 again...
That's where that number is coming from in the first place. The idea was
that
I am tracking an average of the absolute errors, so the worst thing I should
be seeing is a jump from predicted_latency - error to predicted_latency
+error.
That's 2 * latency_error_usec. Because I am tracking an average, the actual
jump might even be larger, so I changed the factor to 2.5 as a safety
margin.
>
> What's that "adjust_time / 2 / base_rate"? The unit of that is seconds
> squared, doesn't make sense to me...
No, it isn't. I know it looks strange, but there is a second that you do
not see.
The smallest amount of latency correction possible at a given base_rate and
adjust_time is one sample per second of adjust_time. And that is the second
you cannot see in the term above (because it is 1).
It does not make sense to try to correct the latency when you are less than
half of that distance away from the desired latency because then you end
up further away from the target.
It does not account for a large part of the deadband, but when I tested my
patch I went down to sample rates of 50 Hz and below where it is
significant.
>
> Are you claiming that this results in adjustment that is as close to
> optimal as physics allows? If so, what's the basis for that claim?
>
Yes, within the restrictions of the used model. The deadband accounts
for
1) The error of the measurement (first term)
2) The minimum correctable latency amount (second term)
3) A general safety margin (third term)
So when I am within the bounds of the deadband it does not make sense to
try to correct the latency because I cannot be sure that I end up with a
better
value.
But overall Alexander convinced me that the deadband is not necessary, so
it will vanish in the next version of the patch. It is replaced by a
Kalman filter
to smooth down the measured latency.
Additionally the current patch set does not take the rate difference between
source and sink into account (it assumes that both are running exactly
at the
same frequency) and will so lead to sawtooth changes of the latency if this
assumption is not met. This is fixed in my new version.
I sent you a few results for the new version earlier today.
