| Message ID | d662ea2787c63d291afd01da86c59f662ebee769.1434458391.git.DirtY.iCE.hu@gmail.com |
|---|---|
| State | New |
| Headers | show |
Copying Eric for additional QAPI schema expertise. My questions inline, pretty sure they show my ignorance. "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 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 v1 patch: > * every time-related field now take usecs (and removed -usecs, -millis suffixes) > * fixed inconsisten optional marking, language issues > > 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 | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 228 insertions(+), 2 deletions(-) > create mode 100644 qapi/audio.json > > diff --git a/Makefile b/Makefile > index 3f97904..ac566fa 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 106008c..e751ea3 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..2851689 > --- /dev/null > +++ b/qapi/audio.json > @@ -0,0 +1,223 @@ > +# -*- mode: python -*- > +# > +# Copyright (C) 2015 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. > + > +## > +# @AudiodevNoOptions > +# > +# The none, coreaudio, sdl and spice audio backend have no options. > +# > +# Since: 2.4 > +## > +{ 'struct': 'AudiodevNoOptions', > + '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 Who picks the default, QEMU or ALSA? > +# > +# @try-poll: #optional attempt to use poll mode What happens when the attempt fails? > +# > +# Since: 2.4 > +## > +{ 'struct': 'AudiodevAlsaPerDirectionOptions', > + 'data': { > + '*dev': 'str', > + '*try-poll': 'bool' } } > + > +## > +# @AudiodevAlsaOptions > +# > +# Options of the alsa audio backend. > +# > +# @in: options of the capture stream > +# > +# @out: 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' } } Do we have an established term for a "direction"? If not: the use with 'in' and 'out' tempts me to name the type AudiodevAlsaIOOptions. Just rambling, use what you think is best. > + > +## > +# @AudiodevDsoundOptions > +# > +# Options of the dsound audio backend. > +# > +# @latency: #optional add extra latency to playback (in microseconds) We already use seconds, milliseconds and nanoseconds. You make the zoo complete. > +# > +# Since: 2.4 > +## > +{ 'struct': 'AudiodevDsoundOptions', > + 'data': { > + '*latency': 'int' } } > + > +## > +# @AudiodevOssPerDirectionOptions > +# > +# Options of the oss backend that are used for both playback and recording. > +# > +# @dev: #optional path of the oss device Who picks the default, QEMU or OSS? For ALSA, you documented @dev to be "the name", here it's "path". Intentional difference? > +# > +# @try-poll: #optional attempt to use poll mode > +# > +# Since: 2.4 > +## > +{ 'struct': 'AudiodevOssPerDirectionOptions', > + 'data': { > + '*dev': 'str', > + '*try-poll': 'bool' } } Same as for ALSA. Keeping them separate is fine with me. > + > +## > +# @AudiodevOssOptions > +# > +# Options of the oss audio backend. > +# > +# @in: options of the capture stream > +# > +# @out: options of the playback stream > +# > +# @mmap: #optional try using memory mapped access What happens when the attempt fails? Should this be called try-mmap? > +# > +# @exclusive: #optional open device in exclusive mode (vmix won't work) > +# > +# @dsp-policy: #optional set the timing policy of the device, -1 to use fragment > +# mode (option ignored on some platforms) What are the possible values besides -1? > +# > +# 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 Who picks the defaults, QEMU or PA? > +# > +# 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' } } Who picks the default? > + > + > +## > +# @AudiodevBackendOptions > +# > +# A discriminated record of audio backends. > +# > +# Since: 2.4 > +## > +{ 'union': 'AudiodevBackendOptions', > + 'data': { > + 'none': 'AudiodevNoOptions', > + 'alsa': 'AudiodevAlsaOptions', > + 'coreaudio': 'AudiodevNoOptions', > + 'dsound': 'AudiodevDsoundOptions', > + 'oss': 'AudiodevOssOptions', > + 'pa': 'AudiodevPaOptions', > + 'sdl': 'AudiodevNoOptions', > + 'spice': 'AudiodevNoOptions', > + '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 format to use when using fixed settings Are these guys used when @fixed-settings is off? > +# > +# @buffer: #optional the buffer size (in microseconds) @buffer suggests this is a buffer, not a buffer length given as time span. @buffer-len? > +# > +# @buffer-count: #optional number of buffers > +# > +# Since: 2.4 > +## > +{ 'struct': 'AudiodevPerDirectionOptions', > + 'data': { > + '*fixed-settings': 'bool', > + '*frequency': 'int', > + '*channels': 'int', > + '*voices': 'int', > + '*format': 'AudioFormat', > + '*buffer': 'int', > + '*buffer-count': 'int' } } > + > +## > +# @Audiodev > +# > +# Captures the configuration of an audio backend. > +# > +# @id: identifier of the backend > +# > +# @in: options of the capture stream > +# > +# @out: options of the playback stream > +# > +# @timer-period: #optional timer period (in microseconds, 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' } } Have you considered making this a flat union, similar ro BlockdevOptions? Don't get deceived by the number of my questions, this is solid work.
2015-06-17 09:46 keltezéssel, Markus Armbruster írta: > Copying Eric for additional QAPI schema expertise. > > My questions inline, pretty sure they show my ignorance. > > "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 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 v1 patch: >> * every time-related field now take usecs (and removed -usecs, -millis suffixes) >> * fixed inconsisten optional marking, language issues >> >> 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 | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 228 insertions(+), 2 deletions(-) >> create mode 100644 qapi/audio.json >> >> diff --git a/Makefile b/Makefile >> index 3f97904..ac566fa 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 106008c..e751ea3 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..2851689 >> --- /dev/null >> +++ b/qapi/audio.json >> @@ -0,0 +1,223 @@ >> +# -*- mode: python -*- >> +# >> +# Copyright (C) 2015 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. >> + >> +## >> +# @AudiodevNoOptions >> +# >> +# The none, coreaudio, sdl and spice audio backend have no options. >> +# >> +# Since: 2.4 >> +## >> +{ 'struct': 'AudiodevNoOptions', >> + '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 > > Who picks the default, QEMU or ALSA? It defaults to "default", which tells alsa to use the default device... > >> +# >> +# @try-poll: #optional attempt to use poll mode > > What happens when the attempt fails? It falls back to non polling (timer based) mode. >> +# >> +# Since: 2.4 >> +## >> +{ 'struct': 'AudiodevAlsaPerDirectionOptions', >> + 'data': { >> + '*dev': 'str', >> + '*try-poll': 'bool' } } >> + >> +## >> +# @AudiodevAlsaOptions >> +# >> +# Options of the alsa audio backend. >> +# >> +# @in: options of the capture stream >> +# >> +# @out: 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' } } > > Do we have an established term for a "direction"? > > If not: the use with 'in' and 'out' tempts me to name the type > AudiodevAlsaIOOptions. > > Just rambling, use what you think is best. I don't know, so far nobody rambled about my naming... > >> + >> +## >> +# @AudiodevDsoundOptions >> +# >> +# Options of the dsound audio backend. >> +# >> +# @latency: #optional add extra latency to playback (in microseconds) > > We already use seconds, milliseconds and nanoseconds. You make the zoo > complete. Hmm. The audio backend previously used microseconds, milliseconds, Hertz, frames, samples and bytes as time measurement, now at least everything use microseconds. Milliseconds precision is not enough, while nanosecond precision doesn't really make sense imho. >> +# >> +# Since: 2.4 >> +## >> +{ 'struct': 'AudiodevDsoundOptions', >> + 'data': { >> + '*latency': 'int' } } >> + >> +## >> +# @AudiodevOssPerDirectionOptions >> +# >> +# Options of the oss backend that are used for both playback and recording. >> +# >> +# @dev: #optional path of the oss device > > Who picks the default, QEMU or OSS? It defaults to "/dev/dsp". > For ALSA, you documented @dev to be "the name", here it's "path". > Intentional difference? Yes. For oss you have to provide a filesystem path of the audio device (like /dev/dsp), for alsa you provide an alsa specific name, like "default", "hw:1,0", "hdmi:CARD=NVidia,DEV=1", whatever. >> +# >> +# @try-poll: #optional attempt to use poll mode >> +# >> +# Since: 2.4 >> +## >> +{ 'struct': 'AudiodevOssPerDirectionOptions', >> + 'data': { >> + '*dev': 'str', >> + '*try-poll': 'bool' } } > > Same as for ALSA. Keeping them separate is fine with me. This is the same as alsa's try-poll. They're separate because only these two backend use this setting. > >> + >> +## >> +# @AudiodevOssOptions >> +# >> +# Options of the oss audio backend. >> +# >> +# @in: options of the capture stream >> +# >> +# @out: options of the playback stream >> +# >> +# @mmap: #optional try using memory mapped access > > What happens when the attempt fails? It should fall back to non-mmapped access (should because when I checked, it was buggy, at least on linux with alsa emulated oss on pulseaudio alsa device...) > Should this be called try-mmap? Agree. >> +# >> +# @exclusive: #optional open device in exclusive mode (vmix won't work) >> +# >> +# @dsp-policy: #optional set the timing policy of the device, -1 to use fragment >> +# mode (option ignored on some platforms) > > What are the possible values besides -1? It should be a number between 0 and 10 (according to this page: http://manuals.opensound.com/developer/SNDCTL_DSP_POLICY.html) > >> +# >> +# 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 > > Who picks the defaults, QEMU or PA? PA >> +# >> +# 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' } } > > Who picks the default? It defaults to "qemu.wav" >> + >> + >> +## >> +# @AudiodevBackendOptions >> +# >> +# A discriminated record of audio backends. >> +# >> +# Since: 2.4 >> +## >> +{ 'union': 'AudiodevBackendOptions', >> + 'data': { >> + 'none': 'AudiodevNoOptions', >> + 'alsa': 'AudiodevAlsaOptions', >> + 'coreaudio': 'AudiodevNoOptions', >> + 'dsound': 'AudiodevDsoundOptions', >> + 'oss': 'AudiodevOssOptions', >> + 'pa': 'AudiodevPaOptions', >> + 'sdl': 'AudiodevNoOptions', >> + 'spice': 'AudiodevNoOptions', >> + '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 format to use when using fixed settings > > Are these guys used when @fixed-settings is off? No. >> +# >> +# @buffer: #optional the buffer size (in microseconds) > > @buffer suggests this is a buffer, not a buffer length given as time > span. @buffer-len? Ok. (It used to be called buffer-usecs before I changed everything to microseconds.) > >> +# >> +# @buffer-count: #optional number of buffers >> +# >> +# Since: 2.4 >> +## >> +{ 'struct': 'AudiodevPerDirectionOptions', >> + 'data': { >> + '*fixed-settings': 'bool', >> + '*frequency': 'int', >> + '*channels': 'int', >> + '*voices': 'int', >> + '*format': 'AudioFormat', >> + '*buffer': 'int', >> + '*buffer-count': 'int' } } >> + >> +## >> +# @Audiodev >> +# >> +# Captures the configuration of an audio backend. >> +# >> +# @id: identifier of the backend >> +# >> +# @in: options of the capture stream >> +# >> +# @out: options of the playback stream >> +# >> +# @timer-period: #optional timer period (in microseconds, 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' } } > > Have you considered making this a flat union, similar ro > BlockdevOptions? Not really. If you qapi masters out there think it's better, then I will convert it. > Don't get deceived by the number of my questions, this is solid work. >
"Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: >> Copying Eric for additional QAPI schema expertise. >> >> My questions inline, pretty sure they show my ignorance. >> >> "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 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 v1 patch: >>> * every time-related field now take usecs (and removed -usecs, -millis suffixes) >>> * fixed inconsisten optional marking, language issues >>> >>> 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 | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 228 insertions(+), 2 deletions(-) >>> create mode 100644 qapi/audio.json >>> >>> diff --git a/Makefile b/Makefile >>> index 3f97904..ac566fa 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 106008c..e751ea3 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..2851689 >>> --- /dev/null >>> +++ b/qapi/audio.json >>> @@ -0,0 +1,223 @@ >>> +# -*- mode: python -*- >>> +# >>> +# Copyright (C) 2015 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. >>> + >>> +## >>> +# @AudiodevNoOptions >>> +# >>> +# The none, coreaudio, sdl and spice audio backend have no options. >>> +# >>> +# Since: 2.4 >>> +## >>> +{ 'struct': 'AudiodevNoOptions', >>> + '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 >> >> Who picks the default, QEMU or ALSA? > > It defaults to "default", which tells alsa to use the default device... Then make it # @dev: #optional the name of the alsa device to use (default 'default') >> >>> +# >>> +# @try-poll: #optional attempt to use poll mode >> >> What happens when the attempt fails? > > It falls back to non polling (timer based) mode. Okay, assuming that's the user interface we want. >>> +# >>> +# Since: 2.4 >>> +## >>> +{ 'struct': 'AudiodevAlsaPerDirectionOptions', >>> + 'data': { >>> + '*dev': 'str', >>> + '*try-poll': 'bool' } } >>> + >>> +## >>> +# @AudiodevAlsaOptions >>> +# >>> +# Options of the alsa audio backend. >>> +# >>> +# @in: options of the capture stream >>> +# >>> +# @out: 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' } } >> >> Do we have an established term for a "direction"? >> >> If not: the use with 'in' and 'out' tempts me to name the type >> AudiodevAlsaIOOptions. >> >> Just rambling, use what you think is best. > > I don't know, so far nobody rambled about my naming... > >> >>> + >>> +## >>> +# @AudiodevDsoundOptions >>> +# >>> +# Options of the dsound audio backend. >>> +# >>> +# @latency: #optional add extra latency to playback (in microseconds) >> >> We already use seconds, milliseconds and nanoseconds. You make the zoo >> complete. > > Hmm. The audio backend previously used microseconds, milliseconds, > Hertz, frames, samples and bytes as time measurement, now at least > everything use microseconds. Milliseconds precision is not enough, > while nanosecond precision doesn't really make sense imho. I'm just poking fun at our inability to pick a time unit for QAPI/QMP. No objection to your use of microseconds. >>> +# >>> +# Since: 2.4 >>> +## >>> +{ 'struct': 'AudiodevDsoundOptions', >>> + 'data': { >>> + '*latency': 'int' } } >>> + >>> +## >>> +# @AudiodevOssPerDirectionOptions >>> +# >>> +# Options of the oss backend that are used for both playback and recording. >>> +# >>> +# @dev: #optional path of the oss device >> >> Who picks the default, QEMU or OSS? > > It defaults to "/dev/dsp". > >> For ALSA, you documented @dev to be "the name", here it's "path". >> Intentional difference? > > Yes. For oss you have to provide a filesystem path of the audio device > (like /dev/dsp), for alsa you provide an alsa specific name, like > "default", "hw:1,0", "hdmi:CARD=NVidia,DEV=1", whatever. I don't like calling a filename a path, even though we already do in many places. Let's do: # @dev: #optional file name of the oss device (default '/dev/dsp') >>> +# >>> +# @try-poll: #optional attempt to use poll mode >>> +# >>> +# Since: 2.4 >>> +## >>> +{ 'struct': 'AudiodevOssPerDirectionOptions', >>> + 'data': { >>> + '*dev': 'str', >>> + '*try-poll': 'bool' } } >> >> Same as for ALSA. Keeping them separate is fine with me. > > This is the same as alsa's try-poll. They're separate because only > these two backend use this setting. >> >>> + >>> +## >>> +# @AudiodevOssOptions >>> +# >>> +# Options of the oss audio backend. >>> +# >>> +# @in: options of the capture stream >>> +# >>> +# @out: options of the playback stream >>> +# >>> +# @mmap: #optional try using memory mapped access >> >> What happens when the attempt fails? > > It should fall back to non-mmapped access (should because when I > checked, it was buggy, at least on linux with alsa emulated oss on > pulseaudio alsa device...) Okay, assuming that's the user interface we want. >> Should this be called try-mmap? > Agree. > >>> +# >>> +# @exclusive: #optional open device in exclusive mode (vmix won't work) >>> +# >>> +# @dsp-policy: #optional set the timing policy of the device, -1 >>> to use fragment >>> +# mode (option ignored on some platforms) >> >> What are the possible values besides -1? > > It should be a number between 0 and 10 (according to this page: > http://manuals.opensound.com/developer/SNDCTL_DSP_POLICY.html) Please cover that in your comment. >>> +# >>> +# 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 >> >> Who picks the defaults, QEMU or PA? > > PA Is there a way to explicitly ask for the PA default? Something like source=default? >>> +# >>> +# 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' } } >> >> Who picks the default? > > It defaults to "qemu.wav" Make it # @path: #optional path of the wav file to record (default 'qemu.wav') >>> + >>> + >>> +## >>> +# @AudiodevBackendOptions >>> +# >>> +# A discriminated record of audio backends. >>> +# >>> +# Since: 2.4 >>> +## >>> +{ 'union': 'AudiodevBackendOptions', >>> + 'data': { >>> + 'none': 'AudiodevNoOptions', >>> + 'alsa': 'AudiodevAlsaOptions', >>> + 'coreaudio': 'AudiodevNoOptions', >>> + 'dsound': 'AudiodevDsoundOptions', >>> + 'oss': 'AudiodevOssOptions', >>> + 'pa': 'AudiodevPaOptions', >>> + 'sdl': 'AudiodevNoOptions', >>> + 'spice': 'AudiodevNoOptions', >>> + '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 format to use when using fixed settings >> >> Are these guys used when @fixed-settings is off? > > No. If @fixed-settings, are the other three all required? If not, what are their defaults? >>> +# >>> +# @buffer: #optional the buffer size (in microseconds) >> >> @buffer suggests this is a buffer, not a buffer length given as time >> span. @buffer-len? > > Ok. (It used to be called buffer-usecs before I changed everything to > microseconds.) > >> >>> +# >>> +# @buffer-count: #optional number of buffers >>> +# >>> +# Since: 2.4 >>> +## >>> +{ 'struct': 'AudiodevPerDirectionOptions', >>> + 'data': { >>> + '*fixed-settings': 'bool', >>> + '*frequency': 'int', >>> + '*channels': 'int', >>> + '*voices': 'int', >>> + '*format': 'AudioFormat', >>> + '*buffer': 'int', >>> + '*buffer-count': 'int' } } >>> + >>> +## >>> +# @Audiodev >>> +# >>> +# Captures the configuration of an audio backend. >>> +# >>> +# @id: identifier of the backend >>> +# >>> +# @in: options of the capture stream >>> +# >>> +# @out: options of the playback stream >>> +# >>> +# @timer-period: #optional timer period (in microseconds, 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' } } >> >> Have you considered making this a flat union, similar ro >> BlockdevOptions? > > Not really. If you qapi masters out there think it's better, then I > will convert it. Related: discussion about flattening in review of PATCH 2. >> Don't get deceived by the number of my questions, this is solid work. >>
2015-06-17 13:48 keltezéssel, Markus Armbruster írta: > "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > >> 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: >>> Copying Eric for additional QAPI schema expertise. >>> >>> My questions inline, pretty sure they show my ignorance. >>> >>> "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 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 v1 patch: >>>> * every time-related field now take usecs (and removed -usecs, -millis suffixes) >>>> * fixed inconsisten optional marking, language issues >>>> >>>> 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 | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 228 insertions(+), 2 deletions(-) >>>> create mode 100644 qapi/audio.json >>>> >>>> diff --git a/Makefile b/Makefile >>>> index 3f97904..ac566fa 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 106008c..e751ea3 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..2851689 >>>> --- /dev/null >>>> +++ b/qapi/audio.json >>>> @@ -0,0 +1,223 @@ >>>> +# -*- mode: python -*- >>>> +# >>>> +# Copyright (C) 2015 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. >>>> + >>>> +## >>>> +# @AudiodevNoOptions >>>> +# >>>> +# The none, coreaudio, sdl and spice audio backend have no options. >>>> +# >>>> +# Since: 2.4 >>>> +## >>>> +{ 'struct': 'AudiodevNoOptions', >>>> + '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 >>> >>> Who picks the default, QEMU or ALSA? >> >> It defaults to "default", which tells alsa to use the default device... > > Then make it > > # @dev: #optional the name of the alsa device to use (default 'default') > >>> >>>> +# >>>> +# @try-poll: #optional attempt to use poll mode >>> >>> What happens when the attempt fails? >> >> It falls back to non polling (timer based) mode. > > Okay, assuming that's the user interface we want. > >>>> +# >>>> +# Since: 2.4 >>>> +## >>>> +{ 'struct': 'AudiodevAlsaPerDirectionOptions', >>>> + 'data': { >>>> + '*dev': 'str', >>>> + '*try-poll': 'bool' } } >>>> + >>>> +## >>>> +# @AudiodevAlsaOptions >>>> +# >>>> +# Options of the alsa audio backend. >>>> +# >>>> +# @in: options of the capture stream >>>> +# >>>> +# @out: 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' } } >>> >>> Do we have an established term for a "direction"? >>> >>> If not: the use with 'in' and 'out' tempts me to name the type >>> AudiodevAlsaIOOptions. >>> >>> Just rambling, use what you think is best. >> >> I don't know, so far nobody rambled about my naming... >> >>> >>>> + >>>> +## >>>> +# @AudiodevDsoundOptions >>>> +# >>>> +# Options of the dsound audio backend. >>>> +# >>>> +# @latency: #optional add extra latency to playback (in microseconds) >>> >>> We already use seconds, milliseconds and nanoseconds. You make the zoo >>> complete. >> >> Hmm. The audio backend previously used microseconds, milliseconds, >> Hertz, frames, samples and bytes as time measurement, now at least >> everything use microseconds. Milliseconds precision is not enough, >> while nanosecond precision doesn't really make sense imho. > > I'm just poking fun at our inability to pick a time unit for QAPI/QMP. > No objection to your use of microseconds. > >>>> +# >>>> +# Since: 2.4 >>>> +## >>>> +{ 'struct': 'AudiodevDsoundOptions', >>>> + 'data': { >>>> + '*latency': 'int' } } >>>> + >>>> +## >>>> +# @AudiodevOssPerDirectionOptions >>>> +# >>>> +# Options of the oss backend that are used for both playback and recording. >>>> +# >>>> +# @dev: #optional path of the oss device >>> >>> Who picks the default, QEMU or OSS? >> >> It defaults to "/dev/dsp". >> >>> For ALSA, you documented @dev to be "the name", here it's "path". >>> Intentional difference? >> >> Yes. For oss you have to provide a filesystem path of the audio device >> (like /dev/dsp), for alsa you provide an alsa specific name, like >> "default", "hw:1,0", "hdmi:CARD=NVidia,DEV=1", whatever. > > I don't like calling a filename a path, even though we already do in > many places. Let's do: > > # @dev: #optional file name of the oss device (default '/dev/dsp') > >>>> +# >>>> +# @try-poll: #optional attempt to use poll mode >>>> +# >>>> +# Since: 2.4 >>>> +## >>>> +{ 'struct': 'AudiodevOssPerDirectionOptions', >>>> + 'data': { >>>> + '*dev': 'str', >>>> + '*try-poll': 'bool' } } >>> >>> Same as for ALSA. Keeping them separate is fine with me. >> >> This is the same as alsa's try-poll. They're separate because only >> these two backend use this setting. >>> >>>> + >>>> +## >>>> +# @AudiodevOssOptions >>>> +# >>>> +# Options of the oss audio backend. >>>> +# >>>> +# @in: options of the capture stream >>>> +# >>>> +# @out: options of the playback stream >>>> +# >>>> +# @mmap: #optional try using memory mapped access >>> >>> What happens when the attempt fails? >> >> It should fall back to non-mmapped access (should because when I >> checked, it was buggy, at least on linux with alsa emulated oss on >> pulseaudio alsa device...) > > Okay, assuming that's the user interface we want. > >>> Should this be called try-mmap? >> Agree. >> >>>> +# >>>> +# @exclusive: #optional open device in exclusive mode (vmix won't work) >>>> +# >>>> +# @dsp-policy: #optional set the timing policy of the device, -1 >>>> to use fragment >>>> +# mode (option ignored on some platforms) >>> >>> What are the possible values besides -1? >> >> It should be a number between 0 and 10 (according to this page: >> http://manuals.opensound.com/developer/SNDCTL_DSP_POLICY.html) > > Please cover that in your comment. > >>>> +# >>>> +# 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 >>> >>> Who picks the defaults, QEMU or PA? >> >> PA > > Is there a way to explicitly ask for the PA default? Something like > source=default? Not really right now. The default is a NULL pointer (pulseaudio api wise), so unless we add an arbitrary keyword (like default), it's not possible to ask explicitly for the default. (But omitting them will choose the default, of course.) >>>> +# >>>> +# 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' } } >>> >>> Who picks the default? >> >> It defaults to "qemu.wav" > > Make it > > # @path: #optional path of the wav file to record (default 'qemu.wav') > >>>> + >>>> + >>>> +## >>>> +# @AudiodevBackendOptions >>>> +# >>>> +# A discriminated record of audio backends. >>>> +# >>>> +# Since: 2.4 >>>> +## >>>> +{ 'union': 'AudiodevBackendOptions', >>>> + 'data': { >>>> + 'none': 'AudiodevNoOptions', >>>> + 'alsa': 'AudiodevAlsaOptions', >>>> + 'coreaudio': 'AudiodevNoOptions', >>>> + 'dsound': 'AudiodevDsoundOptions', >>>> + 'oss': 'AudiodevOssOptions', >>>> + 'pa': 'AudiodevPaOptions', >>>> + 'sdl': 'AudiodevNoOptions', >>>> + 'spice': 'AudiodevNoOptions', >>>> + '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 format to use when using fixed settings >>> >>> Are these guys used when @fixed-settings is off? >> >> No. > > If @fixed-settings, are the other three all required? If not, what are > their defaults? No, they all have defaults: 44100 Hz, 2 channels and s16 format. I guess I should also document it... >>>> +# >>>> +# @buffer: #optional the buffer size (in microseconds) >>> >>> @buffer suggests this is a buffer, not a buffer length given as time >>> span. @buffer-len? >> >> Ok. (It used to be called buffer-usecs before I changed everything to >> microseconds.) >> >>> >>>> +# >>>> +# @buffer-count: #optional number of buffers >>>> +# >>>> +# Since: 2.4 >>>> +## >>>> +{ 'struct': 'AudiodevPerDirectionOptions', >>>> + 'data': { >>>> + '*fixed-settings': 'bool', >>>> + '*frequency': 'int', >>>> + '*channels': 'int', >>>> + '*voices': 'int', >>>> + '*format': 'AudioFormat', >>>> + '*buffer': 'int', >>>> + '*buffer-count': 'int' } } >>>> + >>>> +## >>>> +# @Audiodev >>>> +# >>>> +# Captures the configuration of an audio backend. >>>> +# >>>> +# @id: identifier of the backend >>>> +# >>>> +# @in: options of the capture stream >>>> +# >>>> +# @out: options of the playback stream >>>> +# >>>> +# @timer-period: #optional timer period (in microseconds, 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' } } >>> >>> Have you considered making this a flat union, similar ro >>> BlockdevOptions? >> >> Not really. If you qapi masters out there think it's better, then I >> will convert it. > > Related: discussion about flattening in review of PATCH 2. > >>> Don't get deceived by the number of my questions, this is solid work. >>>
"Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > 2015-06-17 13:48 keltezéssel, Markus Armbruster írta: >> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >> >>> 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: >>>> Copying Eric for additional QAPI schema expertise. >>>> >>>> My questions inline, pretty sure they show my ignorance. >>>> >>>> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes: [...] >>>>> +## >>>>> +# @AudiodevPaOptions >>>>> +# >>>>> +# Options of the pa (PulseAudio) audio backend. >>>>> +# >>>>> +# @server: #optional PulseAudio server address >>>>> +# >>>>> +# @sink: #optional sink device name >>>>> +# >>>>> +# @source: #optional source device name >>>> >>>> Who picks the defaults, QEMU or PA? >>> >>> PA >> >> Is there a way to explicitly ask for the PA default? Something like >> source=default? > > Not really right now. The default is a NULL pointer (pulseaudio api > wise), so unless we add an arbitrary keyword (like default), it's not > possible to ask explicitly for the default. (But omitting them will > choose the default, of course.) Treating an empty string like NULL should get us a way to ask for the default, and a way to document the default concisely, like (default '') plus a suitable explanation what '' means. I'm not saying you should do that. I'm saying whatever you do, document what happens when an optional parameter is absent :) >>>>> +# >>>>> +# 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' } } >>>> >>>> Who picks the default? >>> >>> It defaults to "qemu.wav" >> >> Make it >> >> # @path: #optional path of the wav file to record (default 'qemu.wav') >> >>>>> + >>>>> + >>>>> +## >>>>> +# @AudiodevBackendOptions >>>>> +# >>>>> +# A discriminated record of audio backends. >>>>> +# >>>>> +# Since: 2.4 >>>>> +## >>>>> +{ 'union': 'AudiodevBackendOptions', >>>>> + 'data': { >>>>> + 'none': 'AudiodevNoOptions', >>>>> + 'alsa': 'AudiodevAlsaOptions', >>>>> + 'coreaudio': 'AudiodevNoOptions', >>>>> + 'dsound': 'AudiodevDsoundOptions', >>>>> + 'oss': 'AudiodevOssOptions', >>>>> + 'pa': 'AudiodevPaOptions', >>>>> + 'sdl': 'AudiodevNoOptions', >>>>> + 'spice': 'AudiodevNoOptions', >>>>> + '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 format to use when using fixed settings >>>> >>>> Are these guys used when @fixed-settings is off? >>> >>> No. >> >> If @fixed-settings, are the other three all required? If not, what are >> their defaults? > > No, they all have defaults: 44100 Hz, 2 channels and s16 format. Okay, this sort of explains why you have @fixed-settings. My first thought was that @fixed-settings is redundant, because we can have any of @frequency, @channels, @format imply fixed settings. Except that doesn't let you ask for the *default* fixed settings, as you have to specify at least one. What's the default for @fixed-settings? What if I specify frequency, channels or format together with explicit fixed-settings: false? > I > guess I should also document it... Yes, please. >>>>> +# >>>>> +# @buffer: #optional the buffer size (in microseconds) >>>> >>>> @buffer suggests this is a buffer, not a buffer length given as time >>>> span. @buffer-len? >>> >>> Ok. (It used to be called buffer-usecs before I changed everything to >>> microseconds.) >>> >>>> >>>>> +# >>>>> +# @buffer-count: #optional number of buffers >>>>> +# >>>>> +# Since: 2.4 >>>>> +## >>>>> +{ 'struct': 'AudiodevPerDirectionOptions', >>>>> + 'data': { >>>>> + '*fixed-settings': 'bool', >>>>> + '*frequency': 'int', >>>>> + '*channels': 'int', >>>>> + '*voices': 'int', >>>>> + '*format': 'AudioFormat', >>>>> + '*buffer': 'int', >>>>> + '*buffer-count': 'int' } } [...]
2015-06-17 15:37 keltezéssel, Markus Armbruster írta: > "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > >> 2015-06-17 13:48 keltezéssel, Markus Armbruster írta: >>> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >>> >>>> 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: >>>>> Copying Eric for additional QAPI schema expertise. >>>>> >>>>> My questions inline, pretty sure they show my ignorance. >>>>> >>>>> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes: > [...] >>>>>> +## >>>>>> +# @AudiodevPaOptions >>>>>> +# >>>>>> +# Options of the pa (PulseAudio) audio backend. >>>>>> +# >>>>>> +# @server: #optional PulseAudio server address >>>>>> +# >>>>>> +# @sink: #optional sink device name >>>>>> +# >>>>>> +# @source: #optional source device name >>>>> >>>>> Who picks the defaults, QEMU or PA? >>>> >>>> PA >>> >>> Is there a way to explicitly ask for the PA default? Something like >>> source=default? >> >> Not really right now. The default is a NULL pointer (pulseaudio api >> wise), so unless we add an arbitrary keyword (like default), it's not >> possible to ask explicitly for the default. (But omitting them will >> choose the default, of course.) > > Treating an empty string like NULL should get us a way to ask for the > default, and a way to document the default concisely, like (default '') > plus a suitable explanation what '' means. > > I'm not saying you should do that. I'm saying whatever you do, document > what happens when an optional parameter is absent :) > >>>>>> +# >>>>>> +# 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' } } >>>>> >>>>> Who picks the default? >>>> >>>> It defaults to "qemu.wav" >>> >>> Make it >>> >>> # @path: #optional path of the wav file to record (default 'qemu.wav') >>> >>>>>> + >>>>>> + >>>>>> +## >>>>>> +# @AudiodevBackendOptions >>>>>> +# >>>>>> +# A discriminated record of audio backends. >>>>>> +# >>>>>> +# Since: 2.4 >>>>>> +## >>>>>> +{ 'union': 'AudiodevBackendOptions', >>>>>> + 'data': { >>>>>> + 'none': 'AudiodevNoOptions', >>>>>> + 'alsa': 'AudiodevAlsaOptions', >>>>>> + 'coreaudio': 'AudiodevNoOptions', >>>>>> + 'dsound': 'AudiodevDsoundOptions', >>>>>> + 'oss': 'AudiodevOssOptions', >>>>>> + 'pa': 'AudiodevPaOptions', >>>>>> + 'sdl': 'AudiodevNoOptions', >>>>>> + 'spice': 'AudiodevNoOptions', >>>>>> + '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 format to use when using fixed settings >>>>> >>>>> Are these guys used when @fixed-settings is off? >>>> >>>> No. >>> >>> If @fixed-settings, are the other three all required? If not, what are >>> their defaults? >> >> No, they all have defaults: 44100 Hz, 2 channels and s16 format. > > Okay, this sort of explains why you have @fixed-settings. > > My first thought was that @fixed-settings is redundant, because we can > have any of @frequency, @channels, @format imply fixed settings. Except > that doesn't let you ask for the *default* fixed settings, as you have > to specify at least one. > > What's the default for @fixed-settings? It's on by default. > What if I specify frequency, channels or format together with explicit > fixed-settings: false? They will be ignored. The audio system currently work like this: when an audio frontend wants to open an output with some format (frequency, channels, format) it checks fixed-settings. If it's false, it will just open the stream with the frontend specified settings. If it's true, it'll convert it into the format specified by @frequency, @channels, @format, then pass this converted/recoded stream to the backend. > >> I >> guess I should also document it... > > Yes, please. > >>>>>> +# >>>>>> +# @buffer: #optional the buffer size (in microseconds) >>>>> >>>>> @buffer suggests this is a buffer, not a buffer length given as time >>>>> span. @buffer-len? >>>> >>>> Ok. (It used to be called buffer-usecs before I changed everything to >>>> microseconds.) >>>> >>>>> >>>>>> +# >>>>>> +# @buffer-count: #optional number of buffers >>>>>> +# >>>>>> +# Since: 2.4 >>>>>> +## >>>>>> +{ 'struct': 'AudiodevPerDirectionOptions', >>>>>> + 'data': { >>>>>> + '*fixed-settings': 'bool', >>>>>> + '*frequency': 'int', >>>>>> + '*channels': 'int', >>>>>> + '*voices': 'int', >>>>>> + '*format': 'AudioFormat', >>>>>> + '*buffer': 'int', >>>>>> + '*buffer-count': 'int' } } > [...] >
On 06/17/2015 05:48 AM, Markus Armbruster wrote: > "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > >> 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: >>> Copying Eric for additional QAPI schema expertise. >>> >>>> +{ 'struct': 'Audiodev', >>>> + 'data': { >>>> + '*id': 'str', >>>> + 'in': 'AudiodevPerDirectionOptions', >>>> + 'out': 'AudiodevPerDirectionOptions', >>>> + '*timer-period': 'int', >>>> + 'opts': 'AudiodevBackendOptions' } } >>> >>> Have you considered making this a flat union, similar ro >>> BlockdevOptions? >> >> Not really. If you qapi masters out there think it's better, then I >> will convert it. > > Related: discussion about flattening in review of PATCH 2. Indeed - I think a flat union makes for nicer command line structure, and fewer {} nesting in QMP structure. I still need to spend time on the overall thread, but wanted to chime in on this comment up front.
"Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > 2015-06-17 15:37 keltezéssel, Markus Armbruster írta: >> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >> >>> 2015-06-17 13:48 keltezéssel, Markus Armbruster írta: >>>> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >>>> >>>>> 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: [...] >>>>>>> +## >>>>>>> +# @AudiodevBackendOptions >>>>>>> +# >>>>>>> +# A discriminated record of audio backends. >>>>>>> +# >>>>>>> +# Since: 2.4 >>>>>>> +## >>>>>>> +{ 'union': 'AudiodevBackendOptions', >>>>>>> + 'data': { >>>>>>> + 'none': 'AudiodevNoOptions', >>>>>>> + 'alsa': 'AudiodevAlsaOptions', >>>>>>> + 'coreaudio': 'AudiodevNoOptions', >>>>>>> + 'dsound': 'AudiodevDsoundOptions', >>>>>>> + 'oss': 'AudiodevOssOptions', >>>>>>> + 'pa': 'AudiodevPaOptions', >>>>>>> + 'sdl': 'AudiodevNoOptions', >>>>>>> + 'spice': 'AudiodevNoOptions', >>>>>>> + '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 format to use when using fixed settings >>>>>> >>>>>> Are these guys used when @fixed-settings is off? >>>>> >>>>> No. >>>> >>>> If @fixed-settings, are the other three all required? If not, what are >>>> their defaults? >>> >>> No, they all have defaults: 44100 Hz, 2 channels and s16 format. >> >> Okay, this sort of explains why you have @fixed-settings. >> >> My first thought was that @fixed-settings is redundant, because we can >> have any of @frequency, @channels, @format imply fixed settings. Except >> that doesn't let you ask for the *default* fixed settings, as you have >> to specify at least one. >> >> What's the default for @fixed-settings? > > It's on by default. > >> What if I specify frequency, channels or format together with explicit >> fixed-settings: false? > > They will be ignored. > > The audio system currently work like this: when an audio frontend > wants to open an output with some format (frequency, channels, format) > it checks fixed-settings. If it's false, it will just open the stream > with the frontend specified settings. If it's true, it'll convert it > into the format specified by @frequency, @channels, @format, then pass > this converted/recoded stream to the backend. So user typically specifies either fixed-settings=off, or any combination of the other three (including none of them). Correct? We could reject the non-sensical combination of fixed-settings=off plus any of the other three instead of silently ignoring their values. Matter of taste, your choice. Whatever you do, make sure to document how these four work together. Thank you for educating me so patiently.
2015-06-17 18:06 keltezéssel, Markus Armbruster írta: > "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > >> 2015-06-17 15:37 keltezéssel, Markus Armbruster írta: >>> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >>> >>>> 2015-06-17 13:48 keltezéssel, Markus Armbruster írta: >>>>> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >>>>> >>>>>> 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: > [...] >>>>>>>> +## >>>>>>>> +# @AudiodevBackendOptions >>>>>>>> +# >>>>>>>> +# A discriminated record of audio backends. >>>>>>>> +# >>>>>>>> +# Since: 2.4 >>>>>>>> +## >>>>>>>> +{ 'union': 'AudiodevBackendOptions', >>>>>>>> + 'data': { >>>>>>>> + 'none': 'AudiodevNoOptions', >>>>>>>> + 'alsa': 'AudiodevAlsaOptions', >>>>>>>> + 'coreaudio': 'AudiodevNoOptions', >>>>>>>> + 'dsound': 'AudiodevDsoundOptions', >>>>>>>> + 'oss': 'AudiodevOssOptions', >>>>>>>> + 'pa': 'AudiodevPaOptions', >>>>>>>> + 'sdl': 'AudiodevNoOptions', >>>>>>>> + 'spice': 'AudiodevNoOptions', >>>>>>>> + '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 format to use when using fixed settings >>>>>>> >>>>>>> Are these guys used when @fixed-settings is off? >>>>>> >>>>>> No. >>>>> >>>>> If @fixed-settings, are the other three all required? If not, what are >>>>> their defaults? >>>> >>>> No, they all have defaults: 44100 Hz, 2 channels and s16 format. >>> >>> Okay, this sort of explains why you have @fixed-settings. >>> >>> My first thought was that @fixed-settings is redundant, because we can >>> have any of @frequency, @channels, @format imply fixed settings. Except >>> that doesn't let you ask for the *default* fixed settings, as you have >>> to specify at least one. >>> >>> What's the default for @fixed-settings? >> >> It's on by default. >> >>> What if I specify frequency, channels or format together with explicit >>> fixed-settings: false? >> >> They will be ignored. >> >> The audio system currently work like this: when an audio frontend >> wants to open an output with some format (frequency, channels, format) >> it checks fixed-settings. If it's false, it will just open the stream >> with the frontend specified settings. If it's true, it'll convert it >> into the format specified by @frequency, @channels, @format, then pass >> this converted/recoded stream to the backend. > > So user typically specifies either fixed-settings=off, or any > combination of the other three (including none of them). Correct? > > We could reject the non-sensical combination of fixed-settings=off plus > any of the other three instead of silently ignoring their values. > Matter of taste, your choice. > > Whatever you do, make sure to document how these four work together. > > Thank you for educating me so patiently. The audio backend currently works like that you can pass any non-sensical values to it, like negative frequency, or 'kdp' count of channels, it will silently fallback to some default value, or just fail, but qemu will continue to run. We can make the new config more strict (and we should, I think), so if you have any idea where should we be more strict (without creating a backward compatibility headache), don't hesitate to point it out.
"Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: > 2015-06-17 18:06 keltezéssel, Markus Armbruster írta: >> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >> >>> 2015-06-17 15:37 keltezéssel, Markus Armbruster írta: >>>> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >>>> >>>>> 2015-06-17 13:48 keltezéssel, Markus Armbruster írta: >>>>>> "Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes: >>>>>> >>>>>>> 2015-06-17 09:46 keltezéssel, Markus Armbruster írta: >> [...] >>>>>>>>> +## >>>>>>>>> +# @AudiodevBackendOptions >>>>>>>>> +# >>>>>>>>> +# A discriminated record of audio backends. >>>>>>>>> +# >>>>>>>>> +# Since: 2.4 >>>>>>>>> +## >>>>>>>>> +{ 'union': 'AudiodevBackendOptions', >>>>>>>>> + 'data': { >>>>>>>>> + 'none': 'AudiodevNoOptions', >>>>>>>>> + 'alsa': 'AudiodevAlsaOptions', >>>>>>>>> + 'coreaudio': 'AudiodevNoOptions', >>>>>>>>> + 'dsound': 'AudiodevDsoundOptions', >>>>>>>>> + 'oss': 'AudiodevOssOptions', >>>>>>>>> + 'pa': 'AudiodevPaOptions', >>>>>>>>> + 'sdl': 'AudiodevNoOptions', >>>>>>>>> + 'spice': 'AudiodevNoOptions', >>>>>>>>> + '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 format to use when using fixed settings >>>>>>>> >>>>>>>> Are these guys used when @fixed-settings is off? >>>>>>> >>>>>>> No. >>>>>> >>>>>> If @fixed-settings, are the other three all required? If not, what are >>>>>> their defaults? >>>>> >>>>> No, they all have defaults: 44100 Hz, 2 channels and s16 format. >>>> >>>> Okay, this sort of explains why you have @fixed-settings. >>>> >>>> My first thought was that @fixed-settings is redundant, because we can >>>> have any of @frequency, @channels, @format imply fixed settings. Except >>>> that doesn't let you ask for the *default* fixed settings, as you have >>>> to specify at least one. >>>> >>>> What's the default for @fixed-settings? >>> >>> It's on by default. >>> >>>> What if I specify frequency, channels or format together with explicit >>>> fixed-settings: false? >>> >>> They will be ignored. >>> >>> The audio system currently work like this: when an audio frontend >>> wants to open an output with some format (frequency, channels, format) >>> it checks fixed-settings. If it's false, it will just open the stream >>> with the frontend specified settings. If it's true, it'll convert it >>> into the format specified by @frequency, @channels, @format, then pass >>> this converted/recoded stream to the backend. >> >> So user typically specifies either fixed-settings=off, or any >> combination of the other three (including none of them). Correct? >> >> We could reject the non-sensical combination of fixed-settings=off plus >> any of the other three instead of silently ignoring their values. >> Matter of taste, your choice. >> >> Whatever you do, make sure to document how these four work together. >> >> Thank you for educating me so patiently. > > The audio backend currently works like that you can pass any > non-sensical values to it, like negative frequency, or 'kdp' count of > channels, it will silently fallback to some default value, or just > fail, but qemu will continue to run. We can make the new config more > strict (and we should, I think), so if you have any idea where should > we be more strict (without creating a backward compatibility > headache), don't hesitate to point it out. When a sensible default value exists, making the parameter optional is usually a good idea. We should refuse to start on non-sensical configuration, not silently substitute defaults or disable the affected component (here: audio backend). I can't give you more specific guidance, because I'm an audio ignoramus :)
diff --git a/Makefile b/Makefile index 3f97904..ac566fa 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 106008c..e751ea3 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..2851689 --- /dev/null +++ b/qapi/audio.json @@ -0,0 +1,223 @@ +# -*- mode: python -*- +# +# Copyright (C) 2015 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. + +## +# @AudiodevNoOptions +# +# The none, coreaudio, sdl and spice audio backend have no options. +# +# Since: 2.4 +## +{ 'struct': 'AudiodevNoOptions', + '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: options of the capture stream +# +# @out: 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: #optional add extra latency to playback (in microseconds) +# +# Since: 2.4 +## +{ 'struct': 'AudiodevDsoundOptions', + 'data': { + '*latency': '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: options of the capture stream +# +# @out: options of the playback stream +# +# @mmap: #optional try using memory mapped access +# +# @exclusive: #optional open device in exclusive mode (vmix won't 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': 'AudiodevNoOptions', + 'alsa': 'AudiodevAlsaOptions', + 'coreaudio': 'AudiodevNoOptions', + 'dsound': 'AudiodevDsoundOptions', + 'oss': 'AudiodevOssOptions', + 'pa': 'AudiodevPaOptions', + 'sdl': 'AudiodevNoOptions', + 'spice': 'AudiodevNoOptions', + '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 format to use when using fixed settings +# +# @buffer: #optional the buffer size (in microseconds) +# +# @buffer-count: #optional number of buffers +# +# Since: 2.4 +## +{ 'struct': 'AudiodevPerDirectionOptions', + 'data': { + '*fixed-settings': 'bool', + '*frequency': 'int', + '*channels': 'int', + '*voices': 'int', + '*format': 'AudioFormat', + '*buffer': 'int', + '*buffer-count': 'int' } } + +## +# @Audiodev +# +# Captures the configuration of an audio backend. +# +# @id: identifier of the backend +# +# @in: options of the capture stream +# +# @out: options of the playback stream +# +# @timer-period: #optional timer period (in microseconds, 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' } }
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 v1 patch: * every time-related field now take usecs (and removed -usecs, -millis suffixes) * fixed inconsisten optional marking, language issues 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 | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 qapi/audio.json