On 11/21/2017 03:59 AM, Pavel Hrdina wrote:
On Mon, Nov 20, 2017 at 05:39:38PM -0500, John Ferlan wrote:
On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
Setting the default audio output depends on specific graphic device but requires having sound device configured as well and it's the sound device that handles the audio.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 84 +++++++++++++++------- .../qemuxml2argv-clock-france.args | 2 +- 2 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb72db33ba..e1ef1b05fa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4442,10 +4442,62 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, }
+static void +qemuBuildSoundAudioEnv(virCommandPtr cmd, + const virDomainDef *def, + virQEMUDriverConfigPtr cfg) +{ + if (def->ngraphics == 0) { + if (cfg->nogfxAllowHostAudio) + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + else + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } else { + switch (def->graphics[def->ngraphics - 1]->type) {
So it's the "last" graphics device that then defines "how" this all works? Makes sense for QEMU_AUDIO_DRV since whichever is last would be the winner and get the chicken dinner, but...
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + /* If using SDL for video, then we should just let it + * use QEMU's host audio drivers, possibly SDL too + * User can set these two before starting libvirtd + */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
... if there was more than one graphics device defined, where one was SDL and it wasn't the last one, then theoretically at least this would not be defined.
This is intentional, I should have mentioned it in the commit message. The original design was just wrong, nothing else. Setting "SDL_AUDIODRIVER" if the SDL audio output is not used is pointless and we shouldn't do it. I can move this change to separate patch.
Pavel
I figured you had gone through the history - I certain hadn't ;-)... I would say by this point in the review I was still "missing" the final detail regarding the bug you're working on O:) - that the audio was being 'tied to' that last device only. Guess I was thinking it could somehow be magically applied to "any" device. Anyway, long way of saying - it's fine here. Adding a bit more detail to the commit message will help though. John