[PULL 0/9] Audio 20200923 patches

[PULL 0/9] Audio 20200923 patches

[Gerd Hoffmann]

The following changes since commit 0fc0142828b5bc965790a1c5c6e241897d3387cb:

  Merge remote-tracking branch 'remotes/kraxel/tags/input-20200921-pull-reque=
st' into staging (2020-09-22 21:11:10 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/audio-20200923-pull-request

for you to fetch changes up to 5e626fa7364bc2d7db5f6c8e59150dee1689b98a:

  audio: build spiceaudio as module (2020-09-23 08:36:50 +0200)

----------------------------------------------------------------
audio: various buffering fixes.
audio: build spiceaudio as module.

----------------------------------------------------------------

Gerd Hoffmann (2):
  audio: remove qemu_spice_audio_init()
  audio: build spiceaudio as module

Volker R=C3=BCmelin (7):
  audio: handle buf =3D=3D NULL in put_buffer_out()
  audio/audio: fix video playback slowdown with spiceaudio
  audio/spiceaudio: always rate limit playback stream
  audio: align audio_generic_read with audio_pcm_hw_run_in
  audio: remove unnecessary calls to put_buffer_in
  audio: align audio_generic_write with audio_pcm_hw_run_out
  audio: run downstream playback queue unconditionally

 include/ui/qemu-spice.h |  1 -
 audio/audio.c           | 80 ++++++++++++++++++++++++++++++++++-------
 audio/spiceaudio.c      | 26 ++++++--------
 ui/spice-core.c         |  1 -
 audio/meson.build       |  4 +--
 5 files changed, 80 insertions(+), 32 deletions(-)

--=20
2.27.0

[PULL 1/9] audio: handle buf == NULL in put_buffer_out()

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

With the next patch all audio backends put_buffer_out() functions
have to handle the buf == NULL case, provided the get_buffer_out()
function may return buf = NULL and size > 0.

It turns out that all audio backends get_buffer_out() functions
either can't return buf = NULL or return buf = NULL and size = 0
at the same time. The only exception is the spiceaudio backend
where size may be uninitialized.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-id: 20200920171729.15861-1-vr_qemu@t-online.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/spiceaudio.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index b6b5da4812f2..c8d81ba44290 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -135,6 +135,7 @@ static void *line_out_get_buffer(HWVoiceOut *hw, size_t *size)
             (out->fsize - out->fpos) * hw->info.bytes_per_frame);
     } else {
         audio_rate_start(&out->rate);
+        *size = LINE_OUT_SAMPLES << 2;
     }
     return out->frame + out->fpos;
 }
@@ -143,12 +144,14 @@ static size_t line_out_put_buffer(HWVoiceOut *hw, void *buf, size_t size)
 {
     SpiceVoiceOut *out = container_of(hw, SpiceVoiceOut, hw);
 
-    assert(buf == out->frame + out->fpos && out->fpos <= out->fsize);
-    out->fpos += size >> 2;
+    if (buf) {
+        assert(buf == out->frame + out->fpos && out->fpos <= out->fsize);
+        out->fpos += size >> 2;
 
-    if (out->fpos == out->fsize) { /* buffer full */
-        spice_server_playback_put_samples(&out->sin, out->frame);
-        out->frame = NULL;
+        if (out->fpos == out->fsize) { /* buffer full */
+            spice_server_playback_put_samples(&out->sin, out->frame);
+            out->frame = NULL;
+        }
     }
 
     return size;
-- 
2.27.0

[PULL 2/9] audio/audio: fix video playback slowdown with spiceaudio

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

This patch allows the audio backends get_buffer_out() functions
to drop audio data and mitigates a bug reported on the qemu-devel
mailing list.

https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg03832.html

The new rules for the variables buf and size returned by
get_buffer_out() are:
size == 0: Downstream playback buffer is full. Retry later.
size > 0, buf != NULL: Copy size bytes to buf for playback.
size > 0, buf == NULL: Drop size bytes.

The audio playback rate with spiceaudio for the no audio case is
too fast, but that's what we had before commit fb35c2cec5
"audio/dsound: fix invalid parameters error". The complete fix
comes with the next patch.

Reported-by: Qi Zhou <atmgnd@outlook.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-id: 20200920171729.15861-2-vr_qemu@t-online.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 1a68cfaafb9f..7b660dd0c4d0 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1091,12 +1091,15 @@ static size_t audio_pcm_hw_run_out(HWVoiceOut *hw, size_t live)
     while (live) {
         size_t size, decr, proc;
         void *buf = hw->pcm_ops->get_buffer_out(hw, &size);
-        if (!buf || size == 0) {
+
+        if (size == 0) {
             break;
         }
 
         decr = MIN(size / hw->info.bytes_per_frame, live);
-        audio_pcm_hw_clip_out(hw, buf, decr);
+        if (buf) {
+            audio_pcm_hw_clip_out(hw, buf, decr);
+        }
         proc = hw->pcm_ops->put_buffer_out(hw, buf,
                                            decr * hw->info.bytes_per_frame) /
             hw->info.bytes_per_frame;
-- 
2.27.0

[PULL 3/9] audio/spiceaudio: always rate limit playback stream

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

The playback rate with the spiceaudio backend is currently too
fast if there's no spice client connected or the spice client
can't play audio. Rate limit the audio playback stream in all
cases. To calculate the rate correctly the limiter has to know
the maximum buffer size.

Fixes: 8c198ff065 ("spiceaudio: port to the new audio backend api")
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-id: 20200920171729.15861-3-vr_qemu@t-online.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c      |  3 ++-
 audio/spiceaudio.c | 10 ++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 7b660dd0c4d0..d5891e1928bc 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1089,7 +1089,8 @@ static size_t audio_pcm_hw_run_out(HWVoiceOut *hw, size_t live)
     size_t clipped = 0;
 
     while (live) {
-        size_t size, decr, proc;
+        size_t size = live * hw->info.bytes_per_frame;
+        size_t decr, proc;
         void *buf = hw->pcm_ops->get_buffer_out(hw, &size);
 
         if (size == 0) {
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index c8d81ba44290..c062742622e4 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -130,13 +130,11 @@ static void *line_out_get_buffer(HWVoiceOut *hw, size_t *size)
     }
 
     if (out->frame) {
-        *size = audio_rate_get_bytes(
-            &hw->info, &out->rate,
-            (out->fsize - out->fpos) * hw->info.bytes_per_frame);
-    } else {
-        audio_rate_start(&out->rate);
-        *size = LINE_OUT_SAMPLES << 2;
+        *size = MIN((out->fsize - out->fpos) << 2, *size);
     }
+
+    *size = audio_rate_get_bytes(&hw->info, &out->rate, *size);
+
     return out->frame + out->fpos;
 }
 
-- 
2.27.0

[PULL 4/9] audio: align audio_generic_read with audio_pcm_hw_run_in

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

The function audio_generic_read should work exactly like
audio_pcm_hw_run_in. It's a very similar function working
on a different buffer.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-id: 20200920171729.15861-4-vr_qemu@t-online.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index d5891e1928bc..5c47a036028c 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1495,12 +1495,23 @@ size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size)
 
 size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size)
 {
-    void *src = hw->pcm_ops->get_buffer_in(hw, &size);
+    size_t total = 0;
 
-    memcpy(buf, src, size);
-    hw->pcm_ops->put_buffer_in(hw, src, size);
+    while (total < size) {
+        size_t src_size = size - total;
+        void *src = hw->pcm_ops->get_buffer_in(hw, &src_size);
 
-    return size;
+        if (src_size == 0) {
+            hw->pcm_ops->put_buffer_in(hw, src, src_size);
+            break;
+        }
+
+        memcpy((char *)buf + total, src, src_size);
+        hw->pcm_ops->put_buffer_in(hw, src, src_size);
+        total += src_size;
+    }
+
+    return total;
 }
 
 static int audio_driver_init(AudioState *s, struct audio_driver *drv,
-- 
2.27.0

[PULL 5/9] audio: remove unnecessary calls to put_buffer_in

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

This patch removes unnecessary calls to the pcm_ops function
put_buffer_in(). No audio backend needs this call if the
returned length of pcm_ops function get_buffer_in() is zero.

For the DirectSound backend this prevents a call to
dsound_unlock_in() without a preceding call to dsound_lock_in().
While Windows doesn't complain it seems wrong anyway.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-id: 20200920171729.15861-5-vr_qemu@t-online.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 5c47a036028c..341edc4d0012 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1261,7 +1261,6 @@ static size_t audio_pcm_hw_run_in(HWVoiceIn *hw, size_t samples)
 
         assert(size % hw->info.bytes_per_frame == 0);
         if (size == 0) {
-            hw->pcm_ops->put_buffer_in(hw, buf, size);
             break;
         }
 
@@ -1502,7 +1501,6 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size)
         void *src = hw->pcm_ops->get_buffer_in(hw, &src_size);
 
         if (src_size == 0) {
-            hw->pcm_ops->put_buffer_in(hw, src, src_size);
             break;
         }
 
-- 
2.27.0

[PULL 6/9] audio: align audio_generic_write with audio_pcm_hw_run_out

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

The function audio_generic_write should work exactly like
audio_pcm_hw_run_out. It's a very similar function working on a
different buffer.

This patch significantly reduces the number of drop-outs with
the DirectSound backend. To hear the difference start qemu with
-audiodev dsound,id=audio0,out.mixing-engine=off and play a
song in the guest with and without this patch.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-id: 20200920171729.15861-6-vr_qemu@t-online.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 341edc4d0012..8587e3d15225 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1484,12 +1484,34 @@ size_t audio_generic_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
 
 size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size)
 {
-    size_t dst_size, copy_size;
-    void *dst = hw->pcm_ops->get_buffer_out(hw, &dst_size);
-    copy_size = MIN(size, dst_size);
+    size_t total = 0;
 
-    memcpy(dst, buf, copy_size);
-    return hw->pcm_ops->put_buffer_out(hw, dst, copy_size);
+    while (total < size) {
+        size_t dst_size = size - total;
+        size_t copy_size, proc;
+        void *dst = hw->pcm_ops->get_buffer_out(hw, &dst_size);
+
+        if (dst_size == 0) {
+            break;
+        }
+
+        copy_size = MIN(size - total, dst_size);
+        if (dst) {
+            memcpy(dst, (char *)buf + total, copy_size);
+        }
+        proc = hw->pcm_ops->put_buffer_out(hw, dst, copy_size);
+        total += proc;
+
+        if (proc == 0 || proc < copy_size) {
+            break;
+        }
+    }
+
+    if (hw->pcm_ops->run_buffer_out) {
+        hw->pcm_ops->run_buffer_out(hw);
+    }
+
+    return total;
 }
 
 size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size)
-- 
2.27.0

[PULL 7/9] audio: run downstream playback queue unconditionally

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

Run the downstream playback queue even if there are no samples
in the mixing engine buffer. The downstream queue may still have
queued samples.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-id: 20200920171729.15861-7-vr_qemu@t-online.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index 8587e3d15225..6ff3f168d7b7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1186,6 +1186,9 @@ static void audio_run_out (AudioState *s)
                     }
                 }
             }
+            if (hw->pcm_ops->run_buffer_out) {
+                hw->pcm_ops->run_buffer_out(hw);
+            }
             continue;
         }
 
-- 
2.27.0

[PULL 8/9] audio: remove qemu_spice_audio_init()

[Gerd Hoffmann]

Handle the spice special case in audio_init instead.

With the qemu_spice_audio_init() symbol dependency being
gone we can build spiceaudio as module.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200916084117.21828-2-kraxel@redhat.com
---
 include/ui/qemu-spice.h |  1 -
 audio/audio.c           | 16 ++++++++++++++++
 audio/spiceaudio.c      |  5 -----
 ui/spice-core.c         |  1 -
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 8c23dfe71797..12474d88f40e 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -29,7 +29,6 @@ extern int using_spice;
 
 void qemu_spice_init(void);
 void qemu_spice_input_init(void);
-void qemu_spice_audio_init(void);
 void qemu_spice_display_init(void);
 int qemu_spice_display_add_client(int csock, int skipauth, int tls);
 int qemu_spice_add_interface(SpiceBaseInstance *sin);
diff --git a/audio/audio.c b/audio/audio.c
index 6ff3f168d7b7..46578e4a583b 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -34,6 +34,7 @@
 #include "qemu/module.h"
 #include "sysemu/replay.h"
 #include "sysemu/runstate.h"
+#include "ui/qemu-spice.h"
 #include "trace.h"
 
 #define AUDIO_CAP "audio"
@@ -1696,6 +1697,21 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
     /* silence gcc warning about uninitialized variable */
     AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
 
+    if (using_spice) {
+        /*
+         * When using spice allow the spice audio driver being picked
+         * as default.
+         *
+         * Temporary hack.  Using audio devices without explicit
+         * audiodev= property is already deprecated.  Same goes for
+         * the -soundhw switch.  Once this support gets finally
+         * removed we can also drop the concept of a default audio
+         * backend and this can go away.
+         */
+        driver = audio_driver_lookup("spice");
+        driver->can_be_default = 1;
+    }
+
     if (dev) {
         /* -audiodev option */
         legacy_config = false;
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index c062742622e4..ed6dff1dcc41 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -311,11 +311,6 @@ static struct audio_driver spice_audio_driver = {
     .voice_size_in  = sizeof (SpiceVoiceIn),
 };
 
-void qemu_spice_audio_init (void)
-{
-    spice_audio_driver.can_be_default = 1;
-}
-
 static void register_audio_spice(void)
 {
     audio_driver_register(&spice_audio_driver);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ecc2ec2c55c2..10aa309f78f7 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -804,7 +804,6 @@ void qemu_spice_init(void)
     qemu_spice_add_interface(&spice_migrate.base);
 
     qemu_spice_input_init();
-    qemu_spice_audio_init();
 
     qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
     qemu_spice_display_stop();
-- 
2.27.0

[PULL 9/9] audio: build spiceaudio as module

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200916084117.21828-3-kraxel@redhat.com
---
 audio/meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/meson.build b/audio/meson.build
index 15c06ba0453b..18a831129ed0 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -7,7 +7,6 @@ softmmu_ss.add(files(
   'wavcapture.c',
 ))
 
-softmmu_ss.add(when: [spice, 'CONFIG_SPICE'], if_true: files('spiceaudio.c'))
 softmmu_ss.add(when: [coreaudio, 'CONFIG_AUDIO_COREAUDIO'], if_true: files('coreaudio.c'))
 softmmu_ss.add(when: [dsound, 'CONFIG_AUDIO_DSOUND'], if_true: files('dsoundaudio.c'))
 softmmu_ss.add(when: ['CONFIG_AUDIO_WIN_INT'], if_true: files('audio_win_int.c'))
@@ -18,7 +17,8 @@ foreach m : [
   ['CONFIG_AUDIO_OSS', 'oss', oss, 'ossaudio.c'],
   ['CONFIG_AUDIO_PA', 'pa', pulse, 'paaudio.c'],
   ['CONFIG_AUDIO_SDL', 'sdl', sdl, 'sdlaudio.c'],
-  ['CONFIG_AUDIO_JACK', 'jack', jack, 'jackaudio.c']
+  ['CONFIG_AUDIO_JACK', 'jack', jack, 'jackaudio.c'],
+  ['CONFIG_SPICE', 'spice', spice, 'spiceaudio.c']
 ]
   if config_host.has_key(m[0])
     module_ss = ss.source_set()
-- 
2.27.0

Re: [PULL 0/9] Audio 20200923 patches

[Peter Maydell]

On Wed, 23 Sep 2020 at 10:12, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 0fc0142828b5bc965790a1c5c6e241897d3387cb:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/input-20200921-pull-reque=
> st' into staging (2020-09-22 21:11:10 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/audio-20200923-pull-request
>
> for you to fetch changes up to 5e626fa7364bc2d7db5f6c8e59150dee1689b98a:
>
>   audio: build spiceaudio as module (2020-09-23 08:36:50 +0200)
>
> ----------------------------------------------------------------
> audio: various buffering fixes.
> audio: build spiceaudio as module.
>
> ----------------------------------------------------------------
>
> Gerd Hoffmann (2):
>   audio: remove qemu_spice_audio_init()
>   audio: build spiceaudio as module
>
> Volker R=C3=BCmelin (7):
>   audio: handle buf =3D=3D NULL in put_buffer_out()

UTF-8 damage in your workflow somewhere. Commits in the
actual pullreq seem OK, though.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM