Re: [PATCH 10/11] alsaaudio: change default playback settings

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Sunday, December 18, 2022 6:15:38 PM CET Volker Rümelin wrote:
> The currently used default playback settings in the ALSA audio
> backend are a bit unfortunate. With a few emulated audio devices,
> audio playback does not work properly. Here is a short part of
> the debug log while audio is playing (elapsed time in seconds).

Which emulated devices are these?

> audio: Elapsed since last alsa run (running): 0.046244
> audio: Elapsed since last alsa run (running): 0.023137
> audio: Elapsed since last alsa run (running): 0.023170
> audio: Elapsed since last alsa run (running): 0.023650
> audio: Elapsed since last alsa run (running): 0.060802
> audio: Elapsed since last alsa run (running): 0.031931
> 
> For some audio devices the time of more than 23ms between updates
> is too long.
> 
> Set the period time to 5.8ms so that the maximum time between
> two updates typically does not exceed 11ms. This roughly matches
> the 10ms period time when doing playback with the audio timer.
> After this patch the debug log looks like this.

And what about dynamically adapting that value instead of reducing period time
for everyone by default?

23ms is usually a good trade off between low latency, CPU load and potential
for audio dropouts.

> audio: Elapsed since last alsa run (running): 0.011919
> audio: Elapsed since last alsa run (running): 0.005788
> audio: Elapsed since last alsa run (running): 0.005995
> audio: Elapsed since last alsa run (running): 0.011069
> audio: Elapsed since last alsa run (running): 0.005901
> audio: Elapsed since last alsa run (running): 0.006084
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>  audio/alsaaudio.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
> index 5f50dfa0bf..0cc982e61f 100644
> --- a/audio/alsaaudio.c
> +++ b/audio/alsaaudio.c
> @@ -913,17 +913,14 @@ static void *alsa_audio_init(Audiodev *dev)
>      alsa_init_per_direction(aopts->in);
>      alsa_init_per_direction(aopts->out);
>  
> -    /*
> -     * need to define them, as otherwise alsa produces no sound
> -     * doesn't set has_* so alsa_open can identify it wasn't set by the user
> -     */
> +    /* don't set has_* so alsa_open can identify it wasn't set by the user */
>      if (!dev->u.alsa.out->has_period_length) {
> -        /* 1024 frames assuming 44100Hz */
> -        dev->u.alsa.out->period_length = 1024 * 1000000 / 44100;
> +        /* 256 frames assuming 44100Hz */
> +        dev->u.alsa.out->period_length = 5805;
>      }
>      if (!dev->u.alsa.out->has_buffer_length) {
>          /* 4096 frames assuming 44100Hz */
> -        dev->u.alsa.out->buffer_length = 4096ll * 1000000 / 44100;
> +        dev->u.alsa.out->buffer_length = 92880;

Not a big fan of magic numbers, as it makes code less readable.

>      }
>  
>      /*
>