From d4b70fa4ede25734d2ff76503c735e6fcee6cfda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 20 Sep 2020 19:17:21 +0200 Subject: [PATCH 1/9] audio: handle buf == NULL in put_buffer_out() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-id: 20200920171729.15861-1-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/spiceaudio.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c index b6b5da4812..c8d81ba442 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; From 4c3356f96557e848f0323772f9502d60817682dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 20 Sep 2020 19:17:22 +0200 Subject: [PATCH 2/9] audio/audio: fix video playback slowdown with spiceaudio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Volker Rümelin Message-id: 20200920171729.15861-2-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/audio.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 1a68cfaafb..7b660dd0c4 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; From aec6d0dc4ef243b957115b3c3aef39c348fefb25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 20 Sep 2020 19:17:23 +0200 Subject: [PATCH 3/9] audio/spiceaudio: always rate limit playback stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-id: 20200920171729.15861-3-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- 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 7b660dd0c4..d5891e1928 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 c8d81ba442..c062742622 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; } From b9896dc5becca1c6c2824d1ef8bcf00f5c74a6a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 20 Sep 2020 19:17:24 +0200 Subject: [PATCH 4/9] audio: align audio_generic_read with audio_pcm_hw_run_in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-id: 20200920171729.15861-4-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/audio.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index d5891e1928..5c47a03602 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, From ac221f45e3c4fc7823a8e913a6926ca8509c2526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 20 Sep 2020 19:17:25 +0200 Subject: [PATCH 5/9] audio: remove unnecessary calls to put_buffer_in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-id: 20200920171729.15861-5-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/audio.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 5c47a03602..341edc4d00 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; } From 2d8823077e56598da1ad6adf7e5760f84057bad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 20 Sep 2020 19:17:26 +0200 Subject: [PATCH 6/9] audio: align audio_generic_write with audio_pcm_hw_run_out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-id: 20200920171729.15861-6-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/audio.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 341edc4d00..8587e3d152 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) From a8a98cfd42f70ac1b725ee25e23154291228b330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 20 Sep 2020 19:17:27 +0200 Subject: [PATCH 7/9] audio: run downstream playback queue unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-id: 20200920171729.15861-7-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/audio.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/audio/audio.c b/audio/audio.c index 8587e3d152..6ff3f168d7 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; } From f0c4555edfdd2088aab04b5779912d52b6b90454 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 16 Sep 2020 10:41:16 +0200 Subject: [PATCH 8/9] audio: remove qemu_spice_audio_init() 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 Message-id: 20200916084117.21828-2-kraxel@redhat.com --- audio/audio.c | 16 ++++++++++++++++ audio/spiceaudio.c | 5 ----- include/ui/qemu-spice.h | 1 - ui/spice-core.c | 1 - 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 6ff3f168d7..46578e4a58 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 c062742622..ed6dff1dcc 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/include/ui/qemu-spice.h b/include/ui/qemu-spice.h index 8c23dfe717..12474d88f4 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/ui/spice-core.c b/ui/spice-core.c index ecc2ec2c55..10aa309f78 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(); From 5e626fa7364bc2d7db5f6c8e59150dee1689b98a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 16 Sep 2020 10:41:17 +0200 Subject: [PATCH 9/9] audio: build spiceaudio as module Signed-off-by: Gerd Hoffmann 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 15c06ba045..18a831129e 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()