At Wed, 25 Feb 2009 18:34:31 +0000,
Clive Messer wrote:
>
> On Wednesday 25 Feb 2009 18:13:47 Takashi Iwai wrote:
>
> > > > diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
> > > > index e9ce092..5211d1c 100644
> > > > --- a/src/pcm/pcm_hw.c
> > > > +++ b/src/pcm/pcm_hw.c
> > > > @@ -131,6 +131,10 @@ struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t
> > > > *pcm) static int sync_ptr1(snd_pcm_hw_t *hw, unsigned int flags)
> > > > {
> > > > int err;
> > > > + long old_hwptr, new_hwptr;
> > > > + long old_applptr, new_applptr;
> > > > + old_hwptr = hw->sync_ptr->s.status.hw_ptr;
> > > > + old_applptr = hw->sync_ptr->c.control.appl_ptr;
> > > > hw->sync_ptr->flags = flags;
> > > > err = ioctl((hw)->fd, SNDRV_PCM_IOCTL_SYNC_PTR,
> > > > (hw)->sync_ptr); if (err < 0) {
> > > > @@ -138,6 +142,11 @@ static int sync_ptr1(snd_pcm_hw_t *hw, unsigned
> > > > int flags) SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed");
> > > > return err;
> > > > }
> > > > + new_hwptr = hw->sync_ptr->s.status.hw_ptr;
> > > > + new_applptr = hw->sync_ptr->c.control.appl_ptr;
> > > > + printf("sync_ptr1: %ld(%ld), %ld(%ld)\n",
> > > > + new_hwptr, new_hwptr - old_hwptr,
> > > > + new_applptr, new_applptr - old_applptr);
> > > > return 0;
> > > > }
> > > >
>
> With sync_ptr I cannot reproduce the very large numbers. Here's a typical log.
> (Attached).
Hrm, but still an unexpected jump is found.
If you build the driver with CONFIG_SND_PCM_XRUN_DEBUG=y, you must
have /proc/asound/card0/pcm0p/xrun_debug. Echo 1 to this (as root)
echo 1 > /proc/asound/card0/pcm0p/xrun_debug
and try the program, see whether any debug message appears.
If any message appears, it means basically an unstable hardware.
Also, the below is a patch I tried to clean up and improve the
handling. Give it a try (and do echo 1 above for testing).
thanks,
Takashi
---
>From 6568cc56af28ff0371509e5354dcce43ecd9b4fd Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@xxxxxxx>
Date: Tue, 3 Mar 2009 17:00:15 +0100
Subject: [PATCH] ALSA: Rewrite hw_ptr updaters
Clean up and improve snd_pcm_update_hw_ptr*() functions.
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
sound/core/pcm_lib.c | 125 +++++++++++++++++++++++++++++++++-----------------
1 files changed, 82 insertions(+), 43 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 9216910..7d13eee 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -125,19 +125,23 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
}
}
+#ifdef CONFIG_SND_PCM_XRUN_DEBUG
+#define xrun_debug(substream) ((substream)->pstr->xrun_debug)
+#else
+#define xrun_debug(substream) 0
+#endif
+
static void xrun(struct snd_pcm_substream *substream)
{
snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
-#ifdef CONFIG_SND_PCM_XRUN_DEBUG
- if (substream->pstr->xrun_debug) {
+ if (xrun_debug(substream)) {
snd_printd(KERN_DEBUG "XRUN: pcmC%dD%d%c\n",
substream->pcm->card->number,
substream->pcm->device,
substream->stream ? 'c' : 'p');
- if (substream->pstr->xrun_debug > 1)
+ if (xrun_debug(substream) > 1)
dump_stack();
}
-#endif
}
static inline snd_pcm_uframes_t snd_pcm_update_hw_ptr_pos(struct snd_pcm_substream *substream,
@@ -182,11 +186,26 @@ static inline int snd_pcm_update_hw_ptr_post(struct snd_pcm_substream *substream
return 0;
}
+static void hw_ptr_error(struct snd_pcm_substream *substream,
+ const char *fmt, ...)
+{
+ if (xrun_debug(substream)) {
+ if (printk_ratelimit()) {
+ va_list args;
+ va_start(args, fmt);
+ vprintk(fmt, args);
+ va_end(args);
+ if (xrun_debug(substream) > 1)
+ dump_stack();
+ }
+ }
+}
+
static inline int snd_pcm_update_hw_ptr_interrupt(struct snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
snd_pcm_uframes_t pos;
- snd_pcm_uframes_t new_hw_ptr, hw_ptr_interrupt;
+ snd_pcm_uframes_t new_hw_ptr, hw_ptr_interrupt, hw_base;
snd_pcm_sframes_t delta;
pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
@@ -194,36 +213,47 @@ static inline int snd_pcm_update_hw_ptr_interrupt(struct snd_pcm_substream *subs
xrun(substream);
return -EPIPE;
}
- if (runtime->period_size == runtime->buffer_size)
- goto __next_buf;
- new_hw_ptr = runtime->hw_ptr_base + pos;
+ hw_base = runtime->hw_ptr_base;
+ new_hw_ptr = hw_base + pos;
hw_ptr_interrupt = runtime->hw_ptr_interrupt + runtime->period_size;
-
- delta = hw_ptr_interrupt - new_hw_ptr;
- if (delta > 0) {
- if ((snd_pcm_uframes_t)delta < runtime->buffer_size / 2) {
-#ifdef CONFIG_SND_PCM_XRUN_DEBUG
- if (runtime->periods > 1 && substream->pstr->xrun_debug) {
- snd_printd(KERN_ERR "Unexpected hw_pointer value [1] (stream = %i, delta: -%ld, max jitter = %ld): wrong interrupt acknowledge?\n", substream->stream, (long) delta, runtime->buffer_size / 2);
- if (substream->pstr->xrun_debug > 1)
- dump_stack();
- }
-#endif
- return 0;
+ delta = new_hw_ptr - hw_ptr_interrupt;
+ if (hw_ptr_interrupt == runtime->boundary)
+ hw_ptr_interrupt = 0;
+ if (delta < 0) {
+ delta += runtime->buffer_size;
+ if (delta < 0) {
+ hw_ptr_error(substream,
+ "Unexpected hw_pointer value "
+ "(stream=%i, pos=%ld, intr_ptr=%ld)\n",
+ substream->stream, (long)pos,
+ (long)hw_ptr_interrupt);
+ /* rebase to interrupt position */
+ hw_base = new_hw_ptr = hw_ptr_interrupt;
+ delta = 0;
+ } else {
+ hw_base += runtime->buffer_size;
+ if (hw_base == runtime->boundary)
+ hw_base = 0;
+ new_hw_ptr = hw_base + pos;
}
- __next_buf:
- runtime->hw_ptr_base += runtime->buffer_size;
- if (runtime->hw_ptr_base == runtime->boundary)
- runtime->hw_ptr_base = 0;
- new_hw_ptr = runtime->hw_ptr_base + pos;
}
-
+ if (delta > runtime->period_size) {
+ hw_ptr_error(substream,
+ "Lost interrupts? "
+ "(stream=%i, delta=%ld, intr_ptr=%ld)\n",
+ substream->stream, (long)delta,
+ (long)hw_ptr_interrupt);
+ /* rebase hw_ptr_interrupt */
+ hw_ptr_interrupt =
+ new_hw_ptr - new_hw_ptr % runtime->period_size;
+ }
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
runtime->silence_size > 0)
snd_pcm_playback_silence(substream, new_hw_ptr);
+ runtime->hw_ptr_base = hw_base;
runtime->status->hw_ptr = new_hw_ptr;
- runtime->hw_ptr_interrupt = new_hw_ptr - new_hw_ptr % runtime->period_size;
+ runtime->hw_ptr_interrupt = hw_ptr_interrupt;
return snd_pcm_update_hw_ptr_post(substream, runtime);
}
@@ -233,7 +263,7 @@ int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
snd_pcm_uframes_t pos;
- snd_pcm_uframes_t old_hw_ptr, new_hw_ptr;
+ snd_pcm_uframes_t old_hw_ptr, new_hw_ptr, hw_base;
snd_pcm_sframes_t delta;
old_hw_ptr = runtime->status->hw_ptr;
@@ -242,29 +272,38 @@ int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream)
xrun(substream);
return -EPIPE;
}
- new_hw_ptr = runtime->hw_ptr_base + pos;
-
- delta = old_hw_ptr - new_hw_ptr;
- if (delta > 0) {
- if ((snd_pcm_uframes_t)delta < runtime->buffer_size / 2) {
-#ifdef CONFIG_SND_PCM_XRUN_DEBUG
- if (runtime->periods > 2 && substream->pstr->xrun_debug) {
- snd_printd(KERN_ERR "Unexpected hw_pointer value [2] (stream = %i, delta: -%ld, max jitter = %ld): wrong interrupt acknowledge?\n", substream->stream, (long) delta, runtime->buffer_size / 2);
- if (substream->pstr->xrun_debug > 1)
- dump_stack();
- }
-#endif
+ hw_base = runtime->hw_ptr_base;
+ new_hw_ptr = hw_base + pos;
+
+ delta = new_hw_ptr - old_hw_ptr;
+ if (delta < 0) {
+ delta += runtime->buffer_size;
+ if (delta < 0) {
+ hw_ptr_error(substream,
+ "Unexpected hw_pointer value [2] "
+ "(stream=%i, pos=%ld, old_ptr=%ld)\n",
+ substream->stream, (long)pos,
+ (long)old_hw_ptr);
return 0;
}
- runtime->hw_ptr_base += runtime->buffer_size;
- if (runtime->hw_ptr_base == runtime->boundary)
- runtime->hw_ptr_base = 0;
- new_hw_ptr = runtime->hw_ptr_base + pos;
+ hw_base += runtime->buffer_size;
+ if (hw_base == runtime->boundary)
+ hw_base = 0;
+ new_hw_ptr = hw_base + pos;
+ }
+ if (delta > runtime->period_size && runtime->periods > 1) {
+ hw_ptr_error(substream,
+ "hw_ptr skipping! "
+ "(pos=%ld, delta=%ld, period=%ld)\n",
+ (long)pos, (long)delta,
+ (long)runtime->period_size);
+ return 0;
}
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
runtime->silence_size > 0)
snd_pcm_playback_silence(substream, new_hw_ptr);
+ runtime->hw_ptr_base = hw_base;
runtime->status->hw_ptr = new_hw_ptr;
return snd_pcm_update_hw_ptr_post(substream, runtime);
--
1.6.1.3
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
