| Message ID | 20231005125815.66082-6-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | audio: redo default audio backend creation | expand |
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) > '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' > >
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) >> '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' >> >> > >
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. ===================
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. > =================== > >
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
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
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) ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
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(-)