[RFC PATCH] audio: proper support for float samples in mixeng

[RFC PATCH] audio: proper support for float samples in mixeng

[Kővágó, Zoltán]

This adds proper support for float samples in mixeng by adding a new
audio format for it.

Limitations: only native endianness is supported.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---

This patch is meant to be applied on top of "[PATCH] coreaudio: fix coreaudio
playback" by Volker Rümelin, available at:
https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg00114.html

For more information, please refer to that thread.

---
 qapi/audio.json        |  2 +-
 audio/audio_int.h      |  3 +-
 audio/audio_template.h | 41 ++++++++++++--------
 audio/mixeng.h         |  8 ++--
 audio/alsaaudio.c      | 17 ++++++++
 audio/audio.c          | 56 ++++++++++++++++++---------
 audio/coreaudio.c      |  7 +---
 audio/mixeng.c         | 88 ++++++++++++++++++++++++++----------------
 audio/paaudio.c        |  9 +++++
 audio/sdlaudio.c       | 28 ++++++++++++++
 10 files changed, 180 insertions(+), 79 deletions(-)

diff --git a/qapi/audio.json b/qapi/audio.json
index 83312b2339..d8c507cced 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -276,7 +276,7 @@
 # Since: 4.0
 ##
 { 'enum': 'AudioFormat',
-  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
+  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32', 'f32' ] }
 
 ##
 # @AudiodevDriver:
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 5ba2078346..cd92e48163 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -40,7 +40,8 @@ struct audio_callback {
 
 struct audio_pcm_info {
     int bits;
-    int sign;
+    bool is_signed;
+    bool is_float;
     int freq;
     int nchannels;
     int bytes_per_frame;
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 0336d2670c..7013d3041f 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -153,15 +153,23 @@ static int glue (audio_pcm_sw_init_, TYPE) (
     sw->ratio = ((int64_t) sw->info.freq << 32) / sw->hw->info.freq;
 #endif
 
+    if (sw->info.is_float) {
 #ifdef DAC
-    sw->conv = mixeng_conv
+        sw->conv = mixeng_conv_float[sw->info.nchannels == 2];
 #else
-    sw->clip = mixeng_clip
+        sw->clip = mixeng_clip_float[sw->info.nchannels == 2];
 #endif
-        [sw->info.nchannels == 2]
-        [sw->info.sign]
-        [sw->info.swap_endianness]
-        [audio_bits_to_index (sw->info.bits)];
+    } else {
+#ifdef DAC
+        sw->conv = mixeng_conv
+#else
+        sw->clip = mixeng_clip
+#endif
+            [sw->info.nchannels == 2]
+            [sw->info.is_signed]
+            [sw->info.swap_endianness]
+            [audio_bits_to_index(sw->info.bits)];
+    }
 
     sw->name = g_strdup (name);
     err = glue (audio_pcm_sw_alloc_resources_, TYPE) (sw);
@@ -276,22 +284,23 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
         goto err1;
     }
 
-    if (s->dev->driver == AUDIODEV_DRIVER_COREAUDIO) {
+    if (hw->info.is_float) {
 #ifdef DAC
-        hw->clip = clip_natural_float_from_stereo;
+        hw->clip = mixeng_clip_float[hw->info.nchannels == 2];
 #else
-        hw->conv = conv_natural_float_to_stereo;
+        hw->conv = mixeng_conv_float[hw->info.nchannels == 2];
 #endif
-    } else
+    } else {
 #ifdef DAC
-    hw->clip = mixeng_clip
+        hw->clip = mixeng_clip
 #else
-    hw->conv = mixeng_conv
+        hw->conv = mixeng_conv
 #endif
-        [hw->info.nchannels == 2]
-        [hw->info.sign]
-        [hw->info.swap_endianness]
-        [audio_bits_to_index (hw->info.bits)];
+            [hw->info.nchannels == 2]
+            [hw->info.is_signed]
+            [hw->info.swap_endianness]
+            [audio_bits_to_index(hw->info.bits)];
+    }
 
     glue(audio_pcm_hw_alloc_resources_, TYPE)(hw);
 
diff --git a/audio/mixeng.h b/audio/mixeng.h
index 7ef61763e8..2dcd6df245 100644
--- a/audio/mixeng.h
+++ b/audio/mixeng.h
@@ -38,13 +38,13 @@ typedef struct st_sample st_sample;
 typedef void (t_sample) (struct st_sample *dst, const void *src, int samples);
 typedef void (f_sample) (void *dst, const struct st_sample *src, int samples);
 
+/* indices: [stereo][signed][swap endiannes][8, 16 or 32-bits] */
 extern t_sample *mixeng_conv[2][2][2][3];
 extern f_sample *mixeng_clip[2][2][2][3];
 
-void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
-                                  int samples);
-void clip_natural_float_from_stereo(void *dst, const struct st_sample *src,
-                                    int samples);
+/* indices: [stereo] */
+extern t_sample *mixeng_conv_float[2];
+extern f_sample *mixeng_clip_float[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/alsaaudio.c b/audio/alsaaudio.c
index f37ce1ce85..768b896a93 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -307,6 +307,13 @@ static snd_pcm_format_t aud_to_alsafmt (AudioFormat fmt, int endianness)
             return SND_PCM_FORMAT_U32_LE;
         }
 
+    case AUDIO_FORMAT_F32:
+        if (endianness) {
+            return SND_PCM_FORMAT_FLOAT_BE;
+        } else {
+            return SND_PCM_FORMAT_FLOAT_LE;
+        }
+
     default:
         dolog ("Internal logic error: Bad audio format %d\n", fmt);
 #ifdef DEBUG_AUDIO
@@ -370,6 +377,16 @@ static int alsa_to_audfmt (snd_pcm_format_t alsafmt, AudioFormat *fmt,
         *fmt = AUDIO_FORMAT_U32;
         break;
 
+    case SND_PCM_FORMAT_FLOAT_LE:
+        *endianness = 0;
+        *fmt = AUDIO_FORMAT_F32;
+        break;
+
+    case SND_PCM_FORMAT_FLOAT_BE:
+        *endianness = 1;
+        *fmt = AUDIO_FORMAT_F32;
+        break;
+
     default:
         dolog ("Unrecognized audio format %d\n", alsafmt);
         return -1;
diff --git a/audio/audio.c b/audio/audio.c
index f63f39769a..53fdb42ec7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -218,6 +218,9 @@ static void audio_print_settings (struct audsettings *as)
     case AUDIO_FORMAT_U32:
         AUD_log (NULL, "U32");
         break;
+    case AUDIO_FORMAT_F32:
+        AUD_log (NULL, "F32");
+        break;
     default:
         AUD_log (NULL, "invalid(%d)", as->fmt);
         break;
@@ -252,6 +255,7 @@ static int audio_validate_settings (struct audsettings *as)
     case AUDIO_FORMAT_U16:
     case AUDIO_FORMAT_S32:
     case AUDIO_FORMAT_U32:
+    case AUDIO_FORMAT_F32:
         break;
     default:
         invalid = 1;
@@ -264,24 +268,28 @@ static int audio_validate_settings (struct audsettings *as)
 
 static int audio_pcm_info_eq (struct audio_pcm_info *info, struct audsettings *as)
 {
-    int bits = 8, sign = 0;
+    int bits = 8;
+    bool is_signed = false, is_float = false;
 
     switch (as->fmt) {
     case AUDIO_FORMAT_S8:
-        sign = 1;
+        is_signed = true;
         /* fall through */
     case AUDIO_FORMAT_U8:
         break;
 
     case AUDIO_FORMAT_S16:
-        sign = 1;
+        is_signed = true;
         /* fall through */
     case AUDIO_FORMAT_U16:
         bits = 16;
         break;
 
+    case AUDIO_FORMAT_F32:
+        is_float = true;
+        /* fall through */
     case AUDIO_FORMAT_S32:
-        sign = 1;
+        is_signed = true;
         /* fall through */
     case AUDIO_FORMAT_U32:
         bits = 32;
@@ -292,33 +300,38 @@ static int audio_pcm_info_eq (struct audio_pcm_info *info, struct audsettings *a
     }
     return info->freq == as->freq
         && info->nchannels == as->nchannels
-        && info->sign == sign
+        && info->is_signed == is_signed
+        && info->is_float == is_float
         && info->bits == bits
         && info->swap_endianness == (as->endianness != AUDIO_HOST_ENDIANNESS);
 }
 
 void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as)
 {
-    int bits = 8, sign = 0, mul;
+    int bits = 8, mul;
+    bool is_signed = false, is_float = false;
 
     switch (as->fmt) {
     case AUDIO_FORMAT_S8:
-        sign = 1;
+        is_signed = true;
         /* fall through */
     case AUDIO_FORMAT_U8:
         mul = 1;
         break;
 
     case AUDIO_FORMAT_S16:
-        sign = 1;
+        is_signed = true;
         /* fall through */
     case AUDIO_FORMAT_U16:
         bits = 16;
         mul = 2;
         break;
 
+    case AUDIO_FORMAT_F32:
+        is_float = true;
+        /* fall through */
     case AUDIO_FORMAT_S32:
-        sign = 1;
+        is_signed = true;
         /* fall through */
     case AUDIO_FORMAT_U32:
         bits = 32;
@@ -331,7 +344,8 @@ void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as)
 
     info->freq = as->freq;
     info->bits = bits;
-    info->sign = sign;
+    info->is_signed = is_signed;
+    info->is_float = is_float;
     info->nchannels = as->nchannels;
     info->bytes_per_frame = as->nchannels * mul;
     info->bytes_per_second = info->freq * info->bytes_per_frame;
@@ -344,7 +358,7 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len)
         return;
     }
 
-    if (info->sign) {
+    if (info->is_signed || info->is_float) {
         memset(buf, 0x00, len * info->bytes_per_frame);
     }
     else {
@@ -770,8 +784,9 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size)
 #ifdef DEBUG_AUDIO
 static void audio_pcm_print_info (const char *cap, struct audio_pcm_info *info)
 {
-    dolog ("%s: bits %d, sign %d, freq %d, nchan %d\n",
-           cap, info->bits, info->sign, info->freq, info->nchannels);
+    dolog("%s: bits %d, sign %d, float %d, freq %d, nchan %d\n",
+          cap, info->bits, info->is_signed, info->is_float, info->freq,
+          info->nchannels);
 }
 #endif
 
@@ -1837,11 +1852,15 @@ CaptureVoiceOut *AUD_add_capture(
 
         cap->buf = g_malloc0_n(hw->mix_buf->size, hw->info.bytes_per_frame);
 
-        hw->clip = mixeng_clip
-            [hw->info.nchannels == 2]
-            [hw->info.sign]
-            [hw->info.swap_endianness]
-            [audio_bits_to_index (hw->info.bits)];
+        if (hw->info.is_float) {
+            hw->clip = mixeng_clip_float[hw->info.nchannels == 2];
+        } else {
+            hw->clip = mixeng_clip
+                [hw->info.nchannels == 2]
+                [hw->info.is_signed]
+                [hw->info.swap_endianness]
+                [audio_bits_to_index(hw->info.bits)];
+        }
 
         QLIST_INSERT_HEAD (&s->cap_head, cap, entries);
         QLIST_INSERT_HEAD (&cap->cb_head, cb, entries);
@@ -2080,6 +2099,7 @@ int audioformat_bytes_per_sample(AudioFormat fmt)
 
     case AUDIO_FORMAT_U32:
     case AUDIO_FORMAT_S32:
+    case AUDIO_FORMAT_F32:
         return 4;
 
     case AUDIO_FORMAT__MAX:
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 0049db97fa..f1a009610c 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -491,14 +491,9 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
         return -1;
     }
 
-    /*
-     * The canonical audio format for CoreAudio on macOS is float. Currently
-     * there is no generic code for AUDIO_FORMAT_F32 in qemu. Here we select
-     * AUDIO_FORMAT_S32 instead because only the sample size has to match.
-     */
     fake_as = *as;
     as = &fake_as;
-    as->fmt = AUDIO_FORMAT_S32;
+    as->fmt = AUDIO_FORMAT_F32;
     audio_pcm_init_info (&hw->info, as);
 
     status = coreaudio_get_voice(&core->outputDeviceID);
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 16b646d48c..c14b0d874c 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -267,55 +267,77 @@ f_sample *mixeng_clip[2][2][2][3] = {
     }
 };
 
-void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
-                                  int samples)
+#ifdef FLOAT_MIXENG
+#define FLOAT_CONV_TO(x) (x)
+#define FLOAT_CONV_FROM(x) (x)
+#else
+static const float float_scale = UINT_MAX;
+#define FLOAT_CONV_TO(x) ((x) * float_scale)
+
+#ifdef RECIPROCAL
+static const float float_scale_reciprocal = 1.f / UINT_MAX;
+#define FLOAT_CONV_FROM(x) ((x) * float_scale_reciprocal)
+#else
+#define FLOAT_CONV_FROM(x) ((x) / float_scale)
+#endif
+#endif
+
+static void conv_natural_float_to_mono(struct st_sample *dst, const void *src,
+                                       int samples)
 {
     float *in = (float *)src;
-#ifndef FLOAT_MIXENG
-    const float scale = UINT_MAX;
-#endif
 
     while (samples--) {
-#ifdef FLOAT_MIXENG
-        dst->l = *in++;
-        dst->r = *in++;
-#else
-        dst->l = *in++ * scale;
-        dst->r = *in++ * scale;
-#endif
+        dst->r = dst->l = FLOAT_CONV_TO(*in++);
+        dst++;
+    }
+}
+
+static void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
+                                         int samples)
+{
+    float *in = (float *)src;
+
+    while (samples--) {
+        dst->l = FLOAT_CONV_TO(*in++);
+        dst->r = FLOAT_CONV_TO(*in++);
         dst++;
     }
 }
 
-void clip_natural_float_from_stereo(void *dst, const struct st_sample *src,
-                                    int samples)
+t_sample *mixeng_conv_float[2] = {
+    conv_natural_float_to_mono,
+    conv_natural_float_to_stereo,
+};
+
+static void clip_natural_float_from_mono(void *dst, const struct st_sample *src,
+                                         int samples)
 {
     float *out = (float *)dst;
-#ifndef FLOAT_MIXENG
-#ifdef RECIPROCAL
-    const float scale = 1.f / UINT_MAX;
-#else
-    const float scale = UINT_MAX;
-#endif
-#endif
 
     while (samples--) {
-#ifdef FLOAT_MIXENG
-        *out++ = src->l;
-        *out++ = src->r;
-#else
-#ifdef RECIPROCAL
-        *out++ = src->l * scale;
-        *out++ = src->r * scale;
-#else
-        *out++ = src->l / scale;
-        *out++ = src->r / scale;
-#endif
-#endif
+        *out++ = FLOAT_CONV_FROM(src->l) + FLOAT_CONV_FROM(src->r);
+        src++;
+    }
+}
+
+static void clip_natural_float_from_stereo(
+    void *dst, const struct st_sample *src, int samples)
+{
+    float *out = (float *)dst;
+
+    while (samples--) {
+        *out++ = FLOAT_CONV_FROM(src->l);
+        *out++ = FLOAT_CONV_FROM(src->r);
         src++;
     }
 }
 
+f_sample *mixeng_clip_float[2] = {
+    clip_natural_float_from_mono,
+    clip_natural_float_from_stereo,
+};
+
 void audio_sample_to_uint64(void *samples, int pos,
                             uint64_t *left, uint64_t *right)
 {
diff --git a/audio/paaudio.c b/audio/paaudio.c
index dbfe48c03a..1278c5a775 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -279,6 +279,9 @@ static pa_sample_format_t audfmt_to_pa (AudioFormat afmt, int endianness)
     case AUDIO_FORMAT_U32:
         format = endianness ? PA_SAMPLE_S32BE : PA_SAMPLE_S32LE;
         break;
+    case AUDIO_FORMAT_F32:
+        format = endianness ? PA_SAMPLE_FLOAT32BE : PA_SAMPLE_FLOAT32LE;
+        break;
     default:
         dolog ("Internal logic error: Bad audio format %d\n", afmt);
         format = PA_SAMPLE_U8;
@@ -304,6 +307,12 @@ static AudioFormat pa_to_audfmt (pa_sample_format_t fmt, int *endianness)
     case PA_SAMPLE_S32LE:
         *endianness = 0;
         return AUDIO_FORMAT_S32;
+    case PA_SAMPLE_FLOAT32BE:
+        *endianness = 1;
+        return AUDIO_FORMAT_F32;
+    case PA_SAMPLE_FLOAT32LE:
+        *endianness = 0;
+        return AUDIO_FORMAT_F32;
     default:
         dolog ("Internal logic error: Bad pa_sample_format %d\n", fmt);
         return AUDIO_FORMAT_U8;
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 5c6bcfcb3e..6af1911db9 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -77,6 +77,14 @@ static int aud_to_sdlfmt (AudioFormat fmt)
     case AUDIO_FORMAT_U16:
         return AUDIO_U16LSB;
 
+    case AUDIO_FORMAT_S32:
+        return AUDIO_S32LSB;
+
+    /* no unsigned 32-bit support in SDL */
+
+    case AUDIO_FORMAT_F32:
+        return AUDIO_F32LSB;
+
     default:
         dolog ("Internal logic error: Bad audio format %d\n", fmt);
 #ifdef DEBUG_AUDIO
@@ -119,6 +127,26 @@ static int sdl_to_audfmt(int sdlfmt, AudioFormat *fmt, int *endianness)
         *fmt = AUDIO_FORMAT_U16;
         break;
 
+    case AUDIO_S32LSB:
+        *endianness = 0;
+        *fmt = AUDIO_FORMAT_S32;
+        break;
+
+    case AUDIO_S32MSB:
+        *endianness = 1;
+        *fmt = AUDIO_FORMAT_S32;
+        break;
+
+    case AUDIO_F32LSB:
+        *endianness = 0;
+        *fmt = AUDIO_FORMAT_F32;
+        break;
+
+    case AUDIO_F32MSB:
+        *endianness = 1;
+        *fmt = AUDIO_FORMAT_F32;
+        break;
+
     default:
         dolog ("Unrecognized SDL audio format %d\n", sdlfmt);
         return -1;
-- 
2.25.0

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Howard Spoelstra]

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

On Sun, Feb 2, 2020 at 8:38 PM Kővágó, Zoltán <dirty.ice.hu@gmail.com>
wrote:

> This adds proper support for float samples in mixeng by adding a new
> audio format for it.
>
> Limitations: only native endianness is supported.
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>
> This patch is meant to be applied on top of "[PATCH] coreaudio: fix
> coreaudio
> playback" by Volker Rümelin, available at:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg00114.html
>
> For more information, please refer to that thread.
>
> ---
>  qapi/audio.json        |  2 +-
>  audio/audio_int.h      |  3 +-
>  audio/audio_template.h | 41 ++++++++++++--------
>  audio/mixeng.h         |  8 ++--
>  audio/alsaaudio.c      | 17 ++++++++
>  audio/audio.c          | 56 ++++++++++++++++++---------
>  audio/coreaudio.c      |  7 +---
>  audio/mixeng.c         | 88 ++++++++++++++++++++++++++----------------
>  audio/paaudio.c        |  9 +++++
>  audio/sdlaudio.c       | 28 ++++++++++++++
>  10 files changed, 180 insertions(+), 79 deletions(-)
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 83312b2339..d8c507cced 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -276,7 +276,7 @@
>  # Since: 4.0
>  ##
>  { 'enum': 'AudioFormat',
> -  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32', 'f32' ] }
>
>  ##
>  # @AudiodevDriver:
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 5ba2078346..cd92e48163 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -40,7 +40,8 @@ struct audio_callback {
>
>  struct audio_pcm_info {
>      int bits;
> -    int sign;
> +    bool is_signed;
> +    bool is_float;
>      int freq;
>      int nchannels;
>      int bytes_per_frame;
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 0336d2670c..7013d3041f 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -153,15 +153,23 @@ static int glue (audio_pcm_sw_init_, TYPE) (
>      sw->ratio = ((int64_t) sw->info.freq << 32) / sw->hw->info.freq;
>  #endif
>
> +    if (sw->info.is_float) {
>  #ifdef DAC
> -    sw->conv = mixeng_conv
> +        sw->conv = mixeng_conv_float[sw->info.nchannels == 2];
>  #else
> -    sw->clip = mixeng_clip
> +        sw->clip = mixeng_clip_float[sw->info.nchannels == 2];
>  #endif
> -        [sw->info.nchannels == 2]
> -        [sw->info.sign]
> -        [sw->info.swap_endianness]
> -        [audio_bits_to_index (sw->info.bits)];
> +    } else {
> +#ifdef DAC
> +        sw->conv = mixeng_conv
> +#else
> +        sw->clip = mixeng_clip
> +#endif
> +            [sw->info.nchannels == 2]
> +            [sw->info.is_signed]
> +            [sw->info.swap_endianness]
> +            [audio_bits_to_index(sw->info.bits)];
> +    }
>
>      sw->name = g_strdup (name);
>      err = glue (audio_pcm_sw_alloc_resources_, TYPE) (sw);
> @@ -276,22 +284,23 @@ static HW *glue(audio_pcm_hw_add_new_,
> TYPE)(AudioState *s,
>          goto err1;
>      }
>
> -    if (s->dev->driver == AUDIODEV_DRIVER_COREAUDIO) {
> +    if (hw->info.is_float) {
>  #ifdef DAC
> -        hw->clip = clip_natural_float_from_stereo;
> +        hw->clip = mixeng_clip_float[hw->info.nchannels == 2];
>  #else
> -        hw->conv = conv_natural_float_to_stereo;
> +        hw->conv = mixeng_conv_float[hw->info.nchannels == 2];
>  #endif
> -    } else
> +    } else {
>  #ifdef DAC
> -    hw->clip = mixeng_clip
> +        hw->clip = mixeng_clip
>  #else
> -    hw->conv = mixeng_conv
> +        hw->conv = mixeng_conv
>  #endif
> -        [hw->info.nchannels == 2]
> -        [hw->info.sign]
> -        [hw->info.swap_endianness]
> -        [audio_bits_to_index (hw->info.bits)];
> +            [hw->info.nchannels == 2]
> +            [hw->info.is_signed]
> +            [hw->info.swap_endianness]
> +            [audio_bits_to_index(hw->info.bits)];
> +    }
>
>      glue(audio_pcm_hw_alloc_resources_, TYPE)(hw);
>
> diff --git a/audio/mixeng.h b/audio/mixeng.h
> index 7ef61763e8..2dcd6df245 100644
> --- a/audio/mixeng.h
> +++ b/audio/mixeng.h
> @@ -38,13 +38,13 @@ typedef struct st_sample st_sample;
>  typedef void (t_sample) (struct st_sample *dst, const void *src, int
> samples);
>  typedef void (f_sample) (void *dst, const struct st_sample *src, int
> samples);
>
> +/* indices: [stereo][signed][swap endiannes][8, 16 or 32-bits] */
>  extern t_sample *mixeng_conv[2][2][2][3];
>  extern f_sample *mixeng_clip[2][2][2][3];
>
> -void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
> -                                  int samples);
> -void clip_natural_float_from_stereo(void *dst, const struct st_sample
> *src,
> -                                    int samples);
> +/* indices: [stereo] */
> +extern t_sample *mixeng_conv_float[2];
> +extern f_sample *mixeng_clip_float[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/alsaaudio.c b/audio/alsaaudio.c
> index f37ce1ce85..768b896a93 100644
> --- a/audio/alsaaudio.c
> +++ b/audio/alsaaudio.c
> @@ -307,6 +307,13 @@ static snd_pcm_format_t aud_to_alsafmt (AudioFormat
> fmt, int endianness)
>              return SND_PCM_FORMAT_U32_LE;
>          }
>
> +    case AUDIO_FORMAT_F32:
> +        if (endianness) {
> +            return SND_PCM_FORMAT_FLOAT_BE;
> +        } else {
> +            return SND_PCM_FORMAT_FLOAT_LE;
> +        }
> +
>      default:
>          dolog ("Internal logic error: Bad audio format %d\n", fmt);
>  #ifdef DEBUG_AUDIO
> @@ -370,6 +377,16 @@ static int alsa_to_audfmt (snd_pcm_format_t alsafmt,
> AudioFormat *fmt,
>          *fmt = AUDIO_FORMAT_U32;
>          break;
>
> +    case SND_PCM_FORMAT_FLOAT_LE:
> +        *endianness = 0;
> +        *fmt = AUDIO_FORMAT_F32;
> +        break;
> +
> +    case SND_PCM_FORMAT_FLOAT_BE:
> +        *endianness = 1;
> +        *fmt = AUDIO_FORMAT_F32;
> +        break;
> +
>      default:
>          dolog ("Unrecognized audio format %d\n", alsafmt);
>          return -1;
> diff --git a/audio/audio.c b/audio/audio.c
> index f63f39769a..53fdb42ec7 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -218,6 +218,9 @@ static void audio_print_settings (struct audsettings
> *as)
>      case AUDIO_FORMAT_U32:
>          AUD_log (NULL, "U32");
>          break;
> +    case AUDIO_FORMAT_F32:
> +        AUD_log (NULL, "F32");
> +        break;
>      default:
>          AUD_log (NULL, "invalid(%d)", as->fmt);
>          break;
> @@ -252,6 +255,7 @@ static int audio_validate_settings (struct audsettings
> *as)
>      case AUDIO_FORMAT_U16:
>      case AUDIO_FORMAT_S32:
>      case AUDIO_FORMAT_U32:
> +    case AUDIO_FORMAT_F32:
>          break;
>      default:
>          invalid = 1;
> @@ -264,24 +268,28 @@ static int audio_validate_settings (struct
> audsettings *as)
>
>  static int audio_pcm_info_eq (struct audio_pcm_info *info, struct
> audsettings *as)
>  {
> -    int bits = 8, sign = 0;
> +    int bits = 8;
> +    bool is_signed = false, is_float = false;
>
>      switch (as->fmt) {
>      case AUDIO_FORMAT_S8:
> -        sign = 1;
> +        is_signed = true;
>          /* fall through */
>      case AUDIO_FORMAT_U8:
>          break;
>
>      case AUDIO_FORMAT_S16:
> -        sign = 1;
> +        is_signed = true;
>          /* fall through */
>      case AUDIO_FORMAT_U16:
>          bits = 16;
>          break;
>
> +    case AUDIO_FORMAT_F32:
> +        is_float = true;
> +        /* fall through */
>      case AUDIO_FORMAT_S32:
> -        sign = 1;
> +        is_signed = true;
>          /* fall through */
>      case AUDIO_FORMAT_U32:
>          bits = 32;
> @@ -292,33 +300,38 @@ static int audio_pcm_info_eq (struct audio_pcm_info
> *info, struct audsettings *a
>      }
>      return info->freq == as->freq
>          && info->nchannels == as->nchannels
> -        && info->sign == sign
> +        && info->is_signed == is_signed
> +        && info->is_float == is_float
>          && info->bits == bits
>          && info->swap_endianness == (as->endianness !=
> AUDIO_HOST_ENDIANNESS);
>  }
>
>  void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings
> *as)
>  {
> -    int bits = 8, sign = 0, mul;
> +    int bits = 8, mul;
> +    bool is_signed = false, is_float = false;
>
>      switch (as->fmt) {
>      case AUDIO_FORMAT_S8:
> -        sign = 1;
> +        is_signed = true;
>          /* fall through */
>      case AUDIO_FORMAT_U8:
>          mul = 1;
>          break;
>
>      case AUDIO_FORMAT_S16:
> -        sign = 1;
> +        is_signed = true;
>          /* fall through */
>      case AUDIO_FORMAT_U16:
>          bits = 16;
>          mul = 2;
>          break;
>
> +    case AUDIO_FORMAT_F32:
> +        is_float = true;
> +        /* fall through */
>      case AUDIO_FORMAT_S32:
> -        sign = 1;
> +        is_signed = true;
>          /* fall through */
>      case AUDIO_FORMAT_U32:
>          bits = 32;
> @@ -331,7 +344,8 @@ void audio_pcm_init_info (struct audio_pcm_info *info,
> struct audsettings *as)
>
>      info->freq = as->freq;
>      info->bits = bits;
> -    info->sign = sign;
> +    info->is_signed = is_signed;
> +    info->is_float = is_float;
>      info->nchannels = as->nchannels;
>      info->bytes_per_frame = as->nchannels * mul;
>      info->bytes_per_second = info->freq * info->bytes_per_frame;
> @@ -344,7 +358,7 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info
> *info, void *buf, int len)
>          return;
>      }
>
> -    if (info->sign) {
> +    if (info->is_signed || info->is_float) {
>          memset(buf, 0x00, len * info->bytes_per_frame);
>      }
>      else {
> @@ -770,8 +784,9 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void
> *buf, size_t size)
>  #ifdef DEBUG_AUDIO
>  static void audio_pcm_print_info (const char *cap, struct audio_pcm_info
> *info)
>  {
> -    dolog ("%s: bits %d, sign %d, freq %d, nchan %d\n",
> -           cap, info->bits, info->sign, info->freq, info->nchannels);
> +    dolog("%s: bits %d, sign %d, float %d, freq %d, nchan %d\n",
> +          cap, info->bits, info->is_signed, info->is_float, info->freq,
> +          info->nchannels);
>  }
>  #endif
>
> @@ -1837,11 +1852,15 @@ CaptureVoiceOut *AUD_add_capture(
>
>          cap->buf = g_malloc0_n(hw->mix_buf->size,
> hw->info.bytes_per_frame);
>
> -        hw->clip = mixeng_clip
> -            [hw->info.nchannels == 2]
> -            [hw->info.sign]
> -            [hw->info.swap_endianness]
> -            [audio_bits_to_index (hw->info.bits)];
> +        if (hw->info.is_float) {
> +            hw->clip = mixeng_clip_float[hw->info.nchannels == 2];
> +        } else {
> +            hw->clip = mixeng_clip
> +                [hw->info.nchannels == 2]
> +                [hw->info.is_signed]
> +                [hw->info.swap_endianness]
> +                [audio_bits_to_index(hw->info.bits)];
> +        }
>
>          QLIST_INSERT_HEAD (&s->cap_head, cap, entries);
>          QLIST_INSERT_HEAD (&cap->cb_head, cb, entries);
> @@ -2080,6 +2099,7 @@ int audioformat_bytes_per_sample(AudioFormat fmt)
>
>      case AUDIO_FORMAT_U32:
>      case AUDIO_FORMAT_S32:
> +    case AUDIO_FORMAT_F32:
>          return 4;
>
>      case AUDIO_FORMAT__MAX:
> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> index 0049db97fa..f1a009610c 100644
> --- a/audio/coreaudio.c
> +++ b/audio/coreaudio.c
> @@ -491,14 +491,9 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct
> audsettings *as,
>          return -1;
>      }
>
> -    /*
> -     * The canonical audio format for CoreAudio on macOS is float.
> Currently
> -     * there is no generic code for AUDIO_FORMAT_F32 in qemu. Here we
> select
> -     * AUDIO_FORMAT_S32 instead because only the sample size has to match.
> -     */
>      fake_as = *as;
>      as = &fake_as;
> -    as->fmt = AUDIO_FORMAT_S32;
> +    as->fmt = AUDIO_FORMAT_F32;
>      audio_pcm_init_info (&hw->info, as);
>
>      status = coreaudio_get_voice(&core->outputDeviceID);
> diff --git a/audio/mixeng.c b/audio/mixeng.c
> index 16b646d48c..c14b0d874c 100644
> --- a/audio/mixeng.c
> +++ b/audio/mixeng.c
> @@ -267,55 +267,77 @@ f_sample *mixeng_clip[2][2][2][3] = {
>      }
>  };
>
> -void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
> -                                  int samples)
> +#ifdef FLOAT_MIXENG
> +#define FLOAT_CONV_TO(x) (x)
> +#define FLOAT_CONV_FROM(x) (x)
> +#else
> +static const float float_scale = UINT_MAX;
> +#define FLOAT_CONV_TO(x) ((x) * float_scale)
> +
> +#ifdef RECIPROCAL
> +static const float float_scale_reciprocal = 1.f / UINT_MAX;
> +#define FLOAT_CONV_FROM(x) ((x) * float_scale_reciprocal)
> +#else
> +#define FLOAT_CONV_FROM(x) ((x) / float_scale)
> +#endif
> +#endif
> +
> +static void conv_natural_float_to_mono(struct st_sample *dst, const void
> *src,
> +                                       int samples)
>  {
>      float *in = (float *)src;
> -#ifndef FLOAT_MIXENG
> -    const float scale = UINT_MAX;
> -#endif
>
>      while (samples--) {
> -#ifdef FLOAT_MIXENG
> -        dst->l = *in++;
> -        dst->r = *in++;
> -#else
> -        dst->l = *in++ * scale;
> -        dst->r = *in++ * scale;
> -#endif
> +        dst->r = dst->l = FLOAT_CONV_TO(*in++);
> +        dst++;
> +    }
> +}
> +
> +static void conv_natural_float_to_stereo(struct st_sample *dst, const
> void *src,
> +                                         int samples)
> +{
> +    float *in = (float *)src;
> +
> +    while (samples--) {
> +        dst->l = FLOAT_CONV_TO(*in++);
> +        dst->r = FLOAT_CONV_TO(*in++);
>          dst++;
>      }
>  }
>
> -void clip_natural_float_from_stereo(void *dst, const struct st_sample
> *src,
> -                                    int samples)
> +t_sample *mixeng_conv_float[2] = {
> +    conv_natural_float_to_mono,
> +    conv_natural_float_to_stereo,
> +};
> +
> +static void clip_natural_float_from_mono(void *dst, const struct
> st_sample *src,
> +                                         int samples)
>  {
>      float *out = (float *)dst;
> -#ifndef FLOAT_MIXENG
> -#ifdef RECIPROCAL
> -    const float scale = 1.f / UINT_MAX;
> -#else
> -    const float scale = UINT_MAX;
> -#endif
> -#endif
>
>      while (samples--) {
> -#ifdef FLOAT_MIXENG
> -        *out++ = src->l;
> -        *out++ = src->r;
> -#else
> -#ifdef RECIPROCAL
> -        *out++ = src->l * scale;
> -        *out++ = src->r * scale;
> -#else
> -        *out++ = src->l / scale;
> -        *out++ = src->r / scale;
> -#endif
> -#endif
> +        *out++ = FLOAT_CONV_FROM(src->l) + FLOAT_CONV_FROM(src->r);
> +        src++;
> +    }
> +}
> +
> +static void clip_natural_float_from_stereo(
> +    void *dst, const struct st_sample *src, int samples)
> +{
> +    float *out = (float *)dst;
> +
> +    while (samples--) {
> +        *out++ = FLOAT_CONV_FROM(src->l);
> +        *out++ = FLOAT_CONV_FROM(src->r);
>          src++;
>      }
>  }
>
> +f_sample *mixeng_clip_float[2] = {
> +    clip_natural_float_from_mono,
> +    clip_natural_float_from_stereo,
> +};
> +
>  void audio_sample_to_uint64(void *samples, int pos,
>                              uint64_t *left, uint64_t *right)
>  {
> diff --git a/audio/paaudio.c b/audio/paaudio.c
> index dbfe48c03a..1278c5a775 100644
> --- a/audio/paaudio.c
> +++ b/audio/paaudio.c
> @@ -279,6 +279,9 @@ static pa_sample_format_t audfmt_to_pa (AudioFormat
> afmt, int endianness)
>      case AUDIO_FORMAT_U32:
>          format = endianness ? PA_SAMPLE_S32BE : PA_SAMPLE_S32LE;
>          break;
> +    case AUDIO_FORMAT_F32:
> +        format = endianness ? PA_SAMPLE_FLOAT32BE : PA_SAMPLE_FLOAT32LE;
> +        break;
>      default:
>          dolog ("Internal logic error: Bad audio format %d\n", afmt);
>          format = PA_SAMPLE_U8;
> @@ -304,6 +307,12 @@ static AudioFormat pa_to_audfmt (pa_sample_format_t
> fmt, int *endianness)
>      case PA_SAMPLE_S32LE:
>          *endianness = 0;
>          return AUDIO_FORMAT_S32;
> +    case PA_SAMPLE_FLOAT32BE:
> +        *endianness = 1;
> +        return AUDIO_FORMAT_F32;
> +    case PA_SAMPLE_FLOAT32LE:
> +        *endianness = 0;
> +        return AUDIO_FORMAT_F32;
>      default:
>          dolog ("Internal logic error: Bad pa_sample_format %d\n", fmt);
>          return AUDIO_FORMAT_U8;
> diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
> index 5c6bcfcb3e..6af1911db9 100644
> --- a/audio/sdlaudio.c
> +++ b/audio/sdlaudio.c
> @@ -77,6 +77,14 @@ static int aud_to_sdlfmt (AudioFormat fmt)
>      case AUDIO_FORMAT_U16:
>          return AUDIO_U16LSB;
>
> +    case AUDIO_FORMAT_S32:
> +        return AUDIO_S32LSB;
> +
> +    /* no unsigned 32-bit support in SDL */
> +
> +    case AUDIO_FORMAT_F32:
> +        return AUDIO_F32LSB;
> +
>      default:
>          dolog ("Internal logic error: Bad audio format %d\n", fmt);
>  #ifdef DEBUG_AUDIO
> @@ -119,6 +127,26 @@ static int sdl_to_audfmt(int sdlfmt, AudioFormat
> *fmt, int *endianness)
>          *fmt = AUDIO_FORMAT_U16;
>          break;
>
> +    case AUDIO_S32LSB:
> +        *endianness = 0;
> +        *fmt = AUDIO_FORMAT_S32;
> +        break;
> +
> +    case AUDIO_S32MSB:
> +        *endianness = 1;
> +        *fmt = AUDIO_FORMAT_S32;
> +        break;
> +
> +    case AUDIO_F32LSB:
> +        *endianness = 0;
> +        *fmt = AUDIO_FORMAT_F32;
> +        break;
> +
> +    case AUDIO_F32MSB:
> +        *endianness = 1;
> +        *fmt = AUDIO_FORMAT_F32;
> +        break;
> +
>      default:
>          dolog ("Unrecognized SDL audio format %d\n", sdlfmt);
>          return -1;
> --
> 2.25.0
>
>
> Hi,

I applied the 2 patches to https://github.com/mcayland/qemu/tree/screamer
to test audio support in qemu-system-ppc running Mac OS 9.2 and OSX 10.5.
Host is OSX Sierra. Coreaudio seems happy with them.

Best,
Howard

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

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Markus Armbruster]

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> This adds proper support for float samples in mixeng by adding a new
> audio format for it.
>
> Limitations: only native endianness is supported.
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>
> This patch is meant to be applied on top of "[PATCH] coreaudio: fix coreaudio
> playback" by Volker Rümelin, available at:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg00114.html
>
> For more information, please refer to that thread.
>
> ---
>  qapi/audio.json        |  2 +-
>  audio/audio_int.h      |  3 +-
>  audio/audio_template.h | 41 ++++++++++++--------
>  audio/mixeng.h         |  8 ++--
>  audio/alsaaudio.c      | 17 ++++++++
>  audio/audio.c          | 56 ++++++++++++++++++---------
>  audio/coreaudio.c      |  7 +---
>  audio/mixeng.c         | 88 ++++++++++++++++++++++++++----------------
>  audio/paaudio.c        |  9 +++++
>  audio/sdlaudio.c       | 28 ++++++++++++++
>  10 files changed, 180 insertions(+), 79 deletions(-)
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 83312b2339..d8c507cced 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -276,7 +276,7 @@
>  # Since: 4.0
>  ##
>  { 'enum': 'AudioFormat',
> -  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32', 'f32' ] }
>  
>  ##
>  # @AudiodevDriver:

For QAPI:
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Volker Rümelin]

> This adds proper support for float samples in mixeng by adding a new
> audio format for it.
>
> Limitations: only native endianness is supported.
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>
> This patch is meant to be applied on top of "[PATCH] coreaudio: fix coreaudio
> playback" by Volker Rümelin, available at:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg00114.html
>
> For more information, please refer to that thread.
>

Hi Zoltán,

I like this patch. This is something I had in mind for 5.1. I will test and review the patch,  but I need a few days.

With best regards,
Volker

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Peter Maydell]

On Sun, 2 Feb 2020 at 19:39, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
>
> This adds proper support for float samples in mixeng by adding a new
> audio format for it.
>
> Limitations: only native endianness is supported.

Could you explain a bit more what this limitation means, please?
In general QEMU behaviour shouldn't depend on the endianness
of the host, ie we should byteswap where necessary.

thanks
-- PMM

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Eric Blake]

On 2/3/20 12:21 AM, Markus Armbruster wrote:
> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:
> 
>> This adds proper support for float samples in mixeng by adding a new
>> audio format for it.
>>
>> Limitations: only native endianness is supported.
>>
>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>> ---
>>
>> This patch is meant to be applied on top of "[PATCH] coreaudio: fix coreaudio
>> playback" by Volker Rümelin, available at:
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg00114.html
>>
>> For more information, please refer to that thread.
>>
>> ---

>> +++ b/qapi/audio.json
>> @@ -276,7 +276,7 @@
>>   # Since: 4.0
>>   ##
>>   { 'enum': 'AudioFormat',
>> -  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
>> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32', 'f32' ] }
>>   
>>   ##
>>   # @AudiodevDriver:
> 
> For QAPI:
> Acked-by: Markus Armbruster <armbru@redhat.com>

Is it worth a comment update mentioning that 'f32' is '(since 5.0)'?


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Zoltán Kővágó]

On 2020-02-03 11:00, Peter Maydell wrote:
> On Sun, 2 Feb 2020 at 19:39, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
>>
>> This adds proper support for float samples in mixeng by adding a new
>> audio format for it.
>>
>> Limitations: only native endianness is supported.
> 
> Could you explain a bit more what this limitation means, please?
> In general QEMU behaviour shouldn't depend on the endianness
> of the host, ie we should byteswap where necessary.

None of the virtual sound cards support float samples (it looks like 
most of them only support 8 and 16 bit, only hda supports 32 bit), it is 
only used for the audio backends (i.e. host side).  In 
audiodev_to_audsettings we set endianness to AUDIO_HOST_ENDIANNESS, so 
audio backends should always use native endian.

So this limitation should only cause problems when an audio backend 
overrides the endian setting.  Wavcapture does it, but it does not 
support float.  Alsa, sdl, puleaudio and oss can also do it if for some 
weird reason it acquires a stream with a different endianness than 
requested.

Regards,
Zoltan

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 2/3/20 12:21 AM, Markus Armbruster wrote:
>> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:
>>
>>> This adds proper support for float samples in mixeng by adding a new
>>> audio format for it.
>>>
>>> Limitations: only native endianness is supported.
>>>
>>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>>> ---
>>>
>>> This patch is meant to be applied on top of "[PATCH] coreaudio: fix coreaudio
>>> playback" by Volker Rümelin, available at:
>>> https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg00114.html
>>>
>>> For more information, please refer to that thread.
>>>
>>> ---
>
>>> +++ b/qapi/audio.json
>>> @@ -276,7 +276,7 @@
>>>   # Since: 4.0
>>>   ##
>>>   { 'enum': 'AudioFormat',
>>> -  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
>>> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32', 'f32' ] }
>>>     ##
>>>   # @AudiodevDriver:
>>
>> For QAPI:
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>
> Is it worth a comment update mentioning that 'f32' is '(since 5.0)'?

Good point; we routinely do that.

Should look like this:

##
# @AudioFormat:
#
# An enumeration of possible audio formats.
#
# @u8: lorem
# @s8: ipsum
# @u16: dolor
# @s16: sit
# @u32: amet
# @s32: consectetur
# @f32: adipisici (since 5.0)
#
# Since: 4.0
##

The generator does not enforce documentation of enum values.

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Peter Maydell]

On Mon, 3 Feb 2020 at 20:38, Zoltán Kővágó <dirty.ice.hu@gmail.com> wrote:
>
> On 2020-02-03 11:00, Peter Maydell wrote:
> > On Sun, 2 Feb 2020 at 19:39, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
> >>
> >> This adds proper support for float samples in mixeng by adding a new
> >> audio format for it.
> >>
> >> Limitations: only native endianness is supported.
> >
> > Could you explain a bit more what this limitation means, please?
> > In general QEMU behaviour shouldn't depend on the endianness
> > of the host, ie we should byteswap where necessary.
>
> None of the virtual sound cards support float samples (it looks like
> most of them only support 8 and 16 bit, only hda supports 32 bit), it is
> only used for the audio backends (i.e. host side).  In
> audiodev_to_audsettings we set endianness to AUDIO_HOST_ENDIANNESS, so
> audio backends should always use native endian.
>
> So this limitation should only cause problems when an audio backend
> overrides the endian setting.  Wavcapture does it, but it does not
> support float.  Alsa, sdl, puleaudio and oss can also do it if for some
> weird reason it acquires a stream with a different endianness than
> requested.

Ah, right, I had missed that this is only used by backends; makes
sense not to worry about non-native endianness then. If you
do a respin of the patch you might add some of that into the commit
message as an explanation of why the limitation isn't a significant one.

thanks
-- PMM

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Gerd Hoffmann]

On Mon, Feb 03, 2020 at 09:38:39PM +0100, Zoltán Kővágó wrote:
> On 2020-02-03 11:00, Peter Maydell wrote:
> > On Sun, 2 Feb 2020 at 19:39, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
> > > 
> > > This adds proper support for float samples in mixeng by adding a new
> > > audio format for it.
> > > 
> > > Limitations: only native endianness is supported.
> > 
> > Could you explain a bit more what this limitation means, please?
> > In general QEMU behaviour shouldn't depend on the endianness
> > of the host, ie we should byteswap where necessary.
> 
> None of the virtual sound cards support float samples (it looks like most of
> them only support 8 and 16 bit, only hda supports 32 bit), it is only used
> for the audio backends (i.e. host side).

Queued patch & added this to the commit message.

thanks,
  Gerd

Re: [RFC PATCH] audio: proper support for float samples in mixeng

[Alexander Bulekov]

Hello,
On 200202 2038, Kővágó, Zoltán wrote:
> -void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
> -                                  int samples)
> +#ifdef FLOAT_MIXENG
> +#define FLOAT_CONV_TO(x) (x)
> +#define FLOAT_CONV_FROM(x) (x)
> +#else
> +static const float float_scale = UINT_MAX;
> +#define FLOAT_CONV_TO(x) ((x) * float_scale)
> +
> +#ifdef RECIPROCAL
> +static const float float_scale_reciprocal = 1.f / UINT_MAX;
> +#define FLOAT_CONV_FROM(x) ((x) * float_scale_reciprocal)
> +#else
> +#define FLOAT_CONV_FROM(x) ((x) / float_scale)
> +#endif
> +#endif

This brings up errors, when building with clang-10+:
error: implicit conversion from 'unsigned int' to 'float' changes value from 4294967295 to 4294967296 [-Werror,-Wimplicit-int-float-conversion]
static const float float_scale = UINT_MAX;

Would this work?
#include <math.h>
#define FLOAT_CONV_TO(x) ((x) * nextafter(0x1p32, 0));
#define FLOAT_CONV_FROM(x) ((x) / nextafter(0x1p32, 0));

I asked on IRC about this and with Richard's help, I tried:
FLOAT_CONV_TO(x) ((x) * (int64_t)UINT32_MAX)
FLOAT_CONV_FROM(x) ((x) / UINT32_MAX)

since dst->l,r are integers when !FLOAT_MIXENG, this might avoid an
int->float->int conversion , but this still raises warning since *in is
a float.

Thanks
-Alex