[pulseaudio-discuss] [PATCH 00/13] loopback: Make module-loopback honor requested latency (v5)

[Alexander E. Patrakov]

19.11.2015 11:48, Georg Chini wrote:
> On 19.11.2015 05:08, Alexander E. Patrakov wrote:
>> 19.11.2015 00:43, Georg Chini wrote:
>>> On 15.11.2015 22:08, Alexander E. Patrakov wrote:
>>>> The second result (https://imgur.com/a/eVahQ ) is with the old module.
>>>> You see that, with it, the rate oscillates wildly and then snaps to
>>>> 44100 Hz. However, the final latency is not correct, and it
>>>> accumulates due to clock mismatch between the two sound cards in the
>>>> desktop. I have not waited enough to see what happens next, but the
>>>> result is clearly invalid.
>>>
>>> I wonder why the phase is not continuously growing in the final plot. I
>>> would expect this
>>> because the controller snaps to the base rate.
>>
>> The controller snaps to the base rate, then waits for the latency
>> difference to accumulate, then unsnaps. That's why the average rate is
>> still correct. Also, the script only looks at the audio waveform at
>> the output and thus cannot catch the situation where the module eats
>> the accumulating latency difference.
>>
>
> Are you sure you uploaded the right image? At least the last image looks
> like
> the old module is doing the best job of them all. The phase looks even
> better than
> in the original recording ...

Yes, this is the right image, but I did say "the result is clearly 
invalid". So please don't look :)

The fact that the old module has buffered all the accumulated clock skew 
has fooled my script, and it is not possible to know this fact by 
looking just at the output waveform.

>
>>>
>>>>
>>>> The next result (https://imgur.com/a/Bi2Yz ) is with the new module,
>>>> as of PATCH 04/13. There is some "rate hunting", and it results to
>>>> some noise in phase. It is generally contained between -150 and 200
>>>> degrees.
>>>>
>>>> Next, let's see (https://imgur.com/a/yQGWL ) what the deadband
>>>> achieves. Result: it helps against rate hunting, but not much against
>>>> phase oscillations. This is because new_rate is snapped to base_rate,
>>>> which would only be correct if the cards actually shared the common
>>>> clock. So here is what happens: the module runs for some time with 1:1
>>>> resampling (i.e. with the rate strictly equal to 44100 Hz), and the
>>>> latency difference slowly drifts due to the clock mismatch. When the
>>>> difference becomes bigger than the error of its measurement, the
>>>> module corrects it, and then runs at 44100 Hz for some time again.
>>>>
>>>
>>> Could you use 2 s as adjust time? 10 seconds is definitely too slow to
>>> achieve good results.
>>> I would expect a similar result but with much smaller amplitude of the
>>> phase oscillation.
>>
>> Will measure this in the evening.
>
> Thank you. Please also use 2 seconds when you test the new code. It works
> with 10 seconds as well but convergence will be much quicker with lower
> adjust times.

Well, I used 1s, to be consistent with my earlier measurement. Hopefully 
this is still valid for your purposes.

So https://imgur.com/a/P5Y0A (old) is with 1s and no deadband, and 
https://imgur.com/a/weW6H (new) is also with 1s, but with deadband. As 
you can see, a modified rate controller (that doesn't correct all the 
latency difference at once) provides better results than a deadband.

>
>>
>>>>
>>>> However, I'd argue that this phase metric can be improved without the
>>>> deadband and beyond what this deadband-based implementation provides.
>>>>
>>>
>>> What is the problem with the dead band? There are physical limitations
>>> to what makes
>>> sense to correct and the dead band just takes care of those limits.
>>
>> There are no such limits once you (or your second-order filter) start
>> averaging previous latency difference measurements.
>
> You are definitely wrong there. This would only be (nearly) true, if the
> error of the latency
> measurement would be small. In practice it isn't and even the best low
> pass filter
> will not remove all of the error. Take a look at the latency error
> values that my
> code provides. It already is a low pass filtered value.

I can, so far, only make a conclusion that I should try a different 
lowpass filter.

> Especially in the first couple of seconds you do not get very reliable
> values. There
> must be something in pulseaudio which is preventing that. Maybe it is
> the time
> smoother that has to settle down. I think you can also see this effect
> in the images
> of the original recording - the phase is drifting during the first 50
> seconds and
> then it is nearly constant.

The phase is drifting because the original latency is too far from the 
desired one. Nonzero latency difference => nonzero rate adjustment as 
compared to the ideal value => phase drift. Or, as phase is directly 
proportional to the latency, it's just the "correction of the big 
initial latency difference" process. So, my plots indeed show how fast 
or how slow the rate controller deals with such bad initial conditions.

> To keep the latency constant you probably do not need the dead band, I
> agree.
> But if you want to control the absolute value, it makes sense.

I think we don't understand each other's argument. So let's keep this 
disagreement for now, until more experimental results appear.

>
>>
>>
>>>
>>>>
>>>> First, see (https://imgur.com/a/P5Y0A ) what happens, at PATCH 04/13,
>>>> if we just decrease adjust_time to 1 second. The rate oscillations
>>>> become bigger, but phase oscillations stay of the same order as with
>>>> the default value of adjust_time (10 seconds). So not good.
>>>>
>>>> Then, let's change the rate controller so that it does not attempt to
>>>> correct all of the latency difference in one step. The value of
>>>> adjust_time is still 1s, but let's aim to correct the difference after
>>>> 10 steps by a factor of 2.71. For simplicity (and for testing purposes
>>>> only), let's destroy the non-linear part of the controller. Here is
>>>> the code:
>>>>
>>>> static uint32_t rate_controller(
>>>>                 uint32_t base_rate,
>>>>                 pa_usec_t adjust_time,
>>>>                 int32_t latency_difference_usec) {
>>>>
>>>>     uint32_t new_rate;
>>>>
>>>>     new_rate = base_rate * (1.0 + 0.1 *
>>>> (double)latency_difference_usec / adjust_time);
>>>>
>>>>     return new_rate;
>>>> }
>>>>
>>>> As you can see (https://imgur.com/a/eZT8L ), the phase oscillations
>>>> are now much more limited!
>>>>
>>>> I have received a private email from Georg where he proposes to use
>>>> the deadband, but snap to something other than base_rate, e.g. to a
>>>> result of some slow controller. For me, this is equivalent to using
>>>> this slow controller, i.e. a good idea.
>>>>
>>>
>>> I'll send you a patch privately later today. I kept the controller and
>>> implemented a more or less
>>> independent controller which takes care of the clock skew / latency
>>> drift. I'd like to discuss
>>> this approach first with you before I send another patch to the list.
>>
>> I have received this email, will also test the filter in the evening.
>>
> Thanks again.

Tested. This secret patch (which essentially provides a new rate 
controller based on the rate estimator, used when the old deadband logic 
snaps) with adjust_time=1 yields https://imgur.com/a/xTFmQ .

It's a pity that the deadband logic sometimes unsnaps spuriously, so I 
changed the 2.5 in the deadband condition with 5.0. Result: 
https://imgur.com/a/WKsT7 , looks better than the original deadband 
logic, but I'd say this needs more investigation (and experiments) from 
my side, because it is not clear whether these oscillations will 
eventually decay. I will lok more into this on weekends.

-- 
Alexander E. Patrakov