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

[Volker Rümelin]

Am 23.01.23 um 08:43 schrieb Philippe Mathieu-Daudé:
> On 21/1/23 10:47, 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).
> >
> > 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.
> >
> > 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
> >
> > Acked-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > 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;
>
> Please use DIV_ROUND_UP():
>
>     DIV_ROUND_UP(1000000ul << 8, 44100);
>
> Or
>
>     DIV_ROUND_UP(512 * 1000000ul, 44100);

Hi,

the corresponding -audiodev alsa command line parameters 
out.buffer-length, in.buffer-length, out.period-length and 
in.period-lenght are specified in microseconds. I prefer that the 
default values use the same unit. I believe that specifying buffer 
lengths in microseconds makes more sense than specifying buffer lengths 
in audio frames.

With best regards,
Volker

> >       }
> >       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;
>
> Ditto:
>
>     DIV_ROUND_UP(1000000ul << 12, 44100);
>
> >       }
> >         /*