All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once
@ 2011-01-24 21:07 Gerd Hoffmann
  2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs Gerd Hoffmann
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2011-01-24 21:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 9cf685d..858ca81 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs
  2011-01-24 21:07 [Qemu-devel] [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once Gerd Hoffmann
@ 2011-01-24 21:07 ` Gerd Hoffmann
  2011-01-25  9:20   ` Alon Levy
  2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 3/3] pulseaudio: tweak config Gerd Hoffmann
  2011-01-25 16:57 ` [Qemu-devel] Re: [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once malc
  2 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2011-01-24 21:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 858ca81..75e3ea0 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [RESEND PATCH 3/3] pulseaudio: tweak config
  2011-01-24 21:07 [Qemu-devel] [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once Gerd Hoffmann
  2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs Gerd Hoffmann
@ 2011-01-24 21:07 ` Gerd Hoffmann
  2011-01-25 16:57 ` [Qemu-devel] Re: [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once malc
  2 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2011-01-24 21:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 75e3ea0..fb4510e 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs
  2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs Gerd Hoffmann
@ 2011-01-25  9:20   ` Alon Levy
  2011-01-25 11:05     ` Gerd Hoffmann
  0 siblings, 1 reply; 6+ messages in thread
From: Alon Levy @ 2011-01-25  9:20 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Mon, Jan 24, 2011 at 10:07:45PM +0100, Gerd Hoffmann wrote:
> 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 858ca81..75e3ea0 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;
> +

What's the default buffer size? Is there any reason to query this on startup
somewhere instead of hard coding it?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs
  2011-01-25  9:20   ` Alon Levy
@ 2011-01-25 11:05     ` Gerd Hoffmann
  0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2011-01-25 11:05 UTC (permalink / raw)
  To: qemu-devel

   Hi,

> What's the default buffer size?

Don't know the exact number, but pulse's default buffer size is quite 
big by design.  Which is fine for most apps (such as mp3 players) where 
loading more sound data into the buffer every second or so is ok.  It 
doesn't work very well for sound card emulation though.  And it doesn't 
work at all if your guest happens to use a ring buffer which holds sound 
data for only 20ms.

 > Is there any reason to query this on startup
 > somewhere instead of hard coding it?

No.  The pulse default values will never ever work for qemu's low 
latency requirements.

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] Re: [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once
  2011-01-24 21:07 [Qemu-devel] [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once Gerd Hoffmann
  2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs Gerd Hoffmann
  2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 3/3] pulseaudio: tweak config Gerd Hoffmann
@ 2011-01-25 16:57 ` malc
  2 siblings, 0 replies; 6+ messages in thread
From: malc @ 2011-01-25 16:57 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Mon, 24 Jan 2011, Gerd Hoffmann wrote:

> 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.
> 
[..snip...]

Thanks, applied.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-01-25 16:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-24 21:07 [Qemu-devel] [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once Gerd Hoffmann
2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 2/3] pulseaudio: setup buffer attrs Gerd Hoffmann
2011-01-25  9:20   ` Alon Levy
2011-01-25 11:05     ` Gerd Hoffmann
2011-01-24 21:07 ` [Qemu-devel] [RESEND PATCH 3/3] pulseaudio: tweak config Gerd Hoffmann
2011-01-25 16:57 ` [Qemu-devel] Re: [RESEND PATCH 1/3] pulseaudio: process 1/4 buffer max at once malc

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.