[Qemu-devel] [PATCH 1/2] Introduce AUD_set_volume

[Qemu-devel] [PATCH 1/2] Introduce AUD_set_volume

[Jan Kiszka]

This patch adds volume and mute control to the AUD interface and cleans
up the related code.

As Andrzej suggested, per-voice volume control is dropped in favor of
two global volumes: PCM input and output. Those two should be easier
mappable on real mixer support. But for now the pre-existing soft-mixer
infrastructure is used.

Partly derived from VirtualBox's mixer support - but cleaner and without
calculation errors. :)

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 audio/alsaaudio.c       |    2 +-
 audio/audio.c           |   39 +++++++++++++++++++++++++++++++++++----
 audio/audio.h           |    8 ++++++++
 audio/audio_int.h       |    5 ++---
 audio/audio_template.h  |    1 -
 audio/dsoundaudio.c     |    4 ++--
 audio/esdaudio.c        |    2 +-
 audio/fmodaudio.c       |    4 ++--
 audio/mixeng.c          |    2 --
 audio/mixeng_template.h |   14 ++------------
 audio/ossaudio.c        |    2 +-
 11 files changed, 54 insertions(+), 29 deletions(-)

Index: b/audio/audio.c
===================================================================
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -111,14 +111,25 @@ static struct {
 
 static AudioState glob_audio_state;
 
-volume_t nominal_volume = {
+volume_t pcm_out_volume = {
     0,
 #ifdef FLOAT_MIXENG
     1.0,
     1.0
 #else
-    UINT_MAX,
-    UINT_MAX
+    INT_MAX,
+    INT_MAX
+#endif
+};
+
+volume_t pcm_in_volume = {
+    0,
+#ifdef FLOAT_MIXENG
+    1.0,
+    1.0
+#else
+    INT_MAX,
+    INT_MAX
 #endif
 };
 
@@ -1040,7 +1051,7 @@ int audio_pcm_sw_write (SWVoiceOut *sw, 
     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, &pcm_out_volume);
     }
 
     while (swlim) {
@@ -1955,3 +1966,23 @@ void AUD_del_capture (CaptureVoiceOut *c
         }
     }
 }
+
+void AUD_set_volume (audmixerctl_t mt, int mute, uint8_t lvol, uint8_t rvol)
+{
+    volume_t *vol;
+
+    switch (mt) {
+    case AUD_MIXER_PCM_OUT:
+        vol  = &pcm_out_volume;
+        break;
+    case AUD_MIXER_LINE_IN:
+        vol  = &pcm_in_volume;
+        break;
+    default:
+        return;
+    }
+
+    vol->mute = mute;
+    vol->l = (((int64_t)lvol) * 0x100000000) / 0xff;
+    vol->r = (((int64_t)rvol) * 0x100000000) / 0xff;
+}
Index: b/audio/audio.h
===================================================================
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -56,6 +56,12 @@ typedef enum {
     AUD_CNOTIFY_DISABLE
 } audcnotification_e;
 
+typedef enum
+{
+    AUD_MIXER_PCM_OUT,
+    AUD_MIXER_LINE_IN
+} audmixerctl_t;
+
 struct audio_capture_ops {
     void (*notify) (void *opaque, audcnotification_e cmd);
     void (*capture) (void *opaque, void *buf, int size);
@@ -141,6 +147,8 @@ int  AUD_is_active_in (SWVoiceIn *sw);
 void     AUD_init_time_stamp_in (SWVoiceIn *sw, QEMUAudioTimeStamp *ts);
 uint64_t AUD_get_elapsed_usec_in (SWVoiceIn *sw, QEMUAudioTimeStamp *ts);
 
+void AUD_set_volume (audmixerctl_t mt, int mute, uint8_t lvol, uint8_t rvol);
+
 static inline void *advance (void *p, int incr)
 {
     uint8_t *d = p;
Index: b/audio/audio_int.h
===================================================================
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -114,7 +114,6 @@ struct SWVoiceOut {
     int empty;
     HWVoiceOut *hw;
     char *name;
-    volume_t vol;
     struct audio_callback callback;
     LIST_ENTRY (SWVoiceOut) entries;
 };
@@ -129,7 +128,6 @@ struct SWVoiceIn {
     f_sample *clip;
     HWVoiceIn *hw;
     char *name;
-    volume_t vol;
     struct audio_callback callback;
     LIST_ENTRY (SWVoiceIn) entries;
 };
@@ -203,7 +201,8 @@ extern struct audio_driver alsa_audio_dr
 extern struct audio_driver coreaudio_audio_driver;
 extern struct audio_driver dsound_audio_driver;
 extern struct audio_driver esd_audio_driver;
-extern volume_t nominal_volume;
+extern volume_t pcm_out_volume;
+extern volume_t pcm_in_volume;
 
 void audio_pcm_init_info (struct audio_pcm_info *info, audsettings_t *as);
 void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len);
Index: b/audio/audio_template.h
===================================================================
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -491,7 +491,6 @@ SW *glue (AUD_open_, TYPE) (
     }
 
     if (sw) {
-        sw->vol = nominal_volume;
         sw->callback.fn = callback_fn;
         sw->callback.opaque = callback_opaque;
 
Index: b/audio/mixeng.c
===================================================================
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -28,8 +28,6 @@
 #define AUDIO_CAP "mixeng"
 #include "audio_int.h"
 
-#define NOVOL
-
 /* 8 bit */
 #define ENDIAN_CONVERSION natural
 #define ENDIAN_CONVERT(v) (v)
Index: b/audio/ossaudio.c
===================================================================
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -680,7 +680,7 @@ static int oss_run_in (HWVoiceIn *hw)
                 }
                 read_samples += nread >> hwshift;
                 hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift,
-                          &nominal_volume);
+                          &pcm_in_volume);
             }
 
             if (bufs[i].len - nread) {
Index: b/audio/mixeng_template.h
===================================================================
--- a/audio/mixeng_template.h
+++ b/audio/mixeng_template.h
@@ -31,15 +31,11 @@
 #define HALF (IN_MAX >> 1)
 #endif
 
-#ifdef NOVOL
-#define VOL(a, b) a
-#else
 #ifdef FLOAT_MIXENG
 #define VOL(a, b) ((a) * (b))
 #else
 #define VOL(a, b) ((a) * (b)) >> 32
 #endif
-#endif
 
 #define ET glue (ENDIAN_CONVERSION, glue (_, IN_T))
 
@@ -113,14 +109,11 @@ static void glue (glue (conv_, ET), _to_
 {
     st_sample_t *out = dst;
     IN_T *in = (IN_T *) src;
-#ifndef NOVOL
+
     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);
@@ -133,14 +126,11 @@ static void glue (glue (conv_, ET), _to_
 {
     st_sample_t *out = dst;
     IN_T *in = (IN_T *) src;
-#ifndef NOVOL
+
     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->r = out->l;
Index: b/audio/alsaaudio.c
===================================================================
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -902,7 +902,7 @@ static int alsa_run_in (HWVoiceIn *hw)
                 }
             }
 
-            hw->conv (dst, src, nread, &nominal_volume);
+            hw->conv (dst, src, nread, &pcm_in_volume);
 
             src = advance (src, nread << hwshift);
             dst += nread;
Index: b/audio/dsoundaudio.c
===================================================================
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -927,11 +927,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, &pcm_in_volume);
     }
 
     if (p2 && len2) {
-        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
+        hw->conv (hw->conv_buf, p2, len2, &pcm_in_volume);
     }
 
     dsound_unlock_in (dscb, p1, p2, blen1, blen2);
Index: b/audio/esdaudio.c
===================================================================
--- a/audio/esdaudio.c
+++ b/audio/esdaudio.c
@@ -368,7 +368,7 @@ static void *qesd_thread_in (void *arg)
             }
 
             hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift,
-                      &nominal_volume);
+                      &pcm_in_volume);
             wpos = (wpos + chunk) % hw->samples;
             to_grab -= chunk;
         }
Index: b/audio/fmodaudio.c
===================================================================
--- a/audio/fmodaudio.c
+++ b/audio/fmodaudio.c
@@ -502,10 +502,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, &pcm_in_volume);
     }
     if (p2 && len2) {
-        hw->conv (hw->conv_buf, p2, len2, &nominal_volume);
+        hw->conv (hw->conv_buf, p2, len2, &pcm_in_volume);
     }
 
     fmod_unlock_sample (fmd->fmod_sample, p1, p2, blen1, blen2);

Re: [Qemu-devel] [PATCH 1/2] Introduce AUD_set_volume

[malc]

On Sun, 27 Apr 2008, Jan Kiszka wrote:

> This patch adds volume and mute control to the AUD interface and cleans
> up the related code.
>
> As Andrzej suggested, per-voice volume control is dropped in favor of
> two global volumes: PCM input and output. Those two should be easier
> mappable on real mixer support. But for now the pre-existing soft-mixer
> infrastructure is used.

If the real machine has N audio cards chances that they are routed
through the same mixer are zero, in other words each and every one has
it's own volume controls/mute state etc.

As for "real mixer" support then i must note that it's nearly
impossible to get this thing working correctly, it's more of an
annoynace than benefit.

>
> Partly derived from VirtualBox's mixer support - but cleaner and without
> calculation errors. :)
>

Shrug.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 1/2] Introduce AUD_set_volume

[andrzej zaborowski]

I'd like to merge the following instead, I see no point in removing
per voice volume control if we're still doing scaling in software.  On
top of that I would apply your wm8750 patch - I have it running now
and volume setting in MusicPal UI works nicely.

The user still needs to disable NOVOL if they really want volume control.

diff --git a/audio/audio.c b/audio/audio.c
index 9e7fe80..1231ec5 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1955,3 +1955,21 @@ void AUD_del_capture (CaptureVoiceOut *cap,
void *cb_opaque)
         }
     }
 }
+
+void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+    if (sw) {
+        sw->vol.mute = mute;
+        sw->vol.l = nominal_volume.l * lvol / 255;
+        sw->vol.r = nominal_volume.r * rvol / 255;
+    }
+}
+
+void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+    if (sw) {
+        sw->vol.mute = mute;
+        sw->vol.l = nominal_volume.l * lvol / 255;
+        sw->vol.r = nominal_volume.r * rvol / 255;
+    }
+}
diff --git a/audio/audio.h b/audio/audio.h
index ec9eee4..ae29b8a 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -124,6 +124,9 @@ int  AUD_is_active_out (SWVoiceOut *sw);
 void     AUD_init_time_stamp_out (SWVoiceOut *sw, QEMUAudioTimeStamp *ts);
 uint64_t AUD_get_elapsed_usec_out (SWVoiceOut *sw, QEMUAudioTimeStamp *ts);

+void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol);
+void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol);
+
 SWVoiceIn *AUD_open_in (
     QEMUSoundCard *card,
     SWVoiceIn *sw,
-- 
Please do not print this email unless absolutely necessary. Spread
environmental awareness.

Re: [Qemu-devel] [PATCH 1/2] Introduce AUD_set_volume

[Jan Kiszka]

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

andrzej zaborowski wrote:
> I'd like to merge the following instead, I see no point in removing
> per voice volume control if we're still doing scaling in software.  On
> top of that I would apply your wm8750 patch - I have it running now
> and volume setting in MusicPal UI works nicely.

No fundamental concerns.

> 
> The user still needs to disable NOVOL if they really want volume control.

What is the "user" here? wm8750? musicpal? I just hope it is not the
real user... :)

[ QEMU audio setup is already weird enough: I recently tried to get ALSA
running to overcome outdated OSS - well, tricky. Needs fixing. ]

> diff --git a/audio/audio.c b/audio/audio.c
> index 9e7fe80..1231ec5 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1955,3 +1955,21 @@ void AUD_del_capture (CaptureVoiceOut *cap,
> void *cb_opaque)
>          }
>      }
>  }
> +
> +void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol)
> +{
> +    if (sw) {
> +        sw->vol.mute = mute;
> +        sw->vol.l = nominal_volume.l * lvol / 255;
> +        sw->vol.r = nominal_volume.r * rvol / 255;

As this scaling looks fairly different from my version, I have to test
and think about it first. But my current feeling is that you do not get
the same range. But the API is fine.

> +    }
> +}
> +
> +void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol)
> +{
> +    if (sw) {
> +        sw->vol.mute = mute;
> +        sw->vol.l = nominal_volume.l * lvol / 255;
> +        sw->vol.r = nominal_volume.r * rvol / 255;
> +    }
> +}
> diff --git a/audio/audio.h b/audio/audio.h
> index ec9eee4..ae29b8a 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -124,6 +124,9 @@ int  AUD_is_active_out (SWVoiceOut *sw);
>  void     AUD_init_time_stamp_out (SWVoiceOut *sw, QEMUAudioTimeStamp *ts);
>  uint64_t AUD_get_elapsed_usec_out (SWVoiceOut *sw, QEMUAudioTimeStamp *ts);
> 
> +void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol);
> +void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol);
> +
>  SWVoiceIn *AUD_open_in (
>      QEMUSoundCard *card,
>      SWVoiceIn *sw,

Jan


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

Re: [Qemu-devel] [PATCH 1/2] Introduce AUD_set_volume

[andrzej zaborowski]

On 02/05/2008, Jan Kiszka <jan.kiszka@web.de> wrote:
> andrzej zaborowski wrote:
>  > The user still needs to disable NOVOL if they really want volume control.
>
> What is the "user" here? wm8750? musicpal? I just hope it is not the
>  real user... :)

Whoever builds qemu.  You don't want it enabled by default and globally.

>
>  [ QEMU audio setup is already weird enough: I recently tried to get ALSA
>  running to overcome outdated OSS - well, tricky. Needs fixing. ]

Yup, I had alsa running but normally (for example now) I just run qemu
through "aoss".

>
>  > diff --git a/audio/audio.c b/audio/audio.c
>  > index 9e7fe80..1231ec5 100644
>  > --- a/audio/audio.c
>  > +++ b/audio/audio.c
>  > @@ -1955,3 +1955,21 @@ void AUD_del_capture (CaptureVoiceOut *cap,
>  > void *cb_opaque)
>  >          }
>  >      }
>  >  }
>  > +
>  > +void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol)
>  > +{
>  > +    if (sw) {
>  > +        sw->vol.mute = mute;
>  > +        sw->vol.l = nominal_volume.l * lvol / 255;
>  > +        sw->vol.r = nominal_volume.r * rvol / 255;

Oops, it may be wrong indeed because it might overflow on machines
with sizeof(int) >= sizeof(int64_t) and no FLOAT_MIXENG.  I first
wrote (nominal_volume.l / 255) * lvol and then immediately changed it,
but that was better.

Regards
-- 
Please do not print this email unless absolutely necessary. Spread
environmental awareness.

Re: [Qemu-devel] [PATCH 1/2] Introduce AUD_set_volume

[Jan Kiszka]

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

andrzej zaborowski wrote:
> On 02/05/2008, Jan Kiszka <jan.kiszka@web.de> wrote:
>> andrzej zaborowski wrote:
>>  > The user still needs to disable NOVOL if they really want volume control.
>>
>> What is the "user" here? wm8750? musicpal? I just hope it is not the
>>  real user... :)
> 
> Whoever builds qemu.  You don't want it enabled by default and globally.

That's the totally wrong way from the usability perspective, IMHO. What
are the remaining downsides of mixer support? If there are any, I guess
they are far less than the advantages, so this switch should at least
become an option which is default off.

> 
>>  [ QEMU audio setup is already weird enough: I recently tried to get ALSA
>>  running to overcome outdated OSS - well, tricky. Needs fixing. ]
> 
> Yup, I had alsa running but normally (for example now) I just run qemu
> through "aoss".

I had problems with parallel usage by several apps, and only true ALSA
mode resolved it. I think AOSS is purely for unconvertible legacy apps,
and I don't want to put QEMU into this category. ;)

Will follow up on this the next days.

> 
>>  > diff --git a/audio/audio.c b/audio/audio.c
>>  > index 9e7fe80..1231ec5 100644
>>  > --- a/audio/audio.c
>>  > +++ b/audio/audio.c
>>  > @@ -1955,3 +1955,21 @@ void AUD_del_capture (CaptureVoiceOut *cap,
>>  > void *cb_opaque)
>>  >          }
>>  >      }
>>  >  }
>>  > +
>>  > +void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol)
>>  > +{
>>  > +    if (sw) {
>>  > +        sw->vol.mute = mute;
>>  > +        sw->vol.l = nominal_volume.l * lvol / 255;
>>  > +        sw->vol.r = nominal_volume.r * rvol / 255;
> 
> Oops, it may be wrong indeed because it might overflow on machines
> with sizeof(int) >= sizeof(int64_t) and no FLOAT_MIXENG.  I first
> wrote (nominal_volume.l / 255) * lvol and then immediately changed it,
> but that was better.

Thought a bit about it (still untested, though), and I think it should
be fine to just fix the initialization of nominal_volume from UINT_MAX
to (1ULL << 32) (because we are shifting back by 32 bits at runtime). I
had that wrong in my patch as well, BTW.

Jan


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