| Message ID | 20200622112914.30454-12-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | audio: deprecate -soundhw | expand |
On 22/06/2020 13.29, Gerd Hoffmann wrote: > Add deprecation message to the audio init function. > > Factor out audio initialization and call that from > both audio init and realize, so setting audiodev via > -global is enough to properly initialize pcspk. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/audio/pcspk.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) [...] > +static int pcspk_audio_init_soundhw(ISABus *bus) > +{ > + PCSpkState *s = pcspk_state; > + > + warn_report("'-soundhw pcspk' is deprecated, " > + "please set a backend using '-global isa-pcspk.audiodev=<name>' instead"); > + return pcspk_audio_init(s); > +} While "-soundhw pcspk" is quite easy to use for the average user, I think the "-global" options will be quite hard to figure out, especially once this deprecation message got removed again when -soundhw has been deleted. Could you maybe add a description how to configure the pc-speaker to docs/system/target-i386-desc.rst.inc, too? Thanks, Thomas
On 6/22/20 2:59 PM, Thomas Huth wrote: > On 22/06/2020 13.29, Gerd Hoffmann wrote: >> Add deprecation message to the audio init function. >> >> Factor out audio initialization and call that from >> both audio init and realize, so setting audiodev via >> -global is enough to properly initialize pcspk. >> >> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> >> --- >> hw/audio/pcspk.c | 24 +++++++++++++++++++++--- >> 1 file changed, 21 insertions(+), 3 deletions(-) > [...] >> +static int pcspk_audio_init_soundhw(ISABus *bus) >> +{ >> + PCSpkState *s = pcspk_state; >> + >> + warn_report("'-soundhw pcspk' is deprecated, " >> + "please set a backend using '-global isa-pcspk.audiodev=<name>' instead"); Markus's "Crazy shit around -global (pardon my french)" series instead suggest to use '-device ...': https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg06667.html Could that work here? >> + return pcspk_audio_init(s); >> +} > > While "-soundhw pcspk" is quite easy to use for the average user, I > think the "-global" options will be quite hard to figure out, especially > once this deprecation message got removed again when -soundhw has been > deleted. Could you maybe add a description how to configure the > pc-speaker to docs/system/target-i386-desc.rst.inc, too? > > Thanks, > Thomas > >
On 22/06/20 14:59, Thomas Huth wrote: >> +static int pcspk_audio_init_soundhw(ISABus *bus) >> +{ >> + PCSpkState *s = pcspk_state; >> + >> + warn_report("'-soundhw pcspk' is deprecated, " >> + "please set a backend using '-global isa-pcspk.audiodev=<name>' instead"); >> + return pcspk_audio_init(s); >> +} > While "-soundhw pcspk" is quite easy to use for the average user, I > think the "-global" options will be quite hard to figure out, especially > once this deprecation message got removed again when -soundhw has been > deleted. Could you maybe add a description how to configure the > pc-speaker to docs/system/target-i386-desc.rst.inc, too? -nic is an example of how a single option can work great for both built-in and custom devices, and perhaps we could design something like that for audio. For example you could have # configure a builtin or default audio device # and an ALSA audiodev -sound alsa # configure an external audio device and a PA audiodev # options other than "model" go to audiodev -sound pa,model=sb16,fixed-settings=off For more information: https://www.qemu.org/2018/05/31/nic-parameter/ I think we can go ahead with the deprecation, but -global and -device are probably not good enough a replacement for actual removal. Paolo
On 22/06/20 15:07, Philippe Mathieu-Daudé wrote: >>> +static int pcspk_audio_init_soundhw(ISABus *bus) >>> +{ >>> + PCSpkState *s = pcspk_state; >>> + >>> + warn_report("'-soundhw pcspk' is deprecated, " >>> + "please set a backend using '-global isa-pcspk.audiodev=<name>' instead"); > Markus's "Crazy shit around -global (pardon my french)" > series instead suggest to use '-device ...': > https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg06667.html > Could that work here? No, this is a different issue. The problem with the floppy is that it conflated two devices in one (controller and drive), while here the device is builtin. In this case -global could be replaced by a machine property, similar to what is done for the block devices that back parallel flash. That however is orthogonal to providing a good CLI for configuring audio. Paolo
On Mon, Jun 22, 2020 at 02:59:10PM +0200, Thomas Huth wrote: > On 22/06/2020 13.29, Gerd Hoffmann wrote: > > Add deprecation message to the audio init function. > > > > Factor out audio initialization and call that from > > both audio init and realize, so setting audiodev via > > -global is enough to properly initialize pcspk. > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > --- > > hw/audio/pcspk.c | 24 +++++++++++++++++++++--- > > 1 file changed, 21 insertions(+), 3 deletions(-) > [...] > > +static int pcspk_audio_init_soundhw(ISABus *bus) > > +{ > > + PCSpkState *s = pcspk_state; > > + > > + warn_report("'-soundhw pcspk' is deprecated, " > > + "please set a backend using '-global isa-pcspk.audiodev=<name>' instead"); > > + return pcspk_audio_init(s); > > +} > > While "-soundhw pcspk" is quite easy to use for the average user, I > think the "-global" options will be quite hard to figure out, especially > once this deprecation message got removed again when -soundhw has been > deleted. See also patch 19/19. > Could you maybe add a description how to configure the > pc-speaker to docs/system/target-i386-desc.rst.inc, too? Makes sense indeed. take care, Gerd
On Mon, Jun 22, 2020 at 03:22:05PM +0200, Paolo Bonzini wrote: > On 22/06/20 14:59, Thomas Huth wrote: > >> +static int pcspk_audio_init_soundhw(ISABus *bus) > >> +{ > >> + PCSpkState *s = pcspk_state; > >> + > >> + warn_report("'-soundhw pcspk' is deprecated, " > >> + "please set a backend using '-global isa-pcspk.audiodev=<name>' instead"); > >> + return pcspk_audio_init(s); > >> +} > > While "-soundhw pcspk" is quite easy to use for the average user, I > > think the "-global" options will be quite hard to figure out, especially > > once this deprecation message got removed again when -soundhw has been > > deleted. Could you maybe add a description how to configure the > > pc-speaker to docs/system/target-i386-desc.rst.inc, too? > > -nic is an example of how a single option can work great for both > built-in and custom devices, and perhaps we could design something like > that for audio. For example you could have > > # configure a builtin or default audio device > # and an ALSA audiodev > -sound alsa > > # configure an external audio device and a PA audiodev > # options other than "model" go to audiodev > -sound pa,model=sb16,fixed-settings=off > > For more information: https://www.qemu.org/2018/05/31/nic-parameter/ > > I think we can go ahead with the deprecation, but -global and -device > are probably not good enough a replacement for actual removal. -global is temporary, see patch 19/19 (guess I should reorder the series and squash 19/19 to fix that confusion). The plan is to have audio work simliar to block. -audiodev creates a backend (like -blockdev). Properties are used to reference them. Device properties in most cases, machine properties for builtin devices (i.e. pcspk works like pflash then). take care, Gerd
On 22/06/20 16:03, Gerd Hoffmann wrote: > > The plan is to have audio work simliar to block. -audiodev creates a > backend (like -blockdev). Properties are used to reference them. > Device properties in most cases, machine properties for builtin devices > (i.e. pcspk works like pflash then). Block still has a "-drive" option to configure both frontend and backend, though. Audio should have the same (and I modeled my example on -nic because that's much better than -drive). Paolo
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c index c37a3878612e..ab290e686783 100644 --- a/hw/audio/pcspk.c +++ b/hw/audio/pcspk.c @@ -28,6 +28,7 @@ #include "audio/audio.h" #include "qemu/module.h" #include "qemu/timer.h" +#include "qemu/error-report.h" #include "hw/timer/i8254.h" #include "migration/vmstate.h" #include "hw/audio/pcspk.h" @@ -112,11 +113,15 @@ static void pcspk_callback(void *opaque, int free) } } -static int pcspk_audio_init(ISABus *bus) +static int pcspk_audio_init(PCSpkState *s) { - PCSpkState *s = pcspk_state; struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0}; + if (s->voice) { + /* already initialized */ + return 0; + } + AUD_register_card(s_spk, &s->card); s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback, &as); @@ -185,6 +190,10 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp) isa_register_ioport(isadev, &s->ioport, s->iobase); + if (s->card.state) { + pcspk_audio_init(s); + } + pcspk_state = s; } @@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = { .class_init = pcspk_class_initfn, }; +static int pcspk_audio_init_soundhw(ISABus *bus) +{ + PCSpkState *s = pcspk_state; + + warn_report("'-soundhw pcspk' is deprecated, " + "please set a backend using '-global isa-pcspk.audiodev=<name>' instead"); + return pcspk_audio_init(s); +} + static void pcspk_register(void) { type_register_static(&pcspk_info); - isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init); + isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw); } type_init(pcspk_register)
Add deprecation message to the audio init function. Factor out audio initialization and call that from both audio init and realize, so setting audiodev via -global is enough to properly initialize pcspk. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/audio/pcspk.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)