[Qemu-devel] audio: Mixing engine and mixer emulation

[Qemu-devel] audio: Mixing engine and mixer emulation

[Michael Walle]

Hi,

the SWVoiceIn volume is never used, although there is AUD_set_volume_in() to 
set it. Would it be possible to add support for it? So we could get the mixer 
emulation for capture devices too?

BTW i see you can only attenuate an audio stream with these volume settings 
(you can only set a max value of 255 and the formula is nominal_value * vol / 
255). Would it be possible to support amplification too?


-- 
 Michael

[Qemu-devel] Re: audio: Mixing engine and mixer emulation

[malc]

On Tue, 4 Jan 2011, Michael Walle wrote:

> 
> Hi,
> 
> the SWVoiceIn volume is never used, although there is AUD_set_volume_in() to 
> set it. Would it be possible to add support for it? So we could get the mixer 
> emulation for capture devices too?
> 
> BTW i see you can only attenuate an audio stream with these volume settings 
> (you can only set a max value of 255 and the formula is nominal_value * vol / 
> 255). Would it be possible to support amplification too?

I've never touched this code since writing it way back then, if you cook
up a patch... (This is a reply to both your questions)

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH] audio: split sample conversion and volume mixing

[Michael Walle]

Refactor the volume mixing, so it can be reused for capturing devices.
Additionally, it removes superfluous multiplications with the nominal
volume within the hardware voice code path.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 audio/alsaaudio.c       |    2 +-
 audio/audio.c           |   11 ++++++-----
 audio/audio_int.h       |    2 +-
 audio/dsoundaudio.c     |    4 ++--
 audio/esdaudio.c        |    3 +--
 audio/fmodaudio.c       |    4 ++--
 audio/mixeng.c          |   25 +++++++++++++++++++++++++
 audio/mixeng.h          |    4 ++--
 audio/mixeng_template.h |   39 +++++++--------------------------------
 audio/ossaudio.c        |    3 +--
 audio/paaudio.c         |    2 +-
 audio/spiceaudio.c      |    5 ++---
 audio/winwaveaudio.c    |    3 +--
 13 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index fef24f6..301a6af 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -1094,7 +1094,7 @@ static int alsa_run_in (HWVoiceIn *hw)
                 }
             }
 
-            hw->conv (dst, src, nread, &nominal_volume);
+            hw->conv (dst, src, nread);
 
             src = advance (src, nread << hwshift);
             dst += nread;
diff --git a/audio/audio.c b/audio/audio.c
index 1707446..1729c0b 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -104,7 +104,7 @@ static struct {
 
 static AudioState glob_audio_state;
 
-struct mixeng_volume nominal_volume = {
+const struct mixeng_volume nominal_volume = {
     .mute = 0,
 #ifdef FLOAT_MIXENG
     .r = 1.0,
@@ -702,13 +702,11 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len)
 /*
  * Capture
  */
-static void noop_conv (struct st_sample *dst, const void *src,
-                       int samples, struct mixeng_volume *vol)
+static void noop_conv (struct st_sample *dst, const void *src, int samples)
 {
     (void) src;
     (void) dst;
     (void) samples;
-    (void) vol;
 }
 
 static CaptureVoiceOut *audio_pcm_capture_find_specific (
@@ -956,6 +954,8 @@ int audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int size)
         total += isamp;
     }
 
+    mixeng_volume (sw->buf, ret, &sw->vol);
+
     sw->clip (buf, sw->buf, ret);
     sw->total_hw_samples_acquired += total;
     return ret << sw->info.shift;
@@ -1037,7 +1037,8 @@ int audio_pcm_sw_write (SWVoiceOut *sw, void *buf, int size)
     swlim = ((int64_t) dead << 32) / sw->ratio;
     swlim = audio_MIN (swlim, samples);
     if (swlim) {
-        sw->conv (sw->buf, buf, swlim, &sw->vol);
+        sw->conv (sw->buf, buf, swlim);
+        mixeng_volume (sw->buf, swlim, &sw->vol);
     }
 
     while (swlim) {
diff --git a/audio/audio_int.h b/audio/audio_int.h
index d66f2c3..2003f8b 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -211,7 +211,7 @@ extern struct audio_driver esd_audio_driver;
 extern struct audio_driver pa_audio_driver;
 extern struct audio_driver spice_audio_driver;
 extern struct audio_driver winwave_audio_driver;
-extern struct mixeng_volume nominal_volume;
+extern const struct mixeng_volume nominal_volume;
 
 void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as);
 void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len);
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index e547955..e2d89fd 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -831,11 +831,11 @@ static int dsound_run_in (HWVoiceIn *hw)
     decr = len1 + len2;
 
     if (p1 && len1) {
-        hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume);
+        hw->conv (hw->conv_buf + hw->wpos, p1, len1);
     }
 
     if (p2 && len2) {
-        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
+        hw->conv (hw->conv_buf, p2, len2);
     }
 
     dsound_unlock_in (dscb, p1, p2, blen1, blen2);
diff --git a/audio/esdaudio.c b/audio/esdaudio.c
index 9a1f2f8..ff97b39 100644
--- a/audio/esdaudio.c
+++ b/audio/esdaudio.c
@@ -346,8 +346,7 @@ static void *qesd_thread_in (void *arg)
                 break;
             }
 
-            hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift,
-                      &nominal_volume);
+            hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift);
             wpos = (wpos + chunk) % hw->samples;
             to_grab -= chunk;
         }
diff --git a/audio/fmodaudio.c b/audio/fmodaudio.c
index 7f08e14..c34cf53 100644
--- a/audio/fmodaudio.c
+++ b/audio/fmodaudio.c
@@ -488,10 +488,10 @@ static int fmod_run_in (HWVoiceIn *hw)
     decr = len1 + len2;
 
     if (p1 && blen1) {
-        hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume);
+        hw->conv (hw->conv_buf + hw->wpos, p1, len1);
     }
     if (p2 && len2) {
-        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
+        hw->conv (hw->conv_buf, p2, len2);
     }
 
     fmod_unlock_sample (fmd->fmod_sample, p1, p2, blen1, blen2);
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 9f1d93f..4a9e8eb 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -333,3 +333,28 @@ void mixeng_clear (struct st_sample *buf, int len)
 {
     memset (buf, 0, len * sizeof (struct st_sample));
 }
+
+void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol)
+{
+#ifdef CONFIG_MIXEMU
+    if (vol->mute) {
+        mixeng_clear (buf, len);
+        return;
+    }
+
+    while (len--) {
+#ifdef FLOAT_MIXENG
+        buf->l = buf->l * vol->l;
+        buf->r = buf->r * vol->r;
+#else
+        buf->l = (buf->l * vol->l) >> 32;
+        buf->r = (buf->r * vol->r) >> 32;
+#endif
+        buf += 1;
+    }
+#else
+    (void) buf;
+    (void) len;
+    (void) vol;
+#endif
+}
diff --git a/audio/mixeng.h b/audio/mixeng.h
index 4af1dd9..9de443b 100644
--- a/audio/mixeng.h
+++ b/audio/mixeng.h
@@ -33,8 +33,7 @@ struct mixeng_volume { int mute; int64_t r; int64_t l; };
 struct st_sample { int64_t l; int64_t r; };
 #endif
 
-typedef void (t_sample) (struct st_sample *dst, const void *src,
-                         int samples, struct mixeng_volume *vol);
+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);
 
 extern t_sample *mixeng_conv[2][2][2][3];
@@ -47,5 +46,6 @@ void st_rate_flow_mix (void *opaque, struct st_sample *ibuf, struct st_sample *o
                        int *isamp, int *osamp);
 void st_rate_stop (void *opaque);
 void mixeng_clear (struct st_sample *buf, int len);
+void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol);
 
 #endif  /* mixeng.h */
diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h
index 5617705..a2d0ef8 100644
--- a/audio/mixeng_template.h
+++ b/audio/mixeng_template.h
@@ -31,16 +31,6 @@
 #define HALF (IN_MAX >> 1)
 #endif
 
-#ifdef CONFIG_MIXEMU
-#ifdef FLOAT_MIXENG
-#define VOL(a, b) ((a) * (b))
-#else
-#define VOL(a, b) ((a) * (b)) >> 32
-#endif
-#else
-#define VOL(a, b) a
-#endif
-
 #define ET glue (ENDIAN_CONVERSION, glue (_, IN_T))
 
 #ifdef FLOAT_MIXENG
@@ -109,40 +99,26 @@ static inline IN_T glue (clip_, ET) (int64_t v)
 #endif
 
 static void glue (glue (conv_, ET), _to_stereo)
-    (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol)
+    (struct st_sample *dst, const void *src, int samples)
 {
     struct st_sample *out = dst;
     IN_T *in = (IN_T *) src;
-#ifdef CONFIG_MIXEMU
-    if (vol->mute) {
-        mixeng_clear (dst, samples);
-        return;
-    }
-#else
-    (void) vol;
-#endif
+
     while (samples--) {
-        out->l = VOL (glue (conv_, ET) (*in++), vol->l);
-        out->r = VOL (glue (conv_, ET) (*in++), vol->r);
+        out->l = glue (conv_, ET) (*in++);
+        out->r = glue (conv_, ET) (*in++);
         out += 1;
     }
 }
 
 static void glue (glue (conv_, ET), _to_mono)
-    (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol)
+    (struct st_sample *dst, const void *src, int samples)
 {
     struct st_sample *out = dst;
     IN_T *in = (IN_T *) src;
-#ifdef CONFIG_MIXEMU
-    if (vol->mute) {
-        mixeng_clear (dst, samples);
-        return;
-    }
-#else
-    (void) vol;
-#endif
+
     while (samples--) {
-        out->l = VOL (glue (conv_, ET) (in[0]), vol->l);
+        out->l = glue (conv_, ET) (in[0]);
         out->r = out->l;
         out += 1;
         in += 1;
@@ -174,4 +150,3 @@ static void glue (glue (clip_, ET), _from_mono)
 
 #undef ET
 #undef HALF
-#undef VOL
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 0439ef5..1fda519 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -784,8 +784,7 @@ static int oss_run_in (HWVoiceIn *hw)
                            hw->info.align + 1);
                 }
                 read_samples += nread >> hwshift;
-                hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift,
-                          &nominal_volume);
+                hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift);
             }
 
             if (bufs[i].len - nread) {
diff --git a/audio/paaudio.c b/audio/paaudio.c
index ff71dac..9cf685d 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -195,7 +195,7 @@ static void *qpa_thread_in (void *arg)
                 return NULL;
             }
 
-            hw->conv (hw->conv_buf + wpos, buf, chunk, &nominal_volume);
+            hw->conv (hw->conv_buf + wpos, buf, chunk);
             wpos = (wpos + chunk) % hw->samples;
             to_grab -= chunk;
         }
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index 373e4c4..a5c0d6b 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -268,11 +268,10 @@ static int line_in_run (HWVoiceIn *hw)
         len[1] = 0;
     }
 
-    hw->conv (hw->conv_buf + hw->wpos, samples, len[0], &nominal_volume);
+    hw->conv (hw->conv_buf + hw->wpos, samples, len[0]);
 
     if (len[1]) {
-        hw->conv (hw->conv_buf, samples + len[0], len[1],
-                  &nominal_volume);
+        hw->conv (hw->conv_buf, samples + len[0], len[1]);
     }
 
     hw->wpos = (hw->wpos + num_samples) % hw->samples;
diff --git a/audio/winwaveaudio.c b/audio/winwaveaudio.c
index cdf483b..e5ad3c6 100644
--- a/audio/winwaveaudio.c
+++ b/audio/winwaveaudio.c
@@ -581,8 +581,7 @@ static int winwave_run_in (HWVoiceIn *hw)
         int conv = audio_MIN (left, decr);
         hw->conv (hw->conv_buf + hw->wpos,
                   advance (wave->pcm_buf, wave->rpos << hw->info.shift),
-                  conv,
-                  &nominal_volume);
+                  conv);
 
         wave->rpos = (wave->rpos + conv) % hw->samples;
         hw->wpos = (hw->wpos + conv) % hw->samples;
-- 
1.7.2.3

[Qemu-devel] Re: [PATCH] audio: split sample conversion and volume mixing

[Jan Kiszka]

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

Am 05.01.2011 01:05, Michael Walle wrote:
> Refactor the volume mixing, so it can be reused for capturing devices.
> Additionally, it removes superfluous multiplications with the nominal
> volume within the hardware voice code path.

At least based on tests done with the Musicpal, I see problems with this
patch, volume control still fine here.

Jan

> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  audio/alsaaudio.c       |    2 +-
>  audio/audio.c           |   11 ++++++-----
>  audio/audio_int.h       |    2 +-
>  audio/dsoundaudio.c     |    4 ++--
>  audio/esdaudio.c        |    3 +--
>  audio/fmodaudio.c       |    4 ++--
>  audio/mixeng.c          |   25 +++++++++++++++++++++++++
>  audio/mixeng.h          |    4 ++--
>  audio/mixeng_template.h |   39 +++++++--------------------------------
>  audio/ossaudio.c        |    3 +--
>  audio/paaudio.c         |    2 +-
>  audio/spiceaudio.c      |    5 ++---
>  audio/winwaveaudio.c    |    3 +--
>  13 files changed, 52 insertions(+), 55 deletions(-)
> 
> diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
> index fef24f6..301a6af 100644
> --- a/audio/alsaaudio.c
> +++ b/audio/alsaaudio.c
> @@ -1094,7 +1094,7 @@ static int alsa_run_in (HWVoiceIn *hw)
>                  }
>              }
>  
> -            hw->conv (dst, src, nread, &nominal_volume);
> +            hw->conv (dst, src, nread);
>  
>              src = advance (src, nread << hwshift);
>              dst += nread;
> diff --git a/audio/audio.c b/audio/audio.c
> index 1707446..1729c0b 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -104,7 +104,7 @@ static struct {
>  
>  static AudioState glob_audio_state;
>  
> -struct mixeng_volume nominal_volume = {
> +const struct mixeng_volume nominal_volume = {
>      .mute = 0,
>  #ifdef FLOAT_MIXENG
>      .r = 1.0,
> @@ -702,13 +702,11 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len)
>  /*
>   * Capture
>   */
> -static void noop_conv (struct st_sample *dst, const void *src,
> -                       int samples, struct mixeng_volume *vol)
> +static void noop_conv (struct st_sample *dst, const void *src, int samples)
>  {
>      (void) src;
>      (void) dst;
>      (void) samples;
> -    (void) vol;
>  }
>  
>  static CaptureVoiceOut *audio_pcm_capture_find_specific (
> @@ -956,6 +954,8 @@ int audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int size)
>          total += isamp;
>      }
>  
> +    mixeng_volume (sw->buf, ret, &sw->vol);
> +
>      sw->clip (buf, sw->buf, ret);
>      sw->total_hw_samples_acquired += total;
>      return ret << sw->info.shift;
> @@ -1037,7 +1037,8 @@ int audio_pcm_sw_write (SWVoiceOut *sw, void *buf, int size)
>      swlim = ((int64_t) dead << 32) / sw->ratio;
>      swlim = audio_MIN (swlim, samples);
>      if (swlim) {
> -        sw->conv (sw->buf, buf, swlim, &sw->vol);
> +        sw->conv (sw->buf, buf, swlim);
> +        mixeng_volume (sw->buf, swlim, &sw->vol);
>      }
>  
>      while (swlim) {
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index d66f2c3..2003f8b 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -211,7 +211,7 @@ extern struct audio_driver esd_audio_driver;
>  extern struct audio_driver pa_audio_driver;
>  extern struct audio_driver spice_audio_driver;
>  extern struct audio_driver winwave_audio_driver;
> -extern struct mixeng_volume nominal_volume;
> +extern const struct mixeng_volume nominal_volume;
>  
>  void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as);
>  void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len);
> diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
> index e547955..e2d89fd 100644
> --- a/audio/dsoundaudio.c
> +++ b/audio/dsoundaudio.c
> @@ -831,11 +831,11 @@ static int dsound_run_in (HWVoiceIn *hw)
>      decr = len1 + len2;
>  
>      if (p1 && len1) {
> -        hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume);
> +        hw->conv (hw->conv_buf + hw->wpos, p1, len1);
>      }
>  
>      if (p2 && len2) {
> -        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
> +        hw->conv (hw->conv_buf, p2, len2);
>      }
>  
>      dsound_unlock_in (dscb, p1, p2, blen1, blen2);
> diff --git a/audio/esdaudio.c b/audio/esdaudio.c
> index 9a1f2f8..ff97b39 100644
> --- a/audio/esdaudio.c
> +++ b/audio/esdaudio.c
> @@ -346,8 +346,7 @@ static void *qesd_thread_in (void *arg)
>                  break;
>              }
>  
> -            hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift,
> -                      &nominal_volume);
> +            hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift);
>              wpos = (wpos + chunk) % hw->samples;
>              to_grab -= chunk;
>          }
> diff --git a/audio/fmodaudio.c b/audio/fmodaudio.c
> index 7f08e14..c34cf53 100644
> --- a/audio/fmodaudio.c
> +++ b/audio/fmodaudio.c
> @@ -488,10 +488,10 @@ static int fmod_run_in (HWVoiceIn *hw)
>      decr = len1 + len2;
>  
>      if (p1 && blen1) {
> -        hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume);
> +        hw->conv (hw->conv_buf + hw->wpos, p1, len1);
>      }
>      if (p2 && len2) {
> -        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
> +        hw->conv (hw->conv_buf, p2, len2);
>      }
>  
>      fmod_unlock_sample (fmd->fmod_sample, p1, p2, blen1, blen2);
> diff --git a/audio/mixeng.c b/audio/mixeng.c
> index 9f1d93f..4a9e8eb 100644
> --- a/audio/mixeng.c
> +++ b/audio/mixeng.c
> @@ -333,3 +333,28 @@ void mixeng_clear (struct st_sample *buf, int len)
>  {
>      memset (buf, 0, len * sizeof (struct st_sample));
>  }
> +
> +void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol)
> +{
> +#ifdef CONFIG_MIXEMU
> +    if (vol->mute) {
> +        mixeng_clear (buf, len);
> +        return;
> +    }
> +
> +    while (len--) {
> +#ifdef FLOAT_MIXENG
> +        buf->l = buf->l * vol->l;
> +        buf->r = buf->r * vol->r;
> +#else
> +        buf->l = (buf->l * vol->l) >> 32;
> +        buf->r = (buf->r * vol->r) >> 32;
> +#endif
> +        buf += 1;
> +    }
> +#else
> +    (void) buf;
> +    (void) len;
> +    (void) vol;
> +#endif
> +}
> diff --git a/audio/mixeng.h b/audio/mixeng.h
> index 4af1dd9..9de443b 100644
> --- a/audio/mixeng.h
> +++ b/audio/mixeng.h
> @@ -33,8 +33,7 @@ struct mixeng_volume { int mute; int64_t r; int64_t l; };
>  struct st_sample { int64_t l; int64_t r; };
>  #endif
>  
> -typedef void (t_sample) (struct st_sample *dst, const void *src,
> -                         int samples, struct mixeng_volume *vol);
> +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);
>  
>  extern t_sample *mixeng_conv[2][2][2][3];
> @@ -47,5 +46,6 @@ void st_rate_flow_mix (void *opaque, struct st_sample *ibuf, struct st_sample *o
>                         int *isamp, int *osamp);
>  void st_rate_stop (void *opaque);
>  void mixeng_clear (struct st_sample *buf, int len);
> +void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol);
>  
>  #endif  /* mixeng.h */
> diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h
> index 5617705..a2d0ef8 100644
> --- a/audio/mixeng_template.h
> +++ b/audio/mixeng_template.h
> @@ -31,16 +31,6 @@
>  #define HALF (IN_MAX >> 1)
>  #endif
>  
> -#ifdef CONFIG_MIXEMU
> -#ifdef FLOAT_MIXENG
> -#define VOL(a, b) ((a) * (b))
> -#else
> -#define VOL(a, b) ((a) * (b)) >> 32
> -#endif
> -#else
> -#define VOL(a, b) a
> -#endif
> -
>  #define ET glue (ENDIAN_CONVERSION, glue (_, IN_T))
>  
>  #ifdef FLOAT_MIXENG
> @@ -109,40 +99,26 @@ static inline IN_T glue (clip_, ET) (int64_t v)
>  #endif
>  
>  static void glue (glue (conv_, ET), _to_stereo)
> -    (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol)
> +    (struct st_sample *dst, const void *src, int samples)
>  {
>      struct st_sample *out = dst;
>      IN_T *in = (IN_T *) src;
> -#ifdef CONFIG_MIXEMU
> -    if (vol->mute) {
> -        mixeng_clear (dst, samples);
> -        return;
> -    }
> -#else
> -    (void) vol;
> -#endif
> +
>      while (samples--) {
> -        out->l = VOL (glue (conv_, ET) (*in++), vol->l);
> -        out->r = VOL (glue (conv_, ET) (*in++), vol->r);
> +        out->l = glue (conv_, ET) (*in++);
> +        out->r = glue (conv_, ET) (*in++);
>          out += 1;
>      }
>  }
>  
>  static void glue (glue (conv_, ET), _to_mono)
> -    (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol)
> +    (struct st_sample *dst, const void *src, int samples)
>  {
>      struct st_sample *out = dst;
>      IN_T *in = (IN_T *) src;
> -#ifdef CONFIG_MIXEMU
> -    if (vol->mute) {
> -        mixeng_clear (dst, samples);
> -        return;
> -    }
> -#else
> -    (void) vol;
> -#endif
> +
>      while (samples--) {
> -        out->l = VOL (glue (conv_, ET) (in[0]), vol->l);
> +        out->l = glue (conv_, ET) (in[0]);
>          out->r = out->l;
>          out += 1;
>          in += 1;
> @@ -174,4 +150,3 @@ static void glue (glue (clip_, ET), _from_mono)
>  
>  #undef ET
>  #undef HALF
> -#undef VOL
> diff --git a/audio/ossaudio.c b/audio/ossaudio.c
> index 0439ef5..1fda519 100644
> --- a/audio/ossaudio.c
> +++ b/audio/ossaudio.c
> @@ -784,8 +784,7 @@ static int oss_run_in (HWVoiceIn *hw)
>                             hw->info.align + 1);
>                  }
>                  read_samples += nread >> hwshift;
> -                hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift,
> -                          &nominal_volume);
> +                hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift);
>              }
>  
>              if (bufs[i].len - nread) {
> diff --git a/audio/paaudio.c b/audio/paaudio.c
> index ff71dac..9cf685d 100644
> --- a/audio/paaudio.c
> +++ b/audio/paaudio.c
> @@ -195,7 +195,7 @@ static void *qpa_thread_in (void *arg)
>                  return NULL;
>              }
>  
> -            hw->conv (hw->conv_buf + wpos, buf, chunk, &nominal_volume);
> +            hw->conv (hw->conv_buf + wpos, buf, chunk);
>              wpos = (wpos + chunk) % hw->samples;
>              to_grab -= chunk;
>          }
> diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
> index 373e4c4..a5c0d6b 100644
> --- a/audio/spiceaudio.c
> +++ b/audio/spiceaudio.c
> @@ -268,11 +268,10 @@ static int line_in_run (HWVoiceIn *hw)
>          len[1] = 0;
>      }
>  
> -    hw->conv (hw->conv_buf + hw->wpos, samples, len[0], &nominal_volume);
> +    hw->conv (hw->conv_buf + hw->wpos, samples, len[0]);
>  
>      if (len[1]) {
> -        hw->conv (hw->conv_buf, samples + len[0], len[1],
> -                  &nominal_volume);
> +        hw->conv (hw->conv_buf, samples + len[0], len[1]);
>      }
>  
>      hw->wpos = (hw->wpos + num_samples) % hw->samples;
> diff --git a/audio/winwaveaudio.c b/audio/winwaveaudio.c
> index cdf483b..e5ad3c6 100644
> --- a/audio/winwaveaudio.c
> +++ b/audio/winwaveaudio.c
> @@ -581,8 +581,7 @@ static int winwave_run_in (HWVoiceIn *hw)
>          int conv = audio_MIN (left, decr);
>          hw->conv (hw->conv_buf + hw->wpos,
>                    advance (wave->pcm_buf, wave->rpos << hw->info.shift),
> -                  conv,
> -                  &nominal_volume);
> +                  conv);
>  
>          wave->rpos = (wave->rpos + conv) % hw->samples;
>          hw->wpos = (hw->wpos + conv) % hw->samples;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

[Qemu-devel] Re: [PATCH] audio: split sample conversion and volume mixing

[malc]

On Mon, 10 Jan 2011, Jan Kiszka wrote:

> Am 05.01.2011 01:05, Michael Walle wrote:
> > Refactor the volume mixing, so it can be reused for capturing devices.
> > Additionally, it removes superfluous multiplications with the nominal
> > volume within the hardware voice code path.
> 
> At least based on tests done with the Musicpal, I see problems with this
> patch, volume control still fine here.

"I see problems" or "I see _no_ problems".

[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: [PATCH] audio: split sample conversion and volume mixing

[Jan Kiszka]

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

Am 10.01.2011 21:52, malc wrote:
> On Mon, 10 Jan 2011, Jan Kiszka wrote:
> 
>> Am 05.01.2011 01:05, Michael Walle wrote:
>>> Refactor the volume mixing, so it can be reused for capturing devices.
>>> Additionally, it removes superfluous multiplications with the nominal
>>> volume within the hardware voice code path.
>>
>> At least based on tests done with the Musicpal, I see problems with this
>> patch, volume control still fine here.
> 
> "I see problems" or "I see _no_ problems".

Sorry, "no problem" of course.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

RE: [Qemu-devel] Re: [PATCH] audio: split sample conversion and volumemixing

[Schildbach, Wolfgang]

The fixed point path for scaling in mixeng_volume() seems to be under by
a factor of two, IMHO. The right shift should be by 31, not 32. (Because
the volume, which I assume is a signed 32 bit integer, can be 0.5 at
most).

Does the code work with FLOAT_MIXENG undefined?

- Wolfgang Schildbach


-----Original Message-----
From: qemu-devel-bounces+wschi=dolby.com@nongnu.org
[mailto:qemu-devel-bounces+wschi=dolby.com@nongnu.org] On Behalf Of Jan
Kiszka
Sent: Monday, January 10, 2011 9:44 PM
To: Michael Walle
Cc: qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH] audio: split sample conversion and
volumemixing

Am 05.01.2011 01:05, Michael Walle wrote:
> Refactor the volume mixing, so it can be reused for capturing devices.
> Additionally, it removes superfluous multiplications with the nominal 
> volume within the hardware voice code path.

At least based on tests done with the Musicpal, I see problems with this
patch, volume control still fine here.

Jan

> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  audio/alsaaudio.c       |    2 +-
>  audio/audio.c           |   11 ++++++-----
>  audio/audio_int.h       |    2 +-
>  audio/dsoundaudio.c     |    4 ++--
>  audio/esdaudio.c        |    3 +--
>  audio/fmodaudio.c       |    4 ++--
>  audio/mixeng.c          |   25 +++++++++++++++++++++++++
>  audio/mixeng.h          |    4 ++--
>  audio/mixeng_template.h |   39
+++++++--------------------------------
>  audio/ossaudio.c        |    3 +--
>  audio/paaudio.c         |    2 +-
>  audio/spiceaudio.c      |    5 ++---
>  audio/winwaveaudio.c    |    3 +--
>  13 files changed, 52 insertions(+), 55 deletions(-)
> 
> diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c index 
> fef24f6..301a6af 100644
> --- a/audio/alsaaudio.c
> +++ b/audio/alsaaudio.c
> @@ -1094,7 +1094,7 @@ static int alsa_run_in (HWVoiceIn *hw)
>                  }
>              }
>  
> -            hw->conv (dst, src, nread, &nominal_volume);
> +            hw->conv (dst, src, nread);
>  
>              src = advance (src, nread << hwshift);
>              dst += nread;
> diff --git a/audio/audio.c b/audio/audio.c index 1707446..1729c0b 
> 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -104,7 +104,7 @@ static struct {
>  
>  static AudioState glob_audio_state;
>  
> -struct mixeng_volume nominal_volume = {
> +const struct mixeng_volume nominal_volume = {
>      .mute = 0,
>  #ifdef FLOAT_MIXENG
>      .r = 1.0,
> @@ -702,13 +702,11 @@ void audio_pcm_info_clear_buf (struct 
> audio_pcm_info *info, void *buf, int len)
>  /*
>   * Capture
>   */
> -static void noop_conv (struct st_sample *dst, const void *src,
> -                       int samples, struct mixeng_volume *vol)
> +static void noop_conv (struct st_sample *dst, const void *src, int 
> +samples)
>  {
>      (void) src;
>      (void) dst;
>      (void) samples;
> -    (void) vol;
>  }
>  
>  static CaptureVoiceOut *audio_pcm_capture_find_specific ( @@ -956,6 
> +954,8 @@ int audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int size)
>          total += isamp;
>      }
>  
> +    mixeng_volume (sw->buf, ret, &sw->vol);
> +
>      sw->clip (buf, sw->buf, ret);
>      sw->total_hw_samples_acquired += total;
>      return ret << sw->info.shift;
> @@ -1037,7 +1037,8 @@ int audio_pcm_sw_write (SWVoiceOut *sw, void
*buf, int size)
>      swlim = ((int64_t) dead << 32) / sw->ratio;
>      swlim = audio_MIN (swlim, samples);
>      if (swlim) {
> -        sw->conv (sw->buf, buf, swlim, &sw->vol);
> +        sw->conv (sw->buf, buf, swlim);
> +        mixeng_volume (sw->buf, swlim, &sw->vol);
>      }
>  
>      while (swlim) {
> diff --git a/audio/audio_int.h b/audio/audio_int.h index 
> d66f2c3..2003f8b 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -211,7 +211,7 @@ extern struct audio_driver esd_audio_driver;  
> extern struct audio_driver pa_audio_driver;  extern struct 
> audio_driver spice_audio_driver;  extern struct audio_driver 
> winwave_audio_driver; -extern struct mixeng_volume nominal_volume;
> +extern const struct mixeng_volume nominal_volume;
>  
>  void audio_pcm_init_info (struct audio_pcm_info *info, struct 
> audsettings *as);  void audio_pcm_info_clear_buf (struct 
> audio_pcm_info *info, void *buf, int len); diff --git 
> a/audio/dsoundaudio.c b/audio/dsoundaudio.c index e547955..e2d89fd 
> 100644
> --- a/audio/dsoundaudio.c
> +++ b/audio/dsoundaudio.c
> @@ -831,11 +831,11 @@ static int dsound_run_in (HWVoiceIn *hw)
>      decr = len1 + len2;
>  
>      if (p1 && len1) {
> -        hw->conv (hw->conv_buf + hw->wpos, p1, len1,
&nominal_volume);
> +        hw->conv (hw->conv_buf + hw->wpos, p1, len1);
>      }
>  
>      if (p2 && len2) {
> -        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
> +        hw->conv (hw->conv_buf, p2, len2);
>      }
>  
>      dsound_unlock_in (dscb, p1, p2, blen1, blen2); diff --git 
> a/audio/esdaudio.c b/audio/esdaudio.c index 9a1f2f8..ff97b39 100644
> --- a/audio/esdaudio.c
> +++ b/audio/esdaudio.c
> @@ -346,8 +346,7 @@ static void *qesd_thread_in (void *arg)
>                  break;
>              }
>  
> -            hw->conv (hw->conv_buf + wpos, buf, nread >>
hw->info.shift,
> -                      &nominal_volume);
> +            hw->conv (hw->conv_buf + wpos, buf, nread >> 
> + hw->info.shift);
>              wpos = (wpos + chunk) % hw->samples;
>              to_grab -= chunk;
>          }
> diff --git a/audio/fmodaudio.c b/audio/fmodaudio.c index 
> 7f08e14..c34cf53 100644
> --- a/audio/fmodaudio.c
> +++ b/audio/fmodaudio.c
> @@ -488,10 +488,10 @@ static int fmod_run_in (HWVoiceIn *hw)
>      decr = len1 + len2;
>  
>      if (p1 && blen1) {
> -        hw->conv (hw->conv_buf + hw->wpos, p1, len1,
&nominal_volume);
> +        hw->conv (hw->conv_buf + hw->wpos, p1, len1);
>      }
>      if (p2 && len2) {
> -        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
> +        hw->conv (hw->conv_buf, p2, len2);
>      }
>  
>      fmod_unlock_sample (fmd->fmod_sample, p1, p2, blen1, blen2); diff

> --git a/audio/mixeng.c b/audio/mixeng.c index 9f1d93f..4a9e8eb 100644
> --- a/audio/mixeng.c
> +++ b/audio/mixeng.c
> @@ -333,3 +333,28 @@ void mixeng_clear (struct st_sample *buf, int 
> len)  {
>      memset (buf, 0, len * sizeof (struct st_sample));  }
> +
> +void mixeng_volume (struct st_sample *buf, int len, struct 
> +mixeng_volume *vol) { #ifdef CONFIG_MIXEMU
> +    if (vol->mute) {
> +        mixeng_clear (buf, len);
> +        return;
> +    }
> +
> +    while (len--) {
> +#ifdef FLOAT_MIXENG
> +        buf->l = buf->l * vol->l;
> +        buf->r = buf->r * vol->r;
> +#else
> +        buf->l = (buf->l * vol->l) >> 32;
> +        buf->r = (buf->r * vol->r) >> 32; #endif
> +        buf += 1;
> +    }
> +#else
> +    (void) buf;
> +    (void) len;
> +    (void) vol;
> +#endif
> +}
> diff --git a/audio/mixeng.h b/audio/mixeng.h index 4af1dd9..9de443b 
> 100644
> --- a/audio/mixeng.h
> +++ b/audio/mixeng.h
> @@ -33,8 +33,7 @@ struct mixeng_volume { int mute; int64_t r; int64_t 
> l; };  struct st_sample { int64_t l; int64_t r; };  #endif
>  
> -typedef void (t_sample) (struct st_sample *dst, const void *src,
> -                         int samples, struct mixeng_volume *vol);
> +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);
>  
>  extern t_sample *mixeng_conv[2][2][2][3]; @@ -47,5 +46,6 @@ void 
> st_rate_flow_mix (void *opaque, struct st_sample *ibuf, struct
st_sample *o
>                         int *isamp, int *osamp);  void st_rate_stop 
> (void *opaque);  void mixeng_clear (struct st_sample *buf, int len);
> +void mixeng_volume (struct st_sample *buf, int len, struct 
> +mixeng_volume *vol);
>  
>  #endif  /* mixeng.h */
> diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h index 
> 5617705..a2d0ef8 100644
> --- a/audio/mixeng_template.h
> +++ b/audio/mixeng_template.h
> @@ -31,16 +31,6 @@
>  #define HALF (IN_MAX >> 1)
>  #endif
>  
> -#ifdef CONFIG_MIXEMU
> -#ifdef FLOAT_MIXENG
> -#define VOL(a, b) ((a) * (b))
> -#else
> -#define VOL(a, b) ((a) * (b)) >> 32
> -#endif
> -#else
> -#define VOL(a, b) a
> -#endif
> -
>  #define ET glue (ENDIAN_CONVERSION, glue (_, IN_T))
>  
>  #ifdef FLOAT_MIXENG
> @@ -109,40 +99,26 @@ static inline IN_T glue (clip_, ET) (int64_t v)  
> #endif
>  
>  static void glue (glue (conv_, ET), _to_stereo)
> -    (struct st_sample *dst, const void *src, int samples, struct
mixeng_volume *vol)
> +    (struct st_sample *dst, const void *src, int samples)
>  {
>      struct st_sample *out = dst;
>      IN_T *in = (IN_T *) src;
> -#ifdef CONFIG_MIXEMU
> -    if (vol->mute) {
> -        mixeng_clear (dst, samples);
> -        return;
> -    }
> -#else
> -    (void) vol;
> -#endif
> +
>      while (samples--) {
> -        out->l = VOL (glue (conv_, ET) (*in++), vol->l);
> -        out->r = VOL (glue (conv_, ET) (*in++), vol->r);
> +        out->l = glue (conv_, ET) (*in++);
> +        out->r = glue (conv_, ET) (*in++);
>          out += 1;
>      }
>  }
>  
>  static void glue (glue (conv_, ET), _to_mono)
> -    (struct st_sample *dst, const void *src, int samples, struct
mixeng_volume *vol)
> +    (struct st_sample *dst, const void *src, int samples)
>  {
>      struct st_sample *out = dst;
>      IN_T *in = (IN_T *) src;
> -#ifdef CONFIG_MIXEMU
> -    if (vol->mute) {
> -        mixeng_clear (dst, samples);
> -        return;
> -    }
> -#else
> -    (void) vol;
> -#endif
> +
>      while (samples--) {
> -        out->l = VOL (glue (conv_, ET) (in[0]), vol->l);
> +        out->l = glue (conv_, ET) (in[0]);
>          out->r = out->l;
>          out += 1;
>          in += 1;
> @@ -174,4 +150,3 @@ static void glue (glue (clip_, ET), _from_mono)
>  
>  #undef ET
>  #undef HALF
> -#undef VOL
> diff --git a/audio/ossaudio.c b/audio/ossaudio.c index 
> 0439ef5..1fda519 100644
> --- a/audio/ossaudio.c
> +++ b/audio/ossaudio.c
> @@ -784,8 +784,7 @@ static int oss_run_in (HWVoiceIn *hw)
>                             hw->info.align + 1);
>                  }
>                  read_samples += nread >> hwshift;
> -                hw->conv (hw->conv_buf + bufs[i].add, p, nread >>
hwshift,
> -                          &nominal_volume);
> +                hw->conv (hw->conv_buf + bufs[i].add, p, nread >> 
> + hwshift);
>              }
>  
>              if (bufs[i].len - nread) { diff --git a/audio/paaudio.c 
> b/audio/paaudio.c index ff71dac..9cf685d 100644
> --- a/audio/paaudio.c
> +++ b/audio/paaudio.c
> @@ -195,7 +195,7 @@ static void *qpa_thread_in (void *arg)
>                  return NULL;
>              }
>  
> -            hw->conv (hw->conv_buf + wpos, buf, chunk,
&nominal_volume);
> +            hw->conv (hw->conv_buf + wpos, buf, chunk);
>              wpos = (wpos + chunk) % hw->samples;
>              to_grab -= chunk;
>          }
> diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c index 
> 373e4c4..a5c0d6b 100644
> --- a/audio/spiceaudio.c
> +++ b/audio/spiceaudio.c
> @@ -268,11 +268,10 @@ static int line_in_run (HWVoiceIn *hw)
>          len[1] = 0;
>      }
>  
> -    hw->conv (hw->conv_buf + hw->wpos, samples, len[0],
&nominal_volume);
> +    hw->conv (hw->conv_buf + hw->wpos, samples, len[0]);
>  
>      if (len[1]) {
> -        hw->conv (hw->conv_buf, samples + len[0], len[1],
> -                  &nominal_volume);
> +        hw->conv (hw->conv_buf, samples + len[0], len[1]);
>      }
>  
>      hw->wpos = (hw->wpos + num_samples) % hw->samples; diff --git 
> a/audio/winwaveaudio.c b/audio/winwaveaudio.c index cdf483b..e5ad3c6 
> 100644
> --- a/audio/winwaveaudio.c
> +++ b/audio/winwaveaudio.c
> @@ -581,8 +581,7 @@ static int winwave_run_in (HWVoiceIn *hw)
>          int conv = audio_MIN (left, decr);
>          hw->conv (hw->conv_buf + hw->wpos,
>                    advance (wave->pcm_buf, wave->rpos <<
hw->info.shift),
> -                  conv,
> -                  &nominal_volume);
> +                  conv);
>  
>          wave->rpos = (wave->rpos + conv) % hw->samples;
>          hw->wpos = (hw->wpos + conv) % hw->samples;

[Qemu-devel] Re: [PATCH] audio: split sample conversion and volumemixing

[Michael Walle]

Hi Wolfgang,

> The fixed point path for scaling in mixeng_volume() seems to be under by
> a factor of two, IMHO. The right shift should be by 31, not 32. (Because
> the volume, which I assume is a signed 32 bit integer, can be 0.5 at
> most).
I must admit i just copied the shift from the original define. But the volume 
and the samples are signed 64bit values.

@malc: any comments on this?

> Does the code work with FLOAT_MIXENG undefined?
At least for my input voice, it works with integer arithmetic. 

-- 
  Michael

[Qemu-devel] Re: [PATCH] audio: split sample conversion and volumemixing

[malc]

On Tue, 11 Jan 2011, Michael Walle wrote:

> 
> Hi Wolfgang,
> 
> > The fixed point path for scaling in mixeng_volume() seems to be under by
> > a factor of two, IMHO. The right shift should be by 31, not 32. (Because
> > the volume, which I assume is a signed 32 bit integer, can be 0.5 at
> > most).
> I must admit i just copied the shift from the original define. But the volume 
> and the samples are signed 64bit values.
> 
> @malc: any comments on this?

I don't understand the problem, the result of multiplication is converted
from 32.32 fixed point to integer by shifting right by 32.. It escapes me
how 31 fits here.

> 
> > Does the code work with FLOAT_MIXENG undefined?
> At least for my input voice, it works with integer arithmetic. 
> 
> 

-- 
mailto:av1474@comtv.ru

[Qemu-devel] RE: [PATCH] audio: split sample conversion and volumemixing

[Schildbach, Wolfgang]

Ah, I assumed that the volume would be stored in a 32 bit integer, and
be represented as a signed Q31 fixed point. Sorry for the noise.

- Wolfgang


-----Original Message-----
From: malc [mailto:av1474@comtv.ru] 
Sent: Wednesday, January 12, 2011 12:15 AM
To: Michael Walle
Cc: Schildbach, Wolfgang; Jan Kiszka; qemu-devel@nongnu.org
Subject: Re: [PATCH] audio: split sample conversion and volumemixing

On Tue, 11 Jan 2011, Michael Walle wrote:

> 
> Hi Wolfgang,
> 
> > The fixed point path for scaling in mixeng_volume() seems to be 
> > under by a factor of two, IMHO. The right shift should be by 31, not

> > 32. (Because the volume, which I assume is a signed 32 bit integer, 
> > can be 0.5 at most).
> I must admit i just copied the shift from the original define. But the

> volume and the samples are signed 64bit values.
> 
> @malc: any comments on this?

I don't understand the problem, the result of multiplication is
converted from 32.32 fixed point to integer by shifting right by 32.. It
escapes me how 31 fits here.

> 
> > Does the code work with FLOAT_MIXENG undefined?
> At least for my input voice, it works with integer arithmetic. 
> 
> 

--
mailto:av1474@comtv.ru

[Qemu-devel] Re: [PATCH] audio: split sample conversion and volume mixing

[malc]

On Wed, 5 Jan 2011, Michael Walle wrote:

> Refactor the volume mixing, so it can be reused for capturing devices.
> Additionally, it removes superfluous multiplications with the nominal
> volume within the hardware voice code path.

Thanks, applied.

[..snip..]

-- 
mailto:av1474@comtv.ru