diff mbox series

Huge avail value caused by inconsistency between snd_pcm_multi_status() and snd_pcm_multi_hwptr_update()

Message ID 20220321112146.07311F800FD@alsa1.perex.cz
State New
Headers show
Series Huge avail value caused by inconsistency between snd_pcm_multi_status() and snd_pcm_multi_hwptr_update() | expand

Commit Message

GitHub issues - opened March 21, 2022, 11:21 a.m. UTC
alsa-project/alsa-lib issue #217 was opened from infmagic2047:

## Symptom
Using the following configuration with plugin `type multi`, `snd_pcm_avail()` may return huge values.
```
pcm.combined {
    type multi;
    slaves.a.pcm "dmix:CARD=0";
    slaves.a.channels 2;
    slaves.b.pcm "dmix:CARD=1";
    slaves.b.channels 2;
    bindings.0.slave a;
    bindings.0.channel 0;
    bindings.1.slave a;
    bindings.1.channel 1;
    bindings.2.slave b;
    bindings.2.channel 0;
    bindings.3.slave b;
    bindings.3.channel 1;
}
```
The problem still occurs even when I set both slaves to `dmix:CARD=0`, so use this to reproduce the issue if you don't have multiple output devices.

It can be verified with `aplay --test-position -D plug:combined /dev/urandom`. The output looks like this:
```
Playing raw data '/dev/urandom' : Unsigned 8 bit, Rate 8000 Hz, Mono
Suspicious buffer position (1 total): avail = 765611936652984468, delay = 2582, buffer = 2730
Suspicious buffer position (2 total): avail = 762797186885877738, delay = 2752, buffer = 2730
Suspicious buffer position (3 total): avail = 759982437118771178, delay = 2752, buffer = 2730
Suspicious buffer position (4 total): avail = 757167687351664448, delay = 2922, buffer = 2730
Suspicious buffer position (5 total): avail = 754352937584557888, delay = 2922, buffer = 2730
Suspicious buffer position (6 total): avail = 751538187817451158, delay = 3092, buffer = 2730
Suspicious buffer position (7 total): avail = 748723438050344598, delay = 3092, buffer = 2730
Suspicious buffer position (8 total): avail = 745908688283237868, delay = 3262, buffer = 2730
Suspicious buffer position (9 total): avail = 743093938516131308, delay = 3262, buffer = 2730
Suspicious buffer position (10 total): avail = 740279188749024578, delay = 3432, buffer = 2730
```

I'm using alsa-lib-1.2.6.1 on Gentoo Linux, kernel version is 5.16.15.

## Cause
For `type multi` plugins, `snd_pcm_multi_status()` returns the hwptr of its master_slave, while `snd_pcm_multi_hwptr_update()` sets the hwptr based on all slaves. They are inconsistent, and can lead to problems for plugins in upper levels.

In this particular case, there is a `type rate` plugin on top of `type multi`. It requires the underlying hwptr to be monotonic, and may either read the hwptr directly (in `snd_pcm_rate_sync_hwptr()`) or use the value from status (in `snd_pcm_rate_status()`). The inconsistency can cause hwptr to decrease, breaking the code of `type rate`.

## Attempted solution
I'm able to fix the problem with the following patch, and haven't run into any problems. I'm unfamiliar with the code though, so the patch may be inefficient or incorrect in some way.
```
```

Issue URL     : https://github.com/alsa-project/alsa-lib/issues/217
Repository URL: https://github.com/alsa-project/alsa-lib
diff mbox series

Patch

diff -uNar --no-dereference alsa-lib-1.2.6.1/src/pcm/pcm_multi.c alsa-lib-1.2.6.1-new/src/pcm/pcm_multi.c
--- alsa-lib-1.2.6.1/src/pcm/pcm_multi.c	2021-12-09 21:17:59.000000000 +0800
+++ alsa-lib-1.2.6.1-new/src/pcm/pcm_multi.c	2022-03-21 16:20:32.096036522 +0800
@@ -388,11 +388,21 @@ 
 	return 0;
 }
 
+static snd_pcm_sframes_t snd_pcm_multi_avail_update(snd_pcm_t *pcm);
 static int snd_pcm_multi_status(snd_pcm_t *pcm, snd_pcm_status_t *status)
 {
 	snd_pcm_multi_t *multi = pcm->private_data;
 	snd_pcm_t *slave = multi->slaves[multi->master_slave].pcm;
-	return snd_pcm_status(slave, status);
+
+	int err = snd_pcm_status(slave, status);
+	if (err < 0)
+		return err;
+	snd_pcm_sframes_t avail = snd_pcm_multi_avail_update(pcm);
+	if (avail < 0)
+		return avail;
+	status->hw_ptr = *pcm->hw.ptr;
+	status->avail = avail;
+	return 0;
 }
 
 static snd_pcm_state_t snd_pcm_multi_state(snd_pcm_t *pcm)