| Message ID | 92b31c6af268b8f2a4cc4ed5b20ee8d0e19f614d.1650874791.git.mkletzan@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | RFC: Remove deprecated audio features | expand |
On Mon, Apr 25, 2022 at 10:21:56AM +0200, Martin Kletzander wrote: > Now that all callers support error reporting with errp and all machine-default > devices use an explicit audiodev, this can be changed. To make the detection > easier make AUD_register_card() return false on error. > > Signed-off-by: Martin Kletzander <mkletzan@redhat.com> > --- > audio/audio.c | 7 +++++-- > audio/audio.h | 2 +- > hw/arm/omap2.c | 3 ++- > hw/audio/ac97.c | 6 +++++- > hw/audio/adlib.c | 7 +++++-- > hw/audio/cs4231a.c | 6 ++++-- > hw/audio/es1370.c | 5 ++++- > hw/audio/gus.c | 4 +++- > hw/audio/hda-codec.c | 5 ++++- > hw/audio/lm4549.c | 4 +++- > hw/audio/pcspk.c | 4 +++- > hw/audio/sb16.c | 6 ++++-- > hw/audio/wm8750.c | 5 ++++- > hw/display/xlnx_dp.c | 6 ++++-- > hw/input/tsc210x.c | 3 ++- > hw/usb/dev-audio.c | 5 ++++- > 16 files changed, 57 insertions(+), 21 deletions(-) > > diff --git a/audio/audio.c b/audio/audio.c > index 671845c65d18..b95aca444382 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1822,15 +1822,18 @@ void audio_free_audiodev_list(AudiodevListHead *head) > } > } > > -void AUD_register_card (const char *name, QEMUSoundCard *card) > +bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp) > { > if (!card->state) { > - card->state = audio_init(NULL, name); > + error_setg(errp, "No audiodev specified for %s", name); > + return false; > } This is a significant change in semantics. qemu-system-x86_64 -device ac97 will currently automatically create a default audio backend for the user, but now it just reports an error. I don't think we want todo this, as allowing 'audiodev' to be optional was an intentionale thing to be more user friendly to casual userss. It lets command line args they use "just work" regardless of which audio subsystem their host OS happens to be using, which wouldn't be the case if we force them to use -audiodev every time. > > card->name = g_strdup (name); > memset (&card->entries, 0, sizeof (card->entries)); > QLIST_INSERT_HEAD(&card->state->card_head, card, entries); > + > + return true; > } With regards, Daniel
On Mon, Apr 25, 2022 at 02:34:08PM +0100, Daniel P. Berrangé wrote: > On Mon, Apr 25, 2022 at 10:21:56AM +0200, Martin Kletzander wrote: > > Now that all callers support error reporting with errp and all machine-default > > devices use an explicit audiodev, this can be changed. To make the detection > > easier make AUD_register_card() return false on error. > > > > Signed-off-by: Martin Kletzander <mkletzan@redhat.com> > > --- > > audio/audio.c | 7 +++++-- > > audio/audio.h | 2 +- > > hw/arm/omap2.c | 3 ++- > > hw/audio/ac97.c | 6 +++++- > > hw/audio/adlib.c | 7 +++++-- > > hw/audio/cs4231a.c | 6 ++++-- > > hw/audio/es1370.c | 5 ++++- > > hw/audio/gus.c | 4 +++- > > hw/audio/hda-codec.c | 5 ++++- > > hw/audio/lm4549.c | 4 +++- > > hw/audio/pcspk.c | 4 +++- > > hw/audio/sb16.c | 6 ++++-- > > hw/audio/wm8750.c | 5 ++++- > > hw/display/xlnx_dp.c | 6 ++++-- > > hw/input/tsc210x.c | 3 ++- > > hw/usb/dev-audio.c | 5 ++++- > > 16 files changed, 57 insertions(+), 21 deletions(-) > > > > diff --git a/audio/audio.c b/audio/audio.c > > index 671845c65d18..b95aca444382 100644 > > --- a/audio/audio.c > > +++ b/audio/audio.c > > @@ -1822,15 +1822,18 @@ void audio_free_audiodev_list(AudiodevListHead *head) > > } > > } > > > > -void AUD_register_card (const char *name, QEMUSoundCard *card) > > +bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp) > > { > > if (!card->state) { > > - card->state = audio_init(NULL, name); > > + error_setg(errp, "No audiodev specified for %s", name); > > + return false; > > } > > This is a significant change in semantics. > > qemu-system-x86_64 -device ac97 > > will currently automatically create a default audio backend for the > user, but now it just reports an error. I don't think we want todo > this, as allowing 'audiodev' to be optional was an intentionale > thing to be more user friendly to casual userss. It lets command > line args they use "just work" regardless of which audio subsystem > their host OS happens to be using, which wouldn't be the case if we > force them to use -audiodev every time. Oh, I missed that we had already deprecated the omission of audiodev with the intent to make it mandatory (having previously tried to make it mandatory earlier) commit 4b3b7793e18e1e3edb90bbc21112e875f9ff826d Author: Kővágó, Zoltán <dirty.ice.hu@gmail.com> Date: Mon Aug 26 21:59:02 2019 +0200 audio: omitting audiodev= parameter is only deprecated > > > > > card->name = g_strdup (name); > > memset (&card->entries, 0, sizeof (card->entries)); > > QLIST_INSERT_HEAD(&card->state->card_head, card, entries); > > + > > + return true; > > } > > With regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| > With regards, Daniel
diff --git a/audio/audio.c b/audio/audio.c index 671845c65d18..b95aca444382 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1822,15 +1822,18 @@ void audio_free_audiodev_list(AudiodevListHead *head) } } -void AUD_register_card (const char *name, QEMUSoundCard *card) +bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp) { if (!card->state) { - card->state = audio_init(NULL, name); + error_setg(errp, "No audiodev specified for %s", name); + return false; } card->name = g_strdup (name); memset (&card->entries, 0, sizeof (card->entries)); QLIST_INSERT_HEAD(&card->state->card_head, card, entries); + + return true; } void AUD_remove_card (QEMUSoundCard *card) diff --git a/audio/audio.h b/audio/audio.h index 335704a4ddb1..9deed8ed6830 100644 --- a/audio/audio.h +++ b/audio/audio.h @@ -94,7 +94,7 @@ typedef struct QEMUAudioTimeStamp { void AUD_vlog (const char *cap, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 0); void AUD_log (const char *cap, const char *fmt, ...) G_GNUC_PRINTF(2, 3); -void AUD_register_card (const char *name, QEMUSoundCard *card); +bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp); void AUD_remove_card (QEMUSoundCard *card); CaptureVoiceOut *AUD_add_capture( AudioState *s, diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c index 4ae524a1a1a6..407c24551c84 100644 --- a/hw/arm/omap2.c +++ b/hw/arm/omap2.c @@ -618,7 +618,8 @@ static struct omap_eac_s *omap_eac_init(struct omap_target_agent_s *ta, s->codec.card.name); } - AUD_register_card("OMAP EAC", &s->codec.card); + /* We can quit here because this only gets called from machine class init */ + AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal); memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac", omap_l4_region_size(ta, 0)); diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c index fd0b3b97d5b5..8242ddb0f93d 100644 --- a/hw/audio/ac97.c +++ b/hw/audio/ac97.c @@ -1345,6 +1345,10 @@ static void ac97_realize(PCIDevice *dev, Error **errp) AC97LinkState *s = AC97(dev); uint8_t *c = s->dev.config; + if (!AUD_register_card ("ac97", &s->card, errp)) { + return; + } + /* TODO: no need to override */ c[PCI_COMMAND] = 0x00; /* pcicmd pci command rw, ro */ c[PCI_COMMAND + 1] = 0x00; @@ -1378,7 +1382,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp) "ac97-nabm", 256); pci_register_bar (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam); pci_register_bar (&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm); - AUD_register_card ("ac97", &s->card); + ac97_on_reset(DEVICE(s)); } diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c index ba1be6c8378d..39932654f7f5 100644 --- a/hw/audio/adlib.c +++ b/hw/audio/adlib.c @@ -269,8 +269,6 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) as.fmt = AUDIO_FORMAT_S16; as.endianness = AUDIO_HOST_ENDIANNESS; - AUD_register_card ("adlib", &s->card); - s->voice = AUD_open_out ( &s->card, s->voice, @@ -285,6 +283,11 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) return; } + if (!AUD_register_card ("adlib", &s->card, errp)) { + Adlib_fini (s); + return; + } + s->samples = AUD_get_buffer_size_out (s->voice) >> SHIFT; s->mixbuf = g_malloc0 (s->samples << SHIFT); diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c index f510b862efbe..d9353a51ec66 100644 --- a/hw/audio/cs4231a.c +++ b/hw/audio/cs4231a.c @@ -676,13 +676,15 @@ static void cs4231a_realizefn (DeviceState *dev, Error **errp) return; } + if (!AUD_register_card ("cs4231a", &s->card, errp)) { + return; + } + s->pic = isa_get_irq(d, s->irq); k = ISADMA_GET_CLASS(s->isa_dma); k->register_channel(s->isa_dma, s->dma, cs_dma_read, s); isa_register_ioport (d, &s->ioports, s->port); - - AUD_register_card ("cs4231a", &s->card); } static Property cs4231a_properties[] = { diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c index 62359b84f279..77a84f83060b 100644 --- a/hw/audio/es1370.c +++ b/hw/audio/es1370.c @@ -867,7 +867,10 @@ static void es1370_realize(PCIDevice *dev, Error **errp) memory_region_init_io (&s->io, OBJECT(s), &es1370_io_ops, s, "es1370", 256); pci_register_bar (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io); - AUD_register_card ("es1370", &s->card); + if (!AUD_register_card ("es1370", &s->card, errp)) { + return; + } + es1370_reset (s); } diff --git a/hw/audio/gus.c b/hw/audio/gus.c index c7710a684b88..f890042baa97 100644 --- a/hw/audio/gus.c +++ b/hw/audio/gus.c @@ -245,7 +245,9 @@ static void gus_realizefn (DeviceState *dev, Error **errp) return; } - AUD_register_card ("gus", &s->card); + if (!AUD_register_card ("gus", &s->card, errp)) { + return; + } as.freq = s->freq; as.nchannels = 2; diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c index e86a2adf31a0..7f8a7fa7ca1b 100644 --- a/hw/audio/hda-codec.c +++ b/hw/audio/hda-codec.c @@ -690,7 +690,10 @@ static void hda_audio_init(HDACodecDevice *hda, a->name = object_get_typename(OBJECT(a)); dprint(a, 1, "%s: cad %d\n", __func__, a->hda.cad); - AUD_register_card("hda", &a->card); + if (!AUD_register_card("hda", &a->card, errp)) { + return; + } + for (i = 0; i < a->desc->nnodes; i++) { node = a->desc->nodes + i; param = hda_codec_find_param(node, AC_PAR_AUDIO_WIDGET_CAP); diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c index 418041bc9c6c..9afb81517e8f 100644 --- a/hw/audio/lm4549.c +++ b/hw/audio/lm4549.c @@ -289,7 +289,9 @@ void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque, lm4549_reset(s); /* Register an audio card */ - AUD_register_card("lm4549", &s->card); + if (!AUD_register_card("lm4549", &s->card, errp)) { + return; + } /* Open a default voice */ as.freq = 48000; diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c index 72bea5fb202a..41bf6a5cfdb3 100644 --- a/hw/audio/pcspk.c +++ b/hw/audio/pcspk.c @@ -122,7 +122,9 @@ static int pcspk_audio_init(PCSpkState *s) return 0; } - AUD_register_card(s_spk, &s->card); + if (!AUD_register_card(s_spk, &s->card, NULL)) { + return -1; + } s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback, &as); if (!s->voice) { diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c index 593da2478c14..dd2131426cd3 100644 --- a/hw/audio/sb16.c +++ b/hw/audio/sb16.c @@ -1400,6 +1400,10 @@ static void sb16_realizefn (DeviceState *dev, Error **errp) SB16State *s = SB16 (dev); IsaDmaClass *k; + if (!AUD_register_card ("sb16", &s->card, errp)) { + return; + } + s->isa_hdma = isa_get_dma(isa_bus_from_device(isadev), s->hdma); s->isa_dma = isa_get_dma(isa_bus_from_device(isadev), s->dma); if (!s->isa_dma || !s->isa_hdma) { @@ -1432,8 +1436,6 @@ static void sb16_realizefn (DeviceState *dev, Error **errp) k->register_channel(s->isa_dma, s->dma, SB_read_DMA, s); s->can_write = 1; - - AUD_register_card ("sb16", &s->card); } static Property sb16_properties[] = { diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c index b5722b37c36b..57954a631442 100644 --- a/hw/audio/wm8750.c +++ b/hw/audio/wm8750.c @@ -624,7 +624,10 @@ static void wm8750_realize(DeviceState *dev, Error **errp) { WM8750State *s = WM8750(dev); - AUD_register_card(CODEC, &s->card); + if (!AUD_register_card(CODEC, &s->card, errp)) { + return; + } + wm8750_reset(I2C_SLAVE(s)); } diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c index b16d6be2b5cc..d6699f00d89e 100644 --- a/hw/display/xlnx_dp.c +++ b/hw/display/xlnx_dp.c @@ -1278,6 +1278,10 @@ static void xlnx_dp_realize(DeviceState *dev, Error **errp) DisplaySurface *surface; struct audsettings as; + if (!AUD_register_card("xlnx_dp.audio", &s->aud_card, errp)) { + return; + } + aux_bus_realize(s->aux_bus); qdev_realize(DEVICE(s->dpcd), BUS(s->aux_bus), &error_fatal); @@ -1296,8 +1300,6 @@ static void xlnx_dp_realize(DeviceState *dev, Error **errp) as.fmt = AUDIO_FORMAT_S16; as.endianness = 0; - AUD_register_card("xlnx_dp.audio", &s->aud_card); - s->amixer_output_stream = AUD_open_out(&s->aud_card, s->amixer_output_stream, "xlnx_dp.audio.out", diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c index f0b02bc72280..1ebf0199bfc7 100644 --- a/hw/input/tsc210x.c +++ b/hw/input/tsc210x.c @@ -1106,7 +1106,8 @@ static void tsc210x_init(TSC210xState *s, s->card.name); } - AUD_register_card(s->name, &s->card); + /* We can quit here because this only gets called from machine class init */ + AUD_register_card(s->name, &s->card, &error_fatal); qemu_register_reset((void *) tsc210x_reset, s); vmstate_register(NULL, 0, vmsd, s); diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c index 8748c1ba0401..d5ac1f8962e3 100644 --- a/hw/usb/dev-audio.c +++ b/hw/usb/dev-audio.c @@ -944,12 +944,15 @@ static void usb_audio_realize(USBDevice *dev, Error **errp) USBAudioState *s = USB_AUDIO(dev); int i; + if (!AUD_register_card(TYPE_USB_AUDIO, &s->card, errp)) { + return; + } + dev->usb_desc = s->multi ? &desc_audio_multi : &desc_audio; usb_desc_create_serial(dev); usb_desc_init(dev); s->dev.opaque = s; - AUD_register_card(TYPE_USB_AUDIO, &s->card); s->out.altset = ALTSET_OFF; s->out.vol.mute = false;
Now that all callers support error reporting with errp and all machine-default devices use an explicit audiodev, this can be changed. To make the detection easier make AUD_register_card() return false on error. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- audio/audio.c | 7 +++++-- audio/audio.h | 2 +- hw/arm/omap2.c | 3 ++- hw/audio/ac97.c | 6 +++++- hw/audio/adlib.c | 7 +++++-- hw/audio/cs4231a.c | 6 ++++-- hw/audio/es1370.c | 5 ++++- hw/audio/gus.c | 4 +++- hw/audio/hda-codec.c | 5 ++++- hw/audio/lm4549.c | 4 +++- hw/audio/pcspk.c | 4 +++- hw/audio/sb16.c | 6 ++++-- hw/audio/wm8750.c | 5 ++++- hw/display/xlnx_dp.c | 6 ++++-- hw/input/tsc210x.c | 3 ++- hw/usb/dev-audio.c | 5 ++++- 16 files changed, 57 insertions(+), 21 deletions(-)