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

Implement a new resampling algorithm in AudioStreamPlaybackResampled #46086

Merged
merged 1 commit into from
Feb 19, 2021

Conversation

ellenhp
Copy link
Contributor

@ellenhp ellenhp commented Feb 16, 2021

Fixes #23544

This implements a new resampling algorithm for MP3s and oggs. The algorithm is described in this paper as "4 point, 4th order optimal" and seems to perform much better than the cubic algorithm used previously. This algorithm seems free to use from the header of the paper. "Distribute, host and use this paper freely." The intent is very clear, and I know Godot uses algorithms from other papers (for example, the SPCAP algorithm) so I'm hoping this will be fine.

This algorithm looks slower just from the increased number of multiplies, but I doubt that it's an issue. I can profile it if people want me to though, and if it is an issue it might make sense to add a project option to let people choose.

To test, download the example project in #23544 and use the ogg file in the project (the wav file playback uses a separate codepath). You may need to increase your volume before the resampling artifacts become obvious. It may also be worth trying different speakers/headphones if you're having trouble reproducing the bug.

@akien-mga akien-mga added this to the 4.0 milestone Feb 16, 2021
@akien-mga akien-mga requested a review from a team February 16, 2021 08:06
@Calinou
Copy link
Member

Calinou commented Feb 16, 2021

and if it is an issue it might make sense to add a project option to let people choose.

In the long term, it would be interesting to add a project setting to configure the audio resampler. We could also have a Nearest option which gives a "punchier" result with low-sample rate sounds. Some people may prefer this kind of sound rendering when working on a game with retro sounds.

That should be discussed in a proposal anyway 🙂

@ellenhp
Copy link
Contributor Author

ellenhp commented Feb 16, 2021

Should I add a proposal for this? Since it's associated with a long-standing bug I thought I'd go ahead and make the PR, but I can go and write up a proposal for it if people want to discuss.

@Calinou
Copy link
Member

Calinou commented Feb 16, 2021

@ellenhp I opened a proposal: godotengine/godot-proposals#2298

I think this PR could be merged as-is for master. We can look at adding audio resampling options later.

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Feb 19, 2021
@reduz
Copy link
Member

reduz commented Feb 19, 2021

@ellenhp Looks great. To be honest I think for this use case going to something better than cubic will most likely not produce any audible results to players, but given this is not really costly, we don't lose anything by implementing it.

@Calinou for these types of streams, I honestly don't think you want to change the interpolation. For regular sample streams maybe for artistic/retro reasons in sound effects but to be honest you can simulate the aliasing or bitcrushing in a separate application and import the sound already decimated.

@akien-mga akien-mga merged commit 6d0c502 into godotengine:master Feb 19, 2021
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Feb 19, 2021
@ellenhp
Copy link
Contributor Author

ellenhp commented Feb 19, 2021

@reduz I think it's an edge case but I did notice a difference in very low-pitched sounds without any high frequency components. The resampling artifacts sound like ringing almost, and it's only apparent when there's no treble for it to hide behind. The first comment in the bug this closed has a good example of it. Which reminds me. Should I do the same for wav streams?

@Calinou
Copy link
Member

Calinou commented Feb 19, 2021

I would prefer if we used the same resampling algorithm for all audio formats. This way, your sounds won't change in "feeling" if you go from one audio format to another.

@ellenhp ellenhp deleted the new_resampling branch February 19, 2021 17:59
@ellenhp
Copy link
Contributor Author

ellenhp commented Feb 19, 2021

Sounds good. I'll do that after work today.

@wingedadventurer
Copy link
Contributor

I'd love if this ends up in 3.2.4. I'm making a rhythm game where audio is very important, but there's very hearable ringing, especially in low end of the spectrum. :(

@akien-mga
Copy link
Member

Well see: #46086 (comment)

You can already use it in 3.2.4 RC 3.

@ellenhp
Copy link
Contributor Author

ellenhp commented Mar 2, 2021

It's in the latest release candidate along with a bunch of other audio fixes I made recently, and unless this is reverted it'll make it into the final release cut too. It should be fixed in RC3 as long as you're using oggs or mp3s. WAV resampling hasn't been switched over to this new algorithm yet (see: #46245), but in my testing I found the ringing there to be far less noticeable. Oggs and mp3s had some really bad ringing. If you test 3.2.4 RC 3 and find this hasn't been solved, comment on #23544 and @ tag me so I get an email about it. It would be useful to know if you have this issue with wav files so I can push on #46245. With my bad ears, I can't tell the difference between before and after for wavs though, so unless someone reports issues I'm fine without #46245.

@wingedadventurer
Copy link
Contributor

Oops, i didn't think it was in RC3 because it didn't get listed there among fixes. My bad.

However i just downloaded RC3 and tested with OGG, and don't notice any reduction to ringing. :(

I'll provide a video that showcases the issue in #23544 then and try with MP3 and WAV as well.

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

Successfully merging this pull request may close these issues.

Bitrate-like artifact when playing low frequency sounds
5 participants