[v4,01/14] qapi: qapi for audio backends

This patch adds structures into qapi to replace the existing
configuration structures used by audio backends currently. This qapi
will be the base of the -audiodev command line parameter (that replaces
the old environment variables based config).

This is not a 1:1 translation of the old options, I've tried to make
them much more consistent (e.g. almost every backend had an option to
specify buffer size, but the name was different for every backend, and
some backends required usecs, while some other required frames, samples
or bytes). Also tried to reduce the number of abbreviations used by the
config keys.

Some of the more important changes:
* use `in` and `out` instead of `ADC` and `DAC`, as the former is more
  user friendly imho
* moved buffer settings into the global setting area (so it's the same
  for all backends that support it. Backends that can't change buffer
  size will simply ignore them). Also using usecs, as it's probably more
  user friendly than samples or bytes.
* try-poll is now an alsa backend specific option (as all other backends
  currently ignore it)

AudiodevPaPerDirectionOptions struct might look pointless right now, but
later we will have new per-direction options, so it's more future-proof
this way.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---

Notes:
    Changes from v2:
    
    * update copyright, version numbers
    * remove #optional
    * per-direction options are now optional (needed for qobject_object_visitor_new_str)
    * removed unnecessary AudiodevNoOptions
    * changed integers to unsigned

 Makefile.objs         |   6 +-
 qapi/audio.json       | 233 ++++++++++++++++++++++++++++++++++++++++++
 qapi/qapi-schema.json |   1 +
 3 files changed, 237 insertions(+), 3 deletions(-)
 create mode 100644 qapi/audio.json

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> This patch adds structures into qapi to replace the existing
> configuration structures used by audio backends currently. This qapi
> will be the base of the -audiodev command line parameter (that replaces
> the old environment variables based config).
>
> This is not a 1:1 translation of the old options, I've tried to make
> them much more consistent (e.g. almost every backend had an option to
> specify buffer size, but the name was different for every backend, and
> some backends required usecs, while some other required frames, samples
> or bytes). Also tried to reduce the number of abbreviations used by the
> config keys.
>
> Some of the more important changes:
> * use `in` and `out` instead of `ADC` and `DAC`, as the former is more
>   user friendly imho
> * moved buffer settings into the global setting area (so it's the same
>   for all backends that support it. Backends that can't change buffer
>   size will simply ignore them). Also using usecs, as it's probably more
>   user friendly than samples or bytes.
> * try-poll is now an alsa backend specific option (as all other backends
>   currently ignore it)
>
> AudiodevPaPerDirectionOptions struct might look pointless right now, but
> later we will have new per-direction options, so it's more future-proof
> this way.
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>
> Notes:
>     Changes from v2:
>     
>     * update copyright, version numbers
>     * remove #optional
>     * per-direction options are now optional (needed for qobject_object_visitor_new_str)
>     * removed unnecessary AudiodevNoOptions
>     * changed integers to unsigned
>
>  Makefile.objs         |   6 +-
>  qapi/audio.json       | 233 ++++++++++++++++++++++++++++++++++++++++++
>  qapi/qapi-schema.json |   1 +
>  3 files changed, 237 insertions(+), 3 deletions(-)
>  create mode 100644 qapi/audio.json
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 67a054b08a..d9ee3fed32 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -1,6 +1,6 @@
> -QAPI_MODULES = block-core block char common crypto introspect job migration
> -QAPI_MODULES += misc net rdma rocker run-state sockets tpm trace transaction
> -QAPI_MODULES += ui
> +QAPI_MODULES = audio block-core block char common crypto introspect job
> +QAPI_MODULES += migration misc net rdma rocker run-state sockets tpm trace
> +QAPI_MODULES += transaction ui
>  
>  #######################################################################
>  # Common libraries for tools and emulators
> diff --git a/qapi/audio.json b/qapi/audio.json
> new file mode 100644
> index 0000000000..bd6e2494bd
> --- /dev/null
> +++ b/qapi/audio.json
> @@ -0,0 +1,233 @@
> +# -*- mode: python -*-
> +#
> +# Copyright (C) 2015-2019 Zoltán Kővágó <DirtY.iCE.hu@gmail.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +##
> +# @AudiodevAlsaPerDirectionOptions:
> +#
> +# Options of the alsa backend that are used for both playback and recording.

Please break your comment lines around column 70.

> +#
> +# @dev: the name of the alsa device to use (default 'default')
> +#
> +# @try-poll: attempt to use poll mode, falling back to non polling access on

non-polling

> +#            failure (default on)

default true

> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevAlsaPerDirectionOptions',
> +  'data': {
> +    '*dev':      'str',
> +    '*try-poll': 'bool' } }
> +
> +##
> +# @AudiodevAlsaOptions:
> +#
> +# Options of the alsa audio backend.
> +#
> +# @alsa-in: options of the capture stream
> +#
> +# @alsa-out: options of the playback stream
> +#
> +# @threshold: set the threshold (in microseconds) when playback starts
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevAlsaOptions',
> +  'data': {
> +    '*alsa-in':   'AudiodevAlsaPerDirectionOptions',
> +    '*alsa-out':  'AudiodevAlsaPerDirectionOptions',
> +    '*threshold': 'uint32' } }
> +
> +##
> +# @AudiodevDsoundOptions:
> +#
> +# Options of the dsound audio backend.
> +#
> +# @latency: add extra latency to playback in microseconds (default 10000)
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevDsoundOptions',
> +  'data': {
> +    '*latency': 'uint32' } }
> +
> +##
> +# @AudiodevOssPerDirectionOptions:
> +#
> +# Options of the oss backend that are used for both playback and recording.
> +#
> +# @dev: file name of the oss device (default '/dev/dsp')

A device special file, unlike AudiodevAlsaPerDirectionOptions member
@dev.  Same name, different meaning.  Doesn't bother me in this case.

> +#
> +# @try-poll: attempt to use poll mode, falling back to non polling access on

non-polling

> +#            failure (default on)

default true

> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevOssPerDirectionOptions',
> +  'data': {
> +    '*dev':      'str',
> +    '*try-poll': 'bool' } }
> +
> +##
> +# @AudiodevOssOptions:
> +#
> +# Options of the oss audio backend.
> +#
> +# @oss-in: options of the capture stream
> +#
> +# @oss-out: options of the playback stream
> +#
> +# @try-mmap: try using memory mapped access, falling back to non
> +#            memory mapped access on failure (default off)

memory-mapped, non-memory-mapped

default false

> +#
> +# @exclusive: open device in exclusive mode (vmix won't work) (default off)

default false

> +#
> +# @dsp-policy: set the timing policy of the device (between 0 and 10, where
> +#              smaller number means smaller latency but higher CPU usage) or -1
> +#              to use fragment mode (option ignored on some platforms)
> +#              (default 5)
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevOssOptions',
> +  'data': {
> +    '*oss-in':     'AudiodevOssPerDirectionOptions',
> +    '*oss-out':    'AudiodevOssPerDirectionOptions',
> +    '*try-mmap':   'bool',
> +    '*exclusive':  'bool',
> +    '*dsp-policy': 'uint32' } }
> +
> +##
> +# @AudiodevPaPerDirectionOptions:
> +#
> +# Options of the pa backend that are used for both playback and recording.
> +#
> +# @name: name of the sink/source to use
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevPaPerDirectionOptions',
> +  'data': {
> +    '*name': 'str' } }

Your "might look pointless" remark made me expect an empty struct.  That
would've been pointless indeed.  Instead, you're wrapping the single
option you already have in a struct, to keep open the door for more
options.  Makes sense.

> +
> +##
> +# @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?

> +
> +
> +##
> +# @AudioFormat:
> +#
> +# An enumeration of possible audio formats.
> +#
> +# Since: 4.0
> +##
> +{ 'enum': 'AudioFormat',
> +  'data': [ 'u8', 's8', 'u16', 's16', 'u32', 's32' ] }
> +
> +##
> +# @AudiodevDriver:
> +#
> +# An enumeration of possible audio backend drivers.
> +#
> +# Since: 4.0
> +##
> +{ 'enum': 'AudiodevDriver',
> +  'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'oss', 'pa', 'sdl', 'spice',
> +            'wav' ] }
> +
> +##
> +# @AudiodevPerDirectionOptions:
> +#
> +# General audio backend options that are used for both playback and recording.
> +#
> +# @fixed-settings: use fixed settings for host input/output. When off,
> +#                  frequency, channels and format must not be specified
> +#                  (default on)

default true

> +#
> +# @frequency: frequency to use when using fixed settings (default 44100)
> +#
> +# @channels: number of channels when using fixed settings (default 2)
> +#
> +# @voices: number of voices to use (default 1)
> +#
> +# @format: sample format to use when using fixed settings (default s16)
> +#
> +# @buffer-len: the buffer size in microseconds
> +#
> +# @buffer-count: number of buffers
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'AudiodevPerDirectionOptions',
> +  'data': {
> +    '*fixed-settings': 'bool',
> +    '*frequency':      'uint32',
> +    '*channels':       'uint32',
> +    '*voices':         'uint32',
> +    '*format':         'AudioFormat',
> +    '*buffer-len':     'uint32',
> +    '*buffer-count':   'uint32' } }
> +
> +##
> +# @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'?

Do @in and @out apply to drivers 'dsound' and 'wav'?

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' } }

> 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.

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> This patch adds structures into qapi to replace the existing
> configuration structures used by audio backends currently. This qapi
> will be the base of the -audiodev command line parameter (that replaces
> the old environment variables based config).
>
> This is not a 1:1 translation of the old options, I've tried to make
> them much more consistent (e.g. almost every backend had an option to
> specify buffer size, but the name was different for every backend, and
> some backends required usecs, while some other required frames, samples
> or bytes). Also tried to reduce the number of abbreviations used by the
> config keys.
>
> Some of the more important changes:
> * use `in` and `out` instead of `ADC` and `DAC`, as the former is more
>   user friendly imho
> * moved buffer settings into the global setting area (so it's the same
>   for all backends that support it. Backends that can't change buffer
>   size will simply ignore them). Also using usecs, as it's probably more
>   user friendly than samples or bytes.
> * try-poll is now an alsa backend specific option (as all other backends
>   currently ignore it)
>
> AudiodevPaPerDirectionOptions struct might look pointless right now, but
> later we will have new per-direction options, so it's more future-proof
> this way.
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
[...]
> diff --git a/qapi/audio.json b/qapi/audio.json
> new file mode 100644
> index 0000000000..bd6e2494bd
> --- /dev/null
> +++ b/qapi/audio.json
[...]
> +##
> +# @AudiodevPerDirectionOptions:
> +#
> +# General audio backend options that are used for both playback and recording.
> +#
> +# @fixed-settings: use fixed settings for host input/output. When off,
> +#                  frequency, channels and format must not be specified
> +#                  (default on)
> +#
> +# @frequency: frequency to use when using fixed settings (default 44100)
> +#
> +# @channels: number of channels when using fixed settings (default 2)
> +#
> +# @voices: number of voices to use (default 1)
> +#
> +# @format: sample format to use when using fixed settings (default s16)

If I specify any of @frequency, @channels, @voices, @format, then I have
to have "fixed-settings": true.  Since true is the default, I don't have
to say it explicitly, which would be redundant and mildly annoying.

However, specifying "fixed-settings": false is *not* redundant, because
it's different from specifying nothing at all, which is the same as
"fixed-settings": true, "frequency": 44100, "channels": 2, "voices": 1,
"format": "s16".

I think I get it.

> +#
> +# @buffer-len: the buffer size in microseconds
> +#
> +# @buffer-count: number of buffers
> +#
> +# Since: 4.0
> +##
[...]

On 2019-01-29 14:33, Markus Armbruster wrote:
> "Kővágó, Zoltán" <dirty.ice.hu@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