[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/2] Make mixer emulation configurable at runtim
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 0/2] Make mixer emulation configurable at runtime |
Date: |
Wed, 28 Aug 2013 11:13:43 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Bandan Das <address@hidden> writes:
> Currently, hda-codec mixer emulation can only be enabled by using the
> "--enable-mixemu" option to configure at compile time. These patches add
> the option to make it configurable at runtime as well.
>
> An example usage could be a qemu cmdline :
> "-device hda-duplex,mixer=(on/off)" although this
> is not part of this change.
Really? I see a new property "mixer" in PATCH 2/2. It's UINT32 rather
than BOOL, though.
Let's review where we are and where your patch takes us.
CONFIG_MIXEMU is set by configure --enable-mixemu (default off). It
does two things:
* It makes mixeng_volume() actually do something. Users:
audio_pcm_sw_read():
if (!(hw->ctl_caps & VOICE_VOLUME_CAP)) {
mixeng_volume (sw->buf, ret, &sw->vol);
}
audio_pcm_sw_write():
if (!(sw->hw->ctl_caps & VOICE_VOLUME_CAP)) {
mixeng_volume (sw->buf, swlim, &sw->vol);
}
Only pulse and the spice audio backends set VOICE_VOLUME_CAP.
Impact on users isn't obvious to me.
* Devices hda-{output,duplex,micro} have a mixer if and only if
CONFIG_MIXEMU is enabled.
Reason: their mixer cannot work without CONFIG_MIXEMU. Advertizing a
broken hardware mixer to guests breaks volume control. Not nice. So,
advertize it only when it works. When CONFIG_MIXEMU is disabled, the
hardware provides no mixer, guests fall back to a software mixer, and
volume control works. Commit d61a4ce.
So, -device hda-duplex gives you *different* devices depending on
whether you enabled mixer emulation. Enabling/disabling QEMU's mixer
emulation changes guest ABI. This was a bad idea. We should have
created either two sets of devices, one with and one without a mixer,
or a device property to control mixer presence.
Your patch implements the latter. And boy is it ugly :)
Questions (not just for you, Bandan):
1. Why is CONFIG_MIXEMU off by default?
We generally default optional code to "on", to avoid bit-rot. We
make exceptions only when the optional code is truly unwanted by a
clear majority of our users. Why would anyone want hda audio devices
without a mixer?
2. Why do we bother providing these devices when CONFIG_MIXEMU off?
Why would anyone want hda audio devices without a mixer? Why
wouldn't anyone who wants hda audio devices also want CONFIG_MIXEMU
enabled?
The only remotely convincing reason I can think of is "we foolishly
provided them, and now we have to keep them for backward
compatibility".
3. Why does CONFIG_MIXEMU exist?
Closest thing to a discussion I could find in the list archives:
http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg02313.html
http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg02285.html
I don't exactly find the argument for CONFIG_MIXEMU convincing.