On Thu, 2010-04-08 at 15:03 +0100, Mark Brown wrote: > 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. I agree this will drift but I _think_ we are probably workable here for most latency sensitive apps as long as the worst case notification delay is 1 period, pointer() is accurate to a few frames and there is a least > 3 or 4 period buffers available for use in our driver buffer (for each direction). Currently the min periods for this driver is 2, so I would probably change to 4 to reduce drift related glitches. Liam -- Freelance Developer, SlimLogic Ltd ASoC and Voltage Regulator Maintainer. http://www.slimlogic.co.uk _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
