[alsa-devel] intel 8x0 ALSA driver in virtualbox - choppy sound

[Jaroslav Kysela]

On Fri, 13 Aug 2010, Takashi Iwai wrote:

>> http://www.alsa-project.org/main/index.php/XRUN_Debug is a good start to
>> see if the virtual machine does receive interrupts in the right time.
>
> Actually, there is no real interrupt, but just the virtual one,
> thus it can be never accurate.  So, rather the question is how the
> driver is tolerant for sloppy IRQ updates.
>
> Usually, when the period size is large and the buffer have many
> periods (not two), the delayed update works more stably.

I know. The current position checking in pcm_lib.c should handle 
delayed updates, but it does not handle well the multiple elapsed() calls 
merged to one time (double interrupt acknowledges). In some cases,
the code things that buffer_size boundary were crossed, thus the hw_ptr 
position update fails.

I wanted to force the driver to be responsible to call the elapsed() 
calls at the right time. But it's true that the interrupt controller and 
other operating system contrainsts and behaviour might influence the 
interrupt processing which is more visible in the virtual environment.

Thinking more about it, we need probably another quick check against 
different timing source to check properly for the buffer_size crossing.

What about this patch (the hw_base gets updates in in_interrupt only when 
delta for next jiffies is bigger than (buffer jiffies / 2)). It should 
handle all double acks from the interrupt callbacks:

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 85f1c6b..dfd9b76 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -278,6 +278,7 @@ struct snd_pcm_runtime {
  	snd_pcm_uframes_t hw_ptr_base;	/* Position at buffer restart */
  	snd_pcm_uframes_t hw_ptr_interrupt; /* Position at interrupt time */
  	unsigned long hw_ptr_jiffies;	/* Time when hw_ptr is updated */
+	unsigned long hw_ptr_buffer_jiffies; /* buffer time in jiffies */
  	snd_pcm_sframes_t delay;	/* extra delay; typically FIFO size */

  	/* -- HW params -- */
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index e23e0e7..a1707cc 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -334,11 +334,15 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
  		/* delta = "expected next hw_ptr" for in_interrupt != 0 */
  		delta = runtime->hw_ptr_interrupt + runtime->period_size;
  		if (delta > new_hw_ptr) {
-			hw_base += runtime->buffer_size;
-			if (hw_base >= runtime->boundary)
-				hw_base = 0;
-			new_hw_ptr = hw_base + pos;
-			goto __delta;
+			/* check for double acknowledged interrupts */
+			hdelta = jiffies - runtime->hw_ptr_jiffies;
+			if (hdelta > runtime->hw_ptr_buffer_jiffies/2) {
+				hw_base += runtime->buffer_size;
+				if (hw_base >= runtime->boundary)
+					hw_base = 0;
+				new_hw_ptr = hw_base + pos;
+				goto __delta;
+			}
  		}
  	}
  	/* new_hw_ptr might be lower than old_hw_ptr in case when */
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index a3b2a64..b5dae26 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -864,6 +864,8 @@ static void snd_pcm_post_start(struct snd_pcm_substream *substream, int state)
  	struct snd_pcm_runtime *runtime = substream->runtime;
  	snd_pcm_trigger_tstamp(substream);
  	runtime->hw_ptr_jiffies = jiffies;
+	runtime->hw_ptr_buffer_jiffies = (runtime->buffer_size * HZ) / 
+							    runtime->rate;
  	runtime->status->state = state;
  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
  	    runtime->silence_size > 0)

-----
Jaroslav Kysela <perex at perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.