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

[Volker Rümelin]

Am 21.12.22 um 12:03 schrieb Christian Schoenebeck:
> 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?

The hda device and sb16. When I wrote this patch two months ago ac97 
also had occasional dropouts, but at the moment ac97 works without issues.

> > 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?

It seems this would be only needed for the ALSA backend. All other 
backends with the exception of OSS are fine with a 10ms period, and the 
ALSA audio backend also uses 10ms with -audiodev 
alsa,out.try-poll=off,in.try-poll=off.

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

Quite often it's longer than 23ms. For the rest of the audio backends a 
timer period of 10ms was selected as a good trade off between CPU load 
and audio dropouts. But you are right, this patch increases the CPU load.

On my system the CPU load is increased by 0.9%. This was measured with a 
Linux guest using rhythmbox for audio playback. The guest was configured 
to use pulseaudio as sound server. The measurement was done with top -b 
-d 10 -n 14 over a period of two minutes. The first and last measurement 
was dropped. The average QEMU CPU load was 10.7% with and 9.8% without 
this patch.

I would prefer a system with a 0.9% increased CPU load where audio just 
works over a system where you have to fine tune audio parameters.

> > 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.

I can't see how this can be improved. The buffer length is unchanged. I 
just evaluated the constant expression to have a time in microseconds 
like the rest of the audio backends. And libasound tells me to use 
5804us for the period length which I rounded up to 5805us. I would 
prefer a period length of 5000us.

./qemu-system-x86_64 -device ich9-intel-hda -device 
hda-duplex,audiodev=audio0 -audiodev 
alsa,id=audio0,out.period-length=5000,out.dev=PCH,,0
alsa: Requested period time 5000 was rejected, using 5804

> >       }
> >   
> >       /*