On Monday, December 26, 2022 4:08:37 PM CET Volker Rümelin wrote:
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.
OK, but all it would need was adjusting dev->timer_period appropriately either
in audio_validate_opts() [audio/audio.c, line 2126] to handle it generalized
or at the end of alsa_audio_init() [audio/alsaaudio.c, line 944] if
specifically for ALSA only, no?
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.
Probably nitpicking as the preceding comment indicates the numbers, but maybe
simply like this?
dev->u.alsa.out->period_length = ceil(256.0 * 1000000.0 / 44100.0);
...
dev->u.alsa.out->buffer_length = ceil(4096.0 * 1000000.0 / 44100.0);
I mean these are number literals passed to a built-in function, so the
compiler should automatically evaluate this constant expression at compile
time, so it should end up in the binary with the same constant value as you
did directly in code, at least if optimization was turned on.