[alsa-devel] [PATCH] ALSA: pcm_lib: avoid timing jitter in snd_pcm_read/write()

[Takashi Iwai]

At Mon, 28 Jun 2010 16:11:21 +0200 (CEST),
Jaroslav Kysela wrote:
> 
> On Mon, 28 Jun 2010, David Dillow wrote:
> 
> > On Mon, 2010-06-28 at 09:47 +0200, Jaroslav Kysela wrote:
> >> On Sat, 26 Jun 2010, David Dillow wrote:
> >>
> >>> When using poll() to wait for the next period -- or avail_min samples --
> >>> one gets a consistent delay for each system call that is usually just a
> >>> little short of the selected period time. However, When using
> >>> snd_pcm_read/write(), one gets a jittery delay that alternates between
> >>> less than a millisecond and approximately two period times. This is
> >>> caused by snd_pcm_lib_{read,write}1() transferring any available samples
> >>> to the user's buffer and adjusting the application pointer prior to
> >>> sleeping to the end of the current period. When the next period
> >>> interrupt occurs, there is then less than avail_min samples remaining to
> >>> be transferred in the period, so we end up sleeping until a second
> >>> period occurs.
> >>>
> >>> This is solved by using runtime->twake as the number of samples needed
> >>> for a wakeup in addition to selecting the proper wait queue to wake in
> >>> snd_pcm_update_state(). This requires twake to be non-zero when used
> >>> by snd_pcm_lib_{read,write}1() even if avail_min is zero.
> >>>
> >>> Signed-off-by: Dave Dillow <dave at thedillows.org>
> >>
> >> Thanks. It looks like another way to get things right. I applied your
> >> patch to my tree.
> >
> > I think we should probably better define the semantics of read/write for
> > sizes < avail_min, == avail_min, and > avail_min. As it is, I fear that
> > this patch makes things worse as I hadn't thought through all of the
> > cases when I posted it. If you've not pushed, please revert. If you
> > have, then it improves one case, but let's makes sure we get the others
> > correct before it hits the mainline.
> 
> I don't see any drawbacks in your solution. When I/O size is greater than 
> avail_min, the behaviour is same as in previous code (avail_min is used 
> for wake_up). For smaller I/O sizes (requests are not aligned to period 
> size), each period interrupt will cause a wake up for the application 
> task. In this case, the application may do own optimization of I/O 
> transfers (sizes).

I pulled to my tree now, too.
(sorry for the late response, as I've been on vacation.)

It looks good to me.  Let's see whether it breaks something ;)


thanks,

Takashi