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

AudioEffectPitchShift deteriorates sample quality "without being used" #20198

Open
Tracked by #76797
rokasv opened this issue Jul 16, 2018 · 7 comments · Fixed by #57985
Open
Tracked by #76797

AudioEffectPitchShift deteriorates sample quality "without being used" #20198

rokasv opened this issue Jul 16, 2018 · 7 comments · Fixed by #57985

Comments

@rokasv
Copy link

rokasv commented Jul 16, 2018

Godot 3.0.2 stable + Windows 10

Continuously playing the same sound through the "Effect" bus in this configuration:
capture
...results in the sample changing sound over time, without ever touching pitch_scale, i.e. it's always = 1.0.

@smeikx
Copy link

smeikx commented Apr 27, 2019

As soon as you activate the effect audio gets processed by an additional algorithm which transforms the original sound; regardless of the effect’s configuration, the input is changed.
A cheap workaround would be to bypass the effect as long as the value is 1. But users could also just not activate the effect as long as they don’t change its value.

Maybe we could tweak the pitch shifting algorithm instead?

For pitch shifting Godot uses SMBPitchShift (servers/audio/effects/audio_effect_pitch_shift.cpp), which states the following:

osamp is the STFT oversampling factor which also determines the overlap between adjacent STFT frames. It should at least be 4 for moderate scaling ratios. A value of 32 is recommended for best quality.

Currently the recommended minimum, 4, is being used. I tried beefing it up to 32 which improved the overall sound quality noticeably, but also added some other artefacts.

I’ll keep playing with the algorithm’s settings until I understand them. Maybe we could expose them in the editor in some way? Maybe through a quality slider?

@smeikx
Copy link

smeikx commented May 1, 2019

I tried beefing it up to 32 which improved the overall sound quality noticeably, but also added some other artefacts.

Turns out these artefacts come from clipping, as the effect makes the output louder.

@SubSage
Copy link
Contributor

SubSage commented Apr 11, 2020

Confirming this issue is still alive as of today, using Godot v3.2.stable.official on linux.

@Calinou Calinou added this to the 4.0 milestone Apr 11, 2020
@EviTRea
Copy link

EviTRea commented Feb 9, 2022

Just notice this should be the same issue causing #55090

@Calinou
Copy link
Member

Calinou commented Feb 9, 2022

I can confirm this on master 9d1626b. The audio gets louder and quality is reduced, which is especially noticeable if you accumulate several AudioEffectPitchShift effects together.

Increasing the oversampling value on each of the AudioEffectPitchShifts from 4 to 16 increases CPU usage significantly, causes audible hitches and doesn't improve quality much.

Minimal reproduction project: test_audioeffectpitchshift_master.zip

Original (good)

simplescreenrecorder-2022-02-09_20.20.55.mp4

4 AudioEffectPitchShifts (bad, and gets worse over time)

Volume was manually reduced in the AudioStreamPlayer node to roughly match the original.

simplescreenrecorder-2022-02-09_20.19.41.mp4

@akien-mga
Copy link
Member

Reopening as #57985 was reverted due to regressions.

@akien-mga akien-mga reopened this Mar 1, 2022
@ronyeh
Copy link
Contributor

ronyeh commented Jul 6, 2022

Hi!

If any devs on the Godot Audio / Pitch Shifting system need me to help with testing or bug repros, please @ or message me. I've been running into issues with my app regarding pitch shifting and rapidly repeating sound effects, so I am very much interested in seeing the audio system improved.

Thanks for all your hard work. Godot is awesome. :-)

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

Successfully merging a pull request may close this issue.

8 participants