diff mbox series

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

Message ID 20230121094735.11644-10-vr_qemu@t-online.de
State New
Headers show
Series audio: more improvements | expand

Commit Message

Volker Rümelin Jan. 21, 2023, 9:47 a.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Jan. 23, 2023, 7:43 a.m. UTC | #1
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);

>       }
>       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);

>       }
>   
>       /*
Volker Rümelin Jan. 24, 2023, 7:34 a.m. UTC | #2
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);
>
>>       }
>>         /*
>
diff mbox series

Patch

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;
     }
 
     /*