On 2019-01-29 14:33, Markus Armbruster wrote: > "Kővágó, Zoltán" <dirty.ice...@gmail.com> writes: [...] >> + >> +## >> +# @AudiodevPaOptions: >> +# >> +# Options of the pa (PulseAudio) audio backend. >> +# >> +# @server: PulseAudio server address (default: let PulseAudio choose) >> +# >> +# @sink: name of the sink to use >> +# >> +# @source: name of the source to use >> +# >> +# Since: 4.0 >> +## >> +{ 'struct': 'AudiodevPaOptions', >> + 'data': { >> + '*server': 'str', >> + '*sink': 'AudiodevPaPerDirectionOptions', >> + '*source': 'AudiodevPaPerDirectionOptions' } } >> + >> +## >> +# @AudiodevWavOptions: >> +# >> +# Options of the wav audio backend. >> +# >> +# @path: name of the wav file to record (default 'qemu.wav') >> +# >> +# Since: 4.0 >> +## >> +{ 'struct': 'AudiodevWavOptions', >> + 'data': { >> + '*path': 'str' } } > > Pattern so far: > > * AudiodevFooOptions > > Foo direction options other options > Alsa alsa-in, alsa-out threshold > Dsound latency > Oss oss-in, oss-out try-map, exclusive, dsp-policy > Pa sink, source server > Wav path > > Any particular reason for naming the direction options differently?
I probably chose that name because in PulseAudio parlance you have sources and sinks, and the current environment variable based setting of pa doesn't use ADC/DAC to refer to source/sink, but now that you mention it it probably makes sense to rename them to pa-in and pa-out, it's more uniform that way. [...] >> +## >> +# @Audiodev: >> +# >> +# Options of an audio backend. >> +# >> +# @id: identifier of the backend >> +# >> +# @driver: the backend driver to use >> +# >> +# @in: options of the capture stream >> +# >> +# @out: options of the playback stream >> +# >> +# @timer-period: timer period (in microseconds, 0: use lowest possible) >> +# >> +# Since: 4.0 >> +## >> +{ 'union': 'Audiodev', >> + 'base': { >> + 'id': 'str', >> + 'driver': 'AudiodevDriver', >> + '*in': 'AudiodevPerDirectionOptions', >> + '*out': 'AudiodevPerDirectionOptions', >> + '*timer-period': 'uint32' }, >> + 'discriminator': 'driver', >> + 'data': { >> + 'alsa': 'AudiodevAlsaOptions', >> + 'dsound': 'AudiodevDsoundOptions', >> + 'oss': 'AudiodevOssOptions', >> + 'pa': 'AudiodevPaOptions', >> + 'wav': 'AudiodevWavOptions' } } > > 'none', 'coreaudio', 'sdl' and 'spice' have no driver-specific options. > > Do all the generic options apply to all drivers? Even 'none'? 'fixed-settings', 'frequency', 'channels', 'voices' and 'format' are used by the mixeng thus they apply to all backends, even 'none'. (Whether it makes any sense to set them is a different story.) 'buffer-len' and 'buffer-count' was introduced as a means to give a more uniform interface to setting buffer sizes, because currently to set buffer size: * in case of alsa: you set microseconds or frames depending on a second environment variable * in case of coreaudio: you set it in frames * in case of dsound, oss: you set it in milliseconds * in case of pa, sdl: you set it in samples * in case of spice, wav: you can't set it I added 'buffer-len' and 'buffer-count' because they seemed like a generic option that *most* backends would support, and I didn't want to create extra backend specific options for them. But if I combine it with your inheritance-like proposal, it's probably not that bad. > Do @in and @out apply to drivers 'dsound' and 'wav'? @out applies to every driver. @in shouldn't apply to drivers without input support, but currently the base of mixeng is created for the input too, it'll just fail if the sound card tries to create a capture stream... > > Per-direction options are split between generic ones in @in and @out, > and driver-specific ones in @alsa-in and @alsa-out / @oss-in and > @oss-out / @sink and @source. > > Perhaps the following would be tidier: make the generic per-direction > options AudiodevPerDirectionOptions the base of driver-specific ones > AudiodevAlsaPerDirectionOptions, AudiodevOssPerDirectionOptions, > AudiodevPaPerDirectionOptions, and then > > { 'struct': 'AudiodevGenericOptions', > 'data': { > '*in': 'AudiodevPerDirectionOptions', > '*out': 'AudiodevPerDirectionOptions' } } > > { 'union': 'Audiodev', > 'base': { > 'id': 'str', > 'driver': 'AudiodevDriver', > '*timer-period': 'uint32' }, > 'discriminator': 'driver', > 'data': { > 'none': 'AudiodevGenericOptions', > 'alsa': 'AudiodevAlsaOptions', > 'coreaudio: 'AudiodevGenericOptions', > 'dsound': 'AudiodevDsoundOptions', > 'oss': 'AudiodevOssOptions', > 'pa': 'AudiodevPaOptions', > 'sdl: 'AudiodevGenericOptions', > 'spice': 'AudiodevGenericOptions', > 'wav': 'AudiodevWavOptions' } } Neat, I'll play around with this a bit. >> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json >> index 3bbdfcee84..d5f9856be7 100644 >> --- a/qapi/qapi-schema.json >> +++ b/qapi/qapi-schema.json >> @@ -95,3 +95,4 @@ >> { 'include': 'trace.json' } >> { 'include': 'introspect.json' } >> { 'include': 'misc.json' } >> +{ 'include': 'audio.json' } > > Looks pretty good. > Regards, Zoltan