GNOME Bugzilla – Bug 788006
audiobasesink: Improve clock skew adjustments
Last modified: 2018-06-06 20:16:11 UTC
Created attachment 360208 [details] [review] audiobasesink: Fix clock skew corrections The external time should be moved only as much as needed to get back to the ideal center point, so that the clock is still allowed to drift both directions after the correction. This fixes excessive back and forth corrections due to the assumption of a linear drift. This also allowed me to reduce the clock skew tolerance dramatically without introducing more corrections. Bug #722259 may be to some extent also be caused by this bug. The enhancement bug #769804 still makes sense in combination with this fix.
Created attachment 360211 [details] [review] audiobasesink: Fix clock skew corrections Fix calculation of driftsamples as well.
Created attachment 360213 [details] [review] audiobasesink: Fix clock skew corrections Fix calculation of driftsamples as well (second attempt).
Review of attachment 360208 [details] [review]: That looks promizing, as this is pretty tricky to test, any chance of implementing some unit test ?
Unless there happen to be unit tests around this logic already, I don't think I have the time to do this. All I can say is that I believe the current logic is flawed in that it assumes a linear drift, and because of that, once it exceeded the allowed drift in one direction, it corrected by the whole drift tolerance value "back", placing the new ideal center on the edge of the window on the other side. If the clock then skewed some more, odds are it skewed outside of the window, triggering yet another correction. This behavior required a skew tolerance to be set much higher than needed. Depending on the alignment threshold property value, this may also avoid an alignment resync due to smaller corrections, which means most skew corrections may now be much more subtle.
Review of attachment 360213 [details] [review]: Please fix the commit message :) You don't "fix" the skew correction, but you improve it so that it doesn't over-compensate. Also please add some comments to the code to explain why you do those calculations there and what the result is (how do we correct, where do we end up?). The idea makes sense in any case.
Created attachment 362765 [details] [review] audiobasesink: Improve clock skew corrections.
We've been running with this for quite a while now and it's amazing how much fewer clock skews we see, actually they're almost entirely gone.
Let's get this in while the development branch is young. I've went through the code and haven't fond anything, and Sebastian comments has been addressed. Attachment 362765 [details] pushed as 7d3c098 - audiobasesink: Improve clock skew corrections.