Re: [Qemu-devel] [PATCH v5 04/14] audio: -audiodev command line option basic implementation
Hi, > And magic errors like "'vfio-pci' is not a valid device model name" > after update. Try a full rebuild then. I see strange errors now and then too, that fixed it. Seems the qemu build fails now and then that an object file needs a rebuild. cheers, Gerd
Re: [Qemu-devel] [PATCH v5 04/14] audio: -audiodev command line option basic implementation
On 2019-03-08 08:21, Markus Armbruster wrote: > "Zoltán Kővágó" writes: > >> On 2019-03-07 16:56, Gerd Hoffmann wrote: >>> On Tue, Feb 26, 2019 at 02:39:38AM +0100, Zoltán Kővágó wrote: On 2019-02-20 22:37, Kővágó, Zoltán wrote: [...] > diff --git a/audio/audio.c b/audio/audio.c > index ce8e6ea8c2..8ad8cbe559 100644 > --- a/audio/audio.c > +++ b/audio/audio.c [...] > @@ -2129,3 +1866,170 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute, > uint8_t lvol, uint8_t rvol) > } > } > } > + > +void audio_create_pdos(Audiodev *dev) > +{ > +switch (dev->driver) { > +#define CASE(DRIVER, driver, pdo_name) \ > +case AUDIODEV_DRIVER_##DRIVER: \ > +dev->u.driver.in = g_malloc0( \ > +sizeof(Audiodev##pdo_name##PerDirectionOptions)); \ This should check has_in before overwriting. It'll work correctly when called from audio_legacy.c, but when using -audiodev it will overwrite the options passed by user (and leak memory) when called from audio_validate_opts. I'll fix it in the next update. >>> >>> Ping. 4.0 freeze is next tuesday. Any chance for a v6 early enough >>> that we have a chance to get the first chunk into 4.0? Monday latest, >>> preferably earlier ... >> >> I'll try to do something this weekend, but I can't promise anything. I >> still haven't got to reading through Markus' comments... > > Quoting myself: "We're down to minor stylistic issues. Good work!" > > Addressing these should not be hard. > I blame it on my horrible time management. Also the recent sdl/audio patches generated some conflicts, I had to solve them first. And magic errors like "'vfio-pci' is not a valid device model name" after update. Regards, Zoltan
Re: [Qemu-devel] [PATCH v5 04/14] audio: -audiodev command line option basic implementation
"Zoltán Kővágó" writes: > On 2019-03-07 16:56, Gerd Hoffmann wrote: >> On Tue, Feb 26, 2019 at 02:39:38AM +0100, Zoltán Kővágó wrote: >>> On 2019-02-20 22:37, Kővágó, Zoltán wrote: >>> [...] diff --git a/audio/audio.c b/audio/audio.c index ce8e6ea8c2..8ad8cbe559 100644 --- a/audio/audio.c +++ b/audio/audio.c >>> [...] @@ -2129,3 +1866,170 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol) } } } + +void audio_create_pdos(Audiodev *dev) +{ +switch (dev->driver) { +#define CASE(DRIVER, driver, pdo_name) \ +case AUDIODEV_DRIVER_##DRIVER: \ +dev->u.driver.in = g_malloc0( \ +sizeof(Audiodev##pdo_name##PerDirectionOptions)); \ >>> This should check has_in before overwriting. It'll work correctly when >>> called from audio_legacy.c, but when using -audiodev it will overwrite >>> the options passed by user (and leak memory) when called from >>> audio_validate_opts. I'll fix it in the next update. >> >> Ping. 4.0 freeze is next tuesday. Any chance for a v6 early enough >> that we have a chance to get the first chunk into 4.0? Monday latest, >> preferably earlier ... > > I'll try to do something this weekend, but I can't promise anything. I > still haven't got to reading through Markus' comments... Quoting myself: "We're down to minor stylistic issues. Good work!" Addressing these should not be hard.
Re: [Qemu-devel] [PATCH v5 04/14] audio: -audiodev command line option basic implementation
On 2019-03-07 16:56, Gerd Hoffmann wrote: > On Tue, Feb 26, 2019 at 02:39:38AM +0100, Zoltán Kővágó wrote: >> On 2019-02-20 22:37, Kővágó, Zoltán wrote: >> [...] >>> diff --git a/audio/audio.c b/audio/audio.c >>> index ce8e6ea8c2..8ad8cbe559 100644 >>> --- a/audio/audio.c >>> +++ b/audio/audio.c >> [...] >>> @@ -2129,3 +1866,170 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute, >>> uint8_t lvol, uint8_t rvol) >>> } >>> } >>> } >>> + >>> +void audio_create_pdos(Audiodev *dev) >>> +{ >>> +switch (dev->driver) { >>> +#define CASE(DRIVER, driver, pdo_name) \ >>> +case AUDIODEV_DRIVER_##DRIVER: \ >>> +dev->u.driver.in = g_malloc0( \ >>> +sizeof(Audiodev##pdo_name##PerDirectionOptions)); \ >> This should check has_in before overwriting. It'll work correctly when >> called from audio_legacy.c, but when using -audiodev it will overwrite >> the options passed by user (and leak memory) when called from >> audio_validate_opts. I'll fix it in the next update. > > Ping. 4.0 freeze is next tuesday. Any chance for a v6 early enough > that we have a chance to get the first chunk into 4.0? Monday latest, > preferably earlier ... I'll try to do something this weekend, but I can't promise anything. I still haven't got to reading through Markus' comments... Regards, Zoltan
Re: [Qemu-devel] [PATCH v5 04/14] audio: -audiodev command line option basic implementation
On Tue, Feb 26, 2019 at 02:39:38AM +0100, Zoltán Kővágó wrote: > On 2019-02-20 22:37, Kővágó, Zoltán wrote: > [...] > > diff --git a/audio/audio.c b/audio/audio.c > > index ce8e6ea8c2..8ad8cbe559 100644 > > --- a/audio/audio.c > > +++ b/audio/audio.c > [...] > > @@ -2129,3 +1866,170 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute, > > uint8_t lvol, uint8_t rvol) > > } > > } > > } > > + > > +void audio_create_pdos(Audiodev *dev) > > +{ > > +switch (dev->driver) { > > +#define CASE(DRIVER, driver, pdo_name) \ > > +case AUDIODEV_DRIVER_##DRIVER: \ > > +dev->u.driver.in = g_malloc0( \ > > +sizeof(Audiodev##pdo_name##PerDirectionOptions)); \ > This should check has_in before overwriting. It'll work correctly when > called from audio_legacy.c, but when using -audiodev it will overwrite > the options passed by user (and leak memory) when called from > audio_validate_opts. I'll fix it in the next update. Ping. 4.0 freeze is next tuesday. Any chance for a v6 early enough that we have a chance to get the first chunk into 4.0? Monday latest, preferably earlier ... thanks, Gerd
Re: [Qemu-devel] [PATCH v5 04/14] audio: -audiodev command line option basic implementation
On 2019-02-20 22:37, Kővágó, Zoltán wrote: [...] > diff --git a/audio/audio.c b/audio/audio.c > index ce8e6ea8c2..8ad8cbe559 100644 > --- a/audio/audio.c > +++ b/audio/audio.c [...] > @@ -2129,3 +1866,170 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute, > uint8_t lvol, uint8_t rvol) > } > } > } > + > +void audio_create_pdos(Audiodev *dev) > +{ > +switch (dev->driver) { > +#define CASE(DRIVER, driver, pdo_name) \ > +case AUDIODEV_DRIVER_##DRIVER: \ > +dev->u.driver.in = g_malloc0( \ > +sizeof(Audiodev##pdo_name##PerDirectionOptions)); \ This should check has_in before overwriting. It'll work correctly when called from audio_legacy.c, but when using -audiodev it will overwrite the options passed by user (and leak memory) when called from audio_validate_opts. I'll fix it in the next update. > +dev->u.driver.has_in = true;\ > +dev->u.driver.out = g_malloc0( \ > +sizeof(AudiodevAlsaPerDirectionOptions)); \ > +dev->u.driver.has_out = true; \ > +break > + > +CASE(NONE, none, ); > +CASE(ALSA, alsa, Alsa); > +CASE(COREAUDIO, coreaudio, Coreaudio); > +CASE(DSOUND, dsound, ); > +CASE(OSS, oss, Oss); > +CASE(PA, pa, Pa); > +CASE(SDL, sdl, ); > +CASE(SPICE, spice, ); > +CASE(WAV, wav, ); > + > +case AUDIODEV_DRIVER__MAX: > +abort(); > +}; > +} > + > +static void audio_validate_per_direction_opts( > +AudiodevPerDirectionOptions *pdo, Error **errp) > +{ > +if (!pdo->has_fixed_settings) { > +pdo->has_fixed_settings = true; > +pdo->fixed_settings = true; > +} > +if (!pdo->fixed_settings && > +(pdo->has_frequency || pdo->has_channels || pdo->has_format)) { > +error_setg(errp, > + "You can't use frequency, channels or format with > fixed-settings=off"); > +return; > +} > + > +if (!pdo->has_frequency) { > +pdo->has_frequency = true; > +pdo->frequency = 44100; > +} > +if (!pdo->has_channels) { > +pdo->has_channels = true; > +pdo->channels = 2; > +} > +if (!pdo->has_voices) { > +pdo->has_voices = true; > +pdo->voices = 1; > +} > +if (!pdo->has_format) { > +pdo->has_format = true; > +pdo->format = AUDIO_FORMAT_S16; > +} > +} > + > +static void audio_validate_opts(Audiodev *dev, Error **errp) > +{ > +Error *err = NULL; > + > +audio_create_pdos(dev); > + > +audio_validate_per_direction_opts(audio_get_pdo_in(dev), ); > +if (err) { > +error_propagate(errp, err); > +return; > +} > + > +audio_validate_per_direction_opts(audio_get_pdo_out(dev), ); > +if (err) { > +error_propagate(errp, err); > +return; > +} > + > +if (!dev->has_timer_period) { > +dev->has_timer_period = true; > +dev->timer_period = 1; /* 100Hz -> 10ms */ > +} > +} > + > +void audio_parse_option(const char *opt) > +{ > +AudiodevListEntry *e; > +Audiodev *dev = NULL; > + > +Visitor *v = qobject_input_visitor_new_str(opt, "driver", _fatal); > +visit_type_Audiodev(v, NULL, , _fatal); > +visit_free(v); > + > +audio_validate_opts(dev, _fatal); > + > +e = g_malloc0(sizeof(AudiodevListEntry)); > +e->dev = dev; > +QSIMPLEQ_INSERT_TAIL(, e, next); > +} > + [...] Regards, Zoltan