[PATCH v2 0/7] audio related fixes for 10.1

[PATCH v2 0/7] audio related fixes for 10.1

[Volker Rümelin]

A few audio related fixes for 10.1.

The virtio-sound device is the first QEMU audio front end that supports floating point samples. The audio subsystem is only partially prepared for this. The commit message of patch 7/7 "audio: add float sample endianness converters" has the details. The new code paths in patch 7/7 are only compile tested. I don't have a big endian host to test.

checkpatch.pl complains about missing space characters in the type punning macros in patch 7/7. I don't agree.

v2: Address Mark's comments

patch 4/7 "hw/audio/asc: fix SIGSEGV in asc_realize()"
Only call AUD_remove_card() to handle cleanup.

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

 audio/audio.c                        | 11 +++-
 audio/audio_template.h               | 12 ++--
 audio/mixeng.c                       | 83 ++++++++++++++++++++++++----
 audio/mixeng.h                       |  6 +-
 hw/audio/asc.c                       |  9 ++-
 tests/functional/test_m68k_q800.py   |  3 +-
 tests/functional/test_m68k_replay.py |  3 +-
 7 files changed, 106 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH v2 1/7] tests/functional: use 'none' audio driver for q800 tests

[Volker Rümelin]

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

[PATCH v2 2/7] audio: fix SIGSEGV in AUD_get_buffer_size_out()

[Volker Rümelin]

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

[PATCH v2 3/7] audio: fix size calculation in AUD_get_buffer_size_out()

[Volker Rümelin]

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

[PATCH v2 4/7] hw/audio/asc: fix SIGSEGV in asc_realize()

[Volker Rümelin]

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

Re: [PATCH v2 4/7] hw/audio/asc: fix SIGSEGV in asc_realize()

[Mark Cave-Ayland]

On 15/05/2025 06:44, Volker Rümelin wrote:

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

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

[PATCH v2 5/7] hw/audio/asc: replace g_malloc0() with g_malloc()

[Volker Rümelin]

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

[PATCH v2 6/7] audio/mixeng: remove unnecessary pointer type casts

[Volker Rümelin]

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

[PATCH v2 7/7] audio: add float sample endianness converters

[Volker Rümelin]

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>
---
 audio/audio.c          |  3 +-
 audio/audio_template.h | 12 ++++---
 audio/mixeng.c         | 75 ++++++++++++++++++++++++++++++++++++++----
 audio/mixeng.h         |  6 ++--
 4 files changed, 82 insertions(+), 14 deletions(-)

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/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.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,
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,
-- 
2.43.0

Re: [PATCH v2 0/7] audio related fixes for 10.1

[Michael Tokarev]

On 15.05.2025 08:42, Volker Rümelin wrote:
> A few audio related fixes for 10.1.
> 
> The virtio-sound device is the first QEMU audio front end that supports floating point samples. The audio subsystem is only partially prepared for this. The commit message of patch 7/7 "audio: add float sample endianness converters" has the details. The new code paths in patch 7/7 are only compile tested. I don't have a big endian host to test.
..
> 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

Is there anything here which is worth to apply to qemu-stable?
(10.0.x is supposed to be an LTS series).

Thanks,

/mjt

Re: [PATCH v2 0/7] audio related fixes for 10.1

[Michael Tokarev]

On 29.05.2025 10:26, Michael Tokarev wrote:
> On 15.05.2025 08:42, Volker Rümelin wrote:
>> A few audio related fixes for 10.1.
>>
>> The virtio-sound device is the first QEMU audio front end that 
>> supports floating point samples. The audio subsystem is only partially 
>> prepared for this. The commit message of patch 7/7 "audio: add float 
>> sample endianness converters" has the details. The new code paths in 
>> patch 7/7 are only compile tested. I don't have a big endian host to 
>> test.
> ..
>> 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
> 
> Is there anything here which is worth to apply to qemu-stable?
> (10.0.x is supposed to be an LTS series).

I'm picking up

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

for the stable series.  Please let me know if I shouldn't.

BTW, Volker, it looks like email from me to your address @t-online
does not work.

Thanks,

/mjt