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

[Georg Chini]

On 19.11.2015 17:47, Alexander E. Patrakov wrote:
> 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.
>

OK, understood.

>>
>>>>
>>>>>
>>>>> 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.

I cannot see that the implementation without deadband is better:
Overall amplitude is nearly twice as large and also fluctuations are
less random with the deadband. Or am I comparing the wrong plots?

>
>>
>>>
>>>>>
>>>>> 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.
>

I am talking about the pure recording. Is the latency tracked at all in 
that case?
Is the rate modified during recording?

>> 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.
>
It is not a "spuriously unsnapping" but part of the regulation process. 
In my opinion the phase jump
comes first and then the controller kicks in to correct it. Take a look 
at the reported latency values
when you run with debugging: First there is a jump in the latency and 
then the controller reacts. That
is exactly how it is supposed to be. It also looks like those jumps are 
no artifacts because the next
reported value is consistent with the regulation. I believe this is 
somehow connected to changing
the sample rate (by only 1 Hz) because it does not occur when the rate 
is kept constant.

Maybe the hardware sometimes takes a millisecond to switch the rate.

I wonder how good you think we can get. Even including the jump, the 
latency is kept within
around 500 usec of the expected value, if you disregard the peak we are 
talking about 200 usec.