Skip to content

[APP BUG]: Audio has brief cut offs due to frequent SDL queue overflows #2015

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

Closed
2 tasks done
C0rn3j opened this issue Jan 2, 2025 · 24 comments
Closed
2 tasks done
Labels
Bloodborne bug Something isn't working

Comments

@C0rn3j
Copy link
Contributor

C0rn3j commented Jan 2, 2025

Checklist

  • I have searched for a similar issue in this repository and did not find one.
  • I am using an official build obtained from releases or updated one of those builds using its in-app updater.

Describe the Bug

Follow up to #1717 since the main issue was fixed.

[Lib.AudioOut] <Warning> sdl_audio.cpp:Output:71: SDL audio queue backed up (38656 queued, 32768 threshold), clearing.

Reproduction Steps

Run Bloodborne, sound cuts off briefly relatively frequently even on a beefy system.

Expected Behavior

No response

Specify OS Version

Arch Linux

@C0rn3j C0rn3j changed the title [APP BUG]: Audio cuts off due to frequent SDL queue overflows [APP BUG]: Audio has brief cut offs due to frequent SDL queue overflows Jan 2, 2025
@Hermiten Hermiten added the bug Something isn't working label Jan 2, 2025
@morten71
Copy link

morten71 commented Jan 2, 2025

I think my problem is the same as this. Since the 48c51bd commit (audio: Accurate audio output timing. (#1986)), I haven't had sound at all, and my log is full of "SDL audio queue backed up" lines.

I have experimented a bit, and found that one of the new things being done in this commit, is calling SDL_SetHint with the buffer size to get low audio latency.

Normally this will be called with a buffer size of 256 frames, corresponding to ~ 5 ms of sound. This is quite a harsh requirement I thought, so I tried hard coding a bigger size like this (in src/core/libraries/audio/sdl_audio.cpp line 21):

if (!SDL_SetHint(SDL_HINT_AUDIO_DEVICE_SAMPLE_FRAMES, "1024")) {
    LOG_WARNING(Lib_AudioOut, "Failed to set SDL audio sample frames hint to {}: {}",
                samples_num_str, SDL_GetError());
}

This makes it work perfectly for me, it seems. Whereas before I never got sound, except randomly maybe once in 20-30 tries, now it works every time.

Maybe my system is more allergic to the low latency requirement than most, but it is definitely a problem for me, and relaxing the requirement a little fixes it.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 2, 2025

It does not sound like you have the same problem, but perhaps it is the same core issue.

My problem manifests similarly to this (but usually sounds more cut-offy rather than jagged like this), I have simulated it here by running stress.

2025-01-02.18-57-29.mp4

ending has a loud smash

@morten71
Copy link

morten71 commented Jan 2, 2025

It does not sound like you have the same problem, but perhaps it is the same core issue.

Yeah I know it doesn't manifest exactly the way you described it (Unfortunately I can't seem to get your video clip to show).

But in my experience, forcing a too strict latency requirement on the audio system, can result in both your scenario and mine. My guess is that your system is just a bit better at handling the requirement than mine is, so I just don't get sound at all.

Do you have the option of testing the change I mentioned? It would be really interesting to see if it makes a difference for you too :-)

Actually an even easier way to test it, is changing line 20 to this:

const auto samples_num_str = std::to_string(port.buffer_frames * 4);

It does the same thing.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 2, 2025

(Unfortunately I can't seem to get your video clip to show).

I don't get why GitHub hates me here, but it does at least play the audio for me here which is the important part.
The video file itself plays fine locally…

Do you have the option of testing the change I mentioned? It would be really interesting to see if it makes a difference for you too :-)

Could you throw it up as a patch for the really lazy person that also uses the PKGBUILD to compile?

My guess is that your system is just a bit better at handling the requirement than mine is, so I just don't get sound at all.

My system is rather beefy and I bump up pipewire buffers on top due to issues with SDL3 in CS2:

default.clock.quantum       = 2048 # 1024
default.clock.min-quantum   = 1024 # 32
default.clock.max-quantum   = 4096

@morten71
Copy link

morten71 commented Jan 2, 2025

I don't get why GitHub hates me here, but it does at least play the audio for me here which is the important part.
The video file itself plays fine locally…

I just see this, and I can't get anything to play there at all.
image

Could you throw it up as a patch for the really lazy person that also uses the PKGBUILD to compile?

I can make an old fashioned patch as a file, but I have a feeling your are thinking of something more clever than that. I haven't learned how these things are done with github yet unfortunately.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 2, 2025

Funny enough, Nextcloud web refuses to play the OBS-remuxed video too - the GH link is here:

https://github.com/user-attachments/assets/a02f2709-8620-4e7c-bcb6-5d637c1046fa

I can make an old fashioned patch as a file, but I have a feeling your are thinking of something more clever than that

Nope, I did mean just a dumb patch diff :D

reworked diff below:

--- a/src/core/libraries/audio/sdl_audio.cpp
+++ b/src/core/libraries/audio/sdl_audio.cpp
@@ -17,7 +17,7 @@
         : frame_size(port.format_info.FrameSize()), guest_buffer_size(port.BufferSize()) {
         // We want the latency for delivering frames out to be as small as possible,
         // so set the sample frames hint to the number of frames per buffer.
-        const auto samples_num_str = std::to_string(port.buffer_frames);
+        const auto samples_num_str = std::to_string(port.buffer_frames * 4);
         if (!SDL_SetHint(SDL_HINT_AUDIO_DEVICE_SAMPLE_FRAMES, samples_num_str.c_str())) {
             LOG_WARNING(Lib_AudioOut, "Failed to set SDL audio sample frames hint to {}: {}",
                         samples_num_str, SDL_GetError());

@morten71
Copy link

morten71 commented Jan 2, 2025

latency_test.patch.zip

it should work, if you run
patch -p0 < latency_test.patch
standing in the root shadPS4 dir

I could download your video now, thanks. To me that video sounds exactly like constant buffer underruns, caused by using too small buffers, trying to get low latency. I'm clearly no expert, but I have definitely heard sound coming out like that many times, when I have tried playing sound from my programs 😅

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 2, 2025

With your change I can sometimes reproduce it, but now ONLY with stress (which does cause very noticeable framedrops - on all cores - I forgot to mention), but it takes a while and it still works almost all the time, even though the game is chugging severely.

I doubt I can now reproduce it during normal gameplay.

It does look like the delay is currently a bit too tight - for the record I have a 4090 and a 7600X.

Maybe it would be nice if the value was both configurable in the GUI and the default bumped up, some people will probably desire the lowest of latencies for some competitive games where they can hold a perfectly stable framerate.

2025-01-02.20-00-05.mp4

^ https://github.com/user-attachments/assets/385852a6-23f4-4a08-bcec-49e1234edab6 - I'm not even sure if it glitches here, this is with stress on all cores.

@morten71
Copy link

morten71 commented Jan 2, 2025

With your change I can sometimes reproduce it, but now ONLY with stress (which does cause very noticeable

have you tried increasing the requested buffer size even more?
Maybe try *8 instead of *4. My change with *4 is still only asking for around 20 ms latency, which is still low. Doubling that to 40 would not be unreasonable in my experience with pulseaudio.

I don't understand why it has an effect on the rest of the game for you, though. I haven't seen that at all. I get smooth 30 fps bloodborne, no matter how small a latency I ask for. It's only the sound that peaces out 😂

Edit: Ah I think I misunderstood. You are deliberately stressing your CPU, to try and make it fail. That would make the whole game chug obviously. Good sign that it is still difficult to make the sound crap out though.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 2, 2025

I don't understand why it has an effect on the rest of the game for you, though.

I am running stress to trigger these issues outside of normal gameplay, as it is a bit hard for me to trigger the issue consistently otherwise.

Stress stresses the CPU, and I am running it on all cores(8 out of 12 threads, CPU has 6 cores with HT), which effectively causes near 100% CPU usage at all times.

Your patch is not problematic.

@morten71
Copy link

morten71 commented Jan 2, 2025

Maybe it would be nice if the value was both configurable in the GUI and the default bumped up, some people will probably desire the lowest of latencies for some competitive games where they can hold a perfectly stable framerate.

I'm not sure about this. Trying to get latencies as low as 5 ms is not typically a reasonable idea on normal PC audio systems, either on linux or on windows. When people need that tight control over audio timing, they have to use more professionally oriented audio chains. On linux that is traditionally Jack, and on windows it used to be ASIO last time I had anything to do with it.

But maybe things have changed, though I would say that what we just saw doesn't make it seem that way, to me :-)

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 2, 2025

Trying to get latencies as low as 5 ms is not typically a reasonable idea on normal PC audio systems, either on linux or on windows

I've seen musicians dedicated enough to do that -

image

image

I can imagine someone playing Guitar Hero being very upset about higher latencies - not sure how realistic that is though.

Maybe simply improving this section to allow setting the value from an env var would do too:

// We want the latency for delivering frames out to be as small as possible,
// so set the sample frames hint to the number of frames per buffer.
const auto samples_num_str = std::to_string(port.buffer_frames);
if (!SDL_SetHint(SDL_HINT_AUDIO_DEVICE_SAMPLE_FRAMES, samples_num_str.c_str())) {
LOG_WARNING(Lib_AudioOut, "Failed to set SDL audio sample frames hint to {}: {}",
samples_num_str, SDL_GetError());
}

@morten71
Copy link

morten71 commented Jan 2, 2025

I've seen musicians dedicated enough to do that -

Musicians, and lots of other people. It's absolutely not uncommon to want low latencies like this, but I'm just saying that it is not reasonable to expect to get them with the normal audio servers on windows and linux. You need Jack instead of pulseaudio, or ASIO instead of the normal windows sound server.

Nowadays pipewire can also act as a jack server, making this possible. But when interfacing with it normally, like shadPS4 does with SDL3, pipewire just acts as a pulseaudio server, again making a 5 ms latency pretty hopeless.

That being said, I would obviously personally have no problem with the requested SDL audio buffer size being exposed to be changed by the user, somehow. I just don't think it will have any actual effect, except it will make everybody try to set it to a stupidly low number. If sound ends up coming out of that, I almost guarantee it is because some link in the chain will have ignored the requested low buffer size, and instead replaces it with a more reasonable working size.

Also I would be extremely surprised if the PS4's sound system can get anywhere close to latencies like this as well. Well maybe not extremely: The PS4 does have the advantage of not having to deal with all the crap a multipurpose OS like linux or windows does. But I will still doubt it can get 5 ms latencies, until I see it :D

@morten71
Copy link

morten71 commented Jan 2, 2025

After thinking about it for a bit, I think maybe I agree with you, that it would be nice to be able to set the requested SDL output buffer size in the settings. Both RPCS3 and PCSX2 have sliders to adjust the output buffer sizes, so people can find the smallest size, that doesn't result in crackling sound.

@hspir404
Copy link
Contributor

hspir404 commented Jan 3, 2025

I'm not 100% sure the buffer size or latency expectations are the root cause of the issue. I think the buffer running out may be due to the program stalling, and I think the stalling may have more to do with problems with thread locking implementation, or how the system's worker threads are handled.

On Windows, the new game FMV on Bloodborne (on current main) hits a bunch of different access violation type issues in low level locking/mutex/semaphore related stuff. Those issue look to be at least partly related to TLS not being initialized (or maybe that's just another symptom of the same underlying problem). Sometimes this results in a crash, other times it results in a big list of these audio buffer warnings and a burst of noise before syncing back with the FMV audio.

I admit that it could be both that the audio buffer size should be configurable, and that some synchronization/initialization bugs need to be squashed.

@morten71
Copy link

morten71 commented Jan 3, 2025

You clearly know a lot more about those details than I do. But I can't help but think, that if there was a problem with the producer/consumer implementation, so that in some cases the consumer thread (the AudioOutputThread function in src/core/libraries/audio/audioout.cpp line 273) is not getting audio data fast enough because of locking issues, then changing the requested output buffer size in the SDL_SetHint call would not help anything... But in our 2 cases, doing that completely solved the problem. And I will still maintain that asking for an output buffer size of 5.33 ms is not reasonable, using the normal sound servers :D

I would guess, that the issue you mention with the FMV sound, is caused by something else, and it just happens to also result in lots of buffer underruns. Would that be a possibility? Did that problem start happening after the same commit, that created the problem in this bug report?

Edit: I just tested letting the intro FMV play in Bloodborne. I am getting no errors or warnings of any kind from the debug output, but I guess that is because I'm using linux, so I don't have that particular problem. But that is another indication that it is unrelated to this bug, I would think?

@hspir404
Copy link
Contributor

hspir404 commented Jan 3, 2025

If extending the buffer size is definitely helping you two out, then maybe there is more than one issue here.

asking for an output buffer size of 5.33 ms is not reasonable

5ms is pretty darn low latency for a game, so trying to hit that as a goal seems excessive, unless everything else is already well in order. So yeah, if the buffer is SMALLER than that, then it sounds like it can stand to be bigger, to me.

Maybe such a low latency will be desirable to get some rhythm game to work nicely, but usually those have a TON of tuning possible to account for high latency in the video chain on consumer TVs anyway, so it'd have a workaround. And on Windows, when doing professional audio gear stuff (where you legit need low latency) you usually have to install special ASIO drivers to get it to be usable. So yeah. If it's 5ms now, that sounds like an excessively small buffer.

(edit: yeah, you already said all this in the thread above... sorry for the repeat of info)

https://wiki.libsdl.org/SDL3/SDL_HINT_AUDIO_DEVICE_SAMPLE_FRAMES

This documentation seems to imply that setting the hint at all is probably a silly idea for most users, and that SDL3 can probably manage it fine on its own.

SDL3 generally decides this value on behalf of the app, but if for some reason the app needs to dictate this (because they want either lower latency or higher throughput AND ARE WILLING TO DEAL WITH what that might require of the app), they can specify it.

SDL will try to accommodate this value, but there is no promise you'll get the buffer size requested. Many platforms won't honor this request at all, or might adjust it.

that is another indication that it is unrelated to this bug, I would think?

The issue I mentioned causes thread starving. But it's pathologically bad, so I agree that it's not entirely related.
I just have a hunch the audio subsystem isn't playing super nicely with threads in every case (the symptoms of which may be being exacerbated by the FMV), which is why I mentioned it.

Those types of bugs are non-deterministic, so they can kinda be hard to track down. I mentioned the FMV because it's a very consistent repro on Windows, so solving that has an off chance of helping with this as well. But yeah, maybe solving that won't help.

@morten71
Copy link

morten71 commented Jan 3, 2025

5ms is pretty darn low latency for a game, so trying to hit that as a goal seems excessive, unless everything else is already well in order. So yeah, if the buffer is SMALLER than that, then it sounds like it can stand to be bigger, to me.

As far as I'm able to tell, the 5 ms comes from the buffer size that the PS4 is sending to the sceAudioOutOutput library call. That buffer size is 8192 bytes, which is 256 frames, because there are 8 channels of 4 bytes per sample each. 256 frames last 5.33 ms at the sample rate of 48000 Hz the PS4 is using.

So the sound comes in buffers of 256 frames from the game/PS4, and the call to the SDL_SetHint function just takes that same buffer size, and requests it for the output buffers. But I sincerely don't think that is ever going to work as intended on any system. On windows I think it is quite simply ignored, and you get completely normal latency buffers, that you would have gotten anyway, without the SetHint call. For some reason the combination of SDL3 and pulseaudio seems to try to honor the request. But yes completely removing the call to SDL_SetHint also solves the problem, which is why I never had problems before this commit :D

@squidbus
Copy link
Collaborator

squidbus commented Jan 3, 2025

But I sincerely don't think that is ever going to work as intended on any system.

This is not true, there are plenty of systems that can handle this. Many emulators using cubeb use cubeb_get_min_latency to configure the buffer size to the smallest possible for low latency, and this can be as low as 128 frames from my experience.

That being said, this is not true for all systems as we can see here. SDL is supposed to be using it only as a hint and if the actual sample buffer is larger, it reports that to us. The fact that we are having this issue indicates either this isn't working as it is supposed to, or SDL is choosing a larger buffer size but it is still insufficient for some reason compared to the larger default.

SDL does not seem to provide any mechanism for querying minimum supported buffer latency, so I'll have to see what I can do to balance out latency and consistency here.

This documentation seems to imply that setting the hint at all is probably a silly idea for most users, and that SDL3 can probably manage it fine on its own.

Also on this, SDL's 'management' of it is simply providing three or four default buffer sizes depending on the sample rate, regardless of the actual audio implementation used. This hint only overrides that default set of sizes, the backend used in SDL is supposed to then adjust the buffer based on the reality of what it can support regardless of the hint, but clearly if we're having this issue then that isn't working for all implementations.

@squidbus
Copy link
Collaborator

squidbus commented Jan 3, 2025

Also I would like to see a log on the latest from someone with this issue as there is more logging about the buffer sizes and queue thresholds.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 3, 2025

@squidbus

[Lib.AudioOut] <Info> sdl_audio.cpp:CalculateQueueThreshold:108: SDL audio buffers: guest = 8192 bytes, host = 8192 bytes, threshold = 32768 bytes
[Lib.AudioOut] <Info> sdl_audio.cpp:CalculateQueueThreshold:108: SDL audio buffers: guest = 8192 bytes, host = 8192 bytes, threshold = 32768 bytes

image

shad_log.zip - it's the same log but cut off since I sigkilled.

@squidbus
Copy link
Collaborator

squidbus commented Jan 3, 2025

Okay so it seems SDL is indeed just accepting the buffer size for you and not adjusting it at all. Well I've opened a PR to remove the hint setting, latency can be revisited if there's a strong need I suppose as the defaults still aren't too bad.

@morten71
Copy link

morten71 commented Jan 4, 2025

This is not true, there are plenty of systems that can handle this. Many emulators using cubeb use cubeb_get_min_latency to configure the buffer size to the smallest possible for low latency, and this can be as low as 128 frames from my experience.

Yeah I should not have talked about all systems. It's been years since I had anything to do with windows, and back then ASIO was the only way to get 5 ms latency. I have no idea what windows can do nowadays, but maybe cubeb uses WASAPI and that makes it possible.

I don't think it's interesting anymore, but anyway I'm getting the same log output as C0rn3j:

[Lib.AudioOut] <Info> audioout.cpp:sceAudioOutOpen:307: id = 255 port_type = Main index = 0 length = 256 sample_rate = 48000 param_type = Float_8CH attr = None
[Lib.AudioOut] <Info> sdl_audio.cpp:CalculateQueueThreshold:110: SDL audio buffers: guest = 8192 bytes, host = 0 bytes, threshold = 32768 bytes
[Lib.AudioOut] <Info> audioout.cpp:sceAudioOutOpen:307: id = 255 port_type = Bgm index = 0 length = 256 sample_rate = 48000 param_type = Float_8CH attr = None
[Lib.AudioOut] <Info> sdl_audio.cpp:CalculateQueueThreshold:110: SDL audio buffers: guest = 8192 bytes, host = 0 bytes, threshold = 32768 bytes

except for the host=0 bytes part, which is reflected by the Buffer Latency: 0 usec part of the output of pactl list sink-inputs:

Sink Input #13
	Driver: protocol-native.c
	Owner Module: 15
	Client: 16
	Sink: 1
	Sample Specification: float32le 8ch 48000Hz
	Channel Map: front-left,front-right,front-center,lfe,rear-left,rear-right,front-left-of-center,front-right-of-center
	Format: pcm, format.sample_format = "\"float32le\""  format.rate = "48000"  format.channels = "8"  format.channel_map = "\"front-left,front-right,front-center,lfe,rear-left,rear-right,front-left-of-center,front-right-of-center\""
	Corked: no
	Mute: no
	Volume: front-left: 65536 / 100% / 0,00 dB,   front-right: 65536 / 100% / 0,00 dB,   front-center: 65536 / 100% / 0,00 dB,   lfe: 65536 / 100% / 0,00 dB,   rear-left: 65536 / 100% / 0,00 dB,   rear-right: 65536 / 100% / 0,00 dB,   front-left-of-center: 65536 / 100% / 0,00 dB,   front-right-of-center: 65536 / 100% / 0,00 dB
	        balance 0,00
	Buffer Latency: 0 usec
	Sink Latency: 13718 usec
	Resample method: soxr-vhq
	Properties:
		media.name = "Audio Stream"
		application.name = "shadPS4"
		native-protocol.peer = "UNIX socket client"
		native-protocol.version = "35"
		application.icon_name = "applications-games"
		application.process.id = "21039"
		application.process.user = "morten"
		application.process.host = "muhfisk"
		application.process.binary = "shadps4"
		application.language = "en_US.utf8"
		window.x11.display = ":0.0"
		application.process.machine_id = "ecdc57ccd2ba22c914d557735b82b9f8"
		application.process.session_id = "2"
		module-stream-restore.id = "sink-input-by-application-name:shadPS4"

and of course I get these all over the place:

[Lib.AudioOut] <Warning> sdl_audio.cpp:Output:71: SDL audio queue backed up (32768 queued, 32768 threshold), clearing.
[Render.Vulkan] <Info> vk_pipeline_cache.cpp:CompileModule:478: Compiling cs shader 0xe2c27f0c (permutation)
[Lib.AudioOut] <Warning> sdl_audio.cpp:Output:71: SDL audio queue backed up (32768 queued, 32768 threshold), clearing.
[Lib.AudioOut] <Warning> sdl_audio.cpp:Output:71: SDL audio queue backed up (32768 queued, 32768 threshold), clearing.
[Lib.AudioOut] <Warning> sdl_audio.cpp:Output:71: SDL audio queue backed up (32768 queued, 32768 threshold), clearing.
[Lib.AudioOut] <Warning> sdl_audio.cpp:Output:71: SDL audio queue backed up (32768 queued, 32768 threshold), clearing.
[Lib.AudioOut] <Warning> sdl_audio.cpp:Output:71: SDL audio queue backed up (32768 queued, 32768 threshold), clearing.
[Kernel.Fs] <Info> file_system.cpp:sceKernelClose:188: Closing /app0/dvdroot_ps4/shader/gxflvershader.shaderbnd.dcx

And here is my shad_log:
shad_log.txt.zip

@C0rn3j
Copy link
Contributor Author

C0rn3j commented Jan 8, 2025

Closing as the PR that resolves the issue has been merged.

@C0rn3j C0rn3j closed this as completed Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bloodborne bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants