From: Markus Armbruster <armbru@redhat.com>
To: "Kővágó Zoltán" <dirty.ice.hu@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
qemu-devel@nongnu.org, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 6/6] audio: -audiodev command line option
Date: Wed, 17 Jun 2015 14:27:47 +0200 [thread overview]
Message-ID: <87wpz24j0c.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <5581579B.2020402@gmail.com> ("Kővágó Zoltán"'s message of "Wed, 17 Jun 2015 13:18:51 +0200")
"Kővágó Zoltán" <dirty.ice.hu@gmail.com> writes:
> 2015-06-17 10:13 keltezéssel, Markus Armbruster írta:
>> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:
>>
>>> This patch adds an -audiodev command line option, and deprecates the QEMU_*
>>> environment variables for audio backend configuration. It's syntax
>>> is similar to
>>> existing options (-netdev, -device, etc):
>>> -audiodev driver_name,property=value,...
>>
>> Sounds really good.
>>
>> Please wrap your commit message lines a bit earlier, around column 70.
>>
>>> Audio drivers now get an Audiodev * as config paramters, instead of
>>> the global
>>> audio_option structs. There is some code in audio/audio_legacy.c
>>> that converts
>>> the old environment variables to audiodev options (this way
>>> backends do not have
>>> to worry about legacy options, also print out them with -audio-help, to ease
>>> migrating to -audiodev).
>>
>> The parenthesis isn't as clear as the rest of your message, probably
>> because it deals with two separate things. Suggest to move out the bit
>> about help into its own paragraph.
>>
>>> Although now it's possible to specify multiple -audiodev options on command
>>> line, multiple audio backends are not supported yet.
>>
>> What happens when I specify multiple -audiodev?
>
> You get an error and qemu terminates.
Unlikely to create backward compatibility trouble. Works for me.
>> How should the command line look like when multiple audio backends are
>> supported?
>
> There's an id property of audiodev, so you can identify them:
> -audiodev alsa,id=foo,... -audiodev pa,id=bar,...
> and audio devices should get an extra parameter, like audiodev or
> something like that:
> -device usb-audio,audiodev=foo -device usb-audio,audiodev=bar
> And you have two cards, one connected to the alsa device and the other
> connected to pulseaudio.
Good, because it's consistent with how we connect other kinds of
frontends/backends.
Multiple audio frontends (device models) can connect to the same audio
backend, can't they?
Is backend property "id" mandatory? Hmm, judging from PATCH 1 it isn't.
It generally is for other kinds of backends, e.g. -netdev, -chardev and
(in the future) -blockdev. It's optional with -drive only because
-drive is crazy.
>> Do we have a clear backward-compatible path from here to there?
>
> Currently if you specify an -audiodev option, the environment
> variables are completely ignored, and it will create an audio backend
> using the specified options. If you do not provide an -audiodev, it
> will initialize the audio subsystem using the old environment
> variables when you add the first sound card (so no -audiodev and no
> sound device means no audio subsystem, just like the old times).
Should we document this? Where?
> About multiple backends: if the user does not specify the id of the
> backend when creating the sound card, just use the first -audiodev
> specified on the command line (or the legacy config, if there's no
> -audiodev). This way we stay backward-compatible (there won't be
> multiple -audiodevs in legacy configs).
Old way (before your patch): there is only one audio backend, and it's
configuration comes from a bunch of environment variables.
New way (we're not there yet): you can have multiple audio backends, and
you connect frontends to backends using backend ID as usual, i.e. id=ID
on the backend, audiodev=ID on the frontend.
Required backward compatibility: the old way still works, i.e. when
there's no new-way backend, and no frontend has an audiodev=..., then we
create a backend configured from the environment.
Anything beyond required backward compatibility should be carefully
judged on its UI merits.
If I understand your description correctly, the plan is to default a
frontend's audiodev to the first backend, and if there is none, create
one configured from the environment. That's definitely beyond required.
Is it a good user interface?
[...]
next prev parent reply other threads:[~2015-06-17 12:27 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 12:49 [Qemu-devel] [PATCH v2 0/6] -audiodev option Kővágó, Zoltán
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 1/6] qapi: qapi for audio backends Kővágó, Zoltán
2015-06-17 7:46 ` Markus Armbruster
2015-06-17 10:54 ` Kővágó Zoltán
2015-06-17 11:48 ` Markus Armbruster
2015-06-17 12:07 ` Kővágó Zoltán
2015-06-17 13:37 ` Markus Armbruster
2015-06-17 13:53 ` Kővágó Zoltán
2015-06-17 16:06 ` Markus Armbruster
2015-06-18 0:21 ` Kővágó Zoltán
2015-06-18 8:51 ` Markus Armbruster
2015-06-17 15:50 ` Eric Blake
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 2/6] qapi: support nested structs in OptsVisitor Kővágó, Zoltán
2015-06-17 7:50 ` Markus Armbruster
2015-06-17 8:41 ` Gerd Hoffmann
2015-06-17 11:01 ` Kővágó Zoltán
2015-06-17 11:50 ` Markus Armbruster
2015-06-17 15:47 ` Eric Blake
2015-06-17 11:18 ` Markus Armbruster
2015-06-17 12:11 ` Kővágó Zoltán
2015-06-17 13:41 ` Markus Armbruster
2015-06-17 14:02 ` Kővágó Zoltán
2015-06-17 16:10 ` Markus Armbruster
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 3/6] opts: do not print separator before first item in qemu_opts_print Kővágó, Zoltán
2015-06-17 7:53 ` Markus Armbruster
2015-06-17 9:02 ` Kevin Wolf
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 4/6] qapi: AllocVisitor Kővágó, Zoltán
2015-06-17 7:56 ` Markus Armbruster
2015-06-17 12:01 ` Kővágó Zoltán
2015-06-17 13:42 ` Markus Armbruster
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 5/6] audio: use qapi AudioFormat instead of audfmt_e Kővágó, Zoltán
2015-06-17 8:01 ` Markus Armbruster
2015-06-17 11:05 ` Kővágó Zoltán
2015-06-17 11:51 ` Markus Armbruster
2015-06-17 16:01 ` Eric Blake
2015-06-16 12:49 ` [Qemu-devel] [PATCH v2 6/6] audio: -audiodev command line option Kővágó, Zoltán
2015-06-17 8:13 ` Markus Armbruster
2015-06-17 11:18 ` Kővágó Zoltán
2015-06-17 12:27 ` Markus Armbruster [this message]
2015-06-17 13:25 ` Kővágó Zoltán
2015-06-17 16:13 ` Markus Armbruster
2015-06-18 6:54 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wpz24j0c.fsf@blackfin.pond.sub.org \
--to=armbru@redhat.com \
--cc=dirty.ice.hu@gmail.com \
--cc=kraxel@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.