Re: [Qemu-devel] [PATCH v2 6/6] audio: -audiodev command line option

[Markus Armbruster]

"Kővágó Zoltán" <address@hidden> writes:

> 2015-06-17 10:13 keltezéssel, Markus Armbruster írta:
>> "Kővágó, Zoltán" <address@hidden> 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?

[...]