Re: [PATCH v4 15/24] audio: add mixing-engine option (documentation)

[Markus Armbruster]

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

> On 2019-10-01 08:23, Markus Armbruster wrote:
>> "Zoltán Kővágó" <address@hidden> writes:
>>
>>> On 2019-09-25 11:49, Markus Armbruster wrote:
>>>> "Zoltán Kővágó" <address@hidden> writes:
>>>>
>>>>> On 2019-09-23 15:08, Markus Armbruster wrote:
>>>>>> "Kővágó, Zoltán" <address@hidden> writes:
>>>>>>
>>>>>>> This will allow us to disable mixeng when we use a decent backend.
>>>>>>>
>>>>>>> Disabling mixeng have a few advantages:
>>>>>>> * we no longer convert the audio output from one format to another, when
>>>>>>>      the underlying audio system would just convert it to a third 
>>>>>>> format.
>>>>>>>      We no longer convert, only the underlying system, when needed.
>>>>>>> * the underlying system probably has better resampling and sample format
>>>>>>>      converting methods anyway...
>>>>>>> * we may support formats that the mixeng currently does not support (S24
>>>>>>>      or float samples, more than two channels)
>>>>>>> * when using an audio server (like pulseaudio) different sound card
>>>>>>>      outputs will show up as separate streams, even if we use only one
>>>>>>>      backend
>>>>>>>
>>>>>>> Disadvantages:
>>>>>>> * audio capturing no longer works (wavcapture, and vnc audio extension)
>>>>>>> * some backends only support a single playback stream or very picky
>>>>>>>      about the audio format.  In this case we can't disable mixeng.
>>>>>>>
>>>>>>> However mixeng is not removed, only made optional, so this shouldn't be
>>>>>>> a big concern.
>>>>>>>
>>>>>>> Signed-off-by: Kővágó, Zoltán <address@hidden>
>>>>>>> ---
>>>>>>>
>>>>>>> Notes:
>>>>>>>        Changes from v1:
>>>>>>>             * renamed mixeng to mixing-engine
>>>>>>>
>>>>>>>     qapi/audio.json | 5 +++++
>>>>>>>     qemu-options.hx | 6 ++++++
>>>>>>>     2 files changed, 11 insertions(+)
>>>>>>>
>>>>>>> diff --git a/qapi/audio.json b/qapi/audio.json
>>>>>>> index 9fefdf5186..0535eff794 100644
>>>>>>> --- a/qapi/audio.json
>>>>>>> +++ b/qapi/audio.json
>>>>>>> @@ -11,6 +11,10 @@
>>>>>>>     # General audio backend options that are used for both playback and
>>>>>>>     # recording.
>>>>>>>     #
>>>>>>> +# @mixing-engine: use QEMU's mixing engine to mix all streams inside 
>>>>>>> QEMU. When
>>>>>>> +#                 set to off, fixed-settings must be also off. Not 
>>>>>>> every backend
>>>>>>> +#                 compatible with the off setting (default on, since 
>>>>>>> 4.2)
>>>>>>> +#
>>>>>>
>>>>>> Last sentence no verb.
>>>>>>
>>>>>> Which backends are compatible?
>>>>>
>>>>> Actually that's a simplification, it depends on a few things.  When
>>>>> mixeng is off, qemu will try to use the same format as the emulated
>>>>> sound card, and if the backend doesn't support that format, it won't
>>>>> work (no audio).  Also attaching multiple sound cards to the same
>>>>> audiodev might not work, if the backend doesn't support multiple
>>>>> playback streams.  If you use pulseaudio, it'll work without problems,
>>>>> if you use alsa, it depends on your device.  If you use a hw: device
>>>>> directly, you'll likely only be able to use one emulated sound card
>>>>> with a few selected audio formats.  If you use dmix: (and plug), alsa
>>>>> will handle the conversion and mixing, so it will work no matter what
>>>>> format the emulated sound card uses.  With OSS the situation is
>>>>> probably similar, it depends on the kernel/hw what works and what not.
>>>>> wav and spice certainly doesn't support multiple streams.  I'm not
>>>>> completely sure about the other backends right now, but I think dsound
>>>>> and coreaudio can handle the necessary sample format conversions and
>>>>> mixing.
>>>>>
>>>>>> What happens when you try the off setting with incompatible backends?
>>>>> See above.
>>>>
>>>> What happens *exactly*?
>>>>
>>>> I'm asking because I'm concerned about the user experience.  When a user
>>>> asks for a combination of things QEMU can't provide, such as mixeng off
>>>> with an incompatible backend, QEMU should fail with a suitable error
>>>> message.  Does it?
>>>
>>> Error handling is not the best in the audio subsystem, if something
>>> fails it generally just prints a warning to the console and continues,
>>> and something will happen...  For example, this is what happens when I
>>> try to open one hw device twice. I ran qemu with:
>>>
>>> -audiodev
>>> alsa,id=foo,in.dev=hw:1,,0,out.mixing-engine=off,out.dev=hw:1,,0
>>> -device piix4-usb-uhci -device usb-audio,audiodev=foo -device
>>> AC97,audiodev=foo
>>>
>>> When the guest tried to initialize the AC97 card, I got an error:
>>>
>>> alsa: Could not initialize DAC
>>> alsa: Failed to open `hw:1,0':
>>> alsa: Reason: Device or resource busy
>>>
>>> And it just continued. And the sound worked, but with wrong sample
>>> rate (AC97 wants 44100 Hz, but USB audio previously opened the alsa
>>> device with 48000 Hz).  I'll fix this bug in the next revision,
>>> audio_pcm_hw_add_* shouldn't fall back to other HWs without mixeng.
>>> But even with that, the result will be that one emulated sound card
>>> will work and the other won't.
>>>
>>> It's not ideal, but fixing it would require a lot of effort.  Right
>>> now, if you specify an invalid audiodev for alsa (even with mixeng),
>>> it'll just print an error to the console and continue without audio.
>>
>> Should we document this general error handling deficiency somehow?
>
> It could be useful to mention it, especially for people who write
> script and expect qemu to fail if they specify an invalid parameter.
> I'm not sure how in-depth this explanation should be however.

Common issue when writing documentation.

One way to tackle it: start unspecific, then add specifics until it
feels useful.

The other direction can also work: write up everything, then cut until
it feels useful.  Makes sense mostly when you need to write up
everything anyway, say for a developer audience, or even just for
yourself.  Nothing promotes clear thinking like writing does[*].

>>>> Sometimes rejecting non-working configurations is impractical.  Is it
>>>> here?
>>>
>>> I think it is.  It depends on the backend, its settings, the frontend
>>> (emulated sound card), and how the guest uses it.  We currently don't
>>> know what formats does a backend support, what formats can a frontend
>>> produce, and even if we would know that, just because a frontend can
>>> produce a format that the backend doesn't understand doesn't mean that
>>> it will actually do it.  For example, right now with this patch series
>>> applied, usb-audio can produce 7.1 audio.  If we want to be strict, it
>>> means we can only use it with backends that support at least 8
>>> channels, even if the user only wants to use stereo audio.
>>>
>>>> If yes, we should call out the problematic configurations in
>>>> documentation.
>>>
>>> I think we should rather list known working configurations, and leave
>>> the others as "try at your own risk" because there's too many things
>>> that can go wrong.  (pulseaudio will work, alsa with dmix too.  Need
>>> to check coreaudio, dsound and oss.  spice and wavcapture won't work.)
>>
>> Far from ideal, but better than nothing.
>>
>> Possibly naive idea: what about automatically falling back to mixeng on
>> when mixeng off doesn't work?  Requires detecting "doesn't work", which
>> I understand just isn't there.  Any other reasons why this couldn't be
>> done?  Way out of scope for this series, of course.
>
> There are two cases when mixeng can help: a) when the frontend
> requires a format not supported by the backend, and b) when multiple
> frontends wants to use the same backend, but the backend doesn't
> support multiple channels.  This fallback logic would help with a) but
> it would be much harder to solve b) (I'm not saying it's impossible,
> but it would require us to convert the already running stream back to
> mixeng).  However, since the goal of this setting is to get rid of
> limitations of mixeng, currently you only want to turn it off if when
> you want to use 5.1 or 7.1 audio, where falling back to mixeng is not
> an option.  (The other use case was when using pulseaudio, you wanted
> different pa streams per emulated sound card, but since you can have
> multiple -audiodevs now, it's no longer necessary to disable mixeng.)

"Currently you only want to turn it off if when you want to use 5.1 or
7.1 audio" is useful advice.  I figure it should be worked into
documentation if it's not already there.

Suggest to record the remainder of your answer for posterity by working
it into the commit message, along with this thought:

> The ideal solution is probably porting as much as possible to
> gstreamer, but this is even more out of scope:
> https://wiki.qemu.org/Internships/ProjectIdeas/AudioGStreamer

Thanks!


[*] "If you're thinking without writing, you only think you're
thinking." Leslie Lamport