Re: [Qemu-devel] [PATCH 1/3] fix: buffer_length is ignored

[Martin Schrodt]

On 3/15/19 9:01 AM, Gerd Hoffmann wrote:
> On Fri, Mar 15, 2019 at 08:49:06AM +0100, Martin Schrodt wrote:
>> Hi,
>>
>> On 3/15/19 8:43 AM, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>>> -        qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 46440);
>>>> +        qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 
>>>> ppdo->buffer_length);
>>>
>>> I'd just use
>>>
>>>     ppdo->has_buffer_length ? ppdo->buffer_length : dev->timer_period * 4
>>>
>>> here.
>>>
>>> cheers,
>>>   Gerd
>>>
>>>
>>
>> I made sure the value is present via the new function
>>
>> static int qpa_validate_per_direction_opts()
>>
>> That way, I can group the setting of all defaults in a single place,
>> which is cleaner from my perspective.
> 
> But you also set has_buffer_length, so we loose the information whenever
> the user has specified a buffer length on the command line or not.
> 
> If you want bundle default calculation in one place I'd suggest adding a
> get_buffer_length() function where you can place the
> "if (has_buffer_length) then { ... } else { ... }" logic.  A simliar
> function for the latency can be placed next to it.
> 

I did so because Zoltans code in audio.c does it the same way.
See audio.c, functions audio_validate_opts() and
audio_validate_per_direction_opts().

I mean I can do a separate function for each, but is it really worth it?