[PATCH 0/5] mostly changes related to audio float samples

[PATCH 0/5] mostly changes related to audio float samples

[Volker Rümelin]

Patch "audio: change naming scheme of FLOAT_CONV macros" and
patch "audio: consistency changes" should have been a review
for ed2a4a7941 "audio: proper support for float samples in
mixeng", but I was too slow.

Patch "audio: change mixing engine float range to [-1.f, 1.f]"
definitely needs to be tested by macOS users. I verified
PulseAudio, SDL2 and ALSA (actually libasound) work, but I
don't want to introduce a regression for CoreAudio. I spent
some time on https://developer.apple.com but I couldn't find
the answer if CoreAudio uses the [-1.f, 1.f] range too. The
results from my favorite search engine suggest the answer is
yes.

@Howard:
I need your help once again. Can you please test if you can
hear (or see) clipping with my patches? Don't forget to set all
volume controls in the guest and on the host to 100% or 0dB to
get a reliable result. Compared to qemu without these patches I
expect the playback volume is a bit louder, but there's still
no clipping.

Volker Rümelin (5):
  qapi/audio: add documentation for AudioFormat
  audio: change naming scheme of FLOAT_CONV macros
  audio: consistency changes
  audio: change mixing engine float range to [-1.f, 1.f]
  audio: fix saturation nonlinearity in clip_* functions

 audio/mixeng.c          | 26 +++++++++++++-------------
 audio/mixeng_template.h | 22 ++++++++++------------
 qapi/audio.json         | 14 ++++++++++++++
 3 files changed, 37 insertions(+), 25 deletions(-)

-- 
2.16.4

[PATCH 1/5] qapi/audio: add documentation for AudioFormat

[Volker Rümelin]

The review for patch ed2a4a7941 "audio: proper support for
float samples in mixeng" suggested this would be a good idea.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 qapi/audio.json | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/qapi/audio.json b/qapi/audio.json
index d8c507cced..07e7b05516 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -273,6 +273,20 @@
 #
 # An enumeration of possible audio formats.
 #
+# @u8: unsigned 8 bit integer
+#
+# @s8: signed 8 bit integer
+#
+# @u16: unsigned 16 bit integer
+#
+# @s16: signed 16 bit integer
+#
+# @u32: unsigned 32 bit integer
+#
+# @s32: signed 32 bit integer
+#
+# @f32: single precision floating point (since 5.0)
+#
 # Since: 4.0
 ##
 { 'enum': 'AudioFormat',
-- 
2.16.4

[PATCH 2/5] audio: change naming scheme of FLOAT_CONV macros

[Volker Rümelin]

This patch changes the naming scheme of the FLOAT_CONV_TO and
FLOAT_CONV_FROM macros to the scheme used in mixeng_template.h.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/mixeng.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index c14b0d874c..b57fad83bf 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -268,17 +268,17 @@ f_sample *mixeng_clip[2][2][2][3] = {
 };
 
 #ifdef FLOAT_MIXENG
-#define FLOAT_CONV_TO(x) (x)
-#define FLOAT_CONV_FROM(x) (x)
+#define CONV_NATURAL_FLOAT(x) (x)
+#define CLIP_NATURAL_FLOAT(x) (x)
 #else
 static const float float_scale = UINT_MAX;
-#define FLOAT_CONV_TO(x) ((x) * float_scale)
+#define CONV_NATURAL_FLOAT(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)
+#define CLIP_NATURAL_FLOAT(x) ((x) * float_scale_reciprocal)
 #else
-#define FLOAT_CONV_FROM(x) ((x) / float_scale)
+#define CLIP_NATURAL_FLOAT(x) ((x) / float_scale)
 #endif
 #endif
 
@@ -288,7 +288,7 @@ static void conv_natural_float_to_mono(struct st_sample *dst, const void *src,
     float *in = (float *)src;
 
     while (samples--) {
-        dst->r = dst->l = FLOAT_CONV_TO(*in++);
+        dst->r = dst->l = CONV_NATURAL_FLOAT(*in++);
         dst++;
     }
 }
@@ -299,8 +299,8 @@ static void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
     float *in = (float *)src;
 
     while (samples--) {
-        dst->l = FLOAT_CONV_TO(*in++);
-        dst->r = FLOAT_CONV_TO(*in++);
+        dst->l = CONV_NATURAL_FLOAT(*in++);
+        dst->r = CONV_NATURAL_FLOAT(*in++);
         dst++;
     }
 }
@@ -316,7 +316,7 @@ static void clip_natural_float_from_mono(void *dst, const struct st_sample *src,
     float *out = (float *)dst;
 
     while (samples--) {
-        *out++ = FLOAT_CONV_FROM(src->l) + FLOAT_CONV_FROM(src->r);
+        *out++ = CLIP_NATURAL_FLOAT(src->l) + CLIP_NATURAL_FLOAT(src->r);
         src++;
     }
 }
@@ -327,8 +327,8 @@ static void clip_natural_float_from_stereo(
     float *out = (float *)dst;
 
     while (samples--) {
-        *out++ = FLOAT_CONV_FROM(src->l);
-        *out++ = FLOAT_CONV_FROM(src->r);
+        *out++ = CLIP_NATURAL_FLOAT(src->l);
+        *out++ = CLIP_NATURAL_FLOAT(src->r);
         src++;
     }
 }
-- 
2.16.4

[PATCH 3/5] audio: consistency changes

[Volker Rümelin]

Change the clip_natural_float_from_mono() function in
audio/mixeng.c to be consistent with the clip_*_from_mono()
functions in audio/mixeng_template.h.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/mixeng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index b57fad83bf..725b529be7 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -316,7 +316,7 @@ static void clip_natural_float_from_mono(void *dst, const struct st_sample *src,
     float *out = (float *)dst;
 
     while (samples--) {
-        *out++ = CLIP_NATURAL_FLOAT(src->l) + CLIP_NATURAL_FLOAT(src->r);
+        *out++ = CLIP_NATURAL_FLOAT(src->l + src->r);
         src++;
     }
 }
-- 
2.16.4

[PATCH 4/5] audio: change mixing engine float range to [-1.f, 1.f]

[Volker Rümelin]

Currently the internal float range of the mixing engine is
[-.5f, .5f]. PulseAudio, SDL2 and libasound use a [-1.f, 1.f]
range. This means with float samples the audio playback volume
is 6dB too low and audio recording signals will be clipped in
most cases.

To avoid another scaling factor in the conv_natural_float_* and
clip_natural_float_* functions with FLOAT_MIXENG defined this
patch changes the mixing engine float range to [-1.f, 1.f].

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/mixeng.c          |  4 ++--
 audio/mixeng_template.h | 17 ++++++++---------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index 725b529be7..739a500449 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -271,11 +271,11 @@ f_sample *mixeng_clip[2][2][2][3] = {
 #define CONV_NATURAL_FLOAT(x) (x)
 #define CLIP_NATURAL_FLOAT(x) (x)
 #else
-static const float float_scale = UINT_MAX;
+static const float float_scale = UINT_MAX / 2.f;
 #define CONV_NATURAL_FLOAT(x) ((x) * float_scale)
 
 #ifdef RECIPROCAL
-static const float float_scale_reciprocal = 1.f / UINT_MAX;
+static const float float_scale_reciprocal = 2.f / UINT_MAX;
 #define CLIP_NATURAL_FLOAT(x) ((x) * float_scale_reciprocal)
 #else
 #define CLIP_NATURAL_FLOAT(x) ((x) / float_scale)
diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h
index 77cc89b9e8..fc8e1d4d9e 100644
--- a/audio/mixeng_template.h
+++ b/audio/mixeng_template.h
@@ -41,32 +41,31 @@ static inline mixeng_real glue (conv_, ET) (IN_T v)
 
 #ifdef RECIPROCAL
 #ifdef SIGNED
-    return nv * (1.f / (mixeng_real) (IN_MAX - IN_MIN));
+    return nv * (2.f / ((mixeng_real)IN_MAX - IN_MIN));
 #else
-    return (nv - HALF) * (1.f / (mixeng_real) IN_MAX);
+    return (nv - HALF) * (2.f / (mixeng_real)IN_MAX);
 #endif
 #else  /* !RECIPROCAL */
 #ifdef SIGNED
-    return nv / (mixeng_real) ((mixeng_real) IN_MAX - IN_MIN);
+    return nv / (((mixeng_real)IN_MAX - IN_MIN) / 2.f);
 #else
-    return (nv - HALF) / (mixeng_real) IN_MAX;
+    return (nv - HALF) / ((mixeng_real)IN_MAX / 2.f);
 #endif
 #endif
 }
 
 static inline IN_T glue (clip_, ET) (mixeng_real v)
 {
-    if (v >= 0.5) {
+    if (v >= 1.f) {
         return IN_MAX;
-    }
-    else if (v < -0.5) {
+    } else if (v < -1.f) {
         return IN_MIN;
     }
 
 #ifdef SIGNED
-    return ENDIAN_CONVERT ((IN_T) (v * ((mixeng_real) IN_MAX - IN_MIN)));
+    return ENDIAN_CONVERT((IN_T)(v * (((mixeng_real)IN_MAX - IN_MIN) / 2.f)));
 #else
-    return ENDIAN_CONVERT ((IN_T) ((v * IN_MAX) + HALF));
+    return ENDIAN_CONVERT((IN_T)((v * ((mixeng_real)IN_MAX / 2.f)) + HALF));
 #endif
 }
 
-- 
2.16.4

[PATCH 5/5] audio: fix saturation nonlinearity in clip_* functions

[Volker Rümelin]

The current positive limit for the saturation nonlinearity is
only correct if the type of the result has 8 bits or less.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/mixeng_template.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h
index fc8e1d4d9e..bc8509e423 100644
--- a/audio/mixeng_template.h
+++ b/audio/mixeng_template.h
@@ -83,10 +83,9 @@ static inline int64_t glue (conv_, ET) (IN_T v)
 
 static inline IN_T glue (clip_, ET) (int64_t v)
 {
-    if (v >= 0x7f000000) {
+    if (v >= 0x7fffffffLL) {
         return IN_MAX;
-    }
-    else if (v < -2147483648LL) {
+    } else if (v < -2147483648LL) {
         return IN_MIN;
     }
 
-- 
2.16.4

Re: [PATCH 0/5] mostly changes related to audio float samples

[Howard Spoelstra]

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

On Mon, Mar 2, 2020 at 8:08 PM Volker Rümelin <vr_qemu@t-online.de> wrote:

> Patch "audio: change naming scheme of FLOAT_CONV macros" and
> patch "audio: consistency changes" should have been a review
> for ed2a4a7941 "audio: proper support for float samples in
> mixeng", but I was too slow.
>
> Patch "audio: change mixing engine float range to [-1.f, 1.f]"
> definitely needs to be tested by macOS users. I verified
> PulseAudio, SDL2 and ALSA (actually libasound) work, but I
> don't want to introduce a regression for CoreAudio. I spent
> some time on https://developer.apple.com but I couldn't find
> the answer if CoreAudio uses the [-1.f, 1.f] range too. The
> results from my favorite search engine suggest the answer is
> yes.
>
> @Howard:
> I need your help once again. Can you please test if you can
> hear (or see) clipping with my patches? Don't forget to set all
> volume controls in the guest and on the host to 100% or 0dB to
> get a reliable result. Compared to qemu without these patches I
> expect the playback volume is a bit louder, but there's still
> no clipping.
>
> Volker Rümelin (5):
>   qapi/audio: add documentation for AudioFormat
>   audio: change naming scheme of FLOAT_CONV macros
>   audio: consistency changes
>   audio: change mixing engine float range to [-1.f, 1.f]
>   audio: fix saturation nonlinearity in clip_* functions
>
>  audio/mixeng.c          | 26 +++++++++++++-------------
>  audio/mixeng_template.h | 22 ++++++++++------------
>  qapi/audio.json         | 14 ++++++++++++++
>  3 files changed, 37 insertions(+), 25 deletions(-)
>
> --
> 2.16.4
>
> Hi,

I applied these to Mark's screamer branch to create a new OSX build.
Testing was done by playing system sounds, and an MP3/Internet radio with
QuickTime/iTunes. With or without 6dB, the max volume is way out of my
comfort zone. I hear no real difference in quality compared to a build from
the current screamer branch. If any, it might sound a bit better. That
means there still is some minimal crackling (clipping?) in the high volume
parts of the audio I played with OSX guests, not Mac OS 9.x guests.

So as there is no regression,
tested by: howard spoelstra <hsp.cat7@gmail.com>

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

Re: [PATCH 1/5] qapi/audio: add documentation for AudioFormat

[Markus Armbruster]

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

> The review for patch ed2a4a7941 "audio: proper support for
> float samples in mixeng" suggested this would be a good idea.
>
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>  qapi/audio.json | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index d8c507cced..07e7b05516 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -273,6 +273,20 @@
>  #
>  # An enumeration of possible audio formats.
>  #
> +# @u8: unsigned 8 bit integer
> +#
> +# @s8: signed 8 bit integer
> +#
> +# @u16: unsigned 16 bit integer
> +#
> +# @s16: signed 16 bit integer
> +#
> +# @u32: unsigned 32 bit integer
> +#
> +# @s32: signed 32 bit integer
> +#
> +# @f32: single precision floating point (since 5.0)

floating-point

> +#
>  # Since: 4.0
>  ##
>  { 'enum': 'AudioFormat',

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

Re: [PATCH 0/5] mostly changes related to audio float samples

[Volker Rümelin]

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

Hi Howard,

thank you for testing the patches.

> Hi,
>
> I applied these to Mark's screamer branch to create a new OSX build. Testing was done by playing system sounds, and an MP3/Internet radio with QuickTime/iTunes. With or without 6dB, the max volume is way out of my comfort zone. I hear no real difference in quality compared to a build from the

It wasn't my intention to annoy your neighbours. On my computer I use an analog output for playback. Clipping happens before DA conversion, so I can use the volume control on the amplifier to have normal volume levels.

> current screamer branch. If any, it might sound a bit better. That means there still is some minimal crackling (clipping?) in the high volume parts of the

Crackling and clipping are two different things. Crackling is normally a timing problem where audio samples aren't delivered in time. Clipping produces harmonic distortions. For example a clipped sine wave will sound metallic because of the harmonic distortions. The difference between a clipped and a pure sine wave is clearly audible.

> audio I played with OSX guests, not Mac OS 9.x guests.  
>
> So as there is no regression,
> tested by: howard spoelstra <hsp.cat7@gmail.com <mailto:hsp.cat7@gmail.com>>

With best regards,
Volker

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