[PATCH 0/7] audio: redo default audio backend creation

[PATCH 0/7] audio: redo default audio backend creation

[Paolo Bonzini]

Currently, AUD_register_card picks the audio backend from either:

- the first audiodev that was created

- the audio_prio_list[] array, which can be customized at
  configure time

This series instead extends -audio to define a default audio
backend if no "model" is used.  This preserves simple command line
use where a single "-audio" option applies to all audio devices
and captures, and also uses a single QAPI-based configuration
syntax for both -audio and -audiodev.

The current hack to use the first -audiodev as a default audio
device is removed.  For migration purposes, the first audiodev
is suggested in case of an error:

  ./qemu-system-x86_64 -device sb16 -audiodev pa,id=default
  qemu-system-x86_64: -device sb16: no default audio driver available
  Perhaps you wanted to set audiodev=default?

VNC is changed to reintroduce use of the default audio backend;
still, compared to before the cleanup effort this will not be
enabled if -nodefaults is use, which is an improvement as it
removes magic.

Paolo


Paolo Bonzini (7):
  audio: error hints need a trailing \n
  audio: disable default backends if -audio/-audiodev is used
  audio: extract audio_define_default
  audio: extend -audio to allow creating a default backend
  audio: do not use first -audiodev as default audio device
  audio: reintroduce default audio backend for VNC
  audio, qtest: get rid of QEMU_AUDIO_DRV

 audio/audio.c                   | 85 +++++++++++++--------------------
 audio/audio.h                   |  3 ++
 docs/about/deprecated.rst       |  6 ---
 docs/about/removed-features.rst | 14 ++++--
 qemu-options.hx                 | 29 +++++++----
 system/vl.c                     | 34 ++++++++-----
 tests/qtest/libqtest.c          |  4 +-
 ui/vnc.c                        |  2 +
 8 files changed, 93 insertions(+), 84 deletions(-)

-- 
2.41.0

[PATCH 1/7] audio: error hints need a trailing \n

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/audio/audio.c b/audio/audio.c
index 730bf2498dc..98621576d95 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1833,7 +1833,7 @@ bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
             card->state = audio_init(NULL, errp);
             if (!card->state) {
                 if (!QSIMPLEQ_EMPTY(&audiodevs)) {
-                    error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?",
+                    error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?\n",
                                       QSIMPLEQ_FIRST(&audiodevs)->dev->id);
                 }
                 return false;
-- 
2.41.0

[PATCH 2/7] audio: disable default backends if -audio/-audiodev is used

[Paolo Bonzini]

Match what is done for other options, for example -monitor.  Require
the user to specify a backend if one is specified on the command line.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c                   | 9 +--------
 audio/audio.h                   | 1 +
 docs/about/removed-features.rst | 4 ++++
 system/vl.c                     | 7 +++++++
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 98621576d95..67a7e25254d 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1688,14 +1688,10 @@ static const VMStateDescription vmstate_audio = {
 
 static void audio_validate_opts(Audiodev *dev, Error **errp);
 
-static void audio_create_default_audiodevs(void)
+void audio_create_default_audiodevs(void)
 {
     const char *drvname = getenv("QEMU_AUDIO_DRV");
 
-    if (!defaults_enabled()) {
-        return;
-    }
-
     /* QEMU_AUDIO_DRV=none is used by libqtest.  */
     if (drvname && !g_str_equal(drvname, "none")) {
         error_report("Please use -audiodev instead of QEMU_AUDIO_*");
@@ -1827,9 +1823,6 @@ bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
             }
             card->state = QTAILQ_FIRST(&audio_states);
         } else {
-            if (QSIMPLEQ_EMPTY(&default_audiodevs)) {
-                audio_create_default_audiodevs();
-            }
             card->state = audio_init(NULL, errp);
             if (!card->state) {
                 if (!QSIMPLEQ_EMPTY(&audiodevs)) {
diff --git a/audio/audio.h b/audio/audio.h
index 80f3f92124d..53b1d7e6227 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -170,6 +170,7 @@ void audio_sample_from_uint64(void *samples, int pos,
 
 void audio_define(Audiodev *audio);
 void audio_parse_option(const char *opt);
+void audio_create_default_audiodevs(void);
 void audio_init_audiodevs(void);
 void audio_help(void);
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index e83ed087f6b..58c94392c65 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -442,6 +442,10 @@ line using a ``secret`` object instance.
 The ``-audiodev`` and ``-audio`` command line options are now the only
 way to specify audio backend settings.
 
+QEMU does not create default audio backends anymore if any of the
+``-audiodev``, ``-audio`` or ``-nodefaults`` options are used on the
+command line.
+
 Creating vnc without ``audiodev=`` property (removed in 8.2)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/system/vl.c b/system/vl.c
index 98e071e63bb..7ca92d4490d 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -184,6 +184,7 @@ static const char *qtest_log;
 static bool opt_one_insn_per_tb;
 
 static int has_defaults = 1;
+static int default_audio = 1;
 static int default_serial = 1;
 static int default_parallel = 1;
 static int default_monitor = 1;
@@ -1327,6 +1328,7 @@ static void qemu_disable_default_devices(void)
         default_sdcard = 0;
     }
     if (!has_defaults) {
+        default_audio = 0;
         default_monitor = 0;
         default_net = 0;
         default_vga = 0;
@@ -1963,6 +1965,9 @@ static void qemu_create_early_backends(void)
      */
     configure_blockdev(&bdo_queue, machine_class, snapshot);
     audio_init_audiodevs();
+    if (default_audio) {
+        audio_create_default_audiodevs();
+    }
 }
 
 
@@ -2925,6 +2930,7 @@ void qemu_init(int argc, char **argv)
                 break;
 #endif
             case QEMU_OPTION_audiodev:
+                default_audio = 0;
                 audio_parse_option(optarg);
                 break;
             case QEMU_OPTION_audio: {
@@ -2933,6 +2939,7 @@ void qemu_init(int argc, char **argv)
                 Audiodev *dev = NULL;
                 Visitor *v;
                 QDict *dict = keyval_parse(optarg, "driver", &help, &error_fatal);
+                default_audio = 0;
                 if (help || (qdict_haskey(dict, "driver") &&
                              is_help_option(qdict_get_str(dict, "driver")))) {
                     audio_help();
-- 
2.41.0

[PATCH 3/7] audio: extract audio_define_default

[Paolo Bonzini]

It will be used soon to define a default audio device from the
command line.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c | 19 ++++++++++++-------
 audio/audio.h |  1 +
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 67a7e25254d..186cc4d336e 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1686,8 +1686,6 @@ static const VMStateDescription vmstate_audio = {
     }
 };
 
-static void audio_validate_opts(Audiodev *dev, Error **errp);
-
 void audio_create_default_audiodevs(void)
 {
     const char *drvname = getenv("QEMU_AUDIO_DRV");
@@ -1706,7 +1704,6 @@ void audio_create_default_audiodevs(void)
         if (audio_driver_lookup(audio_prio_list[i])) {
             QDict *dict = qdict_new();
             Audiodev *dev = NULL;
-            AudiodevListEntry *e;
             Visitor *v;
 
             qdict_put_str(dict, "driver", audio_prio_list[i]);
@@ -1717,10 +1714,7 @@ void audio_create_default_audiodevs(void)
             visit_type_Audiodev(v, NULL, &dev, &error_fatal);
             visit_free(v);
 
-            audio_validate_opts(dev, &error_abort);
-            e = g_new0(AudiodevListEntry, 1);
-            e->dev = dev;
-            QSIMPLEQ_INSERT_TAIL(&default_audiodevs, e, next);
+            audio_define_default(dev, &error_abort);
         }
     }
 }
@@ -2165,6 +2159,17 @@ void audio_define(Audiodev *dev)
     QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
 }
 
+void audio_define_default(Audiodev *dev, Error **errp)
+{
+    AudiodevListEntry *e;
+
+    audio_validate_opts(dev, errp);
+
+    e = g_new0(AudiodevListEntry, 1);
+    e->dev = dev;
+    QSIMPLEQ_INSERT_TAIL(&default_audiodevs, e, next);
+}
+
 void audio_init_audiodevs(void)
 {
     AudiodevListEntry *e;
diff --git a/audio/audio.h b/audio/audio.h
index 53b1d7e6227..6da5e8f6f0f 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -169,6 +169,7 @@ void audio_sample_from_uint64(void *samples, int pos,
                             uint64_t left, uint64_t right);
 
 void audio_define(Audiodev *audio);
+void audio_define_default(Audiodev *dev, Error **errp);
 void audio_parse_option(const char *opt);
 void audio_create_default_audiodevs(void);
 void audio_init_audiodevs(void);
-- 
2.41.0

[PATCH 4/7] audio: extend -audio to allow creating a default backend

[Paolo Bonzini]

If "-audio BACKEND" is used without a model, the resulting backend
will be used whenever the audiodev property is not specified.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.hx | 29 +++++++++++++++++++++--------
 system/vl.c     | 27 +++++++++++++++------------
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 9ce8a5b9578..5f4c240a1e2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -728,20 +728,22 @@ ERST
 
 
 DEF("audio", HAS_ARG, QEMU_OPTION_audio,
+    "-audio [driver=]driver[,prop[=value][,...]]\n"
+    "                specifies default audio backend when `audiodev` is not\n"
+    "                used to create a machine or sound device;"
+    "                options are the same as for -audiodev\n"
     "-audio [driver=]driver,model=value[,prop[=value][,...]]\n"
     "                specifies the audio backend and device to use;\n"
     "                apart from 'model', options are the same as for -audiodev.\n"
     "                use '-audio model=help' to show possible devices.\n",
     QEMU_ARCH_ALL)
 SRST
-``-audio [driver=]driver,model=value[,prop[=value][,...]]``
-    This option is a shortcut for configuring both the guest audio
-    hardware and the host audio backend in one go.
-    The driver option is the same as with the corresponding ``-audiodev`` option below.
-    The guest hardware model can be set with ``model=modelname``.
-
-    Use ``driver=help`` to list the available drivers,
-    and ``model=help`` to list the available device types.
+``-audio [driver=]driver[,model=value][,prop[=value][,...]]``
+    If the ``model`` option is specified, ``-audio`` is a shortcut
+    for configuring both the guest audio hardware and the host audio
+    backend in one go. The guest hardware model can be set with
+    ``model=modelname``.  Use ``model=help`` to list the available
+    device types.
 
     The following two example do exactly the same, to show how ``-audio``
     can be used to shorten the command line length:
@@ -750,6 +752,17 @@ SRST
 
         |qemu_system| -audiodev pa,id=pa -device sb16,audiodev=pa
         |qemu_system| -audio pa,model=sb16
+
+    If the ``model`` option is not specified, ``-audio`` is used to
+    configure a default audio backend that will be used whenever the
+    ``audiodev`` property is not set on a device or machine.  In
+    particular, ``-audio none`` ensures that no audio is produced even
+    for machines that have embedded sound hardware.
+
+    In both cases, the driver option is the same as with the corresponding
+    ``-audiodev`` option below.  Use ``driver=help`` to list the available
+    drivers.
+
 ERST
 
 DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
diff --git a/system/vl.c b/system/vl.c
index 7ca92d4490d..72be5a49764 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2935,7 +2935,7 @@ void qemu_init(int argc, char **argv)
                 break;
             case QEMU_OPTION_audio: {
                 bool help;
-                char *model;
+                char *model = NULL;
                 Audiodev *dev = NULL;
                 Visitor *v;
                 QDict *dict = keyval_parse(optarg, "driver", &help, &error_fatal);
@@ -2948,22 +2948,25 @@ void qemu_init(int argc, char **argv)
                 if (!qdict_haskey(dict, "id")) {
                     qdict_put_str(dict, "id", "audiodev0");
                 }
-                if (!qdict_haskey(dict, "model")) {
-                    error_setg(&error_fatal, "Parameter 'model' is missing");
-                }
-                model = g_strdup(qdict_get_str(dict, "model"));
-                qdict_del(dict, "model");
-                if (is_help_option(model)) {
-                    show_valid_soundhw();
-                    exit(0);
+                if (qdict_haskey(dict, "model")) {
+                    model = g_strdup(qdict_get_str(dict, "model"));
+                    qdict_del(dict, "model");
+                    if (is_help_option(model)) {
+                        show_valid_soundhw();
+                        exit(0);
+                    }
                 }
                 v = qobject_input_visitor_new_keyval(QOBJECT(dict));
                 qobject_unref(dict);
                 visit_type_Audiodev(v, NULL, &dev, &error_fatal);
                 visit_free(v);
-                audio_define(dev);
-                select_soundhw(model, dev->id);
-                g_free(model);
+                if (model) {
+                    audio_define(dev);
+                    select_soundhw(model, dev->id);
+                    g_free(model);
+                } else {
+                    audio_define_default(dev, &error_fatal);
+                }
                 break;
             }
             case QEMU_OPTION_h:
-- 
2.41.0

[PATCH 5/7] audio: do not use first -audiodev as default audio device

[Paolo Bonzini]

It is now possible to specify the options for the default audio device
using -audio, so there is no need anymore to use a fake -audiodev option.

Remove the fall back to QTAILQ_FIRST(&audio_states), instead remember the
AudioState that was created from default_audiodevs and use that one.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c                   | 25 +++++++------------------
 docs/about/deprecated.rst       |  6 ------
 docs/about/removed-features.rst |  8 ++++++++
 3 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 186cc4d336e..de37ad7c074 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -104,6 +104,7 @@ static audio_driver *audio_driver_lookup(const char *name)
 
 static QTAILQ_HEAD(AudioStateHead, AudioState) audio_states =
     QTAILQ_HEAD_INITIALIZER(audio_states);
+static AudioState *default_audio_state;
 
 const struct mixeng_volume nominal_volume = {
     .mute = 0,
@@ -1660,6 +1661,7 @@ static void free_audio_state(AudioState *s)
 
 void audio_cleanup(void)
 {
+    default_audio_state = NULL;
     while (!QTAILQ_EMPTY(&audio_states)) {
         AudioState *s = QTAILQ_FIRST(&audio_states);
         QTAILQ_REMOVE(&audio_states, s, list);
@@ -1760,6 +1762,7 @@ static AudioState *audio_init(Audiodev *dev, Error **errp)
             goto out;
         }
     } else {
+        assert(!default_audio_state);
         for (;;) {
             AudiodevListEntry *e = QSIMPLEQ_FIRST(&default_audiodevs);
             if (!e) {
@@ -1801,24 +1804,9 @@ out:
 bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
 {
     if (!card->state) {
-        if (!QTAILQ_EMPTY(&audio_states)) {
-            /*
-             * FIXME: once it is possible to create an arbitrary
-             * default device via -audio DRIVER,OPT=VALUE (no "model"),
-             * replace this special case with the default AudioState*,
-             * storing it in a separate global.  For now, keep the
-             * warning to encourage moving off magic use of the first
-             * -audiodev.
-             */
-            if (QSIMPLEQ_EMPTY(&default_audiodevs)) {
-                dolog("Device %s: audiodev default parameter is deprecated, please "
-                      "specify audiodev=%s\n", name,
-                      QTAILQ_FIRST(&audio_states)->dev->id);
-            }
-            card->state = QTAILQ_FIRST(&audio_states);
-        } else {
-            card->state = audio_init(NULL, errp);
-            if (!card->state) {
+        if (!default_audio_state) {
+            default_audio_state = audio_init(NULL, errp);
+            if (!default_audio_state) {
                 if (!QSIMPLEQ_EMPTY(&audiodevs)) {
                     error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?\n",
                                       QSIMPLEQ_FIRST(&audiodevs)->dev->id);
@@ -1826,6 +1814,7 @@ bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
                 return false;
             }
         }
+        card->state = default_audio_state;
     }
 
     card->name = g_strdup (name);
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 2f51cf770ae..d59bcf36230 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -37,12 +37,6 @@ coverage.
 System emulator command line arguments
 --------------------------------------
 
-Creating sound card devices without ``audiodev=`` property (since 4.2)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-When not using the deprecated legacy audio config, each sound card
-should specify an ``audiodev=`` property.
-
 Short-form boolean options (since 6.0)
 ''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 58c94392c65..27639370f96 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -442,10 +442,18 @@ line using a ``secret`` object instance.
 The ``-audiodev`` and ``-audio`` command line options are now the only
 way to specify audio backend settings.
 
+Using ``-audiodev`` to define the default audio backend (removed in 8.2)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
 QEMU does not create default audio backends anymore if any of the
 ``-audiodev``, ``-audio`` or ``-nodefaults`` options are used on the
 command line.
 
+If an audio backend is created with ``-audiodev``, each sound card
+that wants to use it should specify an ``audiodev=``
+property.  Previously, the first audiodev command line option would be
+used as a fallback.
+
 Creating vnc without ``audiodev=`` property (removed in 8.2)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
-- 
2.41.0

Re: [PATCH 5/7] audio: do not use first -audiodev as default audio device

[BALATON Zoltan]

On Thu, 5 Oct 2023, Paolo Bonzini wrote:
> It is now possible to specify the options for the default audio device
> using -audio, so there is no need anymore to use a fake -audiodev option.
>
> Remove the fall back to QTAILQ_FIRST(&audio_states), instead remember the
> AudioState that was created from default_audiodevs and use that one.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> audio/audio.c                   | 25 +++++++------------------
> docs/about/deprecated.rst       |  6 ------
> docs/about/removed-features.rst |  8 ++++++++
> 3 files changed, 15 insertions(+), 24 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 186cc4d336e..de37ad7c074 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -104,6 +104,7 @@ static audio_driver *audio_driver_lookup(const char *name)
>
> static QTAILQ_HEAD(AudioStateHead, AudioState) audio_states =
>     QTAILQ_HEAD_INITIALIZER(audio_states);
> +static AudioState *default_audio_state;
>
> const struct mixeng_volume nominal_volume = {
>     .mute = 0,
> @@ -1660,6 +1661,7 @@ static void free_audio_state(AudioState *s)
>
> void audio_cleanup(void)
> {
> +    default_audio_state = NULL;
>     while (!QTAILQ_EMPTY(&audio_states)) {
>         AudioState *s = QTAILQ_FIRST(&audio_states);
>         QTAILQ_REMOVE(&audio_states, s, list);
> @@ -1760,6 +1762,7 @@ static AudioState *audio_init(Audiodev *dev, Error **errp)
>             goto out;
>         }
>     } else {
> +        assert(!default_audio_state);
>         for (;;) {
>             AudiodevListEntry *e = QSIMPLEQ_FIRST(&default_audiodevs);
>             if (!e) {
> @@ -1801,24 +1804,9 @@ out:
> bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
> {
>     if (!card->state) {
> -        if (!QTAILQ_EMPTY(&audio_states)) {
> -            /*
> -             * FIXME: once it is possible to create an arbitrary
> -             * default device via -audio DRIVER,OPT=VALUE (no "model"),
> -             * replace this special case with the default AudioState*,
> -             * storing it in a separate global.  For now, keep the
> -             * warning to encourage moving off magic use of the first
> -             * -audiodev.
> -             */
> -            if (QSIMPLEQ_EMPTY(&default_audiodevs)) {
> -                dolog("Device %s: audiodev default parameter is deprecated, please "
> -                      "specify audiodev=%s\n", name,
> -                      QTAILQ_FIRST(&audio_states)->dev->id);
> -            }
> -            card->state = QTAILQ_FIRST(&audio_states);
> -        } else {
> -            card->state = audio_init(NULL, errp);
> -            if (!card->state) {
> +        if (!default_audio_state) {
> +            default_audio_state = audio_init(NULL, errp);
> +            if (!default_audio_state) {
>                 if (!QSIMPLEQ_EMPTY(&audiodevs)) {
>                     error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?\n",
>                                       QSIMPLEQ_FIRST(&audiodevs)->dev->id);
> @@ -1826,6 +1814,7 @@ bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
>                 return false;
>             }
>         }
> +        card->state = default_audio_state;
>     }
>
>     card->name = g_strdup (name);
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 2f51cf770ae..d59bcf36230 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -37,12 +37,6 @@ coverage.
> System emulator command line arguments
> --------------------------------------
>
> -Creating sound card devices without ``audiodev=`` property (since 4.2)
> -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -When not using the deprecated legacy audio config, each sound card
> -should specify an ``audiodev=`` property.
> -
> Short-form boolean options (since 6.0)
> ''''''''''''''''''''''''''''''''''''''
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 58c94392c65..27639370f96 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -442,10 +442,18 @@ line using a ``secret`` object instance.
> The ``-audiodev`` and ``-audio`` command line options are now the only
> way to specify audio backend settings.
>
> +Using ``-audiodev`` to define the default audio backend (removed in 8.2)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> QEMU does not create default audio backends anymore if any of the
> ``-audiodev``, ``-audio`` or ``-nodefaults`` options are used on the
> command line.

Maybe this needs further updating because -audio can now define the 
default and is what should be used instead of -audiodev but this is not 
clear from this documentation.

Regards,
BALATON Zoltan

> +If an audio backend is created with ``-audiodev``, each sound card
> +that wants to use it should specify an ``audiodev=``
> +property.  Previously, the first audiodev command line option would be
> +used as a fallback.
> +
> Creating vnc without ``audiodev=`` property (removed in 8.2)
> ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>
>

Re: [PATCH 5/7] audio: do not use first -audiodev as default audio device

[BALATON Zoltan]

On Thu, 5 Oct 2023, BALATON Zoltan wrote:
> On Thu, 5 Oct 2023, Paolo Bonzini wrote:
>> It is now possible to specify the options for the default audio device
>> using -audio, so there is no need anymore to use a fake -audiodev option.
>> 
>> Remove the fall back to QTAILQ_FIRST(&audio_states), instead remember the
>> AudioState that was created from default_audiodevs and use that one.
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> audio/audio.c                   | 25 +++++++------------------
>> docs/about/deprecated.rst       |  6 ------
>> docs/about/removed-features.rst |  8 ++++++++
>> 3 files changed, 15 insertions(+), 24 deletions(-)
>> 
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 186cc4d336e..de37ad7c074 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -104,6 +104,7 @@ static audio_driver *audio_driver_lookup(const char 
>> *name)
>> 
>> static QTAILQ_HEAD(AudioStateHead, AudioState) audio_states =
>>     QTAILQ_HEAD_INITIALIZER(audio_states);
>> +static AudioState *default_audio_state;
>> 
>> const struct mixeng_volume nominal_volume = {
>>     .mute = 0,
>> @@ -1660,6 +1661,7 @@ static void free_audio_state(AudioState *s)
>> 
>> void audio_cleanup(void)
>> {
>> +    default_audio_state = NULL;
>>     while (!QTAILQ_EMPTY(&audio_states)) {
>>         AudioState *s = QTAILQ_FIRST(&audio_states);
>>         QTAILQ_REMOVE(&audio_states, s, list);
>> @@ -1760,6 +1762,7 @@ static AudioState *audio_init(Audiodev *dev, Error 
>> **errp)
>>             goto out;
>>         }
>>     } else {
>> +        assert(!default_audio_state);
>>         for (;;) {
>>             AudiodevListEntry *e = QSIMPLEQ_FIRST(&default_audiodevs);
>>             if (!e) {
>> @@ -1801,24 +1804,9 @@ out:
>> bool AUD_register_card (const char *name, QEMUSoundCard *card, Error 
>> **errp)
>> {
>>     if (!card->state) {
>> -        if (!QTAILQ_EMPTY(&audio_states)) {
>> -            /*
>> -             * FIXME: once it is possible to create an arbitrary
>> -             * default device via -audio DRIVER,OPT=VALUE (no "model"),
>> -             * replace this special case with the default AudioState*,
>> -             * storing it in a separate global.  For now, keep the
>> -             * warning to encourage moving off magic use of the first
>> -             * -audiodev.
>> -             */
>> -            if (QSIMPLEQ_EMPTY(&default_audiodevs)) {
>> -                dolog("Device %s: audiodev default parameter is 
>> deprecated, please "
>> -                      "specify audiodev=%s\n", name,
>> -                      QTAILQ_FIRST(&audio_states)->dev->id);
>> -            }
>> -            card->state = QTAILQ_FIRST(&audio_states);
>> -        } else {
>> -            card->state = audio_init(NULL, errp);
>> -            if (!card->state) {
>> +        if (!default_audio_state) {
>> +            default_audio_state = audio_init(NULL, errp);
>> +            if (!default_audio_state) {
>>                 if (!QSIMPLEQ_EMPTY(&audiodevs)) {
>>                     error_append_hint(errp, "Perhaps you wanted to set 
>> audiodev=%s?\n",
>>                                       QSIMPLEQ_FIRST(&audiodevs)->dev->id);
>> @@ -1826,6 +1814,7 @@ bool AUD_register_card (const char *name, 
>> QEMUSoundCard *card, Error **errp)
>>                 return false;
>>             }
>>         }
>> +        card->state = default_audio_state;
>>     }
>>
>>     card->name = g_strdup (name);
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 2f51cf770ae..d59bcf36230 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -37,12 +37,6 @@ coverage.
>> System emulator command line arguments
>> --------------------------------------
>> 
>> -Creating sound card devices without ``audiodev=`` property (since 4.2)
>> -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>> -
>> -When not using the deprecated legacy audio config, each sound card
>> -should specify an ``audiodev=`` property.
>> -
>> Short-form boolean options (since 6.0)
>> ''''''''''''''''''''''''''''''''''''''
>> 
>> diff --git a/docs/about/removed-features.rst 
>> b/docs/about/removed-features.rst
>> index 58c94392c65..27639370f96 100644
>> --- a/docs/about/removed-features.rst
>> +++ b/docs/about/removed-features.rst
>> @@ -442,10 +442,18 @@ line using a ``secret`` object instance.
>> The ``-audiodev`` and ``-audio`` command line options are now the only
>> way to specify audio backend settings.
>> 
>> +Using ``-audiodev`` to define the default audio backend (removed in 8.2)
>> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>> +
>> QEMU does not create default audio backends anymore if any of the
>> ``-audiodev``, ``-audio`` or ``-nodefaults`` options are used on the
>> command line.
>
> Maybe this needs further updating because -audio can now define the default 
> and is what should be used instead of -audiodev but this is not clear from 
> this documentation.

And while at it, maybe also mention machine audiodev property here as a 
way to set audiodev of embedded devices.

Regards,
BALATON Zoltan

>> +If an audio backend is created with ``-audiodev``, each sound card
>> +that wants to use it should specify an ``audiodev=``
>> +property.  Previously, the first audiodev command line option would be
>> +used as a fallback.
>> +
>> Creating vnc without ``audiodev=`` property (removed in 8.2)
>> ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>> 
>> 
>
>

Re: [PATCH 5/7] audio: do not use first -audiodev as default audio device

[Paolo Bonzini]

On Thu, Oct 5, 2023 at 3:39 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
> >> QEMU does not create default audio backends anymore if any of the
> >> ``-audiodev``, ``-audio`` or ``-nodefaults`` options are used on the
> >> command line.
> >
> > Maybe this needs further updating because -audio can now define the default
> > and is what should be used instead of -audiodev but this is not clear from
> > this documentation.
>
> And while at it, maybe also mention machine audiodev property here as a
> way to set audiodev of embedded devices.

Sure, here is my next attempt:

===================
Using ``-audiodev`` to define the default audio backend (removed in 8.2)
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
If no audiodev property is specified, previous versions would use the
first ``-audiodev`` command line option as a fallback.  Starting with
version 8.2, audio backends created with ``-audiodev`` will only be
used by clients (sound cards, machines with embedded sound hardware, VNC)
that refer to it in an ``audiodev=`` property.

In order to configure a default audio backend, use the ``-audio``
command line option without specifying a ``model``; while previous
versions of QEMU required a model, starting with version 8.2
QEMU does not require a model and will not create any sound card
in this case.

Note that the default audio backend must be configured on the command
line if the ``-nodefaults`` options is used.
===================

Re: [PATCH 5/7] audio: do not use first -audiodev as default audio device

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]

On Thu, 5 Oct 2023, Paolo Bonzini wrote:
> On Thu, Oct 5, 2023 at 3:39 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>> QEMU does not create default audio backends anymore if any of the
>>>> ``-audiodev``, ``-audio`` or ``-nodefaults`` options are used on the
>>>> command line.
>>>
>>> Maybe this needs further updating because -audio can now define the default
>>> and is what should be used instead of -audiodev but this is not clear from
>>> this documentation.
>>
>> And while at it, maybe also mention machine audiodev property here as a
>> way to set audiodev of embedded devices.
>
> Sure, here is my next attempt:

Much better, thanks. Maybe some more small clarifications as below:

> ===================
> Using ``-audiodev`` to define the default audio backend (removed in 8.2)
> ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> If no audiodev property is specified, previous versions would use the
> first ``-audiodev`` command line option as a fallback.  Starting with
> version 8.2, audio backends created with ``-audiodev`` will only be
> used by clients (sound cards, machines with embedded sound hardware, VNC)

machines with embedded sound hardware that can be set with the audiodev 
machine property

> that refer to it in an ``audiodev=`` property.
>
> In order to configure a default audio backend, use the ``-audio``
> command line option without specifying a ``model``; while previous
> versions of QEMU required a model, starting with version 8.2

required a model for -audio but starting with version 8.2

I'm still not sure users will get it without additional explanation 
somewhere explicitly saying that if you now get an error with -audiodev 
driver then you may now need to use -audio driver instead (hopefully the 
error will say that) but this is now detailed enough to at least try to 
explain that.

Regards,
BALATON Zoltan

> QEMU does not require a model and will not create any sound card
> in this case.
>
> Note that the default audio backend must be configured on the command
> line if the ``-nodefaults`` options is used.
> ===================
>
>

Re: [PATCH 5/7] audio: do not use first -audiodev as default audio device

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

Il gio 5 ott 2023, 17:40 BALATON Zoltan <balaton@eik.bme.hu> ha scritto:

> Much better, thanks. Maybe some more small clarifications as below:
>
> > ===================
> > Using ``-audiodev`` to define the default audio backend (removed in 8.2)
> > ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> > If no audiodev property is specified, previous versions would use the
> > first ``-audiodev`` command line option as a fallback.  Starting with
> > version 8.2, audio backends created with ``-audiodev`` will only be
> > used by clients (sound cards, machines with embedded sound hardware, VNC)
>
> machines with embedded sound hardware that can be set with the audiodev
> machine property
>

-M audiodev needs to be documented in the release notes, not in removed
features.

I'm still not sure users will get it without additional explanation
> somewhere explicitly saying that if you now get an error with -audiodev
> driver then you may now need to use -audio driver instead (hopefully the
> error will say that)


Currently the error says to add audiodev=, I can change that to propose
both.

Paolo

[-- Attachment #2: Type: text/html, Size: 2097 bytes --]

Re: [PATCH 5/7] audio: do not use first -audiodev as default audio device

[BALATON Zoltan]

On Thu, 5 Oct 2023, Paolo Bonzini wrote:
> Il gio 5 ott 2023, 17:40 BALATON Zoltan <balaton@eik.bme.hu> ha scritto:
>
>> Much better, thanks. Maybe some more small clarifications as below:
>>
>>> ===================
>>> Using ``-audiodev`` to define the default audio backend (removed in 8.2)
>>> ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>>> If no audiodev property is specified, previous versions would use the
>>> first ``-audiodev`` command line option as a fallback.  Starting with
>>> version 8.2, audio backends created with ``-audiodev`` will only be
>>> used by clients (sound cards, machines with embedded sound hardware, VNC)
>>
>> machines with embedded sound hardware that can be set with the audiodev
>> machine property
>>
>
> -M audiodev needs to be documented in the release notes, not in removed
> features.

The more places it's documented the better, peopla don't read docs anyway.

> I'm still not sure users will get it without additional explanation
>> somewhere explicitly saying that if you now get an error with -audiodev
>> driver then you may now need to use -audio driver instead (hopefully the
>> error will say that)
>
>
> Currently the error says to add audiodev=, I can change that to propose
> both.

That would help as likely the error will be the only thing the users will 
come across so if it tells them what to do that's the least annoyance.

Regards,
BALATON Zoltan

[PATCH 6/7] audio: reintroduce default audio backend for VNC

[Paolo Bonzini]

Make VNC use the default backend again if one is defined.
The recently introduced support for disabling the VNC audio
extension is still used, in case no default backend exists.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c                   | 28 ++++++++++++++++++----------
 audio/audio.h                   |  1 +
 docs/about/removed-features.rst | 10 ++--------
 ui/vnc.c                        |  2 ++
 4 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index de37ad7c074..ca0df29555a 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1801,20 +1801,28 @@ out:
     return NULL;
 }
 
+AudioState *audio_get_default_audio_state(Error **errp)
+{
+    if (!default_audio_state) {
+        default_audio_state = audio_init(NULL, errp);
+        if (!default_audio_state) {
+            if (!QSIMPLEQ_EMPTY(&audiodevs)) {
+                error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?\n",
+                                  QSIMPLEQ_FIRST(&audiodevs)->dev->id);
+            }
+        }
+    }
+
+    return default_audio_state;
+}
+
 bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
 {
     if (!card->state) {
-        if (!default_audio_state) {
-            default_audio_state = audio_init(NULL, errp);
-            if (!default_audio_state) {
-                if (!QSIMPLEQ_EMPTY(&audiodevs)) {
-                    error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?\n",
-                                      QSIMPLEQ_FIRST(&audiodevs)->dev->id);
-                }
-                return false;
-            }
+        card->state = audio_get_default_audio_state(errp);
+        if (!card->state) {
+            return false;
         }
-        card->state = default_audio_state;
     }
 
     card->name = g_strdup (name);
diff --git a/audio/audio.h b/audio/audio.h
index 6da5e8f6f0f..fcc22307bee 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -176,6 +176,7 @@ void audio_init_audiodevs(void);
 void audio_help(void);
 
 AudioState *audio_state_by_name(const char *name, Error **errp);
+AudioState *audio_get_default_audio_state(Error **errp);
 const char *audio_get_id(QEMUSoundCard *card);
 
 #define DEFINE_AUDIO_PROPERTIES(_s, _f)         \
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 27639370f96..a5e5a1ba1a5 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -449,17 +449,11 @@ QEMU does not create default audio backends anymore if any of the
 ``-audiodev``, ``-audio`` or ``-nodefaults`` options are used on the
 command line.
 
-If an audio backend is created with ``-audiodev``, each sound card
-that wants to use it should specify an ``audiodev=``
+If an audio backend is created with ``-audiodev``, each audio client
+(a sound card or VNC) that wants to use it should specify an ``audiodev=``
 property.  Previously, the first audiodev command line option would be
 used as a fallback.
 
-Creating vnc without ``audiodev=`` property (removed in 8.2)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-When using vnc, you should specify an ``audiodev=`` property if
-you plan to transmit audio through the VNC protocol.
-
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
 
diff --git a/ui/vnc.c b/ui/vnc.c
index acbb88c4ca0..7f4c471503a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4185,6 +4185,8 @@ void vnc_display_open(const char *id, Error **errp)
         if (!vd->audio_state) {
             goto fail;
         }
+    } else {
+        vd->audio_state = audio_get_default_audio_state(NULL);
     }
 
     device_id = qemu_opt_get(opts, "display");
-- 
2.41.0

[PATCH 7/7] audio, qtest: get rid of QEMU_AUDIO_DRV

[Paolo Bonzini]

Default audio devices can now be created with "-audio".  Tests for
soundcards were already using "-audiodev" if they want to specify a
particular backend, for the others remove the last remnants of
legacy audio configuration.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c          | 12 ------------
 tests/qtest/libqtest.c |  4 +---
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index ca0df29555a..feecbc1889a 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1690,19 +1690,7 @@ static const VMStateDescription vmstate_audio = {
 
 void audio_create_default_audiodevs(void)
 {
-    const char *drvname = getenv("QEMU_AUDIO_DRV");
-
-    /* QEMU_AUDIO_DRV=none is used by libqtest.  */
-    if (drvname && !g_str_equal(drvname, "none")) {
-        error_report("Please use -audiodev instead of QEMU_AUDIO_*");
-        exit(1);
-    }
-
     for (int i = 0; audio_prio_list[i]; i++) {
-        if (drvname && !g_str_equal(drvname, audio_prio_list[i])) {
-            continue;
-        }
-
         if (audio_driver_lookup(audio_prio_list[i])) {
             QDict *dict = qdict_new();
             Audiodev *dev = NULL;
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index b1eba71ffe5..3f94a4f4773 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -421,9 +421,6 @@ static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...)
         int sig = SIGKILL;
         procctl(P_PID, getpid(), PROC_PDEATHSIG_CTL, &sig);
 #endif /* __FreeBSD__ */
-        if (!g_setenv("QEMU_AUDIO_DRV", "none", true)) {
-            exit(1);
-        }
         execlp("/bin/sh", "sh", "-c", command->str, NULL);
         exit(1);
     }
@@ -464,6 +461,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
                          "-chardev socket,path=%s,id=char0 "
                          "-mon chardev=char0,mode=control "
                          "-display none "
+                         "-audio none "
                          "%s"
                          " -accel qtest",
                          socket_path,
-- 
2.41.0

Re: [PATCH 0/7] audio: redo default audio backend creation

[Marc-André Lureau]

On Thu, Oct 5, 2023 at 5:01 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Currently, AUD_register_card picks the audio backend from either:
>
> - the first audiodev that was created
>
> - the audio_prio_list[] array, which can be customized at
>   configure time
>
> This series instead extends -audio to define a default audio
> backend if no "model" is used.  This preserves simple command line
> use where a single "-audio" option applies to all audio devices
> and captures, and also uses a single QAPI-based configuration
> syntax for both -audio and -audiodev.
>
> The current hack to use the first -audiodev as a default audio
> device is removed.  For migration purposes, the first audiodev
> is suggested in case of an error:
>
>   ./qemu-system-x86_64 -device sb16 -audiodev pa,id=default
>   qemu-system-x86_64: -device sb16: no default audio driver available
>   Perhaps you wanted to set audiodev=default?
>
> VNC is changed to reintroduce use of the default audio backend;
> still, compared to before the cleanup effort this will not be
> enabled if -nodefaults is use, which is an improvement as it
> removes magic.
>
> Paolo
>
>
> Paolo Bonzini (7):
>   audio: error hints need a trailing \n
>   audio: disable default backends if -audio/-audiodev is used
>   audio: extract audio_define_default
>   audio: extend -audio to allow creating a default backend
>   audio: do not use first -audiodev as default audio device
>   audio: reintroduce default audio backend for VNC
>   audio, qtest: get rid of QEMU_AUDIO_DRV
>

Series:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>  audio/audio.c                   | 85 +++++++++++++--------------------
>  audio/audio.h                   |  3 ++
>  docs/about/deprecated.rst       |  6 ---
>  docs/about/removed-features.rst | 14 ++++--
>  qemu-options.hx                 | 29 +++++++----
>  system/vl.c                     | 34 ++++++++-----
>  tests/qtest/libqtest.c          |  4 +-
>  ui/vnc.c                        |  2 +
>  8 files changed, 93 insertions(+), 84 deletions(-)
>
> --
> 2.41.0
>
>


-- 
Marc-André Lureau