[RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw

Paolo Bonzini posted 6 patches 3 years, 9 months ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
[RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
Posted by Paolo Bonzini 3 years, 9 months ago
-audio is used like "-audio pa,model=sb16".  It is almost as simple as
-soundhw, but it reuses the -audiodev parsing machinery and attaches an
audiodev to the newly-created device.  The main 'feature' is that
it knows about adding the codec device for model=intel-hda, and adding
the audiodev to the codec device.

In the future, it could be extended to support default models or
builtin devices, just like -nic, or even a default backend.  For now,
keep it simple.

JSON parsing is not supported for -audio.  This is okay because the
option is targeted at end users, not programs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c                   |  8 +++++-
 audio/audio.h                   |  1 +
 docs/about/deprecated.rst       |  9 ------
 docs/about/removed-features.rst |  7 +++++
 hw/audio/intel-hda.c            |  5 ++--
 hw/audio/soundhw.c              | 12 +++++---
 include/hw/audio/soundhw.h      |  4 +--
 qemu-options.hx                 | 51 ++++++++++++++++-----------------
 softmmu/vl.c                    | 28 ++++++++++++++++--
 9 files changed, 76 insertions(+), 49 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 9e91a5a4f2..a02f3ce5c6 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2099,13 +2099,19 @@ static void audio_validate_opts(Audiodev *dev, Error **errp)
 
 void audio_parse_option(const char *opt)
 {
-    AudiodevListEntry *e;
     Audiodev *dev = NULL;
 
     Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
     visit_type_Audiodev(v, NULL, &dev, &error_fatal);
     visit_free(v);
 
+    audio_define(dev);
+}
+
+void audio_define(Audiodev *dev)
+{
+    AudiodevListEntry *e;
+
     audio_validate_opts(dev, &error_fatal);
 
     e = g_new0(AudiodevListEntry, 1);
diff --git a/audio/audio.h b/audio/audio.h
index 3d5ecdecd5..b5e17cd218 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -168,6 +168,7 @@ void audio_sample_to_uint64(const void *samples, int pos,
 void audio_sample_from_uint64(void *samples, int pos,
                             uint64_t left, uint64_t right);
 
+void audio_define(Audiodev *audio);
 void audio_parse_option(const char *opt);
 void audio_init_audiodevs(void);
 void audio_legacy_help(void);
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 896e5a97ab..70885d09f3 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -39,15 +39,6 @@ should specify an ``audiodev=`` property.  Additionally, when using
 vnc, you should specify an ``audiodev=`` property if you plan to
 transmit audio through the VNC protocol.
 
-Creating sound card devices using ``-soundhw`` (since 5.1)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-Sound card devices should be created using ``-device`` instead.  The
-names are the same for most devices.  The exceptions are ``hda`` which
-needs two devices (``-device intel-hda -device hda-duplex``) and
-``pcspk`` which can be activated using ``-machine
-pcspk-audiodev=<name>``.
-
 ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 3f324d0536..eabc6c63ac 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -632,6 +632,13 @@ tripped up the CI testing and was suspected to be quite broken. For that
 reason the maintainers strongly suspected no one actually used it.
 
 
+Creating sound card devices using ``-soundhw`` (removed in 7.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Sound card devices should be created using ``-device`` or ``-audio``.
+The exception is ``pcspk`` which can be activated using ``-machine
+pcspk-audiodev=<name>``.
+
 TCG introspection features
 --------------------------
 
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index bc77e3d8c9..f38117057b 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1311,17 +1311,16 @@ static const TypeInfo hda_codec_device_type_info = {
  * create intel hda controller with codec attached to it,
  * so '-soundhw hda' works.
  */
-static int intel_hda_and_codec_init(PCIBus *bus)
+static int intel_hda_and_codec_init(PCIBus *bus, const char *audiodev)
 {
     DeviceState *controller;
     BusState *hdabus;
     DeviceState *codec;
 
-    warn_report("'-soundhw hda' is deprecated, "
-                "please use '-device intel-hda -device hda-duplex' instead");
     controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
     hdabus = QLIST_FIRST(&controller->child_bus);
     codec = qdev_new("hda-duplex");
+    qdev_prop_set_string(codec, "audiodev", audiodev);
     qdev_realize_and_unref(codec, hdabus, &error_fatal);
     return 0;
 }
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index d81ae91136..e979be08ce 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -27,6 +27,7 @@
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qom/object.h"
+#include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
 #include "hw/pci/pci.h"
 #include "hw/audio/soundhw.h"
@@ -36,14 +37,14 @@ struct soundhw {
     const char *descr;
     const char *typename;
     int isa;
-    int (*init_pci) (PCIBus *bus);
+    int (*init_pci) (PCIBus *bus, const char *audiodev);
 };
 
 static struct soundhw soundhw[9];
 static int soundhw_count;
 
 void pci_register_soundhw(const char *name, const char *descr,
-                          int (*init_pci)(PCIBus *bus))
+                          int (*init_pci)(PCIBus *bus, const char *audiodev))
 {
     assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
     soundhw[soundhw_count].name = name;
@@ -80,8 +81,9 @@ void show_valid_soundhw(void)
 }
 
 static struct soundhw *selected = NULL;
+static const char *audiodev_id;
 
-void select_soundhw(const char *optarg)
+void select_soundhw(const char *optarg, const char *audiodev)
 {
     struct soundhw *c;
 
@@ -92,6 +94,7 @@ void select_soundhw(const char *optarg)
     for (c = soundhw; c->name; ++c) {
         if (g_str_equal(c->name, optarg)) {
             selected = c;
+            audiodev_id = audiodev;
             break;
         }
     }
@@ -126,10 +129,11 @@ void soundhw_init(void)
 
     if (c->typename) {
         DeviceState *dev = qdev_new(c->typename);
+        qdev_prop_set_string(dev, "audiodev", audiodev_id);
         qdev_realize_and_unref(dev, bus, &error_fatal);
     } else {
         assert(!c->isa);
-        c->init_pci(pci_bus);
+        c->init_pci(pci_bus, audiodev_id);
     }
 }
 
diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
index dec5c0cdca..270717a06a 100644
--- a/include/hw/audio/soundhw.h
+++ b/include/hw/audio/soundhw.h
@@ -2,12 +2,12 @@
 #define HW_SOUNDHW_H
 
 void pci_register_soundhw(const char *name, const char *descr,
-                          int (*init_pci)(PCIBus *bus));
+                          int (*init_pci)(PCIBus *bus, const char *audiodev));
 void deprecated_register_soundhw(const char *name, const char *descr,
                                  int isa, const char *typename);
 
 void soundhw_init(void);
 void show_valid_soundhw(void);
-void select_soundhw(const char *optarg);
+void select_soundhw(const char *optarg, const char *audiodev);
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index bc196808ae..862263d435 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -661,6 +661,30 @@ SRST
     (deprecated) environment variables.
 ERST
 
+DEF("audio", HAS_ARG, QEMU_OPTION_audio,
+    "-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 host backend options are the same as with the corresponding
+    ``-audiodev`` options below. 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:
+
+    .. parsed-literal::
+
+        |qemu_system| -audiodev pa,id=pa -device sb16,audiodev=pa
+        |qemu_system| -audio pa,model=sb16
+ERST
+
 DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
     "-audiodev [driver=]driver,id=id[,prop[=value][,...]]\n"
     "                specifies the audio backend to use\n"
@@ -892,33 +916,6 @@ SRST
         ``qemu.wav``.
 ERST
 
-DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
-    "-soundhw c1,... enable audio support\n"
-    "                and only specified sound cards (comma separated list)\n"
-    "                use '-soundhw help' to get the list of supported cards\n"
-    "                use '-soundhw all' to enable all of them\n", QEMU_ARCH_ALL)
-SRST
-``-soundhw card1[,card2,...] or -soundhw all``
-    Enable audio and selected sound hardware. Use 'help' to print all
-    available sound hardware. For example:
-
-    .. parsed-literal::
-
-        |qemu_system_x86| -soundhw sb16,adlib disk.img
-        |qemu_system_x86| -soundhw es1370 disk.img
-        |qemu_system_x86| -soundhw ac97 disk.img
-        |qemu_system_x86| -soundhw hda disk.img
-        |qemu_system_x86| -soundhw all disk.img
-        |qemu_system_x86| -soundhw help
-
-    Note that Linux's i810\_audio OSS kernel (for AC97) module might
-    require manually specifying clocking.
-
-    ::
-
-        modprobe i810_audio clocking=48000
-ERST
-
 DEF("device", HAS_ARG, QEMU_OPTION_device,
     "-device driver[,prop[=value][,...]]\n"
     "                add device (based on driver)\n"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5bea0eb3eb..979bbda5aa 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -116,6 +116,8 @@
 #include "crypto/init.h"
 #include "sysemu/replay.h"
 #include "qapi/qapi-events-run-state.h"
+#include "qapi/qapi-types-audio.h"
+#include "qapi/qapi-visit-audio.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "qapi/qapi-visit-compat.h"
 #include "qapi/qapi-visit-ui.h"
@@ -3018,13 +3020,33 @@ void qemu_init(int argc, char **argv, char **envp)
             case QEMU_OPTION_audiodev:
                 audio_parse_option(optarg);
                 break;
-            case QEMU_OPTION_soundhw:
-                if (is_help_option(optarg)) {
+            case QEMU_OPTION_audio: {
+                QDict *dict = keyval_parse(optarg, "driver", NULL, &error_fatal);
+                char *model;
+                Audiodev *dev = NULL;
+                Visitor *v;
+
+                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);
                 }
-                select_soundhw (optarg);
+                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);
                 break;
+            }
             case QEMU_OPTION_h:
                 help(0);
                 break;
-- 
2.35.1
Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
Posted by Martin Kletzander 3 years, 9 months ago
On Wed, Apr 27, 2022 at 01:32:25PM +0200, Paolo Bonzini wrote:
>-audio is used like "-audio pa,model=sb16".  It is almost as simple as
>-soundhw, but it reuses the -audiodev parsing machinery and attaches an
>audiodev to the newly-created device.  The main 'feature' is that
>it knows about adding the codec device for model=intel-hda, and adding
>the audiodev to the codec device.
>
>In the future, it could be extended to support default models or
>builtin devices, just like -nic, or even a default backend.  For now,
>keep it simple.
>
>JSON parsing is not supported for -audio.  This is okay because the
>option is targeted at end users, not programs.
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> audio/audio.c                   |  8 +++++-
> audio/audio.h                   |  1 +
> docs/about/deprecated.rst       |  9 ------
> docs/about/removed-features.rst |  7 +++++
> hw/audio/intel-hda.c            |  5 ++--
> hw/audio/soundhw.c              | 12 +++++---
> include/hw/audio/soundhw.h      |  4 +--
> qemu-options.hx                 | 51 ++++++++++++++++-----------------
> softmmu/vl.c                    | 28 ++++++++++++++++--
> 9 files changed, 76 insertions(+), 49 deletions(-)
>

[...]

>diff --git a/softmmu/vl.c b/softmmu/vl.c
>index 5bea0eb3eb..979bbda5aa 100644
>--- a/softmmu/vl.c
>+++ b/softmmu/vl.c
>@@ -3018,13 +3020,33 @@ void qemu_init(int argc, char **argv, char **envp)
>             case QEMU_OPTION_audiodev:
>                 audio_parse_option(optarg);
>                 break;
>-            case QEMU_OPTION_soundhw:
>-                if (is_help_option(optarg)) {
>+            case QEMU_OPTION_audio: {
>+                QDict *dict = keyval_parse(optarg, "driver", NULL, &error_fatal);
>+                char *model;
>+                Audiodev *dev = NULL;
>+                Visitor *v;
>+
>+                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);
>                 }
>-                select_soundhw (optarg);
>+                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);

I was looking at this and thought that you might be creating multiple
audiodevs if there are multiple -audio options.  And I found out that
even with current master it is possible to create multiple audiodevs not
only with the same driver, but even with the same ID (and different
drivers).

I guess that is not preferable, but I can't say for sure how this is
supposed to be handled.

Other than that it looks fine to me.
Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
Posted by Mark Cave-Ayland 3 years, 9 months ago
On 27/04/2022 12:32, Paolo Bonzini wrote:

> -audio is used like "-audio pa,model=sb16".  It is almost as simple as
> -soundhw, but it reuses the -audiodev parsing machinery and attaches an
> audiodev to the newly-created device.  The main 'feature' is that
> it knows about adding the codec device for model=intel-hda, and adding
> the audiodev to the codec device.
> 
> In the future, it could be extended to support default models or
> builtin devices, just like -nic, or even a default backend.  For now,
> keep it simple.
> 
> JSON parsing is not supported for -audio.  This is okay because the
> option is targeted at end users, not programs.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   audio/audio.c                   |  8 +++++-
>   audio/audio.h                   |  1 +
>   docs/about/deprecated.rst       |  9 ------
>   docs/about/removed-features.rst |  7 +++++
>   hw/audio/intel-hda.c            |  5 ++--
>   hw/audio/soundhw.c              | 12 +++++---
>   include/hw/audio/soundhw.h      |  4 +--
>   qemu-options.hx                 | 51 ++++++++++++++++-----------------
>   softmmu/vl.c                    | 28 ++++++++++++++++--
>   9 files changed, 76 insertions(+), 49 deletions(-)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index 9e91a5a4f2..a02f3ce5c6 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -2099,13 +2099,19 @@ static void audio_validate_opts(Audiodev *dev, Error **errp)
>   
>   void audio_parse_option(const char *opt)
>   {
> -    AudiodevListEntry *e;
>       Audiodev *dev = NULL;
>   
>       Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
>       visit_type_Audiodev(v, NULL, &dev, &error_fatal);
>       visit_free(v);
>   
> +    audio_define(dev);
> +}
> +
> +void audio_define(Audiodev *dev)
> +{
> +    AudiodevListEntry *e;
> +
>       audio_validate_opts(dev, &error_fatal);
>   
>       e = g_new0(AudiodevListEntry, 1);
> diff --git a/audio/audio.h b/audio/audio.h
> index 3d5ecdecd5..b5e17cd218 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -168,6 +168,7 @@ void audio_sample_to_uint64(const void *samples, int pos,
>   void audio_sample_from_uint64(void *samples, int pos,
>                               uint64_t left, uint64_t right);
>   
> +void audio_define(Audiodev *audio);
>   void audio_parse_option(const char *opt);
>   void audio_init_audiodevs(void);
>   void audio_legacy_help(void);
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 896e5a97ab..70885d09f3 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -39,15 +39,6 @@ should specify an ``audiodev=`` property.  Additionally, when using
>   vnc, you should specify an ``audiodev=`` property if you plan to
>   transmit audio through the VNC protocol.
>   
> -Creating sound card devices using ``-soundhw`` (since 5.1)
> -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -Sound card devices should be created using ``-device`` instead.  The
> -names are the same for most devices.  The exceptions are ``hda`` which
> -needs two devices (``-device intel-hda -device hda-duplex``) and
> -``pcspk`` which can be activated using ``-machine
> -pcspk-audiodev=<name>``.
> -
>   ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
>   ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>   
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 3f324d0536..eabc6c63ac 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -632,6 +632,13 @@ tripped up the CI testing and was suspected to be quite broken. For that
>   reason the maintainers strongly suspected no one actually used it.
>   
>   
> +Creating sound card devices using ``-soundhw`` (removed in 7.1)
> +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Sound card devices should be created using ``-device`` or ``-audio``.
> +The exception is ``pcspk`` which can be activated using ``-machine
> +pcspk-audiodev=<name>``.
> +
>   TCG introspection features
>   --------------------------
>   
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index bc77e3d8c9..f38117057b 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -1311,17 +1311,16 @@ static const TypeInfo hda_codec_device_type_info = {
>    * create intel hda controller with codec attached to it,
>    * so '-soundhw hda' works.
>    */
> -static int intel_hda_and_codec_init(PCIBus *bus)
> +static int intel_hda_and_codec_init(PCIBus *bus, const char *audiodev)
>   {
>       DeviceState *controller;
>       BusState *hdabus;
>       DeviceState *codec;
>   
> -    warn_report("'-soundhw hda' is deprecated, "
> -                "please use '-device intel-hda -device hda-duplex' instead");
>       controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
>       hdabus = QLIST_FIRST(&controller->child_bus);
>       codec = qdev_new("hda-duplex");
> +    qdev_prop_set_string(codec, "audiodev", audiodev);
>       qdev_realize_and_unref(codec, hdabus, &error_fatal);
>       return 0;
>   }
> diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
> index d81ae91136..e979be08ce 100644
> --- a/hw/audio/soundhw.c
> +++ b/hw/audio/soundhw.c
> @@ -27,6 +27,7 @@
>   #include "qemu/error-report.h"
>   #include "qapi/error.h"
>   #include "qom/object.h"
> +#include "hw/qdev-properties.h"
>   #include "hw/isa/isa.h"
>   #include "hw/pci/pci.h"
>   #include "hw/audio/soundhw.h"
> @@ -36,14 +37,14 @@ struct soundhw {
>       const char *descr;
>       const char *typename;
>       int isa;
> -    int (*init_pci) (PCIBus *bus);
> +    int (*init_pci) (PCIBus *bus, const char *audiodev);
>   };
>   
>   static struct soundhw soundhw[9];
>   static int soundhw_count;
>   
>   void pci_register_soundhw(const char *name, const char *descr,
> -                          int (*init_pci)(PCIBus *bus))
> +                          int (*init_pci)(PCIBus *bus, const char *audiodev))
>   {
>       assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
>       soundhw[soundhw_count].name = name;
> @@ -80,8 +81,9 @@ void show_valid_soundhw(void)
>   }
>   
>   static struct soundhw *selected = NULL;
> +static const char *audiodev_id;
>   
> -void select_soundhw(const char *optarg)
> +void select_soundhw(const char *optarg, const char *audiodev)
>   {
>       struct soundhw *c;
>   
> @@ -92,6 +94,7 @@ void select_soundhw(const char *optarg)
>       for (c = soundhw; c->name; ++c) {
>           if (g_str_equal(c->name, optarg)) {
>               selected = c;
> +            audiodev_id = audiodev;
>               break;
>           }
>       }
> @@ -126,10 +129,11 @@ void soundhw_init(void)
>   
>       if (c->typename) {
>           DeviceState *dev = qdev_new(c->typename);
> +        qdev_prop_set_string(dev, "audiodev", audiodev_id);
>           qdev_realize_and_unref(dev, bus, &error_fatal);
>       } else {
>           assert(!c->isa);
> -        c->init_pci(pci_bus);
> +        c->init_pci(pci_bus, audiodev_id);
>       }
>   }
>   
> diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
> index dec5c0cdca..270717a06a 100644
> --- a/include/hw/audio/soundhw.h
> +++ b/include/hw/audio/soundhw.h
> @@ -2,12 +2,12 @@
>   #define HW_SOUNDHW_H
>   
>   void pci_register_soundhw(const char *name, const char *descr,
> -                          int (*init_pci)(PCIBus *bus));
> +                          int (*init_pci)(PCIBus *bus, const char *audiodev));
>   void deprecated_register_soundhw(const char *name, const char *descr,
>                                    int isa, const char *typename);
>   
>   void soundhw_init(void);
>   void show_valid_soundhw(void);
> -void select_soundhw(const char *optarg);
> +void select_soundhw(const char *optarg, const char *audiodev);
>   
>   #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index bc196808ae..862263d435 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -661,6 +661,30 @@ SRST
>       (deprecated) environment variables.
>   ERST
>   
> +DEF("audio", HAS_ARG, QEMU_OPTION_audio,
> +    "-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 host backend options are the same as with the corresponding
> +    ``-audiodev`` options below. 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:
> +
> +    .. parsed-literal::
> +
> +        |qemu_system| -audiodev pa,id=pa -device sb16,audiodev=pa
> +        |qemu_system| -audio pa,model=sb16
> +ERST
> +
>   DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
>       "-audiodev [driver=]driver,id=id[,prop[=value][,...]]\n"
>       "                specifies the audio backend to use\n"
> @@ -892,33 +916,6 @@ SRST
>           ``qemu.wav``.
>   ERST
>   
> -DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
> -    "-soundhw c1,... enable audio support\n"
> -    "                and only specified sound cards (comma separated list)\n"
> -    "                use '-soundhw help' to get the list of supported cards\n"
> -    "                use '-soundhw all' to enable all of them\n", QEMU_ARCH_ALL)
> -SRST
> -``-soundhw card1[,card2,...] or -soundhw all``
> -    Enable audio and selected sound hardware. Use 'help' to print all
> -    available sound hardware. For example:
> -
> -    .. parsed-literal::
> -
> -        |qemu_system_x86| -soundhw sb16,adlib disk.img
> -        |qemu_system_x86| -soundhw es1370 disk.img
> -        |qemu_system_x86| -soundhw ac97 disk.img
> -        |qemu_system_x86| -soundhw hda disk.img
> -        |qemu_system_x86| -soundhw all disk.img
> -        |qemu_system_x86| -soundhw help
> -
> -    Note that Linux's i810\_audio OSS kernel (for AC97) module might
> -    require manually specifying clocking.
> -
> -    ::
> -
> -        modprobe i810_audio clocking=48000
> -ERST
> -
>   DEF("device", HAS_ARG, QEMU_OPTION_device,
>       "-device driver[,prop[=value][,...]]\n"
>       "                add device (based on driver)\n"
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5bea0eb3eb..979bbda5aa 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -116,6 +116,8 @@
>   #include "crypto/init.h"
>   #include "sysemu/replay.h"
>   #include "qapi/qapi-events-run-state.h"
> +#include "qapi/qapi-types-audio.h"
> +#include "qapi/qapi-visit-audio.h"
>   #include "qapi/qapi-visit-block-core.h"
>   #include "qapi/qapi-visit-compat.h"
>   #include "qapi/qapi-visit-ui.h"
> @@ -3018,13 +3020,33 @@ void qemu_init(int argc, char **argv, char **envp)
>               case QEMU_OPTION_audiodev:
>                   audio_parse_option(optarg);
>                   break;
> -            case QEMU_OPTION_soundhw:
> -                if (is_help_option(optarg)) {
> +            case QEMU_OPTION_audio: {
> +                QDict *dict = keyval_parse(optarg, "driver", NULL, &error_fatal);
> +                char *model;
> +                Audiodev *dev = NULL;
> +                Visitor *v;
> +
> +                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);
>                   }
> -                select_soundhw (optarg);
> +                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);
>                   break;
> +            }
>               case QEMU_OPTION_h:
>                   help(0);
>                   break;

Is it possible to change select_soundhw() to take an AudioDev pointer rather than a 
string, and then add a new qdev_prop_set_audiodev() function similar to 
qdev_prop_set_chr() and qdev_prop_set_netdev()?

In reality the underlying QOM property is still a string, but I think having the 
stronger typing for AudioDev properties is useful and potentially allows for the 
various *dev backend properties to become QOM links in future.


ATB,

Mark.
Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw
Posted by Paolo Bonzini 3 years, 9 months ago
On 4/27/22 15:41, Mark Cave-Ayland wrote:
>> +                select_soundhw(model, dev->id);
>> +                g_free(model);
>>                   break;
>> +            }
>>               case QEMU_OPTION_h:
>>                   help(0);
>>                   break;
> 
> Is it possible to change select_soundhw() to take an AudioDev pointer 
> rather than a string, and then add a new qdev_prop_set_audiodev() 
> function similar to qdev_prop_set_chr() and qdev_prop_set_netdev()?
> 
> In reality the underlying QOM property is still a string, but I think 
> having the stronger typing for AudioDev properties is useful and 
> potentially allows for the various *dev backend properties to become QOM 
> links in future.

I didn't consider that because there are just two uses and I don't 
expect them to grow much, but yes it's possible.

Paolo