Re: [PATCH 06/18] ui/vnc: Require audiodev=
On Mon, Apr 25, 2022 at 10:21:49AM +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
ui/vnc.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index badf1d7664fe..2e7af139b030 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
vd->ledstate = 0;
audiodev = qemu_opt_get(opts, "audiodev");
- if (audiodev) {
- vd->audio_state = audio_state_by_name(audiodev);
- if (!vd->audio_state) {
- error_setg(errp, "Audiodev '%s' not found", audiodev);
- goto fail;
- }
+ if (!audiodev) {
+ error_setg(errp, "Audiodev parameter for vnc required");
+ goto fail;
+ }
I know we deprecated not setting 'audiodev', but I'm not convinced
this is the right approach.
The VNC audio extension is a custom QEMU invention that few VNC
clients have implemented, and even when implemented few turn it
on as it is pretty awful stuttering.
IMHO a better approach could be to leave audiodev optional, but
stop advertizing VNC_ENCODING_AUDIO when it isn't set.
IOW, current situation
-vnc :1 -> enables audio capture from default backend
This patch
-vnc :1 -> error
-vnc :1,audiodev=audio0 -> enable audio capture from 'audio0'
Better:
-vnc :1 -> stop advertizing VNC_ENCODING_AUDIO
-vnc :1,audiodev=audio0 -> enable audio capture from 'audio0'
+
+ vd->audio_state = audio_state_by_name(audiodev);
+ if (!vd->audio_state) {
+ error_setg(errp, "Audiodev '%s' not found", audiodev);
+ goto fail;
}
device_id = qemu_opt_get(opts, "display");
--
2.35.1
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|