[Qemu-devel] [PATCH 0/3] pulseaudio buffering fixes

[Qemu-devel] [PATCH 0/3] pulseaudio buffering fixes

[Gerd Hoffmann]

  Hi,

These patches improve the pulseaudio driver's buffer handling.
Separated from the win7 sound patch series, it is completely
independant stuff.  Except maybe for the fact that the usb-audio
driver magnifies the pulseaudio buffering issues.

Fengguang, can you please have a look?
malc wants your review comments before applying them.

thanks,
  Gerd

Gerd Hoffmann (3):
  pulseaudio: process 1/4 buffer max at once
  pulseaudio: setup buffer attrs
  pulseaudio: tweak config

 audio/paaudio.c |   44 +++++++++++++++++++++-----------------------
 1 files changed, 21 insertions(+), 23 deletions(-)

[Qemu-devel] [PATCH 1/3] pulseaudio: process 1/4 buffer max at once

[Gerd Hoffmann]

Limit the size of data pieces processed by the pulseaudio worker
threads.  Never ever process more than 1/4 of the buffer at once.

Background: The buffer area currently processed by the pulseaudio thread
is blocked, i.e. the main thread (or iothread) can't fill in more data
there.  The buffer processing time is roughly real-time due to the
pa_simple_write() call blocking when the output queue to the pulse
server is full.  Thus processing big chunks at once means blocking
a large part of the buffer for a long time.  This brings high latency
and can lead to dropouts.

When processing the buffer in smaller chunks the rpos handling becomes a
problem though.  The thread reads hw->rpos without knowing whenever
qpa_run_out has already seen the last (small) chunk processed and
updated rpos accordingly.  There is no point in reading hw->rpos though,
pa->rpos can be used instead.  We just need to take care to initialize
pa->rpos before kicking the thread.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/paaudio.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index ff71dac..f99ca73 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -57,9 +57,6 @@ static void *qpa_thread_out (void *arg)
 {
     PAVoiceOut *pa = arg;
     HWVoiceOut *hw = &pa->hw;
-    int threshold;
-
-    threshold = conf.divisor ? hw->samples / conf.divisor : 0;
 
     if (audio_pt_lock (&pa->pt, AUDIO_FUNC)) {
         return NULL;
@@ -73,7 +70,7 @@ static void *qpa_thread_out (void *arg)
                 goto exit;
             }
 
-            if (pa->live > threshold) {
+            if (pa->live > 0) {
                 break;
             }
 
@@ -82,8 +79,8 @@ static void *qpa_thread_out (void *arg)
             }
         }
 
-        decr = to_mix = pa->live;
-        rpos = hw->rpos;
+        decr = to_mix = audio_MIN (pa->live, conf.samples >> 2);
+        rpos = pa->rpos;
 
         if (audio_pt_unlock (&pa->pt, AUDIO_FUNC)) {
             return NULL;
@@ -110,8 +107,8 @@ static void *qpa_thread_out (void *arg)
             return NULL;
         }
 
-        pa->live = 0;
         pa->rpos = rpos;
+        pa->live -= decr;
         pa->decr += decr;
     }
 
@@ -152,9 +149,6 @@ static void *qpa_thread_in (void *arg)
 {
     PAVoiceIn *pa = arg;
     HWVoiceIn *hw = &pa->hw;
-    int threshold;
-
-    threshold = conf.divisor ? hw->samples / conf.divisor : 0;
 
     if (audio_pt_lock (&pa->pt, AUDIO_FUNC)) {
         return NULL;
@@ -168,7 +162,7 @@ static void *qpa_thread_in (void *arg)
                 goto exit;
             }
 
-            if (pa->dead > threshold) {
+            if (pa->dead > 0) {
                 break;
             }
 
@@ -177,8 +171,8 @@ static void *qpa_thread_in (void *arg)
             }
         }
 
-        incr = to_grab = pa->dead;
-        wpos = hw->wpos;
+        incr = to_grab = audio_MIN (pa->dead, conf.samples >> 2);
+        wpos = pa->wpos;
 
         if (audio_pt_unlock (&pa->pt, AUDIO_FUNC)) {
             return NULL;
@@ -323,6 +317,7 @@ static int qpa_init_out (HWVoiceOut *hw, struct audsettings *as)
     audio_pcm_init_info (&hw->info, &obt_as);
     hw->samples = conf.samples;
     pa->pcm_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << hw->info.shift);
+    pa->rpos = hw->rpos;
     if (!pa->pcm_buf) {
         dolog ("Could not allocate buffer (%d bytes)\n",
                hw->samples << hw->info.shift);
@@ -377,6 +372,7 @@ static int qpa_init_in (HWVoiceIn *hw, struct audsettings *as)
     audio_pcm_init_info (&hw->info, &obt_as);
     hw->samples = conf.samples;
     pa->pcm_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << hw->info.shift);
+    pa->wpos = hw->wpos;
     if (!pa->pcm_buf) {
         dolog ("Could not allocate buffer (%d bytes)\n",
                hw->samples << hw->info.shift);
-- 
1.7.1

[Qemu-devel] [PATCH 2/3] pulseaudio: setup buffer attrs

[Gerd Hoffmann]

Request reasonable buffer sizes from pulseaudio.  Without this
pa_simple_write() can block quite long and lead to dropouts,
especially with guests which use small audio ring buffers.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/paaudio.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index f99ca73..61af0c5 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -289,6 +289,7 @@ static int qpa_init_out (HWVoiceOut *hw, struct audsettings *as)
 {
     int error;
     static pa_sample_spec ss;
+    static pa_buffer_attr ba;
     struct audsettings obt_as = *as;
     PAVoiceOut *pa = (PAVoiceOut *) hw;
 
@@ -296,6 +297,15 @@ static int qpa_init_out (HWVoiceOut *hw, struct audsettings *as)
     ss.channels = as->nchannels;
     ss.rate = as->freq;
 
+    /*
+     * qemu audio tick runs at 250 Hz (by default), so processing
+     * data chunks worth 4 ms of sound should be a good fit.
+     */
+    ba.tlength = pa_usec_to_bytes (4 * 1000, &ss);
+    ba.minreq = pa_usec_to_bytes (2 * 1000, &ss);
+    ba.maxlength = -1;
+    ba.prebuf = -1;
+
     obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
 
     pa->s = pa_simple_new (
@@ -306,7 +316,7 @@ static int qpa_init_out (HWVoiceOut *hw, struct audsettings *as)
         "pcm.playback",
         &ss,
         NULL,                   /* channel map */
-        NULL,                   /* buffering attributes */
+        &ba,                    /* buffering attributes */
         &error
         );
     if (!pa->s) {
-- 
1.7.1

[Qemu-devel] [PATCH 3/3] pulseaudio: tweak config

[Gerd Hoffmann]

Zap unused divisor field.
Raise the buffer size default.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/paaudio.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 61af0c5..34bde23 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -33,13 +33,11 @@ typedef struct {
 
 static struct {
     int samples;
-    int divisor;
     char *server;
     char *sink;
     char *source;
 } conf = {
-    .samples = 1024,
-    .divisor = 2,
+    .samples = 4096,
 };
 
 static void GCC_FMT_ATTR (2, 3) qpa_logerr (int err, const char *fmt, ...)
@@ -478,12 +476,6 @@ struct audio_option qpa_options[] = {
         .descr = "buffer size in samples"
     },
     {
-        .name  = "DIVISOR",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.divisor,
-        .descr = "threshold divisor"
-    },
-    {
         .name  = "SERVER",
         .tag   = AUD_OPT_STR,
         .valp  = &conf.server,
-- 
1.7.1