On Thu, Apr 08, 2010 at 03:13:53PM +0200, Sascha Hauer wrote:
On Thu, Apr 08, 2010 at 10:53:25AM +0100, Mark Brown wrote:
Hrm, this looks like it's going to have an issue with clock drift - we're now unconditionally advancing the timer every period, even if the data transfer hasn't pushed through a period of data. This will cause problems on lengthy playbacks (and shorter ones if the clocks are sufficiently out of sync).
We are calling snd_pcm_period_elapsed when at least one period is over. As I see it the worst thing that could happen is that we have not transfered enough data for one period in the timer callback and thus we call snd_pcm_period_elapsed in the next timer callback, so about one period too late, but the comment in sound/core/pcm_lib.c says:
Even if more than one periods have elapsed since the last call, you have to call this only once.
So I think this should be save.
The risk here is fragility caused by delaying the notification.
The issue is that if the period is long enough and/or the application is running too close to where the hardware is then the delay of what's likely to be almost an entire period is likely to glitch - you can end up with a situation where you're essentially notifying immediately before the end of the next period rather than immediately after the end of the current period.
Consider, for example what happens if the hrtimer runs slightly faster than the audio clock. Eventually you'll get:
- The timer runs, period A has a frame or two to go still. - Period A completes, period A' begins. - The timer fires again - period A is notified, period A' is almost complete.
In this situation the completion notifications lag the actual completion of the frame by almost a period (though this lag will go down over time). Most of the time this will still be fine and everything will work OK but I would expect this to cause issues with some applications, especially if they're trying to be latency sensitive.