[alsa-devel] PCM delay compensation

[Takashi Iwai]

At Mon, 23 Feb 2009 17:28:04 +0100,
I wrote:
> 
> At Mon, 23 Feb 2009 17:24:32 +0100,
> Lennart Poettering wrote:
> > 
> > On Mon, 23.02.09 08:45, Takashi Iwai (tiwai at suse.de) wrote:
> > 
> > > 
> > > At Sun, 22 Feb 2009 00:45:04 +0100,
> > > Lennart Poettering wrote:
> > > > 
> > > > On Tue, 07.10.08 17:32, Takashi Iwai (tiwai at suse.de) wrote:
> > > > 
> > > > > Hi,
> > > > 
> > > > Sorry for the overly long delay.
> > > > > 
> > > > > the patch below (to the latest sound git tree) adds the extra delay
> > > > > count for USB-audio driver.  This change will appear as the return
> > > > > value of snd_pcm_delay().
> > > > > 
> > > > > Could you check whether it's appropriate behavior you've wanted?
> > > > 
> > > > I have now tested this patch on the current 2.6.29-rc5 kernel. The
> > > > effect is that snd_pcm_delay() returns always increasing values, as if
> > > > the playback never proceeds. Most movie players which need that call
> > > > to synchronize video frames hence stall completely.
> > > 
> > > OK, that's bad.  However, the increased value of snd_pcm_delay() is
> > > exactly the effect.  The usb-audio always prefetch the buffer in
> > > advance.
> > > 
> > > That means, changing (or "fixing") snd_pcm_delay() would cause many
> > > regressions with many apps -- thus basically we are not allowed to
> > > change the semantics any more at this too late point.
> > > 
> > > I feel it's better to introduce another kernel-side API to give a
> > > better sync/timing information, and mark snd_pcm_delay as obsolete for
> > > future (although we can never deprecate such a basic API).
> > 
> > No, snd_pcm_delay() with this patch is clearly broken: it apparently
> > increases at the same speed as the hw ptr, ignoring the app
> > ptr. i.e. after 5 min of playback the delay will be reported a 5 mins!
> > After 10 min of playback the delay will be reported as 10 mins!
> 
> Ah, then it must be a buggy patch indeed.  It didn't appear in my
> some test cases at that time, so didn't check after that.
> 
> Maybe some delay account in complete callback wasn't correct.

Maybe a stupid typo below (or removed lines accidentally at
merge/rebase)...


Takashi

---
diff --git a/sound/usb/usbaudio.c b/sound/usb/usbaudio.c
index 03384db..7b362d1 100644
--- a/sound/usb/usbaudio.c
+++ b/sound/usb/usbaudio.c
@@ -651,6 +651,8 @@ static int retire_playback_urb(struct snd_usb_substream *subs,
 
 	spin_lock_irqsave(&subs->lock, flags);
 	if (processed > runtime->delay)
+		runtime->delay = 0;
+	else
 		runtime->delay -= processed;
 	spin_unlock_irqrestore(&subs->lock, flags);
 	return 0;