[PATCH 0/5] audio fixes

[PATCH 0/5] audio fixes

[Volker Rümelin]

Here are five patches to fix PulseAudio playback/recording with
the mixing engine off.

Volker Rümelin (5):
  hda-codec: fix playback rate control
  hda-codec: fix recording rate control
  paaudio: drop recording stream in qpa_fini_in
  paaudio: try to drain the recording stream
  paaudio: wait until the recording stream is ready

 audio/paaudio.c      | 70 ++++++++++++++++++++++++++++++++++++----------------
 hw/audio/hda-codec.c |  8 +++---
 2 files changed, 53 insertions(+), 25 deletions(-)

-- 
2.16.4

[PATCH 1/5] hda-codec: fix playback rate control

[Volker Rümelin]

Since commit 1930616b98 "audio: make mixeng optional" the
function hda_audio_output_cb can no longer assume the function
parameter avail contains the free buffer size. With the playback
mixing-engine turned off this leads to a broken playback rate
control and playback buffer drops in regular intervals.

This patch moves down the rate calculation, so the correct
buffer fill level is used for the calculation.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 hw/audio/hda-codec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index f17e8d8dce..768ba31e79 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -338,8 +338,6 @@ static void hda_audio_output_cb(void *opaque, int avail)
         return;
     }
 
-    hda_timer_sync_adjust(st, (wpos - rpos) - to_transfer - (B_SIZE >> 1));
-
     while (to_transfer) {
         uint32_t start = (uint32_t) (rpos & B_MASK);
         uint32_t chunk = (uint32_t) MIN(B_SIZE - start, to_transfer);
@@ -351,6 +349,8 @@ static void hda_audio_output_cb(void *opaque, int avail)
             break;
         }
     }
+
+    hda_timer_sync_adjust(st, (wpos - rpos) - (B_SIZE >> 1));
 }
 
 static void hda_audio_compat_input_cb(void *opaque, int avail)
-- 
2.16.4

[PATCH 2/5] hda-codec: fix recording rate control

[Volker Rümelin]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

Apply previous commit to hda_audio_input_cb for the same
reasons.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 hw/audio/hda-codec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 768ba31e79..e711a99a41 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -265,8 +265,6 @@ static void hda_audio_input_cb(void *opaque, int avail)
 
     int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), avail);
 
-    hda_timer_sync_adjust(st, -((wpos - rpos) + to_transfer - (B_SIZE >> 1)));
-
     while (to_transfer) {
         uint32_t start = (uint32_t) (wpos & B_MASK);
         uint32_t chunk = (uint32_t) MIN(B_SIZE - start, to_transfer);
@@ -278,6 +276,8 @@ static void hda_audio_input_cb(void *opaque, int avail)
             break;
         }
     }
+
+    hda_timer_sync_adjust(st, -((wpos - rpos) - (B_SIZE >> 1)));
 }
 
 static void hda_audio_output_timer(void *opaque)
-- 
2.16.4


[-- Attachment #2: Type: text/html, Size: 1253 bytes --]

[PATCH 3/5] paaudio: drop recording stream in qpa_fini_in

[Volker Rümelin]

Every call to pa_stream_peek which returns a data length > 0
should have a corresponding pa_stream_drop. A call to qpa_read
does not necessarily call pa_stream_drop immediately after a
call to pa_stream_peek. Test in qpa_fini_in if a last
pa_stream_drop is needed.

This prevents following messages in the libvirt log file after
a recording stream gets closed and a new one opened.

pulseaudio: pa_stream_drop failed
pulseaudio: Reason: Bad state
pulseaudio: pa_stream_drop failed
pulseaudio: Reason: Bad state

To reproduce start qemu with
-audiodev pa,id=audio0,in.mixing-engine=off
and in the guest start and stop Audacity several times.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 55a91f8980..b831e0d6e9 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -536,7 +536,6 @@ static void qpa_simple_disconnect(PAConnection *c, pa_stream *stream)
 {
     int err;
 
-    pa_threaded_mainloop_lock(c->mainloop);
     /*
      * wait until actually connects. workaround pa bug #247
      * https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/247
@@ -550,7 +549,6 @@ static void qpa_simple_disconnect(PAConnection *c, pa_stream *stream)
         dolog("Failed to disconnect! err=%d\n", err);
     }
     pa_stream_unref(stream);
-    pa_threaded_mainloop_unlock(c->mainloop);
 }
 
 static void qpa_fini_out (HWVoiceOut *hw)
@@ -558,8 +556,12 @@ static void qpa_fini_out (HWVoiceOut *hw)
     PAVoiceOut *pa = (PAVoiceOut *) hw;
 
     if (pa->stream) {
-        qpa_simple_disconnect(pa->g->conn, pa->stream);
+        PAConnection *c = pa->g->conn;
+
+        pa_threaded_mainloop_lock(c->mainloop);
+        qpa_simple_disconnect(c, pa->stream);
         pa->stream = NULL;
+        pa_threaded_mainloop_unlock(c->mainloop);
     }
 }
 
@@ -568,8 +570,20 @@ static void qpa_fini_in (HWVoiceIn *hw)
     PAVoiceIn *pa = (PAVoiceIn *) hw;
 
     if (pa->stream) {
-        qpa_simple_disconnect(pa->g->conn, pa->stream);
+        PAConnection *c = pa->g->conn;
+
+        pa_threaded_mainloop_lock(c->mainloop);
+        if (pa->read_length) {
+            int r = pa_stream_drop(pa->stream);
+            if (r) {
+                qpa_logerr(pa_context_errno(c->context),
+                           "pa_stream_peek failed\n");
+            }
+            pa->read_length = 0;
+        }
+        qpa_simple_disconnect(c, pa->stream);
         pa->stream = NULL;
+        pa_threaded_mainloop_unlock(c->mainloop);
     }
 }
 
-- 
2.16.4

[PATCH 4/5] paaudio: try to drain the recording stream

[Volker Rümelin]

There is no guarantee a single call to pa_stream_peek every
timer_period microseconds can read a recording stream faster
than the data gets produced at the source. Let qpa_read try to
drain the recording stream.

To reproduce the problem:

Start qemu with -audiodev pa,id=audio0,in.mixing-engine=off

On the host connect the qemu recording stream to the monitor of
a hardware output device. While the problem can also be seen
with a hardware input device, it's obvious with the monitor of
a hardware output device.

In the guest start audio recording with audacity and notice the
slow recording data rate.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index b831e0d6e9..03cf10bb3c 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -156,34 +156,43 @@ static size_t qpa_read(HWVoiceIn *hw, void *data, size_t length)
 {
     PAVoiceIn *p = (PAVoiceIn *) hw;
     PAConnection *c = p->g->conn;
-    size_t l;
-    int r;
+    size_t total = 0;
 
     pa_threaded_mainloop_lock(c->mainloop);
 
     CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
                     "pa_threaded_mainloop_lock failed\n");
 
-    if (!p->read_length) {
-        r = pa_stream_peek(p->stream, &p->read_data, &p->read_length);
-        CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail,
-                           "pa_stream_peek failed\n");
-    }
+    while (total < length) {
+        size_t l;
+        int r;
+
+        if (!p->read_length) {
+            r = pa_stream_peek(p->stream, &p->read_data, &p->read_length);
+            CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail,
+                               "pa_stream_peek failed\n");
+            if (!p->read_length) {
+                /* buffer is empty */
+                break;
+            }
+        }
 
-    l = MIN(p->read_length, length);
-    memcpy(data, p->read_data, l);
+        l = MIN(p->read_length, length - total);
+        memcpy((char *)data + total, p->read_data, l);
 
-    p->read_data += l;
-    p->read_length -= l;
+        p->read_data += l;
+        p->read_length -= l;
+        total += l;
 
-    if (!p->read_length) {
-        r = pa_stream_drop(p->stream);
-        CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail,
-                           "pa_stream_drop failed\n");
+        if (!p->read_length) {
+            r = pa_stream_drop(p->stream);
+            CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail,
+                               "pa_stream_drop failed\n");
+        }
     }
 
     pa_threaded_mainloop_unlock(c->mainloop);
-    return l;
+    return total;
 
 unlock_and_fail:
     pa_threaded_mainloop_unlock(c->mainloop);
-- 
2.16.4

[PATCH 5/5] paaudio: wait until the recording stream is ready

[Volker Rümelin]

Don't call pa_stream_peek before the recording stream is ready.

Information to reproduce the problem.

Start and stop Audacity in the guest several times because the
problem is racy.

libvirt log file:
-audiodev pa,id=audio0,server=localhost,out.latency=30000,
 out.mixing-engine=off,in.mixing-engine=off \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,
 resourcecontrol=deny \
-msg timestamp=on
: Domain id=4 is tainted: custom-argv
char device redirected to /dev/pts/1 (label charserial0)
audio: Device pcspk: audiodev default parameter is deprecated,
 please specify audiodev=audio0
audio: Device hda: audiodev default parameter is deprecated,
 please specify audiodev=audio0
pulseaudio: pa_stream_peek failed
pulseaudio: Reason: Bad state
pulseaudio: pa_stream_peek failed
pulseaudio: Reason: Bad state

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 03cf10bb3c..bb4150bc34 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -162,6 +162,10 @@ static size_t qpa_read(HWVoiceIn *hw, void *data, size_t length)
 
     CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
                     "pa_threaded_mainloop_lock failed\n");
+    if (pa_stream_get_state(p->stream) != PA_STREAM_READY) {
+        /* wait for stream to become ready */
+        goto unlock;
+    }
 
     while (total < length) {
         size_t l;
@@ -191,6 +195,7 @@ static size_t qpa_read(HWVoiceIn *hw, void *data, size_t length)
         }
     }
 
+unlock:
     pa_threaded_mainloop_unlock(c->mainloop);
     return total;
 
-- 
2.16.4

Re: [PATCH 2/5] hda-codec: fix recording rate control

[Gerd Hoffmann]

On Thu, Dec 19, 2019 at 09:04:16PM +0100, Volker Rümelin wrote:
> Apply previous commit to hda_audio_input_cb for the same
> reasons.

This mail is multipart text+html and "git am" can't process it (the
others are text only).  Can you please resend the patches, preferably
with "git send-email" to avoid them being sent as multipart?  They all
look good to me (this series and the 6th patch sent as separate mail).

thanks,
  Gerd

Re: [PATCH 2/5] hda-codec: fix recording rate control

[Volker Rümelin]

> This mail is multipart text+html and "git am" can't process it (the
> others are text only).  Can you please resend the patches, preferably
> with "git send-email" to avoid them being sent as multipart?  They all
> look good to me (this series and the 6th patch sent as separate mail).
>
>
Sorry, I made a mistake here. I will send a version 2 patch series with
git send-email.

With best regards,
Volker