Re: [PATCH 06/18] ui/vnc: Require audiodev=

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Apr 25, 2022 at 10:21:49AM +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan@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 :|