[PULL 0/9] Audio patches

[PULL 0/9] Audio patches

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The following changes since commit 668df86ee8076152320345d8e36be7c95ec0a09a:

  Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging (2025-05-23 09:26:29 -0400)

are available in the Git repository at:

  https://gitlab.com/marcandre.lureau/qemu.git tags/audio-pull-request

for you to fetch changes up to 2bccabe6df5e91145c1313bb79b98200aa13b5ff:

  audio: Reset rate control when adding bytes (2025-05-25 15:25:21 +0200)

----------------------------------------------------------------
Audio patches

- add float sample endianness converters
- various fixes

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

Akihiko Odaki (1):
  audio: Reset rate control when adding bytes

BALATON Zoltan (1):
  alsaaudio: Set try-poll to false by default

Volker Rümelin (7):
  tests/functional: use 'none' audio driver for q800 tests
  audio: fix SIGSEGV in AUD_get_buffer_size_out()
  audio: fix size calculation in AUD_get_buffer_size_out()
  hw/audio/asc: fix SIGSEGV in asc_realize()
  hw/audio/asc: replace g_malloc0() with g_malloc()
  audio/mixeng: remove unnecessary pointer type casts
  audio: add float sample endianness converters

 qapi/audio.json                      |  2 +-
 audio/audio_int.h                    |  1 +
 audio/audio_template.h               | 12 ++--
 audio/mixeng.h                       |  6 +-
 audio/alsaaudio.c                    |  2 +-
 audio/audio.c                        | 25 ++++++---
 audio/mixeng.c                       | 83 ++++++++++++++++++++++++----
 hw/audio/asc.c                       |  9 ++-
 qemu-options.hx                      |  4 +-
 tests/functional/test_m68k_q800.py   |  3 +-
 tests/functional/test_m68k_replay.py |  3 +-
 11 files changed, 119 insertions(+), 31 deletions(-)

-- 
2.49.0

[PULL 1/9] tests/functional: use 'none' audio driver for q800 tests

[marcandre.lureau]

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

Since commit ac13a6b3fd ("audio: add Apple Sound Chip (ASC)
emulation") the Quadra 800 machine has an audio device. It is
not guaranteed that the default audio driver of the audio
subsystem will work correctly on all host systems. Therefore,
the 'none' audio driver should be used in all q800 tests.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2812
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20250515054429.7385-1-vr_qemu@t-online.de>
---
 tests/functional/test_m68k_q800.py   | 3 ++-
 tests/functional/test_m68k_replay.py | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/functional/test_m68k_q800.py b/tests/functional/test_m68k_q800.py
index 400b7aeb5d..b3e655346c 100755
--- a/tests/functional/test_m68k_q800.py
+++ b/tests/functional/test_m68k_q800.py
@@ -25,7 +25,8 @@ def test_m68k_q800(self):
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyS0 vga=off')
         self.vm.add_args('-kernel', kernel_path,
-                         '-append', kernel_command_line)
+                         '-append', kernel_command_line,
+                         '-audio', 'none')
         self.vm.launch()
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
diff --git a/tests/functional/test_m68k_replay.py b/tests/functional/test_m68k_replay.py
index 18c1db539c..213d6ae07e 100755
--- a/tests/functional/test_m68k_replay.py
+++ b/tests/functional/test_m68k_replay.py
@@ -24,7 +24,8 @@ def test_q800(self):
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyS0 vga=off')
         console_pattern = 'No filesystem could mount root'
-        self.run_rr(kernel_path, kernel_command_line, console_pattern)
+        self.run_rr(kernel_path, kernel_command_line, console_pattern,
+                    args=('-audio', 'none'))
 
     ASSET_MCF5208 = Asset(
        'https://qemu-advcal.gitlab.io/qac-best-of-multiarch/download/day07.tar.xz',
-- 
2.49.0

[PULL 2/9] audio: fix SIGSEGV in AUD_get_buffer_size_out()

[marcandre.lureau]

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

As far as the emulated audio devices are concerned the pointer
returned by AUD_open_out() is an opaque handle. This includes
the NULL pointer. In this case, AUD_get_buffer_size_out() should
return a sensible buffer size instead of triggering a segmentation
fault. All other public AUD_*_out() and audio_*_out() functions
handle this case.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20250515054429.7385-2-vr_qemu@t-online.de>
---
 audio/audio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index 41ee11aaad..70ef22b1a4 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -905,6 +905,10 @@ size_t AUD_read(SWVoiceIn *sw, void *buf, size_t size)
 
 int AUD_get_buffer_size_out(SWVoiceOut *sw)
 {
+    if (!sw) {
+        return 0;
+    }
+
     return sw->hw->samples * sw->hw->info.bytes_per_frame;
 }
 
-- 
2.49.0

[PULL 3/9] audio: fix size calculation in AUD_get_buffer_size_out()

[marcandre.lureau]

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

The buffer size calculated by AUD_get_buffer_size_out() is often
incorrect. sw->hw->samples * sw->hw->info.bytes_per_frame is the
size of the mixing engine buffer in audio frames multiplied by
the size of one frame of the audio backend. Due to resampling or
format conversion, the size of the frontend buffer can differ
significantly.

Return the correct buffer size when the mixing engine is used.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20250515054429.7385-3-vr_qemu@t-online.de>
---
 audio/audio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index 70ef22b1a4..3f5baf0cc6 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -909,6 +909,10 @@ int AUD_get_buffer_size_out(SWVoiceOut *sw)
         return 0;
     }
 
+    if (audio_get_pdo_out(sw->s->dev)->mixing_engine) {
+        return sw->resample_buf.size * sw->info.bytes_per_frame;
+    }
+
     return sw->hw->samples * sw->hw->info.bytes_per_frame;
 }
 
-- 
2.49.0

[PULL 4/9] hw/audio/asc: fix SIGSEGV in asc_realize()

[marcandre.lureau]

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

AUD_open_out() may fail and return NULL. This may then lead to
a segmentation fault in memset() below. The memset() behaviour
is undefined if the pointer to the destination object is a null
pointer.

Add the missing error handling code.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20250515054429.7385-4-vr_qemu@t-online.de>
---
 hw/audio/asc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/audio/asc.c b/hw/audio/asc.c
index 18382ccf6a..6721c0d9fb 100644
--- a/hw/audio/asc.c
+++ b/hw/audio/asc.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/timer.h"
+#include "qapi/error.h"
 #include "hw/sysbus.h"
 #include "hw/irq.h"
 #include "audio/audio.h"
@@ -653,6 +654,12 @@ static void asc_realize(DeviceState *dev, Error **errp)
 
     s->voice = AUD_open_out(&s->card, s->voice, "asc.out", s, asc_out_cb,
                             &as);
+    if (!s->voice) {
+        AUD_remove_card(&s->card);
+        error_setg(errp, "Initializing audio stream failed");
+        return;
+    }
+
     s->shift = 1;
     s->samples = AUD_get_buffer_size_out(s->voice) >> s->shift;
     s->mixbuf = g_malloc0(s->samples << s->shift);
-- 
2.49.0

[PULL 5/9] hw/audio/asc: replace g_malloc0() with g_malloc()

[marcandre.lureau]

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

There is no need to allocate initialized memory with g_malloc0()
if it's directly followed by a memset() function call. g_malloc()
is sufficient.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20250515054429.7385-5-vr_qemu@t-online.de>
---
 hw/audio/asc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/asc.c b/hw/audio/asc.c
index 6721c0d9fb..edd42d6d91 100644
--- a/hw/audio/asc.c
+++ b/hw/audio/asc.c
@@ -664,7 +664,7 @@ static void asc_realize(DeviceState *dev, Error **errp)
     s->samples = AUD_get_buffer_size_out(s->voice) >> s->shift;
     s->mixbuf = g_malloc0(s->samples << s->shift);
 
-    s->silentbuf = g_malloc0(s->samples << s->shift);
+    s->silentbuf = g_malloc(s->samples << s->shift);
     memset(s->silentbuf, 0x80, s->samples << s->shift);
 
     /* Add easc registers if required */
-- 
2.49.0

[PULL 6/9] audio/mixeng: remove unnecessary pointer type casts

[marcandre.lureau]

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

A simple assignment automatically converts a void pointer type
to any other pointer type.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20250515054429.7385-6-vr_qemu@t-online.de>
---
 audio/mixeng.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index 69f6549224..13e1ff9b08 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -286,7 +286,7 @@ static const float float_scale_reciprocal = 1.f / ((int64_t)INT32_MAX + 1);
 static void conv_natural_float_to_mono(struct st_sample *dst, const void *src,
                                        int samples)
 {
-    float *in = (float *)src;
+    const float *in = src;
 
     while (samples--) {
         dst->r = dst->l = CONV_NATURAL_FLOAT(*in++);
@@ -297,7 +297,7 @@ static void conv_natural_float_to_mono(struct st_sample *dst, const void *src,
 static void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
                                          int samples)
 {
-    float *in = (float *)src;
+    const float *in = src;
 
     while (samples--) {
         dst->l = CONV_NATURAL_FLOAT(*in++);
@@ -314,7 +314,7 @@ t_sample *mixeng_conv_float[2] = {
 static void clip_natural_float_from_mono(void *dst, const struct st_sample *src,
                                          int samples)
 {
-    float *out = (float *)dst;
+    float *out = dst;
 
     while (samples--) {
         *out++ = CLIP_NATURAL_FLOAT(src->l + src->r);
@@ -325,7 +325,7 @@ static void clip_natural_float_from_mono(void *dst, const struct st_sample *src,
 static void clip_natural_float_from_stereo(
     void *dst, const struct st_sample *src, int samples)
 {
-    float *out = (float *)dst;
+    float *out = dst;
 
     while (samples--) {
         *out++ = CLIP_NATURAL_FLOAT(src->l);
-- 
2.49.0

[PULL 7/9] audio: add float sample endianness converters

[marcandre.lureau]

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

Commit ed2a4a7941 ("audio: proper support for float samples in
mixeng") added support for float audio samples. As there were no
audio frontend devices with float support at that time, the code
was limited to native endian float samples.

When nobody was paying attention, an audio device that supports
floating point samples crept in with commit eb9ad377bb
("virtio-sound: handle control messages and streams").

Add code for the audio subsystem to convert float samples to the
correct endianness.

The type punning code was taken from the PipeWire project.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20250515054429.7385-7-vr_qemu@t-online.de>
---
 audio/audio_template.h | 12 ++++---
 audio/mixeng.h         |  6 ++--
 audio/audio.c          |  3 +-
 audio/mixeng.c         | 75 ++++++++++++++++++++++++++++++++++++++----
 4 files changed, 82 insertions(+), 14 deletions(-)

diff --git a/audio/audio_template.h b/audio/audio_template.h
index 7ccfec0116..c29d79c443 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -174,9 +174,11 @@ static int glue (audio_pcm_sw_init_, TYPE) (
 
     if (sw->info.is_float) {
 #ifdef DAC
-        sw->conv = mixeng_conv_float[sw->info.nchannels == 2];
+        sw->conv = mixeng_conv_float[sw->info.nchannels == 2]
+            [sw->info.swap_endianness];
 #else
-        sw->clip = mixeng_clip_float[sw->info.nchannels == 2];
+        sw->clip = mixeng_clip_float[sw->info.nchannels == 2]
+            [sw->info.swap_endianness];
 #endif
     } else {
 #ifdef DAC
@@ -303,9 +305,11 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
 
     if (hw->info.is_float) {
 #ifdef DAC
-        hw->clip = mixeng_clip_float[hw->info.nchannels == 2];
+        hw->clip = mixeng_clip_float[hw->info.nchannels == 2]
+            [hw->info.swap_endianness];
 #else
-        hw->conv = mixeng_conv_float[hw->info.nchannels == 2];
+        hw->conv = mixeng_conv_float[hw->info.nchannels == 2]
+            [hw->info.swap_endianness];
 #endif
     } else {
 #ifdef DAC
diff --git a/audio/mixeng.h b/audio/mixeng.h
index a5f56d2c26..ead93ac2f7 100644
--- a/audio/mixeng.h
+++ b/audio/mixeng.h
@@ -42,9 +42,9 @@ typedef void (f_sample) (void *dst, const struct st_sample *src, int samples);
 extern t_sample *mixeng_conv[2][2][2][3];
 extern f_sample *mixeng_clip[2][2][2][3];
 
-/* indices: [stereo] */
-extern t_sample *mixeng_conv_float[2];
-extern f_sample *mixeng_clip_float[2];
+/* indices: [stereo][swap endianness] */
+extern t_sample *mixeng_conv_float[2][2];
+extern f_sample *mixeng_clip_float[2][2];
 
 void *st_rate_start (int inrate, int outrate);
 void st_rate_flow(void *opaque, st_sample *ibuf, st_sample *obuf,
diff --git a/audio/audio.c b/audio/audio.c
index 3f5baf0cc6..b58ad74433 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1892,7 +1892,8 @@ CaptureVoiceOut *AUD_add_capture(
         cap->buf = g_malloc0_n(hw->mix_buf.size, hw->info.bytes_per_frame);
 
         if (hw->info.is_float) {
-            hw->clip = mixeng_clip_float[hw->info.nchannels == 2];
+            hw->clip = mixeng_clip_float[hw->info.nchannels == 2]
+                [hw->info.swap_endianness];
         } else {
             hw->clip = mixeng_clip
                 [hw->info.nchannels == 2]
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 13e1ff9b08..703ee5448f 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -283,6 +283,11 @@ static const float float_scale_reciprocal = 1.f / ((int64_t)INT32_MAX + 1);
 #endif
 #endif
 
+#define F32_TO_F32S(v) \
+    bswap32((union { uint32_t i; float f; }){ .f = (v) }.i)
+#define F32S_TO_F32(v) \
+    ((union { uint32_t i; float f; }){ .i = bswap32(v) }.f)
+
 static void conv_natural_float_to_mono(struct st_sample *dst, const void *src,
                                        int samples)
 {
@@ -294,6 +299,17 @@ static void conv_natural_float_to_mono(struct st_sample *dst, const void *src,
     }
 }
 
+static void conv_swap_float_to_mono(struct st_sample *dst, const void *src,
+                                    int samples)
+{
+    const uint32_t *in_f32s = src;
+
+    while (samples--) {
+        dst->r = dst->l = CONV_NATURAL_FLOAT(F32S_TO_F32(*in_f32s++));
+        dst++;
+    }
+}
+
 static void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
                                          int samples)
 {
@@ -306,9 +322,27 @@ static void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
     }
 }
 
-t_sample *mixeng_conv_float[2] = {
-    conv_natural_float_to_mono,
-    conv_natural_float_to_stereo,
+static void conv_swap_float_to_stereo(struct st_sample *dst, const void *src,
+                                      int samples)
+{
+    const uint32_t *in_f32s = src;
+
+    while (samples--) {
+        dst->l = CONV_NATURAL_FLOAT(F32S_TO_F32(*in_f32s++));
+        dst->r = CONV_NATURAL_FLOAT(F32S_TO_F32(*in_f32s++));
+        dst++;
+    }
+}
+
+t_sample *mixeng_conv_float[2][2] = {
+    {
+        conv_natural_float_to_mono,
+        conv_swap_float_to_mono,
+    },
+    {
+        conv_natural_float_to_stereo,
+        conv_swap_float_to_stereo,
+    }
 };
 
 static void clip_natural_float_from_mono(void *dst, const struct st_sample *src,
@@ -322,6 +356,17 @@ static void clip_natural_float_from_mono(void *dst, const struct st_sample *src,
     }
 }
 
+static void clip_swap_float_from_mono(void *dst, const struct st_sample *src,
+                                      int samples)
+{
+    uint32_t *out_f32s = dst;
+
+    while (samples--) {
+        *out_f32s++ = F32_TO_F32S(CLIP_NATURAL_FLOAT(src->l + src->r));
+        src++;
+    }
+}
+
 static void clip_natural_float_from_stereo(
     void *dst, const struct st_sample *src, int samples)
 {
@@ -334,9 +379,27 @@ static void clip_natural_float_from_stereo(
     }
 }
 
-f_sample *mixeng_clip_float[2] = {
-    clip_natural_float_from_mono,
-    clip_natural_float_from_stereo,
+static void clip_swap_float_from_stereo(
+    void *dst, const struct st_sample *src, int samples)
+{
+    uint32_t *out_f32s = dst;
+
+    while (samples--) {
+        *out_f32s++ = F32_TO_F32S(CLIP_NATURAL_FLOAT(src->l));
+        *out_f32s++ = F32_TO_F32S(CLIP_NATURAL_FLOAT(src->r));
+        src++;
+    }
+}
+
+f_sample *mixeng_clip_float[2][2] = {
+    {
+        clip_natural_float_from_mono,
+        clip_swap_float_from_mono,
+    },
+    {
+        clip_natural_float_from_stereo,
+        clip_swap_float_from_stereo,
+    }
 };
 
 void audio_sample_to_uint64(const void *samples, int pos,
-- 
2.49.0

[PULL 8/9] alsaaudio: Set try-poll to false by default

[marcandre.lureau]

From: BALATON Zoltan <balaton@eik.bme.hu>

Quoting Volker Rümelin: "try-poll=on tells the ALSA backend to try to
use an event loop instead of the audio timer. This works most of the
time. But the poll event handler in the ALSA backend has a bug. For
example, if the guest can't provide enough audio frames in time, the
ALSA buffer is only partly full and the event handler will be called
again and again on every iteration of the main loop. This increases
the processor load and the guest has less processor time to provide
new audio frames in time. I have two examples where a guest can't
recover from this situation and the guest seems to hang."

One reproducer I've found is booting MorphOS demo iso on
qemu-system-ppc -machine pegasos2 -audio alsa which should play a
startup sound but instead it freezes. Even when it does not hang it
plays choppy sound. Volker suggested using command line to set
try-poll=off saying: "The try-poll=off arguments are typically
necessary, because the alsa backend has a design issue with
try-poll=on. If the guest can't provide enough audio frames, it's
really unhelpful to ask for new audio frames on every main loop
iteration until the guest can provide enough audio frames. Timer based
playback doesn't have that problem."

But users cannot easily find this option and having a non-working
default is really unhelpful so to make life easier just set it to
false by default which works until the issue with the alsa backend can
be fixed.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[ Marc-André - Updated QAPI and CLI doc ]
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20250316002046.D066A4E6004@zero.eik.bme.hu>
---
 qapi/audio.json   | 2 +-
 audio/alsaaudio.c | 2 +-
 qemu-options.hx   | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/qapi/audio.json b/qapi/audio.json
index dd5a58d13e..49633cf317 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -96,7 +96,7 @@
 # @period-length: the period length in microseconds
 #
 # @try-poll: attempt to use poll mode, falling back to non-polling
-#     access on failure (default true)
+#     access on failure (default false)
 #
 # Since: 4.0
 ##
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index cacae1ea59..9b6c01c0ef 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -899,7 +899,7 @@ static void alsa_enable_in(HWVoiceIn *hw, bool enable)
 static void alsa_init_per_direction(AudiodevAlsaPerDirectionOptions *apdo)
 {
     if (!apdo->has_try_poll) {
-        apdo->try_poll = true;
+        apdo->try_poll = false;
         apdo->has_try_poll = true;
     }
 }
diff --git a/qemu-options.hx b/qemu-options.hx
index aab53bcfe8..7eb8e02b4b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -965,7 +965,7 @@ SRST
         Sets the period length in microseconds.
 
     ``in|out.try-poll=on|off``
-        Attempt to use poll mode with the device. Default is on.
+        Attempt to use poll mode with the device. Default is off.
 
     ``threshold=threshold``
         Threshold (in microseconds) when playback starts. Default is 0.
@@ -1002,7 +1002,7 @@ SRST
     ``in|out.buffer-count=count``
         Sets the count of the buffers.
 
-    ``in|out.try-poll=on|of``
+    ``in|out.try-poll=on|off``
         Attempt to use poll mode with the device. Default is on.
 
     ``try-mmap=on|off``
-- 
2.49.0

[PULL 9/9] audio: Reset rate control when adding bytes

[marcandre.lureau]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Commit 90320051ea99 ("spiceaudio: add a pcm_ops buffer_get_free
function") caused to emit messages saying "Resetting rate control"
frequently when the guest generates no frames.

audio_rate_peek_bytes() resets the rate control when frames < 0 ||
frames > 65536 where frames is the rate-limited number of frames.
Resetting when frames < 0 is sensible as the number simply doesn't make
sense.

There is a problem when frames > 65536. It implies the guest stopped
generating frames for a while so it makes sense to reset the rate
control when the guest resumed generating frames. However, the
commit mentioned earlier broke this assumption by letting spiceaudio
call audio_rate_peek_bytes() whether the guest is generating frames or
not.

Reset the rate control in audio_rate_add_bytes(), which is called only
when actually adding frames, according to the previous call to
audio_rate_peek_bytes() to avoid frequent rate control resets even when
the guest generates no frame.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-Id: <20250317-rate-v1-1-da9df062747c@daynix.com>
---
 audio/audio_int.h |  1 +
 audio/audio.c     | 14 ++++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index 2d079d00a2..f78ca05f92 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -255,6 +255,7 @@ const char *audio_application_name(void);
 typedef struct RateCtl {
     int64_t start_ticks;
     int64_t bytes_sent;
+    int64_t peeked_frames;
 } RateCtl;
 
 void audio_rate_start(RateCtl *rate);
diff --git a/audio/audio.c b/audio/audio.c
index b58ad74433..89f091bc88 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2283,17 +2283,19 @@ size_t audio_rate_peek_bytes(RateCtl *rate, struct audio_pcm_info *info)
     ticks = now - rate->start_ticks;
     bytes = muldiv64(ticks, info->bytes_per_second, NANOSECONDS_PER_SECOND);
     frames = (bytes - rate->bytes_sent) / info->bytes_per_frame;
-    if (frames < 0 || frames > 65536) {
-        AUD_log(NULL, "Resetting rate control (%" PRId64 " frames)\n", frames);
-        audio_rate_start(rate);
-        frames = 0;
-    }
+    rate->peeked_frames = frames;
 
-    return frames * info->bytes_per_frame;
+    return frames < 0 ? 0 : frames * info->bytes_per_frame;
 }
 
 void audio_rate_add_bytes(RateCtl *rate, size_t bytes_used)
 {
+    if (rate->peeked_frames < 0 || rate->peeked_frames > 65536) {
+        AUD_log(NULL, "Resetting rate control (%" PRId64 " frames)\n",
+                rate->peeked_frames);
+        audio_rate_start(rate);
+    }
+
     rate->bytes_sent += bytes_used;
 }
 
-- 
2.49.0

Re: [PULL 0/9] Audio patches

[Stefan Hajnoczi]

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

Applied, thanks.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 8/9] alsaaudio: Set try-poll to false by default

[Markus Armbruster]

marcandre.lureau@redhat.com writes:

> From: BALATON Zoltan <balaton@eik.bme.hu>
>
> Quoting Volker Rümelin: "try-poll=on tells the ALSA backend to try to
> use an event loop instead of the audio timer. This works most of the
> time. But the poll event handler in the ALSA backend has a bug. For
> example, if the guest can't provide enough audio frames in time, the
> ALSA buffer is only partly full and the event handler will be called
> again and again on every iteration of the main loop. This increases
> the processor load and the guest has less processor time to provide
> new audio frames in time. I have two examples where a guest can't
> recover from this situation and the guest seems to hang."
>
> One reproducer I've found is booting MorphOS demo iso on
> qemu-system-ppc -machine pegasos2 -audio alsa which should play a
> startup sound but instead it freezes. Even when it does not hang it
> plays choppy sound. Volker suggested using command line to set
> try-poll=off saying: "The try-poll=off arguments are typically
> necessary, because the alsa backend has a design issue with
> try-poll=on. If the guest can't provide enough audio frames, it's
> really unhelpful to ask for new audio frames on every main loop
> iteration until the guest can provide enough audio frames. Timer based
> playback doesn't have that problem."
>
> But users cannot easily find this option and having a non-working
> default is really unhelpful so to make life easier just set it to
> false by default which works until the issue with the alsa backend can
> be fixed.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> [ Marc-André - Updated QAPI and CLI doc ]
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-Id: <20250316002046.D066A4E6004@zero.eik.bme.hu>
> ---
>  qapi/audio.json   | 2 +-
>  audio/alsaaudio.c | 2 +-
>  qemu-options.hx   | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index dd5a58d13e..49633cf317 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -96,7 +96,7 @@
>  # @period-length: the period length in microseconds
>  #
>  # @try-poll: attempt to use poll mode, falling back to non-polling
> -#     access on failure (default true)
> +#     access on failure (default false)
>  #
>  # Since: 4.0
>  ##

Missed this when it was posted (it wasn't cc'ed to me).  Flipping the
default is technically an incompatible change.  I understand the
justification, and I'm not passing judgement on its validity; that's the
audio maintainers job.  I just want to ask: does this need a release
note?

We normally record incompatible changes in docs/about/deprecated.rst and
then docs/about/removed-features.rst, but these don't fit here.

[...]

Re: [PULL 8/9] alsaaudio: Set try-poll to false by default

[Marc-André Lureau]

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

Hi

On Mon, May 26, 2025 at 8:27 AM Markus Armbruster <armbru@redhat.com> wrote:

> marcandre.lureau@redhat.com writes:
>
> > From: BALATON Zoltan <balaton@eik.bme.hu>
> >
> > Quoting Volker Rümelin: "try-poll=on tells the ALSA backend to try to
> > use an event loop instead of the audio timer. This works most of the
> > time. But the poll event handler in the ALSA backend has a bug. For
> > example, if the guest can't provide enough audio frames in time, the
> > ALSA buffer is only partly full and the event handler will be called
> > again and again on every iteration of the main loop. This increases
> > the processor load and the guest has less processor time to provide
> > new audio frames in time. I have two examples where a guest can't
> > recover from this situation and the guest seems to hang."
> >
> > One reproducer I've found is booting MorphOS demo iso on
> > qemu-system-ppc -machine pegasos2 -audio alsa which should play a
> > startup sound but instead it freezes. Even when it does not hang it
> > plays choppy sound. Volker suggested using command line to set
> > try-poll=off saying: "The try-poll=off arguments are typically
> > necessary, because the alsa backend has a design issue with
> > try-poll=on. If the guest can't provide enough audio frames, it's
> > really unhelpful to ask for new audio frames on every main loop
> > iteration until the guest can provide enough audio frames. Timer based
> > playback doesn't have that problem."
> >
> > But users cannot easily find this option and having a non-working
> > default is really unhelpful so to make life easier just set it to
> > false by default which works until the issue with the alsa backend can
> > be fixed.
> >
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > [ Marc-André - Updated QAPI and CLI doc ]
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Message-Id: <20250316002046.D066A4E6004@zero.eik.bme.hu>
> > ---
> >  qapi/audio.json   | 2 +-
> >  audio/alsaaudio.c | 2 +-
> >  qemu-options.hx   | 4 ++--
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/qapi/audio.json b/qapi/audio.json
> > index dd5a58d13e..49633cf317 100644
> > --- a/qapi/audio.json
> > +++ b/qapi/audio.json
> > @@ -96,7 +96,7 @@
> >  # @period-length: the period length in microseconds
> >  #
> >  # @try-poll: attempt to use poll mode, falling back to non-polling
> > -#     access on failure (default true)
> > +#     access on failure (default false)
> >  #
> >  # Since: 4.0
> >  ##
>
> Missed this when it was posted (it wasn't cc'ed to me).  Flipping the
> default is technically an incompatible change.  I understand the
> justification, and I'm not passing judgement on its validity; that's the
> audio maintainers job.  I just want to ask: does this need a release
> note?
>
>
I doubt anyone will care as long as it doesn't break (that we can't know).

I added a note to https://wiki.qemu.org/ChangeLog/10.1#Audio


We normally record incompatible changes in docs/about/deprecated.rst and
> then docs/about/removed-features.rst, but these don't fit here.
>
> [...]
>
>

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

Re: [PULL 8/9] alsaaudio: Set try-poll to false by default

[Markus Armbruster]

Thank you!

Re: [PULL 3/9] audio: fix size calculation in AUD_get_buffer_size_out()

[Michael Tokarev]

On 25.05.2025 16:27, marcandre.lureau@redhat.com wrote:
> From: Volker Rümelin <vr_qemu@t-online.de>
> 
> The buffer size calculated by AUD_get_buffer_size_out() is often
> incorrect. sw->hw->samples * sw->hw->info.bytes_per_frame is the
> size of the mixing engine buffer in audio frames multiplied by
> the size of one frame of the audio backend. Due to resampling or
> format conversion, the size of the frontend buffer can differ
> significantly.
> 
> Return the correct buffer size when the mixing engine is used.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> Message-Id: <20250515054429.7385-3-vr_qemu@t-online.de>
> ---
>   audio/audio.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index 70ef22b1a4..3f5baf0cc6 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -909,6 +909,10 @@ int AUD_get_buffer_size_out(SWVoiceOut *sw)
>           return 0;
>       }
>   
> +    if (audio_get_pdo_out(sw->s->dev)->mixing_engine) {
> +        return sw->resample_buf.size * sw->info.bytes_per_frame;
> +    }
> +
>       return sw->hw->samples * sw->hw->info.bytes_per_frame;
>   }
>   

Hi!

Is this relevant for 7.2.x qemu-stable series?

Commit v7.2.0-2160-g2c3f9a0a92 "audio: change type and name of the
resample buffer" introduced sw->resample_buf, before that commit
there was sw->buf, of different type.

Maybe whole this series should not be picked up for stable-7.2,
after all?

Thanks,

/mjt

Re: [PULL 3/9] audio: fix size calculation in AUD_get_buffer_size_out()

[Volker Rümelin]

Am 09.07.25 um 04:24 schrieb Michael Tokarev:
> On 25.05.2025 16:27, marcandre.lureau@redhat.com wrote:
>> From: Volker Rümelin <vr_qemu@t-online.de>
>>
>> The buffer size calculated by AUD_get_buffer_size_out() is often
>> incorrect. sw->hw->samples * sw->hw->info.bytes_per_frame is the
>> size of the mixing engine buffer in audio frames multiplied by
>> the size of one frame of the audio backend. Due to resampling or
>> format conversion, the size of the frontend buffer can differ
>> significantly.
>>
>> Return the correct buffer size when the mixing engine is used.
>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
>> Message-Id: <20250515054429.7385-3-vr_qemu@t-online.de>
>> ---
>>   audio/audio.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 70ef22b1a4..3f5baf0cc6 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -909,6 +909,10 @@ int AUD_get_buffer_size_out(SWVoiceOut *sw)
>>           return 0;
>>       }
>>   +    if (audio_get_pdo_out(sw->s->dev)->mixing_engine) {
>> +        return sw->resample_buf.size * sw->info.bytes_per_frame;
>> +    }
>> +
>>       return sw->hw->samples * sw->hw->info.bytes_per_frame;
>>   }
>>   
>
> Hi!
>
> Is this relevant for 7.2.x qemu-stable series?
>
> Commit v7.2.0-2160-g2c3f9a0a92 "audio: change type and name of the
> resample buffer" introduced sw->resample_buf, before that commit
> there was sw->buf, of different type.
>
> Maybe whole this series should not be picked up for stable-7.2,
> after all?
>

Hi,

I would drop this patch in staging-7.2. This issue exists since end of
2004 and it hasn't really been a big problem since then. You can keep
the patch "audio: fix SIGSEGV in AUD_get_buffer_size_out()".

With best regards,
Volker

> Thanks,
>
> /mjt
>
>