From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50652) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z583P-0004Mf-RB for qemu-devel@nongnu.org; Wed, 17 Jun 2015 03:46:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z583K-0004XC-F5 for qemu-devel@nongnu.org; Wed, 17 Jun 2015 03:46:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46806) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z583K-0004VU-7j for qemu-devel@nongnu.org; Wed, 17 Jun 2015 03:46:30 -0400 From: Markus Armbruster References: Date: Wed, 17 Jun 2015 09:46:26 +0200 In-Reply-To: (=?utf-8?B?IkvFkXbDoWfDsywgWm9sdMOhbiIncw==?= message of "Tue, 16 Jun 2015 14:49:04 +0200") Message-ID: <87si9qg4kt.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 1/6] qapi: qapi for audio backends List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?B?S8WRdsOhZ8OzLCBab2x0w6Fu?= Cc: Gerd Hoffmann , qemu-devel@nongnu.org, Michael Roth Copying Eric for additional QAPI schema expertise. My questions inline, pretty sure they show my ignorance. "K=C5=91v=C3=A1g=C3=B3, Zolt=C3=A1n" writes: > This patch adds structures into qapi to replace the existing configuration > structures used by audio backends currently. This qapi will be the base o= f the > -audiodev command line parameter (that replaces the old environment varia= bles > 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 buffe= r 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 us= er > friendly imho > * moved buffer settings into the global setting area (so it's the same fo= r all > backends that support it. Backends that can't change buffer size will s= imply > 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 b= ackends > currently ignore it) > > Signed-off-by: K=C5=91v=C3=A1g=C3=B3, Zolt=C3=A1n > > --- > > Changes from v1 patch: > * every time-related field now take usecs (and removed -usecs, -millis su= ffixes) > * 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/msg02= 427.html > * removed verbose, debug options. Backends should use trace events instea= d. > * 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 AudiodevPerDirection= Options > 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 $@") >=20=20 > qapi-modules =3D $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.js= on \ > - $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.j= son \ > - $(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.j= son >=20=20 > 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' } >=20=20 > +# QAPI audio definitions > +{ 'include': 'qapi/audio.json' } > + > # QAPI block definitions > { 'include': 'qapi/block.json' } >=20=20 > 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=C3=A1n K=C5=91v=C3=A1g=C3=B3 > +# > +# This work is licensed under the terms of the GNU GPL, version 2 or lat= er. > +# 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 record= ing. > +# > +# @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 star= ts > +# > +# 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 recordi= ng. > +# > +# @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 reco= rding. > +# > +# @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.