Re: [Qemu-devel] [PATCH v2 1/6] qapi: qapi for audio backends

[Markus Armbruster]

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

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