[alsa-devel] PCM delay compensation

[Takashi Iwai]

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).


Takashi

> 
> Lennart
> 
> 
> > diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> > index 40c5a6f..dcbdc60 100644
> > --- a/include/sound/pcm.h
> > +++ b/include/sound/pcm.h
> > @@ -269,6 +269,7 @@ struct snd_pcm_runtime {
> >  	snd_pcm_uframes_t avail_max;
> >  	snd_pcm_uframes_t hw_ptr_base;	/* Position at buffer restart */
> >  	snd_pcm_uframes_t hw_ptr_interrupt; /* Position at interrupt time*/
> > +	snd_pcm_sframes_t delay;	/* extra delay; typically FIFO size */
> >  
> >  	/* -- HW params -- */
> >  	snd_pcm_access_t access;	/* access mode */
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index e61e125..df7c3fa 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -597,14 +597,15 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
> >  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> >  		status->avail = snd_pcm_playback_avail(runtime);
> >  		if (runtime->status->state == SNDRV_PCM_STATE_RUNNING ||
> > -		    runtime->status->state == SNDRV_PCM_STATE_DRAINING)
> > +		    runtime->status->state == SNDRV_PCM_STATE_DRAINING) {
> >  			status->delay = runtime->buffer_size - status->avail;
> > -		else
> > +			status->delay += runtime->delay;
> > +		} else
> >  			status->delay = 0;
> >  	} else {
> >  		status->avail = snd_pcm_capture_avail(runtime);
> >  		if (runtime->status->state == SNDRV_PCM_STATE_RUNNING)
> > -			status->delay = status->avail;
> > +			status->delay = status->avail + runtime->delay;
> >  		else
> >  			status->delay = 0;
> >  	}
> > @@ -2423,6 +2424,7 @@ static int snd_pcm_delay(struct snd_pcm_substream *substream,
> >  			n = snd_pcm_playback_hw_avail(runtime);
> >  		else
> >  			n = snd_pcm_capture_avail(runtime);
> > +		n += runtime->delay;
> >  		break;
> >  	case SNDRV_PCM_STATE_XRUN:
> >  		err = -EPIPE;
> > diff --git a/sound/usb/usbaudio.c b/sound/usb/usbaudio.c
> > index 6e70ba4..7d5a103 100644
> > --- a/sound/usb/usbaudio.c
> > +++ b/sound/usb/usbaudio.c
> > @@ -629,6 +629,7 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
> >  	subs->hwptr_done += offs;
> >  	if (subs->hwptr_done >= runtime->buffer_size)
> >  		subs->hwptr_done -= runtime->buffer_size;
> > +	runtime->delay += offs;
> >  	spin_unlock_irqrestore(&subs->lock, flags);
> >  	urb->transfer_buffer_length = offs * stride;
> >  	if (period_elapsed)
> > @@ -638,12 +639,20 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
> >  
> >  /*
> >   * process after playback data complete
> > - * - nothing to do
> > + * - decrease the delay count again
> >   */
> >  static int retire_playback_urb(struct snd_usb_substream *subs,
> >  			       struct snd_pcm_runtime *runtime,
> >  			       struct urb *urb)
> >  {
> > +	unsigned long flags;
> > +	int stride = runtime->frame_bits >> 3;
> > +	int processed = urb->transfer_buffer_length / stride;
> > +
> > +	spin_lock_irqsave(&subs->lock, flags);
> > +	if (processed > runtime->delay)
> > +		runtime->delay -= processed;
> > +	spin_unlock_irqrestore(&subs->lock, flags);
> >  	return 0;
> >  }
> >  
> > @@ -1542,6 +1551,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
> >  	subs->hwptr_done = 0;
> >  	subs->transfer_done = 0;
> >  	subs->phase = 0;
> > +	runtime->delay = 0;
> >  
> >  	/* clear urbs (to be sure) */
> >  	deactivate_urbs(subs, 0, 1);
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
> 
> Lennart
> 
> -- 
> Lennart Poettering                        Red Hat, Inc.
> lennart [at] poettering [dot] net         ICQ# 11060553
> http://0pointer.net/lennart/           GnuPG 0x1A015CC4
>