[RFC] qapi for audio backends

This is a proposal to add structures into qapi-schema.json to replace the
existing configuration structures used by audio backends currently. I'm going to
use it to implement a new way to specify audio backend options (an -audiodev
command line option, instead of environment variables. This will also allow us
to use multiple audio backends in one qemu instance), so the structure used here
will be the basis of the command line syntax.

This is currently more or less a direct translation of the current audio backend
options. I've changed some names, trying to accomplish a more consistent naming
scheme. I wouldn't be surprised if there were options that doesn't work if you
set their value to anything other than the default (for example, log to monitor
can crash qemu, QEMU_DSOUND_RESTOURE_RETRIES has a typo, so probably nobody used
it, etc). I've also tried to reduce copy-paste, when the same set of options can
be given to output and input (QEMU_*_DAC_* and QEMU_*_ADC_* options), also using
in and out instead of ADC and DAC, as in the world of SPDIF and HDMI it's
completely possible that your computer has nothing to do with analog converters.
Plus a non technician user probably has no idea what ADC and DAC stands for.

Any comment is appreciated.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 qapi-schema.json | 330 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 330 insertions(+)

On 06/03/2015 10:48 AM, Kővágó, Zoltán wrote:
> This is a proposal to add structures into qapi-schema.json to replace the
> existing configuration structures used by audio backends currently. I'm going to
> use it to implement a new way to specify audio backend options (an -audiodev
> command line option, instead of environment variables. This will also allow us
> to use multiple audio backends in one qemu instance), so the structure used here
> will be the basis of the command line syntax.
> 
> This is currently more or less a direct translation of the current audio backend
> options. I've changed some names, trying to accomplish a more consistent naming
> scheme. I wouldn't be surprised if there were options that doesn't work if you
> set their value to anything other than the default (for example, log to monitor
> can crash qemu, QEMU_DSOUND_RESTOURE_RETRIES has a typo, so probably nobody used
> it, etc). I've also tried to reduce copy-paste, when the same set of options can
> be given to output and input (QEMU_*_DAC_* and QEMU_*_ADC_* options), also using
> in and out instead of ADC and DAC, as in the world of SPDIF and HDMI it's
> completely possible that your computer has nothing to do with analog converters.
> Plus a non technician user probably has no idea what ADC and DAC stands for.
> 
> Any comment is appreciated.
> 
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>  qapi-schema.json | 330 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 330 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0662a9b..ff67d5a 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3769,3 +3769,333 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @AudiodevNoneOptions
> +#
> +# The dummy audio backend has no options.
> +#
> +# Since: XXX

It's okay to tentatively put 2.4 here, if you are aiming for 2.4.  If
you think it will be a big enough project to miss the current release
window, put 2.5.

> +##
> +{ 'struct': 'AudiodevNoneOptions',
> +  'data': { } }
> +
> +##
> +# @AudiodevAlsaPerDirectionOptions
> +#
> +# Options of the alsa backend that are used for both playback and recording.
> +#
> +# @dev: #optional the name of the alsa device to use.
> +#
> +# @period_size_usec: #optional the period size in microseconds. Must not be
> +#                    specified with @period_size_frames.
> +#
> +# @period_size_frames: #optional the period size in frames. Must not be
> +#                      specified with @period_size_usec.
> +#
> +# @buffer_size_usec: #optional the buffer size in microseconds. Must not be
> +#                    specified with @buffer_size_frames.
> +#
> +# @buffer_size_frames: #optional the buffer size in frames. Must not be
> +#                      specified with @buffer_size_usec.

Can we name these with s/_/-/?  We've documented that QMP prefers dash
unless there is compelling reason or consistency to worry about, and I
don't see the compelling reason here.

> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevAlsaPerDirectionOptions',
> +  'data': {
> +    '*dev':                'str',
> +    '*period_size_usec':   'int',
> +    '*period_size_frames': 'int',
> +    '*buffer_size_usec':   'int',
> +    '*buffer_size_frames': 'int' } }
> +
> +##
> +# @AudiodevAlsaOptions
> +#
> +# Options of the alsa audio backend.
> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @threshold: #optional

Document this.

> +#
> +# @verbose: #optional behave in a more verbose way
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevAlsaOptions',
> +  'data': {
> +    '*in':  'AudiodevAlsaPerDirectionOptions',
> +    '*out': 'AudiodevAlsaPerDirectionOptions',
> +    '*threshold': 'int',
> +    '*verbose':   'bool' } }
> +
> +##
> +# @AudiodevCoreaudioOptions
> +#
> +# Options of the coreaudio backend.
> +#
> +# @buffer_size: #optional size of the buffer in frames
> +#
> +# @buffer_count: #optional number of buffers

Again, dashes would be nicer, if there is no compelling reason otherwise
(I'll quit repeating it).

> +#
> +# Since: XXX

(and I'll quit pointing out XXX in Since lines)

> +##
> +{ 'struct': 'AudiodevCoreaudioOptions',
> +  'data': {
> +    '*buffer_size':  'int',
> +    '*buffer_count': 'int' } }
> +
> +##
> +# @AudiodevDsoundPerDirectionOptions
> +#
> +# Options of the dsound backend that are used for both playback and recording.
> +#
> +# @bufsize: #optional

Document this

> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevDsoundPerDirectionOptions',
> +  'data' : {
> +    '*bufsize': 'int' } }
> +
> +##
> +# @AudiodevDsoundOptions
> +#
> +# Options of the dsound audio backend.
> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @lock_retries: #optional number of times to attempt locking the buffer
> +#
> +# @restore_retries: #optional number of times to attempt restoring the buffer
> +#
> +# @getstatus_retries: #optional number of times to attempt getting status of the

Borders on being a long line (yes, it's exactly 80, but I tend to stick
to < 80)

> +#                     buffer
> +#
> +# @set_primary: #optional set the parameters of primary buffer
> +#
> +# @latency_millis: #optional
> +#
> +# @primary_freq: #optional primary buffer frequency
> +#
> +# @primary_channels: #optional primary buffer number of channels
> +#
> +# @primary_format: #optional primary buffer format
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevDsoundOptions',
> +  'data': {
> +    '*in':                'AudiodevDsoundPerDirectionOptions',
> +    '*out':               'AudiodevDsoundPerDirectionOptions',
> +    '*lock_retries':      'int',
> +    '*restore_retries':   'int',
> +    '*getstatus_retries': 'int',
> +    '*set_primary':       'bool',
> +    '*latency_millis':    'int',
> +    '*primary_freq':      'int',
> +    '*primary_channels':  'int',
> +    '*primary_format':    'AudioFormat' } }
> +
> +##
> +# @AudiodevOssPerDirectionOptions
> +#
> +# Options of the oss backend that are used for both playback and recording.
> +#
> +# @dev: #optional path of the oss device
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevOssPerDirectionOptions',
> +  'data': {
> +    '*dev': 'str' } }
> +
> +##
> +# @AudiodevOssOptions
> +#
> +# Options of the oss audio backend.
> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @fragsize: #optional fragment size in bytes
> +#
> +# @frags: #optional number of fragments
> +#
> +# @mmap: #optional try using memory mapped access
> +#
> +# @exclusive: #optional open device in exclusive mode (vmix wont work)
> +#
> +# @dsp_policy: #optional set the timing policy of the device, -1 to use fragment
> +#              mode (option ignored on some platforms)
> +#
> +# @debug: #optional turn on some debugging messages
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevOssOptions',
> +  'data': {
> +    '*in':         'AudiodevOssPerDirectionOptions',
> +    '*out':        'AudiodevOssPerDirectionOptions',
> +    '*fragsize':   'int',
> +    '*frags':      'int',
> +    '*mmap':       'bool',
> +    '*exclusive':  'bool',
> +    '*dsp_policy': 'int',
> +    '*debug':      'bool' } }
> +
> +##
> +# @AudiodevPaOptions
> +#
> +# Options of the pa audio backend.
> +#
> +# @samples: #optional buffer size in samples
> +#
> +# @server: #optional PulseAudio server address

Worth mentioning that 'pa' == PulseAudio earlier in the docs?

> +#
> +# @sink: #optional sink device name
> +#
> +# @source: #optional source device name
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevPaOptions',
> +  'data': {
> +    '*samples': 'int',
> +    '*server':  'str',
> +    '*sink':    'str',
> +    '*source':  'str' } }
> +
> +##
> +# @AudiodevSdlOptions
> +#
> +# Options of the sdl audio backend. (Note that most options are only changeable
> +# through SDL's environment variables).
> +#
> +# @samples: #optional size of SDL buffer in samples
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevSdlOptions',
> +  'data': {
> +    '*samples': 'int' } }
> +
> +##
> +# @AudiodevSpiceOptions
> +#
> +# The spice audio backend has no options.
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevSpiceOptions',
> +  'data': { } }
> +
> +##
> +# @AudiodevWavOptions
> +#
> +# Options of the wav audio backend.
> +#
> +# @frequency: #optional frequency of the wav file
> +#
> +# @format: #optional sample format of the wav file
> +#
> +# @channels: #optional number of channels of the wav file
> +#
> +# @path: #optional path of the wav file to record.
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevWavOptions',
> +  'data': {
> +    '*frequency': 'int',
> +    '*format':    'AudioFormat',
> +    '*channels':  'int',
> +    '*path':       'str' } }

Inconsistent indentation.

> +
> +
> +##
> +# @AudiodevBackendOptions
> +#
> +# A discriminated record of audio backends.
> +#
> +# Since: XXX
> +##
> +{ 'union': 'AudiodevBackendOptions',
> +  'data': {
> +    'none':      'AudiodevNoneOptions',
> +    'alsa':      'AudiodevAlsaOptions',
> +    'coreaudio': 'AudiodevCoreaudioOptions',
> +    'dsound':    'AudiodevDsoundOptions',
> +    'oss':       'AudiodevOssOptions',
> +    'pa':        'AudiodevPaOptions',
> +    'sdl':       'AudiodevSdlOptions',
> +    'spice':     'AudiodevSpiceOptions',
> +    'wav':       'AudiodevWavOptions' } }

AudiodevNoneOptions and AudiodevSpiceOptions are identical; you could
consolidate them into one struct.  I'd also (someday) like to get to the
point of using anonymous structures in a union, particularly when the
variant adds no additional fields, so that you could do:

'data': {
  'none': {},
  'alsa': 'AudiodevAlsaOptions', ...

but don't know if that is worth doing any sooner than Markus can land
his introspection patches.

> +
> +##
> +# @AudioFormat
> +#
> +# An enumeration of possible audio formats.
> +#
> +# Since: XXX
> +##
> +{ 'enum': 'AudioFormat',
> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
> +
> +##
> +# @AudiodevPerDirectionOptions
> +#
> +# General audio backend options that are used for both playback and recording.
> +#
> +# @fixed_settings: #optional use fixed settings for host DAC/ADC
> +#
> +# @frequency: #optional frequency to use when using fixed settings
> +#
> +# @channels: #optional number of channels when using fixed settings
> +#
> +# @format: #optional sample fortmat to use when using fixed settings
> +#
> +# @try_poll: #optional attempt to use poll mode
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevPerDirectionOptions',
> +  'data': {
> +    '*fixed_settings': 'bool',
> +    '*frequency':      'int',
> +    '*channels':       'int',
> +    '*format':         'AudioFormat',
> +    '*try_poll':       'bool' } }
> +
> +##
> +# @Audiodev
> +#
> +# Captures the configuration of an audio backend.
> +#
> +# @id: identifier of the backend.

Inconsistent on whether you end with '.' (but the whole file is already
that inconsistent, so not too much of a worry)

> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @timer_period: #optional timer period in HZ (0 - use lowest possible)
> +#
> +# @plive: #optional
> +#
> +# @opts: audio backend specific options
> +#
> +# Since XXX
> +##
> +{ 'struct': 'Audiodev',
> +  'data': {
> +    'id':            'str',
> +    '*in':           'AudiodevPerDirectionOptions',
> +    '*out':          'AudiodevPerDirectionOptions',
> +    '*timer_period': 'int',
> +    '*plive':        'bool',
> +    'opts':          'AudiodevBackendOptions' } }
> 

Overall looks fairly reasonable. Is it enough structures to be worth
splitting into a new qapi/audio.json file and merely touch
qapi-schema.json to add an include?

2015-06-03 21:17 keltezéssel, Eric Blake írta:
> On 06/03/2015 10:48 AM, Kővágó, Zoltán wrote:
>> This is a proposal to add structures into qapi-schema.json to replace the
>> existing configuration structures used by audio backends currently. I'm going to
>> use it to implement a new way to specify audio backend options (an -audiodev
>> command line option, instead of environment variables. This will also allow us
>> to use multiple audio backends in one qemu instance), so the structure used here
>> will be the basis of the command line syntax.
>>
>> This is currently more or less a direct translation of the current audio backend
>> options. I've changed some names, trying to accomplish a more consistent naming
>> scheme. I wouldn't be surprised if there were options that doesn't work if you
>> set their value to anything other than the default (for example, log to monitor
>> can crash qemu, QEMU_DSOUND_RESTOURE_RETRIES has a typo, so probably nobody used
>> it, etc). I've also tried to reduce copy-paste, when the same set of options can
>> be given to output and input (QEMU_*_DAC_* and QEMU_*_ADC_* options), also using
>> in and out instead of ADC and DAC, as in the world of SPDIF and HDMI it's
>> completely possible that your computer has nothing to do with analog converters.
>> Plus a non technician user probably has no idea what ADC and DAC stands for.
>>
>> Any comment is appreciated.
>>
>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>> ---
>>   qapi-schema.json | 330 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 330 insertions(+)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 0662a9b..ff67d5a 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3769,3 +3769,333 @@
>>   # Since: 2.1
>>   ##
>>   { 'command': 'rtc-reset-reinjection' }
>> +
>> +##
>> +# @AudiodevNoneOptions
>> +#
>> +# The dummy audio backend has no options.
>> +#
>> +# Since: XXX
>
> It's okay to tentatively put 2.4 here, if you are aiming for 2.4.  If
> you think it will be a big enough project to miss the current release
> window, put 2.5.
>
>> +##
>> +{ 'struct': 'AudiodevNoneOptions',
>> +  'data': { } }
>> +
>> +##
>> +# @AudiodevAlsaPerDirectionOptions
>> +#
>> +# Options of the alsa backend that are used for both playback and recording.
>> +#
>> +# @dev: #optional the name of the alsa device to use.
>> +#
>> +# @period_size_usec: #optional the period size in microseconds. Must not be
>> +#                    specified with @period_size_frames.
>> +#
>> +# @period_size_frames: #optional the period size in frames. Must not be
>> +#                      specified with @period_size_usec.
>> +#
>> +# @buffer_size_usec: #optional the buffer size in microseconds. Must not be
>> +#                    specified with @buffer_size_frames.
>> +#
>> +# @buffer_size_frames: #optional the buffer size in frames. Must not be
>> +#                      specified with @buffer_size_usec.
>
> Can we name these with s/_/-/?  We've documented that QMP prefers dash
> unless there is compelling reason or consistency to worry about, and I
> don't see the compelling reason here.

There's no particular reason other than when I looked through 
qapi-schema.json I've mostly seen underscores. Will fix this.

>
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevAlsaPerDirectionOptions',
>> +  'data': {
>> +    '*dev':                'str',
>> +    '*period_size_usec':   'int',
>> +    '*period_size_frames': 'int',
>> +    '*buffer_size_usec':   'int',
>> +    '*buffer_size_frames': 'int' } }
>> +
>> +##
>> +# @AudiodevAlsaOptions
>> +#
>> +# Options of the alsa audio backend.
>> +#
>> +# @in: #optional options of the capture stream.
>> +#
>> +# @out: #optional options of the playback stream.
>> +#
>> +# @threshold: #optional
>
> Document this.

This (and some other option) currently only has the documentation 
"(undocumented)", but I will try to figure out what they do...

>
>> +#
>> +# @verbose: #optional behave in a more verbose way
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevAlsaOptions',
>> +  'data': {
>> +    '*in':  'AudiodevAlsaPerDirectionOptions',
>> +    '*out': 'AudiodevAlsaPerDirectionOptions',
>> +    '*threshold': 'int',
>> +    '*verbose':   'bool' } }
>> +
>> +##
>> +# @AudiodevCoreaudioOptions
>> +#
>> +# Options of the coreaudio backend.
>> +#
>> +# @buffer_size: #optional size of the buffer in frames
>> +#
>> +# @buffer_count: #optional number of buffers
>
> Again, dashes would be nicer, if there is no compelling reason otherwise
> (I'll quit repeating it).
>
>> +#
>> +# Since: XXX
>
> (and I'll quit pointing out XXX in Since lines)
>
>> +##
>> +{ 'struct': 'AudiodevCoreaudioOptions',
>> +  'data': {
>> +    '*buffer_size':  'int',
>> +    '*buffer_count': 'int' } }
>> +
>> +##
>> +# @AudiodevDsoundPerDirectionOptions
>> +#
>> +# Options of the dsound backend that are used for both playback and recording.
>> +#
>> +# @bufsize: #optional
>
> Document this
>
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevDsoundPerDirectionOptions',
>> +  'data' : {
>> +    '*bufsize': 'int' } }
>> +
>> +##
>> +# @AudiodevDsoundOptions
>> +#
>> +# Options of the dsound audio backend.
>> +#
>> +# @in: #optional options of the capture stream.
>> +#
>> +# @out: #optional options of the playback stream.
>> +#
>> +# @lock_retries: #optional number of times to attempt locking the buffer
>> +#
>> +# @restore_retries: #optional number of times to attempt restoring the buffer
>> +#
>> +# @getstatus_retries: #optional number of times to attempt getting status of the
>
> Borders on being a long line (yes, it's exactly 80, but I tend to stick
> to < 80)
>
>> +#                     buffer
>> +#
>> +# @set_primary: #optional set the parameters of primary buffer
>> +#
>> +# @latency_millis: #optional
>> +#
>> +# @primary_freq: #optional primary buffer frequency
>> +#
>> +# @primary_channels: #optional primary buffer number of channels
>> +#
>> +# @primary_format: #optional primary buffer format
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevDsoundOptions',
>> +  'data': {
>> +    '*in':                'AudiodevDsoundPerDirectionOptions',
>> +    '*out':               'AudiodevDsoundPerDirectionOptions',
>> +    '*lock_retries':      'int',
>> +    '*restore_retries':   'int',
>> +    '*getstatus_retries': 'int',
>> +    '*set_primary':       'bool',
>> +    '*latency_millis':    'int',
>> +    '*primary_freq':      'int',
>> +    '*primary_channels':  'int',
>> +    '*primary_format':    'AudioFormat' } }
>> +
>> +##
>> +# @AudiodevOssPerDirectionOptions
>> +#
>> +# Options of the oss backend that are used for both playback and recording.
>> +#
>> +# @dev: #optional path of the oss device
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevOssPerDirectionOptions',
>> +  'data': {
>> +    '*dev': 'str' } }
>> +
>> +##
>> +# @AudiodevOssOptions
>> +#
>> +# Options of the oss audio backend.
>> +#
>> +# @in: #optional options of the capture stream.
>> +#
>> +# @out: #optional options of the playback stream.
>> +#
>> +# @fragsize: #optional fragment size in bytes
>> +#
>> +# @frags: #optional number of fragments
>> +#
>> +# @mmap: #optional try using memory mapped access
>> +#
>> +# @exclusive: #optional open device in exclusive mode (vmix wont work)
>> +#
>> +# @dsp_policy: #optional set the timing policy of the device, -1 to use fragment
>> +#              mode (option ignored on some platforms)
>> +#
>> +# @debug: #optional turn on some debugging messages
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevOssOptions',
>> +  'data': {
>> +    '*in':         'AudiodevOssPerDirectionOptions',
>> +    '*out':        'AudiodevOssPerDirectionOptions',
>> +    '*fragsize':   'int',
>> +    '*frags':      'int',
>> +    '*mmap':       'bool',
>> +    '*exclusive':  'bool',
>> +    '*dsp_policy': 'int',
>> +    '*debug':      'bool' } }
>> +
>> +##
>> +# @AudiodevPaOptions
>> +#
>> +# Options of the pa audio backend.
>> +#
>> +# @samples: #optional buffer size in samples
>> +#
>> +# @server: #optional PulseAudio server address
>
> Worth mentioning that 'pa' == PulseAudio earlier in the docs?
>
>> +#
>> +# @sink: #optional sink device name
>> +#
>> +# @source: #optional source device name
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevPaOptions',
>> +  'data': {
>> +    '*samples': 'int',
>> +    '*server':  'str',
>> +    '*sink':    'str',
>> +    '*source':  'str' } }
>> +
>> +##
>> +# @AudiodevSdlOptions
>> +#
>> +# Options of the sdl audio backend. (Note that most options are only changeable
>> +# through SDL's environment variables).
>> +#
>> +# @samples: #optional size of SDL buffer in samples
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevSdlOptions',
>> +  'data': {
>> +    '*samples': 'int' } }
>> +
>> +##
>> +# @AudiodevSpiceOptions
>> +#
>> +# The spice audio backend has no options.
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevSpiceOptions',
>> +  'data': { } }
>> +
>> +##
>> +# @AudiodevWavOptions
>> +#
>> +# Options of the wav audio backend.
>> +#
>> +# @frequency: #optional frequency of the wav file
>> +#
>> +# @format: #optional sample format of the wav file
>> +#
>> +# @channels: #optional number of channels of the wav file
>> +#
>> +# @path: #optional path of the wav file to record.
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevWavOptions',
>> +  'data': {
>> +    '*frequency': 'int',
>> +    '*format':    'AudioFormat',
>> +    '*channels':  'int',
>> +    '*path':       'str' } }
>
> Inconsistent indentation.
>
>> +
>> +
>> +##
>> +# @AudiodevBackendOptions
>> +#
>> +# A discriminated record of audio backends.
>> +#
>> +# Since: XXX
>> +##
>> +{ 'union': 'AudiodevBackendOptions',
>> +  'data': {
>> +    'none':      'AudiodevNoneOptions',
>> +    'alsa':      'AudiodevAlsaOptions',
>> +    'coreaudio': 'AudiodevCoreaudioOptions',
>> +    'dsound':    'AudiodevDsoundOptions',
>> +    'oss':       'AudiodevOssOptions',
>> +    'pa':        'AudiodevPaOptions',
>> +    'sdl':       'AudiodevSdlOptions',
>> +    'spice':     'AudiodevSpiceOptions',
>> +    'wav':       'AudiodevWavOptions' } }
>
> AudiodevNoneOptions and AudiodevSpiceOptions are identical; you could
> consolidate them into one struct.  I'd also (someday) like to get to the
> point of using anonymous structures in a union, particularly when the
> variant adds no additional fields, so that you could do:
>
> 'data': {
>    'none': {},
>    'alsa': 'AudiodevAlsaOptions', ...
>
> but don't know if that is worth doing any sooner than Markus can land
> his introspection patches.
>
>> +
>> +##
>> +# @AudioFormat
>> +#
>> +# An enumeration of possible audio formats.
>> +#
>> +# Since: XXX
>> +##
>> +{ 'enum': 'AudioFormat',
>> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
>> +
>> +##
>> +# @AudiodevPerDirectionOptions
>> +#
>> +# General audio backend options that are used for both playback and recording.
>> +#
>> +# @fixed_settings: #optional use fixed settings for host DAC/ADC
>> +#
>> +# @frequency: #optional frequency to use when using fixed settings
>> +#
>> +# @channels: #optional number of channels when using fixed settings
>> +#
>> +# @format: #optional sample fortmat to use when using fixed settings
>> +#
>> +# @try_poll: #optional attempt to use poll mode
>> +#
>> +# Since: XXX
>> +##
>> +{ 'struct': 'AudiodevPerDirectionOptions',
>> +  'data': {
>> +    '*fixed_settings': 'bool',
>> +    '*frequency':      'int',
>> +    '*channels':       'int',
>> +    '*format':         'AudioFormat',
>> +    '*try_poll':       'bool' } }
>> +
>> +##
>> +# @Audiodev
>> +#
>> +# Captures the configuration of an audio backend.
>> +#
>> +# @id: identifier of the backend.
>
> Inconsistent on whether you end with '.' (but the whole file is already
> that inconsistent, so not too much of a worry)
>
>> +#
>> +# @in: #optional options of the capture stream.
>> +#
>> +# @out: #optional options of the playback stream.
>> +#
>> +# @timer_period: #optional timer period in HZ (0 - use lowest possible)
>> +#
>> +# @plive: #optional
>> +#
>> +# @opts: audio backend specific options
>> +#
>> +# Since XXX
>> +##
>> +{ 'struct': 'Audiodev',
>> +  'data': {
>> +    'id':            'str',
>> +    '*in':           'AudiodevPerDirectionOptions',
>> +    '*out':          'AudiodevPerDirectionOptions',
>> +    '*timer_period': 'int',
>> +    '*plive':        'bool',
>> +    'opts':          'AudiodevBackendOptions' } }
>>
>
> Overall looks fairly reasonable. Is it enough structures to be worth
> splitting into a new qapi/audio.json file and merely touch
> qapi-schema.json to add an include?
>

Maybe. Given that there are a few quite short includes already, it 
should be ok...

Hi,

> +##
> +# @AudiodevAlsaPerDirectionOptions
> +#
> +# Options of the alsa backend that are used for both playback and recording.
> +#
> +# @dev: #optional the name of the alsa device to use.
> +#
> +# @period_size_usec: #optional the period size in microseconds. Must not be
> +#                    specified with @period_size_frames.
> +#
> +# @period_size_frames: #optional the period size in frames. Must not be
> +#                      specified with @period_size_usec.

Do we really want keep both?

I think that generally applies to all options:  We should go over them
all, check what they are doing and whenever it makes sense to keep them.

> +# @buffer_size_usec: #optional the buffer size in microseconds. Must not be
> +#                    specified with @buffer_size_frames.
> +#
> +# @buffer_size_frames: #optional the buffer size in frames. Must not be
> +#                      specified with @buffer_size_usec.
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevAlsaPerDirectionOptions',
> +  'data': {
> +    '*dev':                'str',
> +    '*period_size_usec':   'int',
> +    '*period_size_frames': 'int',
> +    '*buffer_size_usec':   'int',
> +    '*buffer_size_frames': 'int' } }
> +
> +##
> +# @AudiodevAlsaOptions
> +#
> +# Options of the alsa audio backend.
> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @threshold: #optional
> +#
> +# @verbose: #optional behave in a more verbose way

qemu is moving to use tracepoints for debugging purposes.
I think we should not have a 'verbose' switch here to enable
debug messages.

> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevAlsaOptions',
> +  'data': {
> +    '*in':  'AudiodevAlsaPerDirectionOptions',
> +    '*out': 'AudiodevAlsaPerDirectionOptions',
> +    '*threshold': 'int',
> +    '*verbose':   'bool' } }
> +
> +##
> +# @AudiodevCoreaudioOptions
> +#
> +# Options of the coreaudio backend.
> +#
> +# @buffer_size: #optional size of the buffer in frames
> +#
> +# @buffer_count: #optional number of buffers
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevCoreaudioOptions',
> +  'data': {
> +    '*buffer_size':  'int',
> +    '*buffer_count': 'int' } }
> +
> +##
> +# @AudiodevDsoundPerDirectionOptions
> +#
> +# Options of the dsound backend that are used for both playback and recording.
> +#
> +# @bufsize: #optional
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevDsoundPerDirectionOptions',
> +  'data' : {
> +    '*bufsize': 'int' } }
> +
> +##
> +# @AudiodevDsoundOptions
> +#
> +# Options of the dsound audio backend.
> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @lock_retries: #optional number of times to attempt locking the buffer
> +#
> +# @restore_retries: #optional number of times to attempt restoring the buffer
> +#
> +# @getstatus_retries: #optional number of times to attempt getting status of the
> +#                     buffer
> +#
> +# @set_primary: #optional set the parameters of primary buffer
> +#
> +# @latency_millis: #optional
> +#
> +# @primary_freq: #optional primary buffer frequency
> +#
> +# @primary_channels: #optional primary buffer number of channels
> +#
> +# @primary_format: #optional primary buffer format
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevDsoundOptions',
> +  'data': {
> +    '*in':                'AudiodevDsoundPerDirectionOptions',
> +    '*out':               'AudiodevDsoundPerDirectionOptions',
> +    '*lock_retries':      'int',
> +    '*restore_retries':   'int',
> +    '*getstatus_retries': 'int',
> +    '*set_primary':       'bool',
> +    '*latency_millis':    'int',
> +    '*primary_freq':      'int',
> +    '*primary_channels':  'int',
> +    '*primary_format':    'AudioFormat' } }
> +
> +##
> +# @AudiodevOssPerDirectionOptions
> +#
> +# Options of the oss backend that are used for both playback and recording.
> +#
> +# @dev: #optional path of the oss device
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevOssPerDirectionOptions',
> +  'data': {
> +    '*dev': 'str' } }
> +
> +##
> +# @AudiodevOssOptions
> +#
> +# Options of the oss audio backend.
> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @fragsize: #optional fragment size in bytes
> +#
> +# @frags: #optional number of fragments

Hmm.  Every backend has options to configure the audio buffers, but it
isn't consistent in any way.

I think we should try to create a AudioBufferOptions struct which then
is used by every backend.  Makes thinks easier for the user, and we can
also have common helper functions which calculate buffer sizes based on
options and format etc.

Something like:

{  'struct': 'AudioBufferOptions',
  'data': {
    '*buffer_usecs' : 'int',
    '*buffer_count' : 'int'
} }

Where buffer_usecs is the size of a single audio buffer,
in microseconds, and buffer_count is the number of buffers.

oss will calculate fragsize from buffer_usecs, frags == buffer_count.

alsa will use buffer_usecs for period_size_usec, and set
buffer_size_usec to buffer_usecs * buffer_count (I think this is how
'period' and 'buffer' are defined for alsa, needs double-check though).

Simliar for the other backends ...

Maybe add fields to the AudiodevPerDirectionOptions struct below instead
of creating a new one.

> +#
> +# @mmap: #optional try using memory mapped access

IIRC this doesn't work everywhere, especially when the oss api is
implemented as library.

Linux had that, but on linux everybody uses alsa these days ...
Dunno about other platforms.

> +# @exclusive: #optional open device in exclusive mode (vmix wont work)
> 
> +# @dsp_policy: #optional set the timing policy of the device, -1 to use fragment
> +#              mode (option ignored on some platforms)

Would be interesting to know on which platforms this actually has an
effect (both options) ...

IIRC 'vmix' was a feature of the commercial, ossaudio driver package.

> +#
> +# @debug: #optional turn on some debugging messages

Same comment as for 'verbose' above.

> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevOssOptions',
> +  'data': {
> +    '*in':         'AudiodevOssPerDirectionOptions',
> +    '*out':        'AudiodevOssPerDirectionOptions',
> +    '*fragsize':   'int',
> +    '*frags':      'int',
> +    '*mmap':       'bool',
> +    '*exclusive':  'bool',
> +    '*dsp_policy': 'int',
> +    '*debug':      'bool' } }
> +
> +##
> +# @AudiodevPaOptions
> +#
> +# Options of the pa audio backend.
> +#
> +# @samples: #optional buffer size in samples
> +#
> +# @server: #optional PulseAudio server address
> +#
> +# @sink: #optional sink device name
> +#
> +# @source: #optional source device name
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevPaOptions',
> +  'data': {
> +    '*samples': 'int',
> +    '*server':  'str',
> +    '*sink':    'str',
> +    '*source':  'str' } }
> +
> +##
> +# @AudiodevSdlOptions
> +#
> +# Options of the sdl audio backend. (Note that most options are only changeable
> +# through SDL's environment variables).
> +#
> +# @samples: #optional size of SDL buffer in samples
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevSdlOptions',
> +  'data': {
> +    '*samples': 'int' } }
> +
> +##
> +# @AudiodevSpiceOptions
> +#
> +# The spice audio backend has no options.
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevSpiceOptions',
> +  'data': { } }
> +
> +##
> +# @AudiodevWavOptions
> +#
> +# Options of the wav audio backend.
> +#
> +# @frequency: #optional frequency of the wav file
> +#
> +# @format: #optional sample format of the wav file
> +#
> +# @channels: #optional number of channels of the wav file
> +#
> +# @path: #optional path of the wav file to record.
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevWavOptions',
> +  'data': {
> +    '*frequency': 'int',
> +    '*format':    'AudioFormat',
> +    '*channels':  'int',
> +    '*path':       'str' } }
> +
> +
> +##
> +# @AudiodevBackendOptions
> +#
> +# A discriminated record of audio backends.
> +#
> +# Since: XXX
> +##
> +{ 'union': 'AudiodevBackendOptions',
> +  'data': {
> +    'none':      'AudiodevNoneOptions',
> +    'alsa':      'AudiodevAlsaOptions',
> +    'coreaudio': 'AudiodevCoreaudioOptions',
> +    'dsound':    'AudiodevDsoundOptions',
> +    'oss':       'AudiodevOssOptions',
> +    'pa':        'AudiodevPaOptions',
> +    'sdl':       'AudiodevSdlOptions',
> +    'spice':     'AudiodevSpiceOptions',
> +    'wav':       'AudiodevWavOptions' } }
> +
> +##
> +# @AudioFormat
> +#
> +# An enumeration of possible audio formats.
> +#
> +# Since: XXX
> +##
> +{ 'enum': 'AudioFormat',
> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
> +
> +##
> +# @AudiodevPerDirectionOptions
> +#
> +# General audio backend options that are used for both playback and recording.
> +#
> +# @fixed_settings: #optional use fixed settings for host DAC/ADC
> +#
> +# @frequency: #optional frequency to use when using fixed settings
> +#
> +# @channels: #optional number of channels when using fixed settings
> +#
> +# @format: #optional sample fortmat to use when using fixed settings
> +#
> +# @try_poll: #optional attempt to use poll mode
> +#
> +# Since: XXX
> +##
> +{ 'struct': 'AudiodevPerDirectionOptions',
> +  'data': {
> +    '*fixed_settings': 'bool',
> +    '*frequency':      'int',
> +    '*channels':       'int',
> +    '*format':         'AudioFormat',
> +    '*try_poll':       'bool' } }
> +
> +##
> +# @Audiodev
> +#
> +# Captures the configuration of an audio backend.
> +#
> +# @id: identifier of the backend.
> +#
> +# @in: #optional options of the capture stream.
> +#
> +# @out: #optional options of the playback stream.
> +#
> +# @timer_period: #optional timer period in HZ (0 - use lowest possible)
> +#
> +# @plive: #optional
> +#
> +# @opts: audio backend specific options
> +#
> +# Since XXX
> +##
> +{ 'struct': 'Audiodev',
> +  'data': {
> +    'id':            'str',
> +    '*in':           'AudiodevPerDirectionOptions',
> +    '*out':          'AudiodevPerDirectionOptions',
> +    '*timer_period': 'int',
> +    '*plive':        'bool',
> +    'opts':          'AudiodevBackendOptions' } }

2015-06-04 09:43 keltezéssel, Gerd Hoffmann írta:
>> +#
>> +# @mmap: #optional try using memory mapped access
>
> IIRC this doesn't work everywhere, especially when the oss api is
> implemented as library.
>
> Linux had that, but on linux everybody uses alsa these days ...
> Dunno about other platforms.

idk, the code tries to mmap first, and if it fails, falls back to non 
mmapped access. But something is broken there, as with QEMU_OSS_MMAP=1 
it fails (on linux with alsa oss emulation with pulseaudio alsa 
emulation...). But it ought to work on linux, according to the comments. 
Maybe it needs the native oss, and not the alsa emulation.

>
>> +# @exclusive: #optional open device in exclusive mode (vmix wont work)
>>
>> +# @dsp_policy: #optional set the timing policy of the device, -1 to use fragment
>> +#              mode (option ignored on some platforms)
>
> Would be interesting to know on which platforms this actually has an
> effect (both options) ...

 From a quick google it looks like whatever platform that have oss4 (the 
dsp_policy). exclusive just adds O_EXCL to open flags.

>
> IIRC 'vmix' was a feature of the commercial, ossaudio driver package.
>