Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sokol_audio ALSA: buffer size initialization problem on some Linux configs #400

Open
xglabs opened this issue Oct 6, 2020 · 19 comments
Open
Assignees

Comments

@xglabs
Copy link

xglabs commented Oct 6, 2020

In function _saudio_backend_init(), snd_pcm_hw_params_set_rate_near() return negative result.

Reproduced on two different computers. Both run Arch current (x86_64):

  • Card: HDA Intel, Chip: Analog Devices AD1989B
  • Card: HDA Intel PCH, Chip: Conexant CX20751/2

The following patch fixes sound (saudio-app and modplay-app work), but I'm not sure that it doesn't hammer something else.

diff --git a/modules/sokol/sokol_audio.h b/modules/sokol/sokol_audio.h
index bf7e4a7..ebafca2 100644
--- a/modules/sokol/sokol_audio.h
+++ b/modules/sokol/sokol_audio.h
@@ -1044,7 +1044,6 @@ _SOKOL_PRIVATE bool _saudio_backend_init(void) {
     snd_pcm_hw_params_any(_saudio.backend.device, params);
     snd_pcm_hw_params_set_access(_saudio.backend.device, params, SND_PCM_ACCESS_RW_INTERLEAVED);
     snd_pcm_hw_params_set_channels(_saudio.backend.device, params, _saudio.num_channels);
-    snd_pcm_hw_params_set_buffer_size(_saudio.backend.device, params, _saudio.buffer_frames);
     if (0 > snd_pcm_hw_params_test_format(_saudio.backend.device, params, SND_PCM_FORMAT_FLOAT_LE)) {
         goto error;
     }
@@ -1056,6 +1055,12 @@ _SOKOL_PRIVATE bool _saudio_backend_init(void) {
     if (0 > snd_pcm_hw_params_set_rate_near(_saudio.backend.device, params, &val, &dir)) {
         goto error;
     }
+    snd_pcm_uframes_t periodsize = _saudio.buffer_frames * 2;
+    rc = snd_pcm_hw_params_set_buffer_size_near(_saudio.backend.device, params, &periodsize);
+    if(rc < 0) {
+        goto error;
+    }
+
     if (0 > snd_pcm_hw_params(_saudio.backend.device, params)) {
         goto error;
     }

I.e. we set rate first, then try to set buffer size. The code was copied from alsa samples. I'm not sure, if we must change anything else (period_size).

P.S. It would be nice, if samples report sound initializion failure somehow. With red screen for example.

@floooh
Copy link
Owner

floooh commented Oct 7, 2020

Hmm, interesting. I guess SoLoud will have the same problem then. That's were the sokol_audio.h backend code is taken from. It works on my Asus Zenbook laptop with Ubuntu (currently at v20.04, but also on v18.x).

Here's the init code from SoLoud:

https://github.com/jarikomppa/soloud/blob/4d72336a8855f3f421f95ec75cda9062da3fe7eb/src/backend/alsa/soloud_alsa.cpp#L132-L143

miniaudio on the other hand seems to call set_rate_near before set_buffer_size_near.

Thanks for the report and suggested fix, I'll try to add it later today!

@floooh floooh self-assigned this Oct 7, 2020
@floooh
Copy link
Owner

floooh commented Oct 7, 2020

Ok I committed a fix, it's quite different from your code though:

8611fae

...but it's hard to test without being able to reproduce the original problem.

I think the order of calls isn't the problem though, but instead that ALSA might not support the requested buffer size on your config. The difference between snd_pcm_hw_params_set_buffer_size() and snd_pcm_hw_params_set_buffer_size_near() is that the first function fails if ALSA doesn't support a buffer of the requested size (for instance because the requested size is too small), while the second function picks the next bigger valid buffer size, and then also returns that new size (so the rest of sokol-audio also needs to use this actual buffer size).

I also changed a hard assert into a soft initialization error if the actual buffer size is no longer a multiple of the sokol-audio fifo packet size (e.g. on my laptop the minimal ALSA buffer size seems to be 96 frames, which is not a multiple of the 128 frames for the ring buffer packet size, hopefully this isn't a problem with "realistic" buffer sizes, and those are always multiple of 128. If not, more fixes are needed.

Can you give the fix a whirl and let me know if this works now on your machines? (if not please reopen this issue).

@floooh floooh closed this as completed Oct 7, 2020
@xglabs
Copy link
Author

xglabs commented Oct 7, 2020

Doesn't work for me:

sokol_audio.h: actual backend buffer size isn't multiple of requested packet size
buffer_frames=1881 packet_frames=128

@floooh
Copy link
Owner

floooh commented Oct 7, 2020

Man that's weird.. Is this one of the sokol-samples or your own code?

What's the initial value for buffer_frames? The sokol-samples use the default buffer size of 2048 frames, it would be very strange if this would result in 1881 allocated frames...

@floooh
Copy link
Owner

floooh commented Oct 7, 2020

1881 isn't even a multiple of 32, 64 or 96... (96 frames seems to be the minimal supported buffer size on my config).

@xglabs
Copy link
Author

xglabs commented Oct 7, 2020

This is from saudio-app.

buffer_frames=2048 packet_frames=128
sokol_audio.h: actual backend buffer size isn't multiple of requested packet size
buffer_frames=1881 packet_frames=128

This is from alsa function call:

buf_frames before snd_pcm_hw_params_set_buffer_size_near() = 2048
buf_frames after snd_pcm_hw_params_set_buffer_size_near() = 1881

@floooh
Copy link
Owner

floooh commented Oct 7, 2020

Hmm... currently I have no clue what to do about this. If ALSA allocates such weird buffer sizes the entire code of pushing audio packets needs to be rewritten... why would any audio hardware require such an odd buffer size?

@floooh
Copy link
Owner

floooh commented Oct 7, 2020

1881 isn't a multiple of any 2^N number, but it's a multiple of 3: 3*627 = 1881. But 627 also isn't a particularly "round number". This is very very odd.

@floooh floooh reopened this Oct 7, 2020
@floooh floooh changed the title sokol_audio: ALSA don't work sokol_audio ALSA: buffer size initialization problem on some Linux configs Oct 7, 2020
@xglabs
Copy link
Author

xglabs commented Oct 7, 2020

It should be noted, that this weird behaviour happens only when sample rate is 44100. For 48000 everything works as expected.

sample_rate before: 48000
sample_rate after: 48000
buf_frames before: 2048
buf_frames after: 2048

@floooh
Copy link
Owner

floooh commented Oct 8, 2020

Aha, interesting! When you try to initialize with sample rate 44100, does the ALSA initialization change this to 48000 (e.g. sample_rate before: 44100 and sample_rate after: 48000?

(44100 / 48000) * 2048 = 1881,6

...so at least this explains where that strange buffer size is coming from. It seems like ALSA adjusts the buffer size to get the same latency if it cannot match the requested sample rate.

1881 samples at 44.1 kHz would cover the same duration as 2048 samples at 48 kHz...

I think this gives me some ideas how to proceed, but I need to study the ALSA API a bit more.

I'm currently busy with D3D11 swapchain stuff, but hopefully I can look into this afterwards.

Thanks for your investigations !

@floooh
Copy link
Owner

floooh commented Oct 12, 2020

Hmm ok, I've been reading up a bit more on how ALSA setup-configuration works, and I did a bit more code cleanup as result, but I don't think this fixes the problems on your machines. Before going into the details, I don't understand why the previous (and current) code doesn't work on your setup, because the setup code should accept a buffer size of 2048 frames, but automatically select 48kHz sample rate even if 44.1 kHz was requested (since this combination: 2048 buffer size and 48kHz seems to be supported on your machine).

The way this ALSA configuration stuff works is that you first setup a structure with all possible parameter combinations:

snd_pcm_hw_params_any(_saudio.backend.device, params);

...all following function calls "restrict" this configuration space more and more to throw out "unwanted" configs.

So step by step (with the new code):

Remove all 'access types' that are not RW_INTERLEAVED:

snd_pcm_hw_params_set_access(_saudio.backend.device, params, SND_PCM_ACCESS_RW_INTERLEAVED);

Remove all configs with sample formats that are not FLOAT_LE:

sokol/sokol_audio.h

Lines 1074 to 1077 in 5b5d9a7

if (0 > snd_pcm_hw_params_set_format(_saudio.backend.device, params, SND_PCM_FORMAT_FLOAT_LE)) {
SOKOL_LOG("sokol_audio.h: float samples not supported");
goto error;
}

Remove all configs with buffer sizes different from the requested buffer size:

sokol/sokol_audio.h

Lines 1078 to 1081 in 5b5d9a7

if (0 > snd_pcm_hw_params_set_buffer_size(_saudio.backend.device, params, _saudio.buffer_frames)) {
SOKOL_LOG("sokol_audio.h: requested buffer size not supported");
goto error;
}

Remove all configs where the channel count doesn't match:

sokol/sokol_audio.h

Lines 1082 to 1085 in 5b5d9a7

if (0 > snd_pcm_hw_params_set_channels(_saudio.backend.device, params, _saudio.num_channels)) {
SOKOL_LOG("sokol_audio.h: requested channel count not supported");
goto error;
}

...and finally remove all configs with a non-matching sample rate, but allow ALSA to pick a "nearby" sample rate:

sokol/sokol_audio.h

Lines 1086 to 1092 in 5b5d9a7

/* let ALSA pick a nearby sampling rate */
rate = _saudio.sample_rate;
dir = 0;
if (0 > snd_pcm_hw_params_set_rate_near(_saudio.backend.device, params, &rate, &dir)) {
SOKOL_LOG("sokol_audio.h: snd_pcm_hw_params_set_rate_near() failed");
goto error;
}

If after any of those steps the remaining "configuration space" is empty (meaning this combination of parameters is not supported by the ALSA driver), the snd_pcm_hw_params_set_* functions return with <0. But it's important to note that the order how those functions are called should not matter (what may happen though is that a different function fails when the order is changed, but in the end this just means that the specific configuration that's requested is not supported).

Now the strange part: if the combination of 2048 sample frames at 48 kHz is supported on your hardware configs, then all config steps should succeed, and the last function snd_pcm_hw_params_set_rate_near() which takes 44.1 KHz as input should pick the "nearby" frequency 48 kHz. This is the part I don't understand.

@xglabs
Copy link
Author

xglabs commented Oct 12, 2020

I've looked at openal-soft ALSA initialization code and borrowed one function call, that you are not using. With this patch, everything works. But I don't understand why...

diff --git a/sokol_audio.h b/sokol_audio.h
index 8df5813..897526a 100644
--- a/sokol_audio.h
+++ b/sokol_audio.h
@@ -1083,6 +1083,11 @@ _SOKOL_PRIVATE bool _saudio_backend_init(void) {
         SOKOL_LOG("sokol_audio.h: requested channel count not supported");
         goto error;
     }
+    /* disable resampler */
+    if(snd_pcm_hw_params_set_rate_resample(_saudio.backend.device, params, 0) < 0) {
+        SOKOL_LOG("set_rate_resample failed");
+        goto error;
+    }
     /* let ALSA pick a nearby sampling rate */
     rate = _saudio.sample_rate;
     dir = 0;

@floooh
Copy link
Owner

floooh commented Oct 12, 2020

...which is also strange because I would except that resampling is a good thing if a requested sample rate isn't supported. But maybe this is what enables picking a nearby sample rate in the first place instead of trying to emulate the requested sample rate by resampling. Thanks for looking into this, I'll try to add this fix tomorrow :)

@xglabs
Copy link
Author

xglabs commented Oct 12, 2020

Oops. I was too fast, to report everything is OK. There is sound, but it is not right:

  • saudio_app works as a beeper, i.e sound is interrupted with pauses.
  • modplay_app just doesn't sound right.

Sorry.

P.S. Initially, I've tried to set '1', but got no sound at all.

@floooh
Copy link
Owner

floooh commented Oct 13, 2020

Hmm this sounds like there's still some disagreement about sample rates and/or buffer sizes, maybe it's just the returned sample rate that's wrong.

I really should try to get my hands on one of those "problematic" laptops. What are those two laptops models you're testing on? Maybe similar configs are sold as very cheap laptops.

PS: and do you know how to get the chipset info on Linux? E.g. what command tells me that I have (for instance) a "Conexant CX20751/2"?

@xglabs
Copy link
Author

xglabs commented Oct 13, 2020

Alsamixer.

Screenshot 2020-10-13 at 16 07 17

Conexant is on Lenovo IdeaPad 100. "Analog Devices AD1989B" on desktop with some ASUS motherboard.

@floooh
Copy link
Owner

floooh commented Oct 15, 2020

...I wonder if the best short-term solution is to simply set the default sample rate in sokol_audio.h to 48kHz. E.g. if "native" 48kHz is more widely supported than 44.1kHz, this would definitely make sense. I'll try to do some research what native sample rates are common...

(PS: only downside would be that I need to add resampling to my plmpeg video decoding sample, because MPEG1 audio frequency is (typically?) 44.1kHz (definitely in the example video I used).

@floooh
Copy link
Owner

floooh commented Oct 15, 2020

...another more robust solution is to rewrite the ring buffer code so that the audio streaming buffer size doesn't need to be a multiple of the ring-buffer packet size... I just remembered that I did something similar in the plmpeg.h sokol-sample... This would allow sokol_audio.h to work with odd buffer sizes (like 1881 frames we've seen here).

I need to think this through a bit, and it will be a little while until I can take care of this.

@oviano
Copy link
Contributor

oviano commented Feb 21, 2021

...another more robust solution is to rewrite the ring buffer code so that the audio streaming buffer size doesn't need to be a multiple of the ring-buffer packet size... I just remembered that I did something similar in the plmpeg.h sokol-sample... This would allow sokol_audio.h to work with odd buffer sizes (like 1881 frames we've seen here).

I need to think this through a bit, and it will be a little while until I can take care of this.

Yes, to this. The AAudio backend could benefit from being able to take a lower latency path where the callback can be called with different values (even within one audio session) depending on the characteristics of the underlying audio device.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants