[Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Marc-André Lureau]

Hello,

This patch series implements client-side audio volume support. This reduces confusion of guest users when volume control is not effective (because mixemu is disabled or because client-side is muted and can't be unmuted by the guest..)

Instead, the backend is responsible for applying volume giving the guest control over the full range of the client, and avoiding multiple level of volume/mute effects.

Although I was mainly interested in having the Spice audio backend support, I also added PulseAudio backend support (which unfortunately requires full-blown API, even after quick discussion with upstream).

Marc-André Lureau (11):
  audio: add VOICE_VOLUME ctl
  audio: don't apply volume effect if backend has VOICE_VOLUME_CAP
  audio: use a nominal volume of UINT_MAX
  hw/ac97: remove USE_MIXER code
  hw/ac97: the volume mask is not only 0x1f
  hw/ac97: add support for volume control
  audio/spice: add support for volume control
  Do not use pa_simple PulseAudio API
  configure: pa_simple is not needed anymore
  Allow controlling volume with PulseAudio backend
  Make mixemu mandatory

 audio/audio.c          |   25 +++-
 audio/audio_int.h      |    6 +
 audio/audio_template.h |    2 +
 audio/mixeng.c         |    6 -
 audio/paaudio.c        |  452 +++++++++++++++++++++++++++++++++++++++++++-----
 audio/spiceaudio.c     |   41 +++++
 configure              |   14 +--
 hw/ac97.c              |  141 ++++++----------
 hw/hda-audio.c         |    4 -
 9 files changed, 531 insertions(+), 160 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH 01/11] audio: add VOICE_VOLUME ctl

[Marc-André Lureau]

Add a new PCM control operation to update the stream volume on the
audio backend.  The argument given is a SWVoiceOut/SWVoiceIn.
---
 audio/audio.c     |   12 ++++++++++++
 audio/audio_int.h |    1 +
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 398763f..d76c342 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2053,17 +2053,29 @@ 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) {
+        HWVoiceOut *hw = sw->hw;
+
         sw->vol.mute = mute;
         sw->vol.l = nominal_volume.l * lvol / 255;
         sw->vol.r = nominal_volume.r * rvol / 255;
+
+        if (hw->pcm_ops->ctl_out) {
+            hw->pcm_ops->ctl_out (hw, VOICE_VOLUME, sw);
+        }
     }
 }
 
 void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol)
 {
     if (sw) {
+        HWVoiceIn *hw = sw->hw;
+
         sw->vol.mute = mute;
         sw->vol.l = nominal_volume.l * lvol / 255;
         sw->vol.r = nominal_volume.r * rvol / 255;
+
+        if (hw->pcm_ops->ctl_in) {
+            hw->pcm_ops->ctl_in (hw, VOICE_VOLUME, sw);
+        }
     }
 }
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 2003f8b..117f95e 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -231,6 +231,7 @@ void audio_run (const char *msg);
 
 #define VOICE_ENABLE 1
 #define VOICE_DISABLE 2
+#define VOICE_VOLUME 3
 
 static inline int audio_ring_dist (int dst, int src, int len)
 {
-- 
1.7.7.6

[Qemu-devel] [PATCH 02/11] audio: don't apply volume effect if backend has VOICE_VOLUME_CAP

[Marc-André Lureau]

If the audio backend is capable of volume control, don't apply
software volume (mixeng_volume ()), but instead, rely on backend
volume control. This will allow guest to have full range volume
control.
---
 audio/audio.c          |    9 +++++++--
 audio/audio_int.h      |    5 +++++
 audio/audio_template.h |    2 ++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index d76c342..bd9237e 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -957,7 +957,9 @@ int audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int size)
         total += isamp;
     }
 
-    mixeng_volume (sw->buf, ret, &sw->vol);
+    if (!(hw->ctl_caps & VOICE_VOLUME_CAP)) {
+        mixeng_volume (sw->buf, ret, &sw->vol);
+    }
 
     sw->clip (buf, sw->buf, ret);
     sw->total_hw_samples_acquired += total;
@@ -1041,7 +1043,10 @@ int audio_pcm_sw_write (SWVoiceOut *sw, void *buf, int size)
     swlim = audio_MIN (swlim, samples);
     if (swlim) {
         sw->conv (sw->buf, buf, swlim);
-        mixeng_volume (sw->buf, swlim, &sw->vol);
+
+        if (!(sw->hw->ctl_caps & VOICE_VOLUME_CAP)) {
+            mixeng_volume (sw->buf, swlim, &sw->vol);
+        }
     }
 
     while (swlim) {
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 117f95e..b9b0676 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -82,6 +82,7 @@ typedef struct HWVoiceOut {
     int samples;
     QLIST_HEAD (sw_out_listhead, SWVoiceOut) sw_head;
     QLIST_HEAD (sw_cap_listhead, SWVoiceCap) cap_head;
+    int ctl_caps;
     struct audio_pcm_ops *pcm_ops;
     QLIST_ENTRY (HWVoiceOut) entries;
 } HWVoiceOut;
@@ -101,6 +102,7 @@ typedef struct HWVoiceIn {
 
     int samples;
     QLIST_HEAD (sw_in_listhead, SWVoiceIn) sw_head;
+    int ctl_caps;
     struct audio_pcm_ops *pcm_ops;
     QLIST_ENTRY (HWVoiceIn) entries;
 } HWVoiceIn;
@@ -150,6 +152,7 @@ struct audio_driver {
     int max_voices_in;
     int voice_size_out;
     int voice_size_in;
+    int ctl_caps;
 };
 
 struct audio_pcm_ops {
@@ -233,6 +236,8 @@ void audio_run (const char *msg);
 #define VOICE_DISABLE 2
 #define VOICE_VOLUME 3
 
+#define VOICE_VOLUME_CAP (1 << VOICE_VOLUME)
+
 static inline int audio_ring_dist (int dst, int src, int len)
 {
     return (dst >= src) ? (dst - src) : (len - src + dst);
diff --git a/audio/audio_template.h b/audio/audio_template.h
index e62a713..519432a 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -263,6 +263,8 @@ static HW *glue (audio_pcm_hw_add_new_, TYPE) (struct audsettings *as)
     }
 
     hw->pcm_ops = drv->pcm_ops;
+    hw->ctl_caps = drv->ctl_caps;
+
     QLIST_INIT (&hw->sw_head);
 #ifdef DAC
     QLIST_INIT (&hw->cap_head);
-- 
1.7.7.6

[Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[Marc-André Lureau]

It's more appropriate to set the maximum value into a fitting integer.
---
 audio/audio.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index bd9237e..06c2384 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -110,8 +110,8 @@ const struct mixeng_volume nominal_volume = {
     .r = 1.0,
     .l = 1.0,
 #else
-    .r = 1ULL << 32,
-    .l = 1ULL << 32,
+    .r = UINT_MAX,
+    .l = UINT_MAX,
 #endif
 };
 
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> It's more appropriate to set the maximum value into a fitting integer.
> ---
>  audio/audio.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index bd9237e..06c2384 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -110,8 +110,8 @@ const struct mixeng_volume nominal_volume = {
>      .r = 1.0,
>      .l = 1.0,
>  #else
> -    .r = 1ULL << 32,
> -    .l = 1ULL << 32,
> +    .r = UINT_MAX,
> +    .l = UINT_MAX,
>  #endif
>  };

How's putting UINT_MAX into int64_t more appropriate? UINT_MAX is
even one less than one in 32.32 fixpoint... I must be missing something
here.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[Marc-André Lureau]

On Mon, Mar 12, 2012 at 9:03 PM, malc <av1474@comtv.ru> wrote:
> How's putting UINT_MAX into int64_t more appropriate? UINT_MAX is
> even one less than one in 32.32 fixpoint... I must be missing something
> here.

Right, the patch series used to have 2^32 -1, so it can fit in a
16bits or 32bits integer.

So what I meant is rather G_MAXUINT32.

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> On Mon, Mar 12, 2012 at 9:03 PM, malc <av1474@comtv.ru> wrote:
> > How's putting UINT_MAX into int64_t more appropriate? UINT_MAX is
> > even one less than one in 32.32 fixpoint... I must be missing something
> > here.
> 
> Right, the patch series used to have 2^32 -1, so it can fit in a
> 16bits or 32bits integer.
> 
> So what I meant is rather G_MAXUINT32.

Just leave it as is, it's perfectly fine. 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[Marc-André Lureau]

On Mon, Mar 12, 2012 at 9:29 PM, malc <av1474@comtv.ru> wrote:
>> So what I meant is rather G_MAXUINT32.
>
> Just leave it as is, it's perfectly fine.

But it's not convenient to fit into a regular integer, as the number
of steps is 2^32 + 1. Why is it preferrable this way rather than a
MAXUINT? I must be missing something. Hardware range for example is
usually  0...2^nbits-1

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> On Mon, Mar 12, 2012 at 9:29 PM, malc <av1474@comtv.ru> wrote:
> >> So what I meant is rather G_MAXUINT32.
> >
> > Just leave it as is, it's perfectly fine.
> 
> But it's not convenient to fit into a regular integer, as the number
> of steps is 2^32 + 1. Why is it preferrable this way rather than a
> MAXUINT? I must be missing something. Hardware range for example is
> usually  0...2^nbits-1
> 

I really do not understand your point at all, mixeng_volume fields
are int64_t's it's irrelevant what you initialize them with, they
are still 64bit integers, and UINT_MAX as i already said is not even
correct, it's 1/(2**32-1) less than one in fixed point.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[Marc-André Lureau]

On Mon, Mar 12, 2012 at 10:00 PM, malc <av1474@comtv.ru> wrote:
> I really do not understand your point at all, mixeng_volume fields
> are int64_t's it's irrelevant what you initialize them with, they
> are still 64bit integers, and UINT_MAX as i already said is not even
> correct, it's 1/(2**32-1) less than one in fixed point.

But the current code does this:

AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol..)

sw->vol.l = nominal_volume.l * lvol / 255;

So the range was [0..2^8-1] and then it becomes [0..2^32], which looks
wrong to me.

And Spice uses [0..2^16-1]. So it is simpler to stay within the range
of an integer..

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> On Mon, Mar 12, 2012 at 10:00 PM, malc <av1474@comtv.ru> wrote:
> > I really do not understand your point at all, mixeng_volume fields
> > are int64_t's it's irrelevant what you initialize them with, they
> > are still 64bit integers, and UINT_MAX as i already said is not even
> > correct, it's 1/(2**32-1) less than one in fixed point.
> 
> But the current code does this:
> 
> AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol..)
> 
> sw->vol.l = nominal_volume.l * lvol / 255;
> 
> So the range was [0..2^8-1] and then it becomes [0..2^32], which looks
> wrong to me.

It actually becomes [-2^63..2^63-1] nominal_volume.l being 64 bit signed
and all.

> 
> And Spice uses [0..2^16-1]. So it is simpler to stay within the range
> of an integer..

I do not get you, i really don't.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[Marc-André Lureau]

On Mon, Mar 12, 2012 at 10:11 PM, malc <av1474@comtv.ru> wrote:
>> AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol..)
>>
>> sw->vol.l = nominal_volume.l * lvol / 255;
>>
>> So the range was [0..2^8-1] and then it becomes [0..2^32], which looks
>> wrong to me.
>
> It actually becomes [-2^63..2^63-1] nominal_volume.l being 64 bit signed
> and all.

uint8_t [0..255] / 255 * 2^32 = [0..2^32]

>> And Spice uses [0..2^16-1]. So it is simpler to stay within the range
>> of an integer..
>
> I do not get you, i really don't.

The audio hw volume range is within a uint8 [0..2^8-1] that is then
scaled into a [0..2^32], it would be easier to stay within a
[0..2^n-1] range all the way.


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> On Mon, Mar 12, 2012 at 10:11 PM, malc <av1474@comtv.ru> wrote:
> >> AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol..)
> >>
> >> sw->vol.l = nominal_volume.l * lvol / 255;
> >>
> >> So the range was [0..2^8-1] and then it becomes [0..2^32], which looks
> >> wrong to me.
> >
> > It actually becomes [-2^63..2^63-1] nominal_volume.l being 64 bit signed
> > and all.
> 
> uint8_t [0..255] / 255 * 2^32 = [0..2^32]
> 

nominal_volume.[lr] is int64_t... i don't get where you take this ^32
from.
 

> >> And Spice uses [0..2^16-1]. So it is simpler to stay within the range
> >> of an integer..
> >
> > I do not get you, i really don't.
> 
> The audio hw volume range is within a uint8 [0..2^8-1] that is then
> scaled into a [0..2^32], it would be easier to stay within a
> [0..2^n-1] range all the way.
> 
> 
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[Marc-André Lureau]

On Mon, Mar 12, 2012 at 10:42 PM, malc <av1474@comtv.ru> wrote:
> nominal_volume.[lr] is int64_t... i don't get where you take this ^32
> from.

from where it is defined, audio/audio.c:

const struct mixeng_volume nominal_volume = {
    .mute = 0,
#ifdef FLOAT_MIXENG
    .r = 1.0,
    .l = 1.0,
#else
    .r = 1ULL << 32,
    .l = 1ULL << 32,
#endif
};


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> On Mon, Mar 12, 2012 at 10:42 PM, malc <av1474@comtv.ru> wrote:
> > nominal_volume.[lr] is int64_t... i don't get where you take this ^32
> > from.
> 
> from where it is defined, audio/audio.c:
> 
> const struct mixeng_volume nominal_volume = {
>     .mute = 0,
> #ifdef FLOAT_MIXENG
>     .r = 1.0,
>     .l = 1.0,
> #else
>     .r = 1ULL << 32,
>     .l = 1ULL << 32,
> #endif
> };
> 

1 << 32 = 1.0 in 64(32.32) fixed point... the type of l/r is int64_t

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 03/11] audio: use a nominal volume of UINT_MAX

[Marc-André Lureau]

On Mon, Mar 12, 2012 at 11:52 PM, malc <av1474@comtv.ru> wrote:
> 1 << 32 = 1.0 in 64(32.32) fixed point... the type of l/r is int64_t

Ok, I'll leave it as is then if it is on purpose.

-- 
Marc-André Lureau

[Qemu-devel] [PATCH 04/11] hw/ac97: remove USE_MIXER code

[Marc-André Lureau]

That code doesn't compile. The interesting bits for volume control are
going to be rewritten in the following patch.
---
 hw/ac97.c |  121 -------------------------------------------------------------
 1 files changed, 0 insertions(+), 121 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index c0fd019..f2804e6 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -437,99 +437,6 @@ static void reset_voices (AC97LinkState *s, uint8_t active[LAST_INDEX])
     AUD_set_active_in (s->voice_mc, active[MC_INDEX]);
 }
 
-#ifdef USE_MIXER
-static void set_volume (AC97LinkState *s, int index,
-                        audmixerctl_t mt, uint32_t val)
-{
-    int mute = (val >> MUTE_SHIFT) & 1;
-    uint8_t rvol = VOL_MASK - (val & VOL_MASK);
-    uint8_t lvol = VOL_MASK - ((val >> 8) & VOL_MASK);
-    rvol = 255 * rvol / VOL_MASK;
-    lvol = 255 * lvol / VOL_MASK;
-
-#ifdef SOFT_VOLUME
-    if (index == AC97_Master_Volume_Mute) {
-        AUD_set_volume_out (s->voice_po, mute, lvol, rvol);
-    }
-    else {
-        AUD_set_volume (mt, &mute, &lvol, &rvol);
-    }
-#else
-    AUD_set_volume (mt, &mute, &lvol, &rvol);
-#endif
-
-    rvol = VOL_MASK - ((VOL_MASK * rvol) / 255);
-    lvol = VOL_MASK - ((VOL_MASK * lvol) / 255);
-    mixer_store (s, index, val);
-}
-
-static audrecsource_t ac97_to_aud_record_source (uint8_t i)
-{
-    switch (i) {
-    case REC_MIC:
-        return AUD_REC_MIC;
-
-    case REC_CD:
-        return AUD_REC_CD;
-
-    case REC_VIDEO:
-        return AUD_REC_VIDEO;
-
-    case REC_AUX:
-        return AUD_REC_AUX;
-
-    case REC_LINE_IN:
-        return AUD_REC_LINE_IN;
-
-    case REC_PHONE:
-        return AUD_REC_PHONE;
-
-    default:
-        dolog ("Unknown record source %d, using MIC\n", i);
-        return AUD_REC_MIC;
-    }
-}
-
-static uint8_t aud_to_ac97_record_source (audrecsource_t rs)
-{
-    switch (rs) {
-    case AUD_REC_MIC:
-        return REC_MIC;
-
-    case AUD_REC_CD:
-        return REC_CD;
-
-    case AUD_REC_VIDEO:
-        return REC_VIDEO;
-
-    case AUD_REC_AUX:
-        return REC_AUX;
-
-    case AUD_REC_LINE_IN:
-        return REC_LINE_IN;
-
-    case AUD_REC_PHONE:
-        return REC_PHONE;
-
-    default:
-        dolog ("Unknown audio recording source %d using MIC\n", rs);
-        return REC_MIC;
-    }
-}
-
-static void record_select (AC97LinkState *s, uint32_t val)
-{
-    uint8_t rs = val & REC_MASK;
-    uint8_t ls = (val >> 8) & REC_MASK;
-    audrecsource_t ars = ac97_to_aud_record_source (rs);
-    audrecsource_t als = ac97_to_aud_record_source (ls);
-    AUD_set_record_source (&als, &ars);
-    rs = aud_to_ac97_record_source (ars);
-    ls = aud_to_ac97_record_source (als);
-    mixer_store (s, AC97_Record_Select, rs | (ls << 8));
-}
-#endif
-
 static void mixer_reset (AC97LinkState *s)
 {
     uint8_t active[LAST_INDEX];
@@ -564,12 +471,6 @@ static void mixer_reset (AC97LinkState *s)
     mixer_store (s, AC97_PCM_LR_ADC_Rate         , 0xbb80);
     mixer_store (s, AC97_MIC_ADC_Rate            , 0xbb80);
 
-#ifdef USE_MIXER
-    record_select (s, 0);
-    set_volume (s, AC97_Master_Volume_Mute, AUD_MIXER_VOLUME  , 0x8000);
-    set_volume (s, AC97_PCM_Out_Volume_Mute, AUD_MIXER_PCM    , 0x8808);
-    set_volume (s, AC97_Line_In_Volume_Mute, AUD_MIXER_LINE_IN, 0x8808);
-#endif
     reset_voices (s, active);
 }
 
@@ -628,20 +529,6 @@ static void nam_writew (void *opaque, uint32_t addr, uint32_t val)
         val |= mixer_load (s, index) & 0xf;
         mixer_store (s, index, val);
         break;
-#ifdef USE_MIXER
-    case AC97_Master_Volume_Mute:
-        set_volume (s, index, AUD_MIXER_VOLUME, val);
-        break;
-    case AC97_PCM_Out_Volume_Mute:
-        set_volume (s, index, AUD_MIXER_PCM, val);
-        break;
-    case AC97_Line_In_Volume_Mute:
-        set_volume (s, index, AUD_MIXER_LINE_IN, val);
-        break;
-    case AC97_Record_Select:
-        record_select (s, val);
-        break;
-#endif
     case AC97_Vendor_ID1:
     case AC97_Vendor_ID2:
         dolog ("Attempt to write vendor ID to %#x\n", val);
@@ -1194,14 +1081,6 @@ static int ac97_post_load (void *opaque, int version_id)
     uint8_t active[LAST_INDEX];
     AC97LinkState *s = opaque;
 
-#ifdef USE_MIXER
-    record_select (s, mixer_load (s, AC97_Record_Select));
-#define V_(a, b) set_volume (s, a, b, mixer_load (s, a))
-    V_ (AC97_Master_Volume_Mute, AUD_MIXER_VOLUME);
-    V_ (AC97_PCM_Out_Volume_Mute, AUD_MIXER_PCM);
-    V_ (AC97_Line_In_Volume_Mute, AUD_MIXER_LINE_IN);
-#undef V_
-#endif
     active[PI_INDEX] = !!(s->bm_regs[PI_INDEX].cr & CR_RPBM);
     active[PO_INDEX] = !!(s->bm_regs[PO_INDEX].cr & CR_RPBM);
     active[MC_INDEX] = !!(s->bm_regs[MC_INDEX].cr & CR_RPBM);
-- 
1.7.7.6

[Qemu-devel] [PATCH 05/11] hw/ac97: the volume mask is not only 0x1f

[Marc-André Lureau]

It's a case by case (see Table 66. AC ‘97 Baseline Audio Register Map)
---
 hw/ac97.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index f2804e6..f7866ed 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -118,7 +118,6 @@ enum {
 #define EACS_VRA 1
 #define EACS_VRM 8
 
-#define VOL_MASK 0x1f
 #define MUTE_SHIFT 15
 
 #define REC_MASK 7
-- 
1.7.7.6

[Qemu-devel] [PATCH 06/11] hw/ac97: add support for volume control

[Marc-André Lureau]

Combine output volume with Master and PCM registers values.
Use default values in mixer_reset ().
Set volume on post-load to update backend values.
---
 hw/ac97.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index f7866ed..227233c 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -436,6 +436,63 @@ static void reset_voices (AC97LinkState *s, uint8_t active[LAST_INDEX])
     AUD_set_active_in (s->voice_mc, active[MC_INDEX]);
 }
 
+static void get_volume (uint16_t vol, uint16_t mask, int inverse,
+                        int *mute, uint8_t *lvol, uint8_t *rvol)
+{
+  *mute = (vol >> MUTE_SHIFT) & 1;
+  *rvol = (255 * (vol & mask)) / mask;
+  *lvol = (255 * ((vol >> 8) & mask)) / mask;
+  if (inverse) {
+    *rvol = 255 - *rvol;
+    *lvol = 255 - *lvol;
+  }
+}
+
+static void update_combined_volume_out (AC97LinkState *s)
+{
+    uint8_t lvol, rvol, plvol, prvol;
+    int mute, pmute;
+
+    get_volume (mixer_load (s, AC97_Master_Volume_Mute), 0x3f, 1,
+                &mute, &lvol, &rvol);
+    /* FIXME: should be 1f according to spec */
+    get_volume (mixer_load (s, AC97_PCM_Out_Volume_Mute), 0x3f, 1,
+                &pmute, &plvol, &prvol);
+
+    mute = mute | pmute;
+    lvol = (lvol * plvol) / 255;
+    rvol = (rvol * prvol) / 255;
+
+    AUD_set_volume_out (s->voice_po, mute, lvol, rvol);
+}
+
+static void update_volume_in(AC97LinkState *s)
+{
+    uint8_t lvol, rvol;
+    int mute;
+
+    get_volume (mixer_load (s, AC97_Record_Gain_Mute), 0x0f, 0,
+                &mute, &lvol, &rvol);
+
+    AUD_set_volume_in (s->voice_pi, mute, lvol, rvol);
+}
+
+static void set_volume (AC97LinkState *s, int index, uint32_t val)
+{
+    mixer_store (s, index, val);
+    if (index == AC97_Master_Volume_Mute || index == AC97_PCM_Out_Volume_Mute)
+      update_combined_volume_out (s);
+    else if (index == AC97_Record_Gain_Mute)
+      update_volume_in (s);
+}
+
+static void record_select (AC97LinkState *s, uint32_t val)
+{
+    uint8_t rs = val & REC_MASK;
+    uint8_t ls = (val >> 8) & REC_MASK;
+    mixer_store (s, AC97_Record_Select, rs | (ls << 8));
+}
+
 static void mixer_reset (AC97LinkState *s)
 {
     uint8_t active[LAST_INDEX];
@@ -470,6 +527,11 @@ static void mixer_reset (AC97LinkState *s)
     mixer_store (s, AC97_PCM_LR_ADC_Rate         , 0xbb80);
     mixer_store (s, AC97_MIC_ADC_Rate            , 0xbb80);
 
+    record_select (s, 0);
+    set_volume (s, AC97_Master_Volume_Mute, 0x8000);
+    set_volume (s, AC97_PCM_Out_Volume_Mute, 0x8808);
+    set_volume (s, AC97_Line_In_Volume_Mute, 0x8808);
+
     reset_voices (s, active);
 }
 
@@ -528,6 +590,15 @@ static void nam_writew (void *opaque, uint32_t addr, uint32_t val)
         val |= mixer_load (s, index) & 0xf;
         mixer_store (s, index, val);
         break;
+    case AC97_PCM_Out_Volume_Mute:
+    case AC97_Master_Volume_Mute:
+    case AC97_Record_Gain_Mute:
+    case AC97_Line_In_Volume_Mute:
+        set_volume (s, index, val);
+        break;
+    case AC97_Record_Select:
+        record_select (s, val);
+        break;
     case AC97_Vendor_ID1:
     case AC97_Vendor_ID2:
         dolog ("Attempt to write vendor ID to %#x\n", val);
@@ -1080,6 +1151,14 @@ static int ac97_post_load (void *opaque, int version_id)
     uint8_t active[LAST_INDEX];
     AC97LinkState *s = opaque;
 
+    record_select (s, mixer_load (s, AC97_Record_Select));
+    set_volume (s, AC97_Master_Volume_Mute,
+                mixer_load (s, AC97_Master_Volume_Mute));
+    set_volume (s, AC97_PCM_Out_Volume_Mute,
+                mixer_load (s, AC97_PCM_Out_Volume_Mute));
+    set_volume (s, AC97_Line_In_Volume_Mute,
+                mixer_load (s, AC97_Line_In_Volume_Mute));
+
     active[PI_INDEX] = !!(s->bm_regs[PI_INDEX].cr & CR_RPBM);
     active[PO_INDEX] = !!(s->bm_regs[PO_INDEX].cr & CR_RPBM);
     active[MC_INDEX] = !!(s->bm_regs[MC_INDEX].cr & CR_RPBM);
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 06/11] hw/ac97: add support for volume control

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> Combine output volume with Master and PCM registers values.
> Use default values in mixer_reset ().
> Set volume on post-load to update backend values.
> ---
>  hw/ac97.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 79 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/ac97.c b/hw/ac97.c
> index f7866ed..227233c 100644
> --- a/hw/ac97.c
> +++ b/hw/ac97.c
> @@ -436,6 +436,63 @@ static void reset_voices (AC97LinkState *s, uint8_t active[LAST_INDEX])
>      AUD_set_active_in (s->voice_mc, active[MC_INDEX]);
>  }
>  
> +static void get_volume (uint16_t vol, uint16_t mask, int inverse,
> +                        int *mute, uint8_t *lvol, uint8_t *rvol)
> +{
> +  *mute = (vol >> MUTE_SHIFT) & 1;
> +  *rvol = (255 * (vol & mask)) / mask;
> +  *lvol = (255 * ((vol >> 8) & mask)) / mask;
> +  if (inverse) {
> +    *rvol = 255 - *rvol;
> +    *lvol = 255 - *lvol;
> +  }
> +}

Indentation is off here.

[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 07/11] audio/spice: add support for volume control

[Marc-André Lureau]

Use Spice server volume control API when available.
---
 audio/spiceaudio.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index f972110..92964ae 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -202,7 +202,26 @@ static int line_out_ctl (HWVoiceOut *hw, int cmd, ...)
         }
         spice_server_playback_stop (&out->sin);
         break;
+    case VOICE_VOLUME:
+        {
+#if ((SPICE_INTERFACE_PLAYBACK_MAJOR >= 1) && (SPICE_INTERFACE_PLAYBACK_MINOR >= 2))
+            SWVoiceOut *sw;
+            va_list ap;
+            uint16_t vol[2];
+
+            va_start (ap, cmd);
+            sw = va_arg (ap, SWVoiceOut *);
+            va_end (ap);
+
+            vol[0] = sw->vol.l >> 16;
+            vol[1] = sw->vol.r >> 16;
+            spice_server_playback_set_volume (&out->sin, 2, vol);
+            spice_server_playback_set_mute (&out->sin, sw->vol.mute);
+#endif
+            break;
+        }
     }
+
     return 0;
 }
 
@@ -304,7 +323,26 @@ static int line_in_ctl (HWVoiceIn *hw, int cmd, ...)
         in->active = 0;
         spice_server_record_stop (&in->sin);
         break;
+    case VOICE_VOLUME:
+        {
+#if ((SPICE_INTERFACE_RECORD_MAJOR >= 2) && (SPICE_INTERFACE_RECORD_MINOR >= 2))
+            SWVoiceIn *sw;
+            va_list ap;
+            uint16_t vol[2];
+
+            va_start (ap, cmd);
+            sw = va_arg (ap, SWVoiceIn *);
+            va_end (ap);
+
+            vol[0] = sw->vol.l >> 16;
+            vol[1] = sw->vol.r >> 16;
+            spice_server_record_set_volume (&in->sin, 2, vol);
+            spice_server_record_set_mute (&in->sin, sw->vol.mute);
+#endif
+            break;
+        }
     }
+
     return 0;
 }
 
@@ -337,6 +375,9 @@ struct audio_driver spice_audio_driver = {
     .max_voices_in  = 1,
     .voice_size_out = sizeof (SpiceVoiceOut),
     .voice_size_in  = sizeof (SpiceVoiceIn),
+#if ((SPICE_INTERFACE_PLAYBACK_MAJOR >= 1) && (SPICE_INTERFACE_PLAYBACK_MINOR >= 2))
+    .ctl_caps       = VOICE_VOLUME_CAP
+#endif
 };
 
 void qemu_spice_audio_init (void)
-- 
1.7.7.6

[Qemu-devel] [PATCH 08/11] Do not use pa_simple PulseAudio API

[Marc-André Lureau]

Unfortunately, pa_simple is a limited API which doesn't let us
retrieve the associated pa_stream. It is needed to control the volume
of the stream.
---
 audio/paaudio.c |  356 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 318 insertions(+), 38 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index d1f3912..beed434 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -2,8 +2,7 @@
 #include "qemu-common.h"
 #include "audio.h"
 
-#include <pulse/simple.h>
-#include <pulse/error.h>
+#include <pulse/pulseaudio.h>
 
 #define AUDIO_CAP "pulseaudio"
 #include "audio_int.h"
@@ -15,7 +14,7 @@ typedef struct {
     int live;
     int decr;
     int rpos;
-    pa_simple *s;
+    pa_stream *stream;
     void *pcm_buf;
     struct audio_pt pt;
 } PAVoiceOut;
@@ -26,17 +25,23 @@ typedef struct {
     int dead;
     int incr;
     int wpos;
-    pa_simple *s;
+    pa_stream *stream;
     void *pcm_buf;
     struct audio_pt pt;
+    const void *read_data;
+    size_t read_index, read_length;
 } PAVoiceIn;
 
-static struct {
+typedef struct {
     int samples;
     char *server;
     char *sink;
     char *source;
-} conf = {
+    pa_threaded_mainloop *mainloop;
+    pa_context *context;
+} paaudio;
+
+static paaudio glob_paaudio = {
     .samples = 4096,
 };
 
@@ -51,6 +56,120 @@ static void GCC_FMT_ATTR (2, 3) qpa_logerr (int err, const char *fmt, ...)
     AUD_log (AUDIO_CAP, "Reason: %s\n", pa_strerror (err));
 }
 
+#define CHECK_SUCCESS_GOTO(c, rerror, expression, label)        \
+    do {                                                        \
+        if (!(expression)) {                                    \
+            if (rerror)                                         \
+                *(rerror) = pa_context_errno((c)->context);     \
+            goto label;                                         \
+        }                                                       \
+    } while(0);
+
+#define CHECK_DEAD_GOTO(c, stream, rerror, label)                       \
+    do {                                                                \
+        if (!(c)->context || !PA_CONTEXT_IS_GOOD(pa_context_get_state((c)->context)) || \
+            !(stream) || !PA_STREAM_IS_GOOD(pa_stream_get_state((stream)))) { \
+            if (((c)->context && pa_context_get_state((c)->context) == PA_CONTEXT_FAILED) || \
+                ((stream) && pa_stream_get_state((stream)) == PA_STREAM_FAILED)) { \
+                if (rerror)                                             \
+                    *(rerror) = pa_context_errno((c)->context);         \
+            } else                                                      \
+                if (rerror)                                             \
+                    *(rerror) = PA_ERR_BADSTATE;                        \
+            goto label;                                                 \
+        }                                                               \
+    } while(0);
+
+static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror)
+{
+    paaudio *g = &glob_paaudio;
+
+    pa_threaded_mainloop_lock (g->mainloop);
+
+    CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+
+    while (length > 0) {
+        size_t l;
+
+        while (!p->read_data) {
+            int r;
+
+            r = pa_stream_peek (p->stream, &p->read_data, &p->read_length);
+            CHECK_SUCCESS_GOTO (g, rerror, r == 0, unlock_and_fail);
+
+            if (!p->read_data) {
+                pa_threaded_mainloop_wait (g->mainloop);
+                CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+            } else
+                p->read_index = 0;
+        }
+
+        l = p->read_length < length ? p->read_length : length;
+        memcpy (data, (const uint8_t*) p->read_data+p->read_index, l);
+
+        data = (uint8_t*) data + l;
+        length -= l;
+
+        p->read_index += l;
+        p->read_length -= l;
+
+        if (!p->read_length) {
+            int r;
+
+            r = pa_stream_drop (p->stream);
+            p->read_data = NULL;
+            p->read_length = 0;
+            p->read_index = 0;
+
+            CHECK_SUCCESS_GOTO (g, rerror, r == 0, unlock_and_fail);
+        }
+    }
+
+    pa_threaded_mainloop_unlock (g->mainloop);
+    return 0;
+
+unlock_and_fail:
+    pa_threaded_mainloop_unlock (g->mainloop);
+    return -1;
+}
+
+static int qpa_simple_write (PAVoiceOut *p, const void *data, size_t length, int *rerror)
+{
+    paaudio *g = &glob_paaudio;
+
+    pa_threaded_mainloop_lock(g->mainloop);
+
+    CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+
+    while (length > 0) {
+        size_t l;
+        int r;
+
+        while (!(l = pa_stream_writable_size (p->stream))) {
+            pa_threaded_mainloop_wait (g->mainloop);
+            CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+        }
+
+        CHECK_SUCCESS_GOTO (g, rerror, l != (size_t) -1, unlock_and_fail);
+
+        if (l > length)
+            l = length;
+
+        r = pa_stream_write (p->stream, data, l, NULL, 0LL, PA_SEEK_RELATIVE);
+        CHECK_SUCCESS_GOTO (g, rerror, r >= 0, unlock_and_fail);
+
+        data = (const uint8_t*) data + l;
+        length -= l;
+    }
+
+    pa_threaded_mainloop_unlock (g->mainloop);
+    return 0;
+
+unlock_and_fail:
+    pa_threaded_mainloop_unlock (g->mainloop);
+    return -1;
+}
+
 static void *qpa_thread_out (void *arg)
 {
     PAVoiceOut *pa = arg;
@@ -77,7 +196,7 @@ static void *qpa_thread_out (void *arg)
             }
         }
 
-        decr = to_mix = audio_MIN (pa->live, conf.samples >> 2);
+        decr = to_mix = audio_MIN (pa->live, glob_paaudio.samples >> 2);
         rpos = pa->rpos;
 
         if (audio_pt_unlock (&pa->pt, AUDIO_FUNC)) {
@@ -91,8 +210,8 @@ static void *qpa_thread_out (void *arg)
 
             hw->clip (pa->pcm_buf, src, chunk);
 
-            if (pa_simple_write (pa->s, pa->pcm_buf,
-                                 chunk << hw->info.shift, &error) < 0) {
+            if (qpa_simple_write (pa, pa->pcm_buf,
+                                  chunk << hw->info.shift, &error) < 0) {
                 qpa_logerr (error, "pa_simple_write failed\n");
                 return NULL;
             }
@@ -169,7 +288,7 @@ static void *qpa_thread_in (void *arg)
             }
         }
 
-        incr = to_grab = audio_MIN (pa->dead, conf.samples >> 2);
+        incr = to_grab = audio_MIN (pa->dead, glob_paaudio.samples >> 2);
         wpos = pa->wpos;
 
         if (audio_pt_unlock (&pa->pt, AUDIO_FUNC)) {
@@ -181,8 +300,8 @@ static void *qpa_thread_in (void *arg)
             int chunk = audio_MIN (to_grab, hw->samples - wpos);
             void *buf = advance (pa->pcm_buf, wpos);
 
-            if (pa_simple_read (pa->s, buf,
-                                chunk << hw->info.shift, &error) < 0) {
+            if (qpa_simple_read (pa, buf,
+                                 chunk << hw->info.shift, &error) < 0) {
                 qpa_logerr (error, "pa_simple_read failed\n");
                 return NULL;
             }
@@ -283,6 +402,104 @@ static audfmt_e pa_to_audfmt (pa_sample_format_t fmt, int *endianness)
     }
 }
 
+static void context_state_cb (pa_context *c, void *userdata)
+{
+    paaudio *g = &glob_paaudio;
+
+    switch (pa_context_get_state(c)) {
+        case PA_CONTEXT_READY:
+        case PA_CONTEXT_TERMINATED:
+        case PA_CONTEXT_FAILED:
+            pa_threaded_mainloop_signal (g->mainloop, 0);
+            break;
+
+        case PA_CONTEXT_UNCONNECTED:
+        case PA_CONTEXT_CONNECTING:
+        case PA_CONTEXT_AUTHORIZING:
+        case PA_CONTEXT_SETTING_NAME:
+            break;
+    }
+}
+
+static void stream_state_cb (pa_stream *s, void * userdata)
+{
+    paaudio *g = &glob_paaudio;
+
+    switch (pa_stream_get_state (s)) {
+
+        case PA_STREAM_READY:
+        case PA_STREAM_FAILED:
+        case PA_STREAM_TERMINATED:
+            pa_threaded_mainloop_signal (g->mainloop, 0);
+            break;
+
+        case PA_STREAM_UNCONNECTED:
+        case PA_STREAM_CREATING:
+            break;
+    }
+}
+
+static void stream_request_cb (pa_stream *s, size_t length, void *userdata)
+{
+    paaudio *g = &glob_paaudio;
+
+    pa_threaded_mainloop_signal (g->mainloop, 0);
+}
+
+static pa_stream* qpa_simple_new (
+        const char *server,
+        const char *name,
+        pa_stream_direction_t dir,
+        const char *dev,
+        const char *stream_name,
+        const pa_sample_spec *ss,
+        const pa_channel_map *map,
+        const pa_buffer_attr *attr,
+        int *rerror)
+{
+    paaudio *g = &glob_paaudio;
+    int r;
+    pa_stream *stream;
+
+    pa_threaded_mainloop_lock (g->mainloop);
+
+    if (!(stream = pa_stream_new (g->context, name, ss, map)))
+        goto fail;
+
+    pa_stream_set_state_callback (stream, stream_state_cb, g);
+    pa_stream_set_read_callback (stream, stream_request_cb, g);
+    pa_stream_set_write_callback (stream, stream_request_cb, g);
+
+    if (dir == PA_STREAM_PLAYBACK)
+        r = pa_stream_connect_playback (stream, dev, attr,
+                                        PA_STREAM_INTERPOLATE_TIMING
+                                        |PA_STREAM_ADJUST_LATENCY
+                                        |PA_STREAM_AUTO_TIMING_UPDATE, NULL, NULL);
+    else
+        r = pa_stream_connect_record (stream, dev, attr,
+                                      PA_STREAM_INTERPOLATE_TIMING
+                                      |PA_STREAM_ADJUST_LATENCY
+                                      |PA_STREAM_AUTO_TIMING_UPDATE);
+
+    if (r < 0)
+      goto fail;
+
+    pa_threaded_mainloop_unlock (g->mainloop);
+
+    return stream;
+
+fail:
+    pa_threaded_mainloop_unlock (g->mainloop);
+
+    if (stream)
+        pa_stream_unref (stream);
+
+    qpa_logerr (pa_context_errno (g->context),
+                "stream_new() failed\n");
+
+    return NULL;
+}
+
 static int qpa_init_out (HWVoiceOut *hw, struct audsettings *as)
 {
     int error;
@@ -306,24 +523,24 @@ static int qpa_init_out (HWVoiceOut *hw, struct audsettings *as)
 
     obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
 
-    pa->s = pa_simple_new (
-        conf.server,
+    pa->stream = qpa_simple_new (
+        glob_paaudio.server,
         "qemu",
         PA_STREAM_PLAYBACK,
-        conf.sink,
+        glob_paaudio.sink,
         "pcm.playback",
         &ss,
         NULL,                   /* channel map */
         &ba,                    /* buffering attributes */
         &error
         );
-    if (!pa->s) {
+    if (!pa->stream) {
         qpa_logerr (error, "pa_simple_new for playback failed\n");
         goto fail1;
     }
 
     audio_pcm_init_info (&hw->info, &obt_as);
-    hw->samples = conf.samples;
+    hw->samples = glob_paaudio.samples;
     pa->pcm_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << hw->info.shift);
     pa->rpos = hw->rpos;
     if (!pa->pcm_buf) {
@@ -342,8 +559,9 @@ static int qpa_init_out (HWVoiceOut *hw, struct audsettings *as)
     g_free (pa->pcm_buf);
     pa->pcm_buf = NULL;
  fail2:
-    pa_simple_free (pa->s);
-    pa->s = NULL;
+    if (pa->stream)
+        pa_stream_unref (pa->stream);
+    pa->stream = NULL;
  fail1:
     return -1;
 }
@@ -361,24 +579,24 @@ static int qpa_init_in (HWVoiceIn *hw, struct audsettings *as)
 
     obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
 
-    pa->s = pa_simple_new (
-        conf.server,
+    pa->stream = qpa_simple_new (
+        glob_paaudio.server,
         "qemu",
         PA_STREAM_RECORD,
-        conf.source,
+        glob_paaudio.source,
         "pcm.capture",
         &ss,
         NULL,                   /* channel map */
         NULL,                   /* buffering attributes */
         &error
         );
-    if (!pa->s) {
+    if (!pa->stream) {
         qpa_logerr (error, "pa_simple_new for capture failed\n");
         goto fail1;
     }
 
     audio_pcm_init_info (&hw->info, &obt_as);
-    hw->samples = conf.samples;
+    hw->samples = glob_paaudio.samples;
     pa->pcm_buf = audio_calloc (AUDIO_FUNC, hw->samples, 1 << hw->info.shift);
     pa->wpos = hw->wpos;
     if (!pa->pcm_buf) {
@@ -397,8 +615,9 @@ static int qpa_init_in (HWVoiceIn *hw, struct audsettings *as)
     g_free (pa->pcm_buf);
     pa->pcm_buf = NULL;
  fail2:
-    pa_simple_free (pa->s);
-    pa->s = NULL;
+    if (pa->stream)
+        pa_stream_unref (pa->stream);
+    pa->stream = NULL;
  fail1:
     return -1;
 }
@@ -413,9 +632,9 @@ static void qpa_fini_out (HWVoiceOut *hw)
     audio_pt_unlock_and_signal (&pa->pt, AUDIO_FUNC);
     audio_pt_join (&pa->pt, &ret, AUDIO_FUNC);
 
-    if (pa->s) {
-        pa_simple_free (pa->s);
-        pa->s = NULL;
+    if (pa->stream) {
+        pa_stream_unref (pa->stream);
+        pa->stream = NULL;
     }
 
     audio_pt_fini (&pa->pt, AUDIO_FUNC);
@@ -433,9 +652,9 @@ static void qpa_fini_in (HWVoiceIn *hw)
     audio_pt_unlock_and_signal (&pa->pt, AUDIO_FUNC);
     audio_pt_join (&pa->pt, &ret, AUDIO_FUNC);
 
-    if (pa->s) {
-        pa_simple_free (pa->s);
-        pa->s = NULL;
+    if (pa->stream) {
+        pa_stream_unref (pa->stream);
+        pa->stream = NULL;
     }
 
     audio_pt_fini (&pa->pt, AUDIO_FUNC);
@@ -460,37 +679,98 @@ static int qpa_ctl_in (HWVoiceIn *hw, int cmd, ...)
 /* common */
 static void *qpa_audio_init (void)
 {
-    return &conf;
+    paaudio *g = &glob_paaudio;
+
+    if (!(g->mainloop = pa_threaded_mainloop_new ()))
+        goto fail;
+
+    if (!(g->context = pa_context_new (pa_threaded_mainloop_get_api (g->mainloop), glob_paaudio.server)))
+        goto fail;
+
+    pa_context_set_state_callback (g->context, context_state_cb, g);
+
+    if (pa_context_connect (g->context, glob_paaudio.server, 0, NULL) < 0) {
+        qpa_logerr (pa_context_errno (g->context),
+                    "pa_context_connect() failed\n");
+        goto fail;
+    }
+
+    pa_threaded_mainloop_lock (g->mainloop);
+
+    if (pa_threaded_mainloop_start (g->mainloop) < 0)
+        goto unlock_and_fail;
+
+    for (;;) {
+        pa_context_state_t state;
+
+        state = pa_context_get_state (g->context);
+
+        if (state == PA_CONTEXT_READY)
+            break;
+
+        if (!PA_CONTEXT_IS_GOOD (state)) {
+            qpa_logerr (pa_context_errno (g->context),
+                        "Wrong context state\n");
+            goto unlock_and_fail;
+        }
+
+        /* Wait until the context is ready */
+        pa_threaded_mainloop_wait (g->mainloop);
+    }
+
+    pa_threaded_mainloop_unlock (g->mainloop);
+
+    return &glob_paaudio;
+
+unlock_and_fail:
+    pa_threaded_mainloop_unlock (g->mainloop);
+fail:
+    AUD_log (AUDIO_CAP, "Failed to initialize PA context");
+    return NULL;
 }
 
 static void qpa_audio_fini (void *opaque)
 {
-    (void) opaque;
+    paaudio *g = opaque;
+
+    if (g->mainloop)
+        pa_threaded_mainloop_stop (g->mainloop);
+
+    if (g->context) {
+        pa_context_disconnect (g->context);
+        pa_context_unref (g->context);
+        g->context = NULL;
+    }
+
+    if (g->mainloop)
+        pa_threaded_mainloop_free (g->mainloop);
+
+    g->mainloop = NULL;
 }
 
 struct audio_option qpa_options[] = {
     {
         .name  = "SAMPLES",
         .tag   = AUD_OPT_INT,
-        .valp  = &conf.samples,
+        .valp  = &glob_paaudio.samples,
         .descr = "buffer size in samples"
     },
     {
         .name  = "SERVER",
         .tag   = AUD_OPT_STR,
-        .valp  = &conf.server,
+        .valp  = &glob_paaudio.server,
         .descr = "server address"
     },
     {
         .name  = "SINK",
         .tag   = AUD_OPT_STR,
-        .valp  = &conf.sink,
+        .valp  = &glob_paaudio.sink,
         .descr = "sink device name"
     },
     {
         .name  = "SOURCE",
         .tag   = AUD_OPT_STR,
-        .valp  = &conf.source,
+        .valp  = &glob_paaudio.source,
         .descr = "source device name"
     },
     { /* End of list */ }
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 08/11] Do not use pa_simple PulseAudio API

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> Unfortunately, pa_simple is a limited API which doesn't let us
> retrieve the associated pa_stream. It is needed to control the volume
> of the stream.
> ---
>  audio/paaudio.c |  356 +++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 318 insertions(+), 38 deletions(-)
> 

I can not test this (do not have pulseaudio on any of the boxen) and so
would have to take your word on whether it works..

This patch is also misses some braces.

[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 09/11] configure: pa_simple is not needed anymore

[Marc-André Lureau]

---
 configure |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 39d2b54..160bf63 100755
--- a/configure
+++ b/configure
@@ -1842,9 +1842,9 @@ for drv in $audio_drv_list; do
     ;;
 
     pa)
-    audio_drv_probe $drv pulse/simple.h "-lpulse-simple -lpulse" \
-        "pa_simple *s = 0; pa_simple_free(s); return 0;"
-    libs_softmmu="-lpulse -lpulse-simple $libs_softmmu"
+    audio_drv_probe $drv pulse/mainloop.h "-lpulse" \
+        "pa_mainloop *m = 0; pa_mainloop_free (m); return 0;"
+    libs_softmmu="-lpulse $libs_softmmu"
     audio_pt_int="yes"
     ;;
 
-- 
1.7.7.6

[Qemu-devel] [PATCH 10/11] Allow controlling volume with PulseAudio backend

[Marc-André Lureau]

---
 audio/paaudio.c |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index beed434..7ddc16d 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -664,15 +664,100 @@ static void qpa_fini_in (HWVoiceIn *hw)
 
 static int qpa_ctl_out (HWVoiceOut *hw, int cmd, ...)
 {
-    (void) hw;
-    (void) cmd;
+    PAVoiceOut *pa = (PAVoiceOut *) hw;
+    pa_operation *op;
+    pa_cvolume v;
+    paaudio *g = &glob_paaudio;
+
+    pa_cvolume_init (&v);
+
+    switch (cmd) {
+    case VOICE_VOLUME:
+        {
+            SWVoiceOut *sw;
+            va_list ap;
+
+            va_start (ap, cmd);
+            sw = va_arg (ap, SWVoiceOut *);
+            va_end (ap);
+
+            v.channels = 2;
+            v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.l) / UINT32_MAX;
+            v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.r) / UINT32_MAX;
+
+            pa_threaded_mainloop_lock (g->mainloop);
+
+            op = pa_context_set_sink_input_volume (g->context,
+                pa_stream_get_index (pa->stream),
+                &v, NULL, NULL);
+            if (!op)
+                qpa_logerr (pa_context_errno (g->context),
+                            "set_sink_input_volume() failed\n");
+            else
+                pa_operation_unref (op);
+
+            op = pa_context_set_sink_input_mute (g->context,
+                pa_stream_get_index (pa->stream),
+               sw->vol.mute, NULL, NULL);
+            if (!op)
+                qpa_logerr (pa_context_errno (g->context),
+                            "set_sink_input_mute() failed\n");
+            else
+                pa_operation_unref (op);
+
+            pa_threaded_mainloop_unlock (g->mainloop);
+        }
+    }
     return 0;
 }
 
 static int qpa_ctl_in (HWVoiceIn *hw, int cmd, ...)
 {
-    (void) hw;
-    (void) cmd;
+    PAVoiceIn *pa = (PAVoiceIn *) hw;
+    pa_operation *op;
+    pa_cvolume v;
+    paaudio *g = &glob_paaudio;
+
+    pa_cvolume_init (&v);
+
+    switch (cmd) {
+    case VOICE_VOLUME:
+        {
+            SWVoiceIn *sw;
+            va_list ap;
+
+            va_start (ap, cmd);
+            sw = va_arg (ap, SWVoiceIn *);
+            va_end (ap);
+
+            v.channels = 2;
+            v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.l) / UINT32_MAX;
+            v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.r) / UINT32_MAX;
+
+            pa_threaded_mainloop_lock (g->mainloop);
+
+            /* FIXME: use the upcoming "set_source_output_{volume,mute}" */
+            op = pa_context_set_source_volume_by_index (g->context,
+                pa_stream_get_device_index (pa->stream),
+                &v, NULL, NULL);
+            if (!op)
+                qpa_logerr (pa_context_errno (g->context),
+                            "set_source_volume() failed\n");
+            else
+                pa_operation_unref(op);
+
+            op = pa_context_set_source_mute_by_index (g->context,
+                pa_stream_get_index (pa->stream),
+                sw->vol.mute, NULL, NULL);
+            if (!op)
+                qpa_logerr (pa_context_errno (g->context),
+                            "set_source_mute() failed\n");
+            else
+                pa_operation_unref (op);
+
+            pa_threaded_mainloop_unlock (g->mainloop);
+        }
+    }
     return 0;
 }
 
@@ -801,5 +886,6 @@ struct audio_driver pa_audio_driver = {
     .max_voices_out = INT_MAX,
     .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof (PAVoiceOut),
-    .voice_size_in  = sizeof (PAVoiceIn)
+    .voice_size_in  = sizeof (PAVoiceIn),
+    .ctl_caps       = VOICE_VOLUME_CAP
 };
-- 
1.7.7.6

[Qemu-devel] [PATCH 11/11] Make mixemu mandatory

[Marc-André Lureau]

Without MIXEMU, volume control on the guest doesn't work (except when
volume is applied by guest "emulation", in Win7 for example).

Instead rely on backend volume support, or fallback on mixeng if
backend doesn't support volume control.
---
 audio/mixeng.c |    6 ------
 configure      |    8 --------
 hw/hda-audio.c |    4 ----
 3 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index 5446be6..0b060e3 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -336,7 +336,6 @@ void mixeng_clear (struct st_sample *buf, int len)
 
 void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol)
 {
-#ifdef CONFIG_MIXEMU
     if (vol->mute) {
         mixeng_clear (buf, len);
         return;
@@ -352,9 +351,4 @@ void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol)
 #endif
         buf += 1;
     }
-#else
-    (void) buf;
-    (void) len;
-    (void) vol;
-#endif
 }
diff --git a/configure b/configure
index 160bf63..09fc408 100755
--- a/configure
+++ b/configure
@@ -177,7 +177,6 @@ darwin_user="no"
 bsd_user="no"
 guest_base=""
 uname_release=""
-mixemu="no"
 aix="no"
 blobs="yes"
 pkgversion=""
@@ -770,8 +769,6 @@ for opt do
   ;;
   --enable-nptl) nptl="yes"
   ;;
-  --enable-mixemu) mixemu="yes"
-  ;;
   --disable-linux-aio) linux_aio="no"
   ;;
   --enable-linux-aio) linux_aio="yes"
@@ -1028,7 +1025,6 @@ echo "  --audio-card-list=LIST   set list of emulated audio cards [$audio_card_l
 echo "                           Available cards: $audio_possible_cards"
 echo "  --block-drv-whitelist=L  set block driver whitelist"
 echo "                           (affects only QEMU, not qemu-img)"
-echo "  --enable-mixemu          enable mixer emulation"
 echo "  --disable-xen            disable xen backend driver support"
 echo "  --enable-xen             enable xen backend driver support"
 echo "  --disable-brlapi         disable BrlAPI"
@@ -2885,7 +2881,6 @@ echo "mingw32 support   $mingw32"
 echo "Audio drivers     $audio_drv_list"
 echo "Extra audio cards $audio_card_list"
 echo "Block whitelist   $block_drv_whitelist"
-echo "Mixer emulation   $mixemu"
 echo "VirtFS support    $virtfs"
 echo "VNC support       $vnc"
 if test "$vnc" = "yes" ; then
@@ -3047,9 +3042,6 @@ if test "$audio_win_int" = "yes" ; then
   echo "CONFIG_AUDIO_WIN_INT=y" >> $config_host_mak
 fi
 echo "CONFIG_BDRV_WHITELIST=$block_drv_whitelist" >> $config_host_mak
-if test "$mixemu" = "yes" ; then
-  echo "CONFIG_MIXEMU=y" >> $config_host_mak
-fi
 if test "$vnc" = "yes" ; then
   echo "CONFIG_VNC=y" >> $config_host_mak
 fi
diff --git a/hw/hda-audio.c b/hw/hda-audio.c
index 8995519..75f1402 100644
--- a/hw/hda-audio.c
+++ b/hw/hda-audio.c
@@ -121,15 +121,11 @@ static void hda_codec_parse_fmt(uint32_t format, struct audsettings *as)
 #define QEMU_HDA_AMP_NONE    (0)
 #define QEMU_HDA_AMP_STEPS   0x4a
 
-#ifdef CONFIG_MIXEMU
 #define QEMU_HDA_AMP_CAPS                                               \
     (AC_AMPCAP_MUTE |                                                   \
      (QEMU_HDA_AMP_STEPS << AC_AMPCAP_OFFSET_SHIFT)    |                \
      (QEMU_HDA_AMP_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) |                \
      (3                  << AC_AMPCAP_STEP_SIZE_SHIFT))
-#else
-#define QEMU_HDA_AMP_CAPS    QEMU_HDA_AMP_NONE
-#endif
 
 /* common: audio output widget */
 static const desc_param common_params_audio_dac[] = {
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> Hello,
> 
> This patch series implements client-side audio volume support. This 
> reduces confusion of guest users when volume control is not effective 
> (because mixemu is disabled or because client-side is muted and can't be 
> unmuted by the guest..)
> 
> Instead, the backend is responsible for applying volume giving the guest 
> control over the full range of the client, and avoiding multiple level 
> of volume/mute effects.
> 
> Although I was mainly interested in having the Spice audio backend 
> support, I also added PulseAudio backend support (which unfortunately 
> requires full-blown API, even after quick discussion with upstream).
> 

I'm okay with this, provided that:

a. Pulse/Spice have per connection volume
b. Other drivers are not affected
c. mixeng is not the default

> Marc-Andr? Lureau (11):
>   audio: add VOICE_VOLUME ctl
>   audio: don't apply volume effect if backend has VOICE_VOLUME_CAP
>   audio: use a nominal volume of UINT_MAX
>   hw/ac97: remove USE_MIXER code
>   hw/ac97: the volume mask is not only 0x1f
>   hw/ac97: add support for volume control
>   audio/spice: add support for volume control
>   Do not use pa_simple PulseAudio API
>   configure: pa_simple is not needed anymore
>   Allow controlling volume with PulseAudio backend
>   Make mixemu mandatory
> 
>  audio/audio.c          |   25 +++-
>  audio/audio_int.h      |    6 +
>  audio/audio_template.h |    2 +
>  audio/mixeng.c         |    6 -
>  audio/paaudio.c        |  452 +++++++++++++++++++++++++++++++++++++++++++-----
>  audio/spiceaudio.c     |   41 +++++
>  configure              |   14 +--
>  hw/ac97.c              |  141 ++++++----------
>  hw/hda-audio.c         |    4 -
>  9 files changed, 531 insertions(+), 160 deletions(-)
> 
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Marc-André Lureau]

Hi Vassili

Thanks for your review!

On Mon, Mar 12, 2012 at 8:44 PM, malc <av1474@comtv.ru> wrote:
> a. Pulse/Spice have per connection volume

Each playback/recorde stream has it's own volume, and that's what we
control. We get the client full range thanks to flat-volume logic
(implemented by PulseAudio and Windows Vista+)

> b. Other drivers are not affected

I don't see yet how, but I will review other drivers ctl_{in,out}
implementations

> c. mixeng is not the default

So you mean the last patch shouldn't be applied? Can you explain the
rationale? Why would we want broken behaviour by default?

cheers

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> Hi Vassili
> 
> Thanks for your review!
> 
> On Mon, Mar 12, 2012 at 8:44 PM, malc <av1474@comtv.ru> wrote:
> > a. Pulse/Spice have per connection volume
> 
> Each playback/recorde stream has it's own volume, and that's what we
> control. We get the client full range thanks to flat-volume logic
> (implemented by PulseAudio and Windows Vista+)

Okay.

> 
> > b. Other drivers are not affected
> 
> I don't see yet how, but I will review other drivers ctl_{in,out}
> implementations

You don't see how they can be affected or you don't see how you can
figure that out?

> 
> > c. mixeng is not the default
> 
> So you mean the last patch shouldn't be applied? Can you explain the
> rationale? Why would we want broken behaviour by default?
> 

Because i hate change.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Marc-André Lureau]

On Mon, Mar 12, 2012 at 9:51 PM, malc <av1474@comtv.ru> wrote:
>>
>> > b. Other drivers are not affected
>>
>> I don't see yet how, but I will review other drivers ctl_{in,out}
>> implementations
>
> You don't see how they can be affected or you don't see how you can
> figure that out?

I don't see how they can be affected, unless they assert on an unknown
pcm_ctrl value VOICE_VOLUME for example (the calling call ignores
return value, so it alsa for example, can return -1 and it will get
ignored).

>> > c. mixeng is not the default
>>
>> So you mean the last patch shouldn't be applied? Can you explain the
>> rationale? Why would we want broken behaviour by default?
>>
>
> Because i hate change.

I do to, except when they fix broken behaviour. More seriously, do you
have other concerns with the mixemu code?

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[malc]

On Mon, 12 Mar 2012, Marc-Andr? Lureau wrote:

> On Mon, Mar 12, 2012 at 9:51 PM, malc <av1474@comtv.ru> wrote:
> >>
> >> > b. Other drivers are not affected
> >>
> >> I don't see yet how, but I will review other drivers ctl_{in,out}
> >> implementations
> >
> > You don't see how they can be affected or you don't see how you can
> > figure that out?
> 
> I don't see how they can be affected, unless they assert on an unknown
> pcm_ctrl value VOICE_VOLUME for example (the calling call ignores
> return value, so it alsa for example, can return -1 and it will get
                 it = in?
> ignored).
> 
> >> > c. mixeng is not the default
> >>
> >> So you mean the last patch shouldn't be applied? Can you explain the
> >> rationale? Why would we want broken behaviour by default?
> >>
> >
> > Because i hate change.
> 
> I do to, except when they fix broken behaviour. More seriously, do you
> have other concerns with the mixemu code?
> 

Sure - it adds overhead.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Gerd Hoffmann]

  Hi,

>> I do to, except when they fix broken behaviour. More seriously, do you
>> have other concerns with the mixemu code?
> 
> Sure - it adds overhead.

The point of this patchset is to kill the overhead if possible, i.e. try
to pass down the volume the guest asked for all the way down to the
hosts's audio hardware.

This patchset starts with spice and pulse, adding support to other audio
backends (oss, alsa, windows, ...) shouldn't be hard, mixemu shouldn't
be used in the common case then.

I think we should remove the mixemu configure option.  It makes code
bitrot.  Patch #4 proves that.  If you want to keep it because of the
overhead or other reasons I'd suggest to make it a runtime option.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Kevin Wolf]

Am 13.03.2012 11:19, schrieb Gerd Hoffmann:
>   Hi,
> 
>>> I do to, except when they fix broken behaviour. More seriously, do you
>>> have other concerns with the mixemu code?
>>
>> Sure - it adds overhead.
> 
> The point of this patchset is to kill the overhead if possible, i.e. try
> to pass down the volume the guest asked for all the way down to the
> hosts's audio hardware.
> 
> This patchset starts with spice and pulse, adding support to other audio
> backends (oss, alsa, windows, ...) shouldn't be hard, mixemu shouldn't
> be used in the common case then.

Then the default should probably to use it at least if the backend
support passing it through? Leaving it broken by default wouldn't be
very nice.

Kevin

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Gerd Hoffmann]

On 03/13/12 11:33, Kevin Wolf wrote:
> Am 13.03.2012 11:19, schrieb Gerd Hoffmann:
>>   Hi,
>>
>>>> I do to, except when they fix broken behaviour. More seriously, do you
>>>> have other concerns with the mixemu code?
>>>
>>> Sure - it adds overhead.
>>
>> The point of this patchset is to kill the overhead if possible, i.e. try
>> to pass down the volume the guest asked for all the way down to the
>> hosts's audio hardware.
>>
>> This patchset starts with spice and pulse, adding support to other audio
>> backends (oss, alsa, windows, ...) shouldn't be hard, mixemu shouldn't
>> be used in the common case then.
> 
> Then the default should probably to use it at least if the backend
> support passing it through? Leaving it broken by default wouldn't be
> very nice.

Agree.  I would make the mixemu runtime switch have an effect only in
case the audio backend doesn't support volume, so the user can pick
between non-working volume control + zero overhead and working volume
control at the price of mixemu overhead then.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[Marc-André Lureau]

On Tue, Mar 13, 2012 at 11:19 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> I think we should remove the mixemu configure option.  It makes code
> bitrot.  Patch #4 proves that.  If you want to keep it because of the
> overhead or other reasons I'd suggest to make it a runtime option.

To be fair, the patch #4 only remove dead code (USE_MIXER wasn't
enabled by mixemu).

Making mixemu a runtime option sounds good to me.

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 00/11] apply volume on client side (v3)

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 587 bytes --]

On Tue, 13 Mar 2012, Marc-Andr? Lureau wrote:

> On Tue, Mar 13, 2012 at 11:19 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > I think we should remove the mixemu configure option.  It makes code
> > bitrot.  Patch #4 proves that.  If you want to keep it because of the
> > overhead or other reasons I'd suggest to make it a runtime option.
> 
> To be fair, the patch #4 only remove dead code (USE_MIXER wasn't
> enabled by mixemu).
> 
> Making mixemu a runtime option sounds good to me.
 
No objection (yet... as i haven't seen how it'(s, d be) implemented)

-- 
mailto:av1474@comtv.ru