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