* [Qemu-devel] [RFC/PATCH] Bad volume scaling with Win7 guest, spice audio, and Qemu Intel HDA codec
@ 2015-04-20 14:37 Wilck, Martin
2015-04-21 13:06 ` Marc-André Lureau
0 siblings, 1 reply; 6+ messages in thread
From: Wilck, Martin @ 2015-04-20 14:37 UTC (permalink / raw)
To: Gerd Hoffmann, Vassili Karpov (malc), marcandre.lureau
Cc: Wilck, Martin, spice-devel, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3356 bytes --]
Hello,
I see a problem with input volume control on a Windows7 guest
using the qemu Intel HDA codec. In short, moving the volume slider for
the input volume from 0% to 1% under Windows results in
the "gain" values in the emulated HW to jump from 0 to 40 (out of 74)
(looking at "st->gain_[left|right]" in hda_audio_command()). This makes
it impossible to control sound volume in the low-gain range using the
windows guest.
Here is a table of Windows slider position vs. actual gain value to
illustrate the problem ("0.5" is a slider position between 0 and 1 that
can be reached with skillful slider shifting, windows still displays the
numerical value "0"):
slider gain
0.5 34
1 40
2 43
3 46
5 48
10 54
20 60
50 68
75 71
89 73
99 73
100 74
For output, the situation is similar but less (1% on Win7 corresponds to
a gain value of 19/74).
I am using the spice driver connected to pulseaudio to control the
volume in the host (PA adds another level of complication which I am
going to address in a posting to pulseaudio-discuss soon). The spice-
I am using a Win7(32bit) guest on a Fedora 21 host.
qemu-system-x86-2.1.3-6.fc21.x86_64
spice-gtk3-0.27-6.fc21.x86_64
pulseaudio-6.0-2.fc21.1.x86_64
By using low-level controls in a Linux VM (e.g. "amixer -c 0 set Capture
$x), I was able to set the gain levels to any value $x between 0 and 74,
just as one would expect. So this is not a problem of QEMU alone.
Rather, it's related to how the Windows HDA codec driver and the QEMU
emulated HW interact. For the Windows side I can only guess, but it
seems that Windows uses a ~30dB overall scale for the min-max range of
the input volume slider (and ~60dB for output). I'm not sure if that's
general Win7 behavior or related to the driver. According to
https://msdn.microsoft.com/en-us/library/windows/hardware/ff536251%
28v=vs.85%29.aspx, audio drivers can set volume-related registry values,
but I didn't find any of those on my system.
I experimentally changed the value of QEMU_HDA_AMP_STEPS in
hw/audio/hda-codec.c from 74 to 32, artificially reducing the dynamic
range of the HDA codec to 32 dB. This improved matters much, allowing me
to set a gain value as low as 4 (-28dB / 12.5%) from the Windows guest
at 1%. Going one step further and using 32dB dynamic range with a 0.5 dB
step size, I could even set a gain value of 5 (-29.5dB / 8%).
The "right" solution for this problem would probably be to implement
proper dB scaling in the the spice client code, as the note in the git
commit suggests
(http://cgit.freedesktop.org/spice/spice/commit/server/snd_worker.c?id=d1758b328811979beb58ff9ddb9cf4f318fa28f7).
I am not sure how difficult this might be. AFAICS it would require
changes in the general QEMU audio API to deal with dB.
While this clean solution is not available, I would suggest to decrease
the dynamic range for the the emulated Amps in the QEMU hda codec code.
AFAICS, that would also match the dynamic range of actual HD Audio HW
better. I am attaching a tentative patch that does exactly that for
input, changing nothing for output (the output volume setting behaves
acceptably for the default QEMU settings, see above). Please comment.
Regards
Martin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: qemu-hda-codec-amp-1.patch --]
[-- Type: text/x-patch; name="qemu-hda-codec-amp-1.patch", Size: 4371 bytes --]
--- qemu-2.1.3/hw/audio/hda-codec-common.h 2015-04-20 14:44:18.621197997 +0200
+++ qemu-2.1.3/hw/audio/hda-codec-common.h.new 2015-04-20 14:43:24.852627519 +0200
@@ -28,16 +28,22 @@
#define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x12)
#define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x22)
#define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x32)
-#define QEMU_HDA_AMP_CAPS \
+#define QEMU_HDA_AMP_OUT_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))
+ (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
+ (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
+ (QEMU_HDA_AMP_OUT_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
+#define QEMU_HDA_AMP_IN_CAPS \
+ (AC_AMPCAP_MUTE | \
+ (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
+ (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
+ (QEMU_HDA_AMP_IN_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
#else
#define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x11)
#define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x21)
#define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x31)
-#define QEMU_HDA_AMP_CAPS QEMU_HDA_AMP_NONE
+#define QEMU_HDA_AMP_OUT_CAPS QEMU_HDA_AMP_NONE
+#define QEMU_HDA_AMP_IN_CAPS QEMU_HDA_AMP_NONE
#endif
@@ -61,7 +67,7 @@ static const desc_param glue(common_para
.val = QEMU_HDA_AMP_NONE,
},{
.id = AC_PAR_AMP_OUT_CAP,
- .val = QEMU_HDA_AMP_CAPS,
+ .val = QEMU_HDA_AMP_OUT_CAPS,
},
};
@@ -86,7 +92,7 @@ static const desc_param glue(common_para
.val = AC_SUPFMT_PCM,
},{
.id = AC_PAR_AMP_IN_CAP,
- .val = QEMU_HDA_AMP_CAPS,
+ .val = QEMU_HDA_AMP_IN_CAPS,
},{
.id = AC_PAR_AMP_OUT_CAP,
.val = QEMU_HDA_AMP_NONE,
@@ -453,4 +459,5 @@ static const desc_codec glue(micro_, PAR
#undef QEMU_HDA_ID_OUTPUT
#undef QEMU_HDA_ID_DUPLEX
#undef QEMU_HDA_ID_MICRO
-#undef QEMU_HDA_AMP_CAPS
+#undef QEMU_HDA_AMP_IN_CAPS
+#undef QEMU_HDA_AMP_OUT_CAPS
--- qemu-2.1.3/hw/audio/hda-codec.c 2015-04-20 15:20:38.069019517 +0200
+++ qemu-2.1.3/hw/audio/hda-codec.c.new 2015-04-20 15:17:32.443070187 +0200
@@ -116,7 +116,17 @@
#define QEMU_HDA_PCM_FORMATS (AC_SUPPCM_BITS_16 | \
0x1fc /* 16 -> 96 kHz */)
#define QEMU_HDA_AMP_NONE (0)
-#define QEMU_HDA_AMP_STEPS 0x4a
+/* Amplifier properties */
+/* Step size: 0: 0.25dB 1: 0.5dB 2: 0.75dB 3: 1dB */
+/*
+ Until we have a way to propagate dB values cleanly to the host
+ e.g. through spice, it is better to use a smaller dynamic range
+ for input. Here we use 0x40 * 0.5 dB = 32dB.
+*/
+#define QEMU_HDA_AMP_OUT_STEPS 0x4a
+#define QEMU_HDA_AMP_OUT_STEP_SIZE 3
+#define QEMU_HDA_AMP_IN_STEPS 0x40
+#define QEMU_HDA_AMP_IN_STEP_SIZE 1
#define PARAM mixemu
#define HDA_MIXER
@@ -258,15 +268,16 @@
left = st->mute_left ? 0 : st->gain_left;
right = st->mute_right ? 0 : st->gain_right;
- left = left * 255 / QEMU_HDA_AMP_STEPS;
- right = right * 255 / QEMU_HDA_AMP_STEPS;
-
if (!st->state->mixer) {
return;
}
if (st->output) {
+ left = left * 255 / QEMU_HDA_AMP_OUT_STEPS;
+ right = right * 255 / QEMU_HDA_AMP_OUT_STEPS;
AUD_set_volume_out(st->voice.out, muted, left, right);
} else {
+ left = left * 255 / QEMU_HDA_AMP_IN_STEPS;
+ right = right * 255 / QEMU_HDA_AMP_IN_STEPS;
AUD_set_volume_in(st->voice.in, muted, left, right);
}
}
@@ -512,8 +512,8 @@
st->node = node;
if (type == AC_WID_AUD_OUT) {
/* unmute output by default */
- st->gain_left = QEMU_HDA_AMP_STEPS;
- st->gain_right = QEMU_HDA_AMP_STEPS;
+ st->gain_left = QEMU_HDA_AMP_OUT_STEPS;
+ st->gain_right = QEMU_HDA_AMP_OUT_STEPS;
st->bpos = sizeof(st->buf);
st->output = true;
} else {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC/PATCH] Bad volume scaling with Win7 guest, spice audio, and Qemu Intel HDA codec
2015-04-20 14:37 [Qemu-devel] [RFC/PATCH] Bad volume scaling with Win7 guest, spice audio, and Qemu Intel HDA codec Wilck, Martin
@ 2015-04-21 13:06 ` Marc-André Lureau
2015-04-21 16:50 ` [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier Martin Wilck
0 siblings, 1 reply; 6+ messages in thread
From: Marc-André Lureau @ 2015-04-21 13:06 UTC (permalink / raw)
To: Martin Wilck
Cc: spice-devel, marcandre lureau, Vassili Karpov (malc),
Gerd Hoffmann, qemu-devel
Hi
----- Original Message -----
> I see a problem with input volume control on a Windows7 guest
> using the qemu Intel HDA codec. In short, moving the volume slider for
> the input volume from 0% to 1% under Windows results in
> the "gain" values in the emulated HW to jump from 0 to 40 (out of 74)
> (looking at "st->gain_[left|right]" in hda_audio_command()). This makes
> it impossible to control sound volume in the low-gain range using the
> windows guest.
>
> Here is a table of Windows slider position vs. actual gain value to
> illustrate the problem ("0.5" is a slider position between 0 and 1 that
> can be reached with skillful slider shifting, windows still displays the
> numerical value "0"):
>
> slider gain
> 0.5 34
> 1 40
> 2 43
> 3 46
> 5 48
> 10 54
> 20 60
> 50 68
> 75 71
> 89 73
> 99 73
> 100 74
>
> For output, the situation is similar but less (1% on Win7 corresponds to
> a gain value of 19/74).
>
> I am using the spice driver connected to pulseaudio to control the
> volume in the host (PA adds another level of complication which I am
> going to address in a posting to pulseaudio-discuss soon). The spice-
>
>
>
> I am using a Win7(32bit) guest on a Fedora 21 host.
> qemu-system-x86-2.1.3-6.fc21.x86_64
> spice-gtk3-0.27-6.fc21.x86_64
> pulseaudio-6.0-2.fc21.1.x86_64
>
> By using low-level controls in a Linux VM (e.g. "amixer -c 0 set Capture
> $x), I was able to set the gain levels to any value $x between 0 and 74,
> just as one would expect. So this is not a problem of QEMU alone.
> Rather, it's related to how the Windows HDA codec driver and the QEMU
> emulated HW interact. For the Windows side I can only guess, but it
> seems that Windows uses a ~30dB overall scale for the min-max range of
> the input volume slider (and ~60dB for output). I'm not sure if that's
> general Win7 behavior or related to the driver. According to
> https://msdn.microsoft.com/en-us/library/windows/hardware/ff536251%
> 28v=vs.85%29.aspx, audio drivers can set volume-related registry values,
> but I didn't find any of those on my system.
>
> I experimentally changed the value of QEMU_HDA_AMP_STEPS in
> hw/audio/hda-codec.c from 74 to 32, artificially reducing the dynamic
> range of the HDA codec to 32 dB. This improved matters much, allowing me
> to set a gain value as low as 4 (-28dB / 12.5%) from the Windows guest
> at 1%. Going one step further and using 32dB dynamic range with a 0.5 dB
> step size, I could even set a gain value of 5 (-29.5dB / 8%).
Very nice! It is way better indeed. I think your patch looks good too.
> The "right" solution for this problem would probably be to implement
> proper dB scaling in the the spice client code, as the note in the git
> commit suggests
> (http://cgit.freedesktop.org/spice/spice/commit/server/snd_worker.c?id=d1758b328811979beb58ff9ddb9cf4f318fa28f7).
> I am not sure how difficult this might be. AFAICS it would require
> changes in the general QEMU audio API to deal with dB.
That would be indeed a welcome improvement.
> While this clean solution is not available, I would suggest to decrease
> the dynamic range for the the emulated Amps in the QEMU hda codec code.
> AFAICS, that would also match the dynamic range of actual HD Audio HW
> better. I am attaching a tentative patch that does exactly that for
> input, changing nothing for output (the output volume setting behaves
> acceptably for the default QEMU settings, see above). Please comment.
Please send a git formatted patch to the ML:
http://wiki.qemu.org/Contribute/SubmitAPatch
thanks a lot
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier
2015-04-21 13:06 ` Marc-André Lureau
@ 2015-04-21 16:50 ` Martin Wilck
2015-04-21 17:14 ` Marc-André Lureau
0 siblings, 1 reply; 6+ messages in thread
From: Martin Wilck @ 2015-04-21 16:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Martin Wilck, mlureau
This patch addresses the following problem:
Moving the input volume slider 0% to 1% in a Win7 guest causes the
the "gain" values in the emulated HDA codec HW to jump from 0 to 40.
This makes it impossible to control sound volume in the low-gain
range using the windows guest.
This is related to how the Windows HDA codec driver and the QEMU
emulated HW interact. It seems that Windows uses a ~30dB overall
scale for the min-max range of the input volume slider.
The "right" solution for this problem would be to implement
proper dB scaling in QEMU and the audio backends (such as spice).
While this clean solution is not available, I suggest to decrease
the dynamic range for the the emulated Amps in the QEMU hda codec.
Experiments showed that with 32dB dynamic range with a 0.5 dB step,
it was possible to set gain values as low as 5 (-29.5dB / 8%).
Actual HW seems to use similar ranges for input amplifiers.
Signed-off-by: Martin Wilck <martin.wilck@ts.fujitsu.com>
---
hw/audio/hda-codec-common.h | 23 +++++++++++++++--------
hw/audio/hda-codec.c | 23 +++++++++++++++++------
2 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/hw/audio/hda-codec-common.h b/hw/audio/hda-codec-common.h
index b4fdb51..d59781b 100644
--- a/hw/audio/hda-codec-common.h
+++ b/hw/audio/hda-codec-common.h
@@ -28,16 +28,22 @@
#define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x12)
#define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x22)
#define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x32)
-#define QEMU_HDA_AMP_CAPS \
+#define QEMU_HDA_AMP_OUT_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))
+ (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
+ (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
+ (QEMU_HDA_AMP_OUT_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
+#define QEMU_HDA_AMP_IN_CAPS \
+ (AC_AMPCAP_MUTE | \
+ (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
+ (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
+ (QEMU_HDA_AMP_IN_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
#else
#define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x11)
#define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x21)
#define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x31)
-#define QEMU_HDA_AMP_CAPS QEMU_HDA_AMP_NONE
+#define QEMU_HDA_AMP_OUT_CAPS QEMU_HDA_AMP_NONE
+#define QEMU_HDA_AMP_IN_CAPS QEMU_HDA_AMP_NONE
#endif
@@ -61,7 +67,7 @@ static const desc_param glue(common_params_audio_dac_, PARAM)[] = {
.val = QEMU_HDA_AMP_NONE,
},{
.id = AC_PAR_AMP_OUT_CAP,
- .val = QEMU_HDA_AMP_CAPS,
+ .val = QEMU_HDA_AMP_OUT_CAPS,
},
};
@@ -86,7 +92,7 @@ static const desc_param glue(common_params_audio_adc_, PARAM)[] = {
.val = AC_SUPFMT_PCM,
},{
.id = AC_PAR_AMP_IN_CAP,
- .val = QEMU_HDA_AMP_CAPS,
+ .val = QEMU_HDA_AMP_IN_CAPS,
},{
.id = AC_PAR_AMP_OUT_CAP,
.val = QEMU_HDA_AMP_NONE,
@@ -453,4 +459,5 @@ static const desc_codec glue(micro_, PARAM) = {
#undef QEMU_HDA_ID_OUTPUT
#undef QEMU_HDA_ID_DUPLEX
#undef QEMU_HDA_ID_MICRO
-#undef QEMU_HDA_AMP_CAPS
+#undef QEMU_HDA_AMP_IN_CAPS
+#undef QEMU_HDA_AMP_OUT_CAPS
diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 3c03ff5..64b4d98 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -116,7 +116,17 @@ static void hda_codec_parse_fmt(uint32_t format, struct audsettings *as)
#define QEMU_HDA_PCM_FORMATS (AC_SUPPCM_BITS_16 | \
0x1fc /* 16 -> 96 kHz */)
#define QEMU_HDA_AMP_NONE (0)
-#define QEMU_HDA_AMP_STEPS 0x4a
+/* Amplifier properties */
+/* Step size: 0: 0.25dB 1: 0.5dB 2: 0.75dB 3: 1dB */
+/*
+ Until we have a way to propagate dB values cleanly to the host
+ e.g. through spice, it is better to use a smaller dynamic range
+ for input. Here we use 0x40 * 0.5 dB = 32dB.
+*/
+#define QEMU_HDA_AMP_OUT_STEPS 0x4a
+#define QEMU_HDA_AMP_OUT_STEP_SIZE 3
+#define QEMU_HDA_AMP_IN_STEPS 0x40
+#define QEMU_HDA_AMP_IN_STEP_SIZE 1
#define PARAM mixemu
#define HDA_MIXER
@@ -258,15 +268,16 @@ static void hda_audio_set_amp(HDAAudioStream *st)
left = st->mute_left ? 0 : st->gain_left;
right = st->mute_right ? 0 : st->gain_right;
- left = left * 255 / QEMU_HDA_AMP_STEPS;
- right = right * 255 / QEMU_HDA_AMP_STEPS;
-
if (!st->state->mixer) {
return;
}
if (st->output) {
+ left = left * 255 / QEMU_HDA_AMP_OUT_STEPS;
+ right = right * 255 / QEMU_HDA_AMP_OUT_STEPS;
AUD_set_volume_out(st->voice.out, muted, left, right);
} else {
+ left = left * 255 / QEMU_HDA_AMP_IN_STEPS;
+ right = right * 255 / QEMU_HDA_AMP_IN_STEPS;
AUD_set_volume_in(st->voice.in, muted, left, right);
}
}
@@ -502,8 +513,8 @@ static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
st->node = node;
if (type == AC_WID_AUD_OUT) {
/* unmute output by default */
- st->gain_left = QEMU_HDA_AMP_STEPS;
- st->gain_right = QEMU_HDA_AMP_STEPS;
+ st->gain_left = QEMU_HDA_AMP_OUT_STEPS;
+ st->gain_right = QEMU_HDA_AMP_OUT_STEPS;
st->bpos = sizeof(st->buf);
st->output = true;
} else {
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier
2015-04-21 16:50 ` [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier Martin Wilck
@ 2015-04-21 17:14 ` Marc-André Lureau
2015-04-22 6:37 ` Gerd Hoffmann
0 siblings, 1 reply; 6+ messages in thread
From: Marc-André Lureau @ 2015-04-21 17:14 UTC (permalink / raw)
To: Martin Wilck; +Cc: qemu-devel
Hi
----- Original Message -----
> This patch addresses the following problem:
>
> Moving the input volume slider 0% to 1% in a Win7 guest causes the
> the "gain" values in the emulated HDA codec HW to jump from 0 to 40.
> This makes it impossible to control sound volume in the low-gain
> range using the windows guest.
> This is related to how the Windows HDA codec driver and the QEMU
> emulated HW interact. It seems that Windows uses a ~30dB overall
> scale for the min-max range of the input volume slider.
>
> The "right" solution for this problem would be to implement
> proper dB scaling in QEMU and the audio backends (such as spice).
>
> While this clean solution is not available, I suggest to decrease
> the dynamic range for the the emulated Amps in the QEMU hda codec.
> Experiments showed that with 32dB dynamic range with a 0.5 dB step,
> it was possible to set gain values as low as 5 (-29.5dB / 8%).
> Actual HW seems to use similar ranges for input amplifiers.
>
> Signed-off-by: Martin Wilck <martin.wilck@ts.fujitsu.com>
> ---
> hw/audio/hda-codec-common.h | 23 +++++++++++++++--------
> hw/audio/hda-codec.c | 23 +++++++++++++++++------
> 2 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/hw/audio/hda-codec-common.h b/hw/audio/hda-codec-common.h
> index b4fdb51..d59781b 100644
> --- a/hw/audio/hda-codec-common.h
> +++ b/hw/audio/hda-codec-common.h
> @@ -28,16 +28,22 @@
> #define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x12)
> #define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x22)
> #define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x32)
> -#define QEMU_HDA_AMP_CAPS \
> +#define QEMU_HDA_AMP_OUT_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))
> + (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
> + (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
> + (QEMU_HDA_AMP_OUT_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
> +#define QEMU_HDA_AMP_IN_CAPS \
> + (AC_AMPCAP_MUTE | \
> + (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
> + (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
> + (QEMU_HDA_AMP_IN_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
> #else
> #define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x11)
> #define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x21)
> #define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x31)
> -#define QEMU_HDA_AMP_CAPS QEMU_HDA_AMP_NONE
> +#define QEMU_HDA_AMP_OUT_CAPS QEMU_HDA_AMP_NONE
> +#define QEMU_HDA_AMP_IN_CAPS QEMU_HDA_AMP_NONE
> #endif
>
>
> @@ -61,7 +67,7 @@ static const desc_param glue(common_params_audio_dac_,
> PARAM)[] = {
> .val = QEMU_HDA_AMP_NONE,
> },{
> .id = AC_PAR_AMP_OUT_CAP,
> - .val = QEMU_HDA_AMP_CAPS,
> + .val = QEMU_HDA_AMP_OUT_CAPS,
> },
> };
>
> @@ -86,7 +92,7 @@ static const desc_param glue(common_params_audio_adc_,
> PARAM)[] = {
> .val = AC_SUPFMT_PCM,
> },{
> .id = AC_PAR_AMP_IN_CAP,
> - .val = QEMU_HDA_AMP_CAPS,
> + .val = QEMU_HDA_AMP_IN_CAPS,
> },{
> .id = AC_PAR_AMP_OUT_CAP,
> .val = QEMU_HDA_AMP_NONE,
I am afraid guest OS won't like it the card description changes after a migration.
This will likely need a new VMState field, for the steps, or the amp caps.
> @@ -453,4 +459,5 @@ static const desc_codec glue(micro_, PARAM) = {
> #undef QEMU_HDA_ID_OUTPUT
> #undef QEMU_HDA_ID_DUPLEX
> #undef QEMU_HDA_ID_MICRO
> -#undef QEMU_HDA_AMP_CAPS
> +#undef QEMU_HDA_AMP_IN_CAPS
> +#undef QEMU_HDA_AMP_OUT_CAPS
> diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
> index 3c03ff5..64b4d98 100644
> --- a/hw/audio/hda-codec.c
> +++ b/hw/audio/hda-codec.c
> @@ -116,7 +116,17 @@ static void hda_codec_parse_fmt(uint32_t format, struct
> audsettings *as)
> #define QEMU_HDA_PCM_FORMATS (AC_SUPPCM_BITS_16 | \
> 0x1fc /* 16 -> 96 kHz */)
> #define QEMU_HDA_AMP_NONE (0)
> -#define QEMU_HDA_AMP_STEPS 0x4a
> +/* Amplifier properties */
> +/* Step size: 0: 0.25dB 1: 0.5dB 2: 0.75dB 3: 1dB */
> +/*
> + Until we have a way to propagate dB values cleanly to the host
> + e.g. through spice, it is better to use a smaller dynamic range
> + for input. Here we use 0x40 * 0.5 dB = 32dB.
> +*/
> +#define QEMU_HDA_AMP_OUT_STEPS 0x4a
> +#define QEMU_HDA_AMP_OUT_STEP_SIZE 3
> +#define QEMU_HDA_AMP_IN_STEPS 0x40
> +#define QEMU_HDA_AMP_IN_STEP_SIZE 1
>
> #define PARAM mixemu
> #define HDA_MIXER
> @@ -258,15 +268,16 @@ static void hda_audio_set_amp(HDAAudioStream *st)
> left = st->mute_left ? 0 : st->gain_left;
> right = st->mute_right ? 0 : st->gain_right;
>
> - left = left * 255 / QEMU_HDA_AMP_STEPS;
> - right = right * 255 / QEMU_HDA_AMP_STEPS;
> -
> if (!st->state->mixer) {
> return;
> }
> if (st->output) {
> + left = left * 255 / QEMU_HDA_AMP_OUT_STEPS;
> + right = right * 255 / QEMU_HDA_AMP_OUT_STEPS;
> AUD_set_volume_out(st->voice.out, muted, left, right);
> } else {
> + left = left * 255 / QEMU_HDA_AMP_IN_STEPS;
> + right = right * 255 / QEMU_HDA_AMP_IN_STEPS;
> AUD_set_volume_in(st->voice.in, muted, left, right);
> }
> }
> @@ -502,8 +513,8 @@ static int hda_audio_init(HDACodecDevice *hda, const
> struct desc_codec *desc)
> st->node = node;
> if (type == AC_WID_AUD_OUT) {
> /* unmute output by default */
> - st->gain_left = QEMU_HDA_AMP_STEPS;
> - st->gain_right = QEMU_HDA_AMP_STEPS;
> + st->gain_left = QEMU_HDA_AMP_OUT_STEPS;
> + st->gain_right = QEMU_HDA_AMP_OUT_STEPS;
> st->bpos = sizeof(st->buf);
> st->output = true;
> } else {
> --
> 2.1.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier
2015-04-21 17:14 ` Marc-André Lureau
@ 2015-04-22 6:37 ` Gerd Hoffmann
2015-04-22 8:01 ` Wilck, Martin
0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2015-04-22 6:37 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Martin Wilck, qemu-devel
Hi,
> > The "right" solution for this problem would be to implement
> > proper dB scaling in QEMU and the audio backends (such as spice).
Can we try this please?
> > While this clean solution is not available, I suggest to decrease
> > the dynamic range for the the emulated Amps in the QEMU hda codec.
> > Experiments showed that with 32dB dynamic range with a 0.5 dB step,
> > it was possible to set gain values as low as 5 (-29.5dB / 8%).
> > Actual HW seems to use similar ranges for input amplifiers.
> I am afraid guest OS won't like it the card description changes after a migration.
> This will likely need a new VMState field, for the steps, or the amp caps.
Correct. This is a guest-visible change, so we have to make this
runtime-switchable for compatibility with older qemu versions. This
quickly becomes a bit messy, so I'd prefer to avoid this.
cheers,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier
2015-04-22 6:37 ` Gerd Hoffmann
@ 2015-04-22 8:01 ` Wilck, Martin
0 siblings, 0 replies; 6+ messages in thread
From: Wilck, Martin @ 2015-04-22 8:01 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Marc-André Lureau, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]
> > > The "right" solution for this problem would be to implement
> > > proper dB scaling in QEMU and the audio backends (such as spice).
>
> Can we try this please?
I would like to, but it's a rather big project and beyond my current
QEMU progamming skills. Support seems to be lacking in the generic QEMU
audio layer.
> > > While this clean solution is not available, I suggest to decrease
> > > the dynamic range for the the emulated Amps in the QEMU hda codec.
> > > Experiments showed that with 32dB dynamic range with a 0.5 dB step,
> > > it was possible to set gain values as low as 5 (-29.5dB / 8%).
> > > Actual HW seems to use similar ranges for input amplifiers.
>
> > I am afraid guest OS won't like it the card description changes after a migration.
> > This will likely need a new VMState field, for the steps, or the amp caps.
>
> Correct. This is a guest-visible change, so we have to make this
> runtime-switchable for compatibility with older qemu versions. This
> quickly becomes a bit messy, so I'd prefer to avoid this.
So what do you suggest? The current situation (not being able to control
volume properly from the guest) is messy and should be fixed. It might
look like a minor problem, but try to run a Windows application that
attempts to auto-regulate input volume (such as Microsoft Lync/Skype) -
it simply doesn't work.
In the case of migration, what has to be avoided is that the guest
attempts to set a gain value outside the allowed range. So I should
modify my patch to use the same number of steps as before (0x4a).
Modified patch is attached.
Now, if the guest migrates from "new" to "old", it will continue to
assume a 0.5dB stepping. Because the HD audio step size is a purely
virtual value (we are not using dB at the moment), this will have the
effect that volume control after migration still behaves well. Likewise,
migrating from "old" to "new" will cause my fix not to become effective
in the "new" environment, because the guest will still assume 1dB
stepping.
Other than that, I can't see what might go wrong during a migration -
but maybe I am overlooking something? I have no environment to test
migration, so my thoughts above are just theoretical.
Martin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-hda-codec-use-smaller-dynamic-range-for-input-amplif.patch --]
[-- Type: text/x-patch; name="0001-hda-codec-use-smaller-dynamic-range-for-input-amplif.patch", Size: 6024 bytes --]
From bc4bbc4a92a8e9fb87aab1641f3868f7345b9b1e Mon Sep 17 00:00:00 2001
From: Martin Wilck <martin.wilck@ts.fujitsu.com>
Date: Tue, 21 Apr 2015 17:55:38 +0200
Subject: [PATCH] hda-codec: use smaller dynamic range for input amplifier
This patch addresses the following problem:
Moving the input volume slider 0% to 1% in a Win7 guest causes the
the "gain" values in the emulated HDA codec HW to jump from 0 to 40.
This makes it impossible to control sound volume in the low-gain
range using the windows guest.
This is related to how the Windows HDA codec driver and the QEMU
emulated HW interact. It seems that Windows uses a ~30dB overall
scale for the min-max range of the input volume slider.
The "right" solution for this problem would be to implement
proper dB scaling in QEMU and the audio backends (such as spice).
While this clean solution is not available, I suggest to decrease
the dynamic range for the the emulated Amps in the QEMU hda codec.
Experiments showed that with 32dB dynamic range with a 0.5 dB step,
it was possible to set gain values as low as 5 (-29.5dB / 8%).
Actual HW seems to use similar ranges for input amplifiers.
Signed-off-by: Martin Wilck <martin.wilck@ts.fujitsu.com>
---
hw/audio/hda-codec-common.h | 23 +++++++++++++++--------
hw/audio/hda-codec.c | 23 +++++++++++++++++------
2 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/hw/audio/hda-codec-common.h b/hw/audio/hda-codec-common.h
index b4fdb51..d59781b 100644
--- a/hw/audio/hda-codec-common.h
+++ b/hw/audio/hda-codec-common.h
@@ -28,16 +28,22 @@
#define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x12)
#define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x22)
#define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x32)
-#define QEMU_HDA_AMP_CAPS \
+#define QEMU_HDA_AMP_OUT_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))
+ (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
+ (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
+ (QEMU_HDA_AMP_OUT_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
+#define QEMU_HDA_AMP_IN_CAPS \
+ (AC_AMPCAP_MUTE | \
+ (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
+ (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
+ (QEMU_HDA_AMP_IN_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
#else
#define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x11)
#define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x21)
#define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x31)
-#define QEMU_HDA_AMP_CAPS QEMU_HDA_AMP_NONE
+#define QEMU_HDA_AMP_OUT_CAPS QEMU_HDA_AMP_NONE
+#define QEMU_HDA_AMP_IN_CAPS QEMU_HDA_AMP_NONE
#endif
@@ -61,7 +67,7 @@ static const desc_param glue(common_params_audio_dac_, PARAM)[] = {
.val = QEMU_HDA_AMP_NONE,
},{
.id = AC_PAR_AMP_OUT_CAP,
- .val = QEMU_HDA_AMP_CAPS,
+ .val = QEMU_HDA_AMP_OUT_CAPS,
},
};
@@ -86,7 +92,7 @@ static const desc_param glue(common_params_audio_adc_, PARAM)[] = {
.val = AC_SUPFMT_PCM,
},{
.id = AC_PAR_AMP_IN_CAP,
- .val = QEMU_HDA_AMP_CAPS,
+ .val = QEMU_HDA_AMP_IN_CAPS,
},{
.id = AC_PAR_AMP_OUT_CAP,
.val = QEMU_HDA_AMP_NONE,
@@ -453,4 +459,5 @@ static const desc_codec glue(micro_, PARAM) = {
#undef QEMU_HDA_ID_OUTPUT
#undef QEMU_HDA_ID_DUPLEX
#undef QEMU_HDA_ID_MICRO
-#undef QEMU_HDA_AMP_CAPS
+#undef QEMU_HDA_AMP_IN_CAPS
+#undef QEMU_HDA_AMP_OUT_CAPS
diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 3c03ff5..44c9a28 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -116,7 +116,17 @@ static void hda_codec_parse_fmt(uint32_t format, struct audsettings *as)
#define QEMU_HDA_PCM_FORMATS (AC_SUPPCM_BITS_16 | \
0x1fc /* 16 -> 96 kHz */)
#define QEMU_HDA_AMP_NONE (0)
-#define QEMU_HDA_AMP_STEPS 0x4a
+/* Amplifier properties */
+/* Step size: 0: 0.25dB 1: 0.5dB 2: 0.75dB 3: 1dB */
+/*
+ Until we have a way to propagate dB values cleanly to the host
+ e.g. through spice, it is better to use a smaller dynamic range
+ for input. Here we use 0x40 * 0.5 dB = 32dB.
+*/
+#define QEMU_HDA_AMP_OUT_STEPS 0x4a
+#define QEMU_HDA_AMP_OUT_STEP_SIZE 3
+#define QEMU_HDA_AMP_IN_STEPS 0x4a
+#define QEMU_HDA_AMP_IN_STEP_SIZE 1
#define PARAM mixemu
#define HDA_MIXER
@@ -258,15 +268,16 @@ static void hda_audio_set_amp(HDAAudioStream *st)
left = st->mute_left ? 0 : st->gain_left;
right = st->mute_right ? 0 : st->gain_right;
- left = left * 255 / QEMU_HDA_AMP_STEPS;
- right = right * 255 / QEMU_HDA_AMP_STEPS;
-
if (!st->state->mixer) {
return;
}
if (st->output) {
+ left = left * 255 / QEMU_HDA_AMP_OUT_STEPS;
+ right = right * 255 / QEMU_HDA_AMP_OUT_STEPS;
AUD_set_volume_out(st->voice.out, muted, left, right);
} else {
+ left = left * 255 / QEMU_HDA_AMP_IN_STEPS;
+ right = right * 255 / QEMU_HDA_AMP_IN_STEPS;
AUD_set_volume_in(st->voice.in, muted, left, right);
}
}
@@ -502,8 +513,8 @@ static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
st->node = node;
if (type == AC_WID_AUD_OUT) {
/* unmute output by default */
- st->gain_left = QEMU_HDA_AMP_STEPS;
- st->gain_right = QEMU_HDA_AMP_STEPS;
+ st->gain_left = QEMU_HDA_AMP_OUT_STEPS;
+ st->gain_right = QEMU_HDA_AMP_OUT_STEPS;
st->bpos = sizeof(st->buf);
st->output = true;
} else {
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-22 8:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-20 14:37 [Qemu-devel] [RFC/PATCH] Bad volume scaling with Win7 guest, spice audio, and Qemu Intel HDA codec Wilck, Martin
2015-04-21 13:06 ` Marc-André Lureau
2015-04-21 16:50 ` [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier Martin Wilck
2015-04-21 17:14 ` Marc-André Lureau
2015-04-22 6:37 ` Gerd Hoffmann
2015-04-22 8:01 ` Wilck, Martin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.