More audio buffering fixes (primarily affects SDL) #12916
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…king SYSPROP_AUDIO_FRAMES_PER_BUFFER into account.
…hanges to the Qt SDL code.
|
Just gonna go ahead and merge, I think it's pretty safe. |
| @@ -248,7 +263,8 @@ unsigned int StereoResampler::Mix(short* samples, unsigned int numSamples, bool | |||
| // Flush cached variable | |||
| m_indexR.store(indexR); | |||
|
|
|||
| return realSamples / 2; | |||
| // TODO: What should we actually return here? | |||
There was a problem hiding this comment.
I do think actually produced samples is best, then padding can be decided per platform.
-[Unknown]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Started with the assumption that something was wrong with the rate adjustment logic, but couldn't find anything, just bad variable names not describing what was going on. Instead it turned out that the target buffer size (which is what lowwatermark really was) simply was lower than the default buffer size in SDL, which can never work reliably, leading to #12705.
Eventually ended up clamping the target buffer size so it's always larger than the SDL buffer size with some margin, and also reduced the SDL buffer size from 2048 to 1024.
So this change could have been a bit smaller but I want to keep the renaming and minor refactorings I did.
I want to later change to SDL_QueueAudio but really wanted to get this solid first.
Ported the changes to the SDL audio code used by Qt, too (we should really share that...)
Fixes #12705 , hopefully.