At Sun, 22 Feb 2009 00:45:04 +0100,
Lennart Poettering wrote:
>
> On Tue, 07.10.08 17:32, Takashi Iwai (tiwai@xxxxxxx) 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@xxxxxxxxxxxxxxxx
> > 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
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
