diff mbox

[07/12] qapi: qapi for audio backends

Message ID 5dee9d7d95eb2260edcc0de0d1090366f159b03f.1434111578.git.DirtY.iCE.hu@gmail.com
State New
Headers show

Commit Message

=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= June 12, 2015, 12:33 p.m. UTC
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 and oss backend specific option (as all other backends
  currently ignore it)

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

---

Changes from v2 RFC patch:
* in, out are no longer optional
* try-poll: moved to alsa and oss (as no other backend used them)
* voices: added (env variables had this option)
* dsound: removed primary buffer related fields

Changes from v1 RFC patch:
* fixed style issues
* moved definitions into a separate file
* documented undocumented options (hopefully)
* removed plive option. It was useless even years ago so it can probably safely
  go away: https://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg02427.html
* removed verbose, debug options. Backends should use trace events instead.
* removed *_retries options from dsound. It's a kludge.
* moved buffer_usecs and buffer_count to the global config options. Some driver
  might ignore it (as they do not expose API to change them).
* wav backend: removed frequecy, format, channels as AudiodevPerDirectionOptions
  already have them.

 Makefile         |   4 +-
 qapi-schema.json |   3 +
 qapi/audio.json  | 217 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 222 insertions(+), 2 deletions(-)
 create mode 100644 qapi/audio.json

Comments

Eric Blake June 12, 2015, 10:11 p.m. UTC | #1
On 06/12/2015 06:33 AM, Kővágó, Zoltán wrote:
> 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 and oss backend specific option (as all other backends
>   currently ignore it)
> 
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> 
> ---

> +++ b/qapi/audio.json
> @@ -0,0 +1,217 @@
> +# -*- mode: python -*-
> +
> +##
> +# @AudiodevNoneOptions

Might be nice to include copyright/license, but this is not the first
.json file missing it.

> +#
> +# The none, coreaudio, sdl and spice audio backend has no options.

s/has/have/

> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'AudiodevNoneOptions',
> +  'data': { } }

Maybe s/None/No/ (since it is shared by backends that have no options),
but I can live with it as-is (since it is used by the 'none' backend).


> +##
> +# @AudiodevAlsaOptions
> +#
> +# Options of the alsa audio backend.
> +#
> +# @in: #optional options of the capture stream
> +#
> +# @out: #optional options of the playback stream

Marked optional here...

> +#
> +# @threshold: #optional set the threshold (in frames) when playback starts
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'AudiodevAlsaOptions',
> +  'data': {
> +    'in':         'AudiodevAlsaPerDirectionOptions',
> +    'out':        'AudiodevAlsaPerDirectionOptions',

...but not here.

> +    '*threshold': 'int' } }
> +
> +##
> +# @AudiodevDsoundOptions
> +#
> +# Options of the dsound audio backend.
> +#
> +# @latency-millis: #optional add extra latency to playback
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'AudiodevDsoundOptions',
> +  'data': {
> +    '*latency-millis': 'int' } }

Style question - should we just call this 'latency', and document the
milliseconds unit in the description? But having the name latency_millis
in C code might not be all that bad, so you may not want to change this one.


> +##
> +# @AudiodevOssOptions
> +#
> +# Options of the oss audio backend.
> +#
> +# @in: #optional options of the capture stream
> +#
> +# @out: #optional options of the playback stream
> +#
> +# @mmap: #optional try using memory mapped access
> +#
> +# @exclusive: #optional open device in exclusive mode (vmix wont work)

s/wont/won't/

> +#
> +# @dsp-policy: #optional set the timing policy of the device, -1 to use fragment
> +#              mode (option ignored on some platforms)
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'AudiodevOssOptions',
> +  'data': {
> +    'in':          'AudiodevOssPerDirectionOptions',
> +    'out':         'AudiodevOssPerDirectionOptions',

Again, inconsistent on the optional marking.


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

s/fortmat/format/

> +#
> +# @buffer-usecs: #optional the buffer size in microseconds
> +#
> +# @buffer-count: #optional nuber of buffers
> +#

s/nuber/number/

> +# Since: 2.4
> +##
> +{ 'struct': 'AudiodevPerDirectionOptions',
> +  'data': {
> +    '*fixed-settings': 'bool',
> +    '*frequency':      'int',
> +    '*channels':       'int',
> +    '*voices':         'int',
> +    '*format':         'AudioFormat',
> +    '*buffer-usecs':   'int',
> +    '*buffer-count':   'int' } }
> +
> +##
> +# @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)
> +#
> +# @opts: audio backend specific options
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'Audiodev',
> +  'data': {
> +    '*id':           'str',
> +    'in':            'AudiodevPerDirectionOptions',
> +    'out':           'AudiodevPerDirectionOptions',

Another mismatch in optional marking.

> +    '*timer-period': 'int',
> +    'opts':          'AudiodevBackendOptions' } }
>
=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= June 12, 2015, 10:59 p.m. UTC | #2
2015-06-13 00:11 keltezéssel, Eric Blake írta:
>> +##
>> +# @AudiodevAlsaOptions
>> +#
>> +# Options of the alsa audio backend.
>> +#
>> +# @in: #optional options of the capture stream
>> +#
>> +# @out: #optional options of the playback stream
>
> Marked optional here...
>
>> +#
>> +# @threshold: #optional set the threshold (in frames) when playback starts
>> +#
>> +# Since: 2.4
>> +##
>> +{ 'struct': 'AudiodevAlsaOptions',
>> +  'data': {
>> +    'in':         'AudiodevAlsaPerDirectionOptions',
>> +    'out':        'AudiodevAlsaPerDirectionOptions',
>
> ...but not here.

Oups. The code is the correct (they are not optional), I forgot updating 
the documentation. (Same goes for the other mismatches).

>
>> +    '*threshold': 'int' } }
>> +
>> +##
>> +# @AudiodevDsoundOptions
>> +#
>> +# Options of the dsound audio backend.
>> +#
>> +# @latency-millis: #optional add extra latency to playback
>> +#
>> +# Since: 2.4
>> +##
>> +{ 'struct': 'AudiodevDsoundOptions',
>> +  'data': {
>> +    '*latency-millis': 'int' } }
>
> Style question - should we just call this 'latency', and document the
> milliseconds unit in the description? But having the name latency_millis
> in C code might not be all that bad, so you may not want to change this one.

There is also a buffer-usecs, so I vote for keeping latency-millis. Also 
there is timer-period in Audiodev. Maybe it should be renamed to 
timer-period-hz, to keep consistency. Or maybe change all of them to usecs.


Other issues acked.

Thanks,
Zoltan
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 2d52536..982563e 100644
--- a/Makefile
+++ b/Makefile
@@ -257,8 +257,8 @@  $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 		"  GEN   $@")
 
 qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
-               $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
-               $(SRC_PATH)/qapi/event.json
+               $(SRC_PATH)/qapi/audio.json  $(SRC_PATH)/qapi/block.json \
+               $(SRC_PATH)/qapi/block-core.json $(SRC_PATH)/qapi/event.json
 
 qapi-types.c qapi-types.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
diff --git a/qapi-schema.json b/qapi-schema.json
index 6e17a5c..26c470a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5,6 +5,9 @@ 
 # QAPI common definitions
 { 'include': 'qapi/common.json' }
 
+# QAPI audio definitions
+{ 'include': 'qapi/audio.json' }
+
 # QAPI block definitions
 { 'include': 'qapi/block.json' }
 
diff --git a/qapi/audio.json b/qapi/audio.json
new file mode 100644
index 0000000..157ccf6
--- /dev/null
+++ b/qapi/audio.json
@@ -0,0 +1,217 @@ 
+# -*- mode: python -*-
+
+##
+# @AudiodevNoneOptions
+#
+# The none, coreaudio, sdl and spice audio backend has no options.
+#
+# Since: 2.4
+##
+{ '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
+#
+# @try-poll: #optional attempt to use poll mode
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevAlsaPerDirectionOptions',
+  'data': {
+    '*dev':      'str',
+    '*try-poll': 'bool' } }
+
+##
+# @AudiodevAlsaOptions
+#
+# Options of the alsa audio backend.
+#
+# @in: #optional options of the capture stream
+#
+# @out: #optional options of the playback stream
+#
+# @threshold: #optional set the threshold (in frames) when playback starts
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevAlsaOptions',
+  'data': {
+    'in':         'AudiodevAlsaPerDirectionOptions',
+    'out':        'AudiodevAlsaPerDirectionOptions',
+    '*threshold': 'int' } }
+
+##
+# @AudiodevDsoundOptions
+#
+# Options of the dsound audio backend.
+#
+# @latency-millis: #optional add extra latency to playback
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevDsoundOptions',
+  'data': {
+    '*latency-millis': 'int' } }
+
+##
+# @AudiodevOssPerDirectionOptions
+#
+# Options of the oss backend that are used for both playback and recording.
+#
+# @dev: #optional path of the oss device
+#
+# @try-poll: #optional attempt to use poll mode
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevOssPerDirectionOptions',
+  'data': {
+    '*dev':      'str',
+    '*try-poll': 'bool' } }
+
+##
+# @AudiodevOssOptions
+#
+# Options of the oss audio backend.
+#
+# @in: #optional options of the capture stream
+#
+# @out: #optional options of the playback stream
+#
+# @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)
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevOssOptions',
+  'data': {
+    'in':          'AudiodevOssPerDirectionOptions',
+    'out':         'AudiodevOssPerDirectionOptions',
+    '*mmap':       'bool',
+    '*exclusive':  'bool',
+    '*dsp-policy': 'int' } }
+
+##
+# @AudiodevPaOptions
+#
+# Options of the pa (PulseAudio) audio backend.
+#
+# @server: #optional PulseAudio server address
+#
+# @sink: #optional sink device name
+#
+# @source: #optional source device name
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevPaOptions',
+  'data': {
+    '*server': 'str',
+    '*sink':   'str',
+    '*source': 'str' } }
+
+##
+# @AudiodevWavOptions
+#
+# Options of the wav audio backend.
+#
+# @path: #optional path of the wav file to record
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevWavOptions',
+  'data': {
+    '*path': 'str' } }
+
+
+##
+# @AudiodevBackendOptions
+#
+# A discriminated record of audio backends.
+#
+# Since: 2.4
+##
+{ 'union': 'AudiodevBackendOptions',
+  'data': {
+    'none':      'AudiodevNoneOptions',
+    'alsa':      'AudiodevAlsaOptions',
+    'coreaudio': 'AudiodevNoneOptions',
+    'dsound':    'AudiodevDsoundOptions',
+    'oss':       'AudiodevOssOptions',
+    'pa':        'AudiodevPaOptions',
+    'sdl':       'AudiodevNoneOptions',
+    'spice':     'AudiodevNoneOptions',
+    'wav':       'AudiodevWavOptions' } }
+
+##
+# @AudioFormat
+#
+# An enumeration of possible audio formats.
+#
+# Since: 2.4
+##
+{ '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
+#
+# @buffer-usecs: #optional the buffer size in microseconds
+#
+# @buffer-count: #optional nuber of buffers
+#
+# Since: 2.4
+##
+{ 'struct': 'AudiodevPerDirectionOptions',
+  'data': {
+    '*fixed-settings': 'bool',
+    '*frequency':      'int',
+    '*channels':       'int',
+    '*voices':         'int',
+    '*format':         'AudioFormat',
+    '*buffer-usecs':   'int',
+    '*buffer-count':   'int' } }
+
+##
+# @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)
+#
+# @opts: audio backend specific options
+#
+# Since: 2.4
+##
+{ 'struct': 'Audiodev',
+  'data': {
+    '*id':           'str',
+    'in':            'AudiodevPerDirectionOptions',
+    'out':           'AudiodevPerDirectionOptions',
+    '*timer-period': 'int',
+    'opts':          'AudiodevBackendOptions' } }