Fix Spectrum Analyzer effect returning jittered values

[Akosmo]

Fixes #67650

Please read the edit below. The original description is a little outdated/inaccurate since I've gotten a better understanding of this issue.

Removes the problematic undocumented variable tap_back_pos, and also removes lines of code related to times FFT is performed and attempts to compensate for that.

The original code records the time FFT was performed, to account for latency, I assume. The only thing it did with that data was going back to a previous FFT result (index) if a long time has passed since the last FFT calculation (hence why jittered values are more apparent on higher FFT sizes). During that process, it also made use of tap_back_pos, an undocumented variable. Changing the variable, only made the problem worse, as it'd make an original variable diff higher, and easier to go back to previous FFT indeces. Because of that, I have removed tap_back_pos, and set/get functions related to it.
I believe those pieces of code were performing as expected, but I see no reason to go back to previous FFT indeces. We have no author notes, so much of the work is based on assumptions.

Note: this is my first time trying to contribute. I may have gotten things wrong, so please, let me know if anything is wrong.

EDIT: considering that it's hard for audio PRs to be merged, and how sort of niche this issue is, I'll do my best to follow the PR template, to properly explain the issue, this PR, and help reviewers:

• Summary of changes: This PR mainly removes a code block (this loop) that makes get_magnitude_for_frequency_range() return magnitudes for older FFT results, after some time since the last time FFT was performed, by subtracting from fft_index, used in fft_history.
  In case the link is broken, the block is the while loop on L169 in audio_effect_spectrum_analyzer.cpp, inside get_magnitude_for_frequency_range().

• Motivation: I have an audio visualizer made in Godot, and from the beginning, I wanted to have an audio spectrum mode. However, the jittered values made the spectrum behave really badly, even with time smoothing of magnitudes. It does not present the Fourier Transform of the signal in a faithful way.

• Technical overview: Because of the behavior described in Summary of changes, the results from get_magnitude_for_frequency_range() would appear jittered. Removing that code block makes the return values of that function APPEAR to update slower, but the frequency of FFT calls and execution of that function is the same as before, except that it's not "filling the gaps" with magnitudes from older FFT results that we don't need anymore. That code block depended on two other things: another code block to capture times of FFT calculations, and an undocumented variable (tap_back_pos), which would start at a low value, and if increased, would make the issue worse. Those two things have also been removed, since they no longer served a purpose, but because tap_back_pos was exposed, I had to create compatibility methods for it.
  Notes: we have no author comments for the original code, so a lot of the work is based on assumptions.
  When FFT is performed, the time for that call is captured, which is later compared to the time get_magnitude_for_frequency_range() is called. If the difference between both is too large, a condition for a while loop, which uses that diff, becomes true. In it, fft_index is subtracted by 1, and some value is subtracted from diff. When diff becomes short enough, the condition for the while loop becomes false. tap_back_pos adds to diff before the while loop, which is why increasing this value would worsen the problem.

• Testing: This PR was tested with the demo project linked by the original poster of the issue this PR fixes, and with an old prototype of my audio visualizer, which has an audio spectrum mode. Both worked correctly, and the spectrum works even better with time smoothing added to magnitude values (this is done in GDScript, not source code). Both projects are in GDScript, and I believe nothing would change in C#, since no matter how often get_magnitude_for_frequency_range() is called, FFT calls would have the same frequency, as it's an internal behavior. Also, WASAPI was used.

• Discussion: The original code made use of AudioServer::get_singleton()->get_output_latency(). To be fair, I still don't know the exact purpose of those blocks, and neither did others. Perhaps a way to compensate for delay? But from what I have seen, everything worked as it was supposed to with that code. I just don't know why it modified fft_index. Either way, testing this PR at different latencies, audio drivers and OS should paint a better picture for how this PR affects existing projects. The only things I can say for sure: magnitudes will update less frequently. That can be mitigated by smoothing results over time, for example, decaying magnitude at a fixed rate, until a new magnitude value is higher than the current value post-decay. And also, set_tap_back_pos no longer has effect, and getting its value will always return the value tap_back_pos was originally initialized at (in C++ code, not in engine, which sets it to 0.1, the minimum allowed).

• Additional work: I believe the code works well. However, as I said, it could use further testing in different audio APIs, just to make sure removing AudioServer::get_singleton()->get_output_latency() didn't break the method for some users, but I doubt that's the case. Testing this issue in C# could be useful, but I don't think it'd make a difference either. If anything, I think it'd just allow faster FFT calls, if that is dependent on user script. But even with high FPS in GDScript, FFT calls seem to happen in the same frequency.

Here are demonstrations of the issue, and the result of this pull request:

Reference (both with a bit of time smoothing, and bottom one at 2048 FFT size and average mode, which is the same size and mode as the videos underneath):

2026-01-23.21-44-35.mp4

Before (4.6 RC 2, tap_back_pos = 0.1, lowest in engine - CW: FLASHING LIGHTS):

2026-01-23.22-50-04.mp4

Before (smoothing enabled):

2026-01-23.23-11-10.mp4

After (this PR):

2026-01-23.22-23-11.mp4

After (smoothing enabled):

2026-01-23.23-42-41.mp4

(Don't mind the quality changes, I had to compress some of the videos)

[goatchurchprime]

There is an official godot demo of the audio spectrum effect https://github.com/godotengine/godot-demo-projects/tree/master/audio/spectrum which is not good enough to show off this bad issue.

As well as being good enough to show off bugs, I believe demo projects should also illustrate what the different parameters do. So, for example, the demo could show 4 spectrum analyzers side by side with different fft_size values so you can visibly see the compromise between smoothness and latency. Also, the sine sweep from the original issue is much better for explaining what an FFT does.

[Akosmo]

Tested with the project provided in the original issue. It's indeed fixed on my end. Other people in the issue page also confirmed that removing the while loop fixed the issue for them.

Would be great to reviews on this. It's kind of a niche part of the engine, but it's also just broken, it seems.
I was initially worried that removing code that dealt with time of FFT capture would break something, but the only thing it was used for was that while loop.
There's the idea of having a "legacy" mode for this, but I personally don't think it's necessary. Not only it seems these changes don't break anything (need reviews), but the current version on stable is already broken.

Also, I agree with @goatchurchprime . We could use a demo project that demonstrates the spectrum analyzer better.

Edit: issue still happens on 4.5.1, by the way.

[Calinou]

Also, the sine sweep from the #67650 (comment) is much better for explaining what an FFT does.

What's the difference between the current demo and a sine sweep? (I'm not an audio specialist.)

[Akosmo]

Also, the sine sweep from the #67650 (comment) is much better for explaining what an FFT does.

What's the difference between the current demo and a sine sweep? (I'm not an audio specialist.)

Not the person you asked, but the Fourier Transform formula takes signal from the time domain to the frequency domain. So basically taking audio samples/frames, and displaying which frequencies lead to those same samples, making the spectrum analyzer. A sweeping sine wave is better at displaying that behavior than a song with multiple instruments, like in the current demo project. It is better because you have a single sound and simple waveshape (sine waves only produce one frequency), with a clear pitch and behavior.

[Bromeon]

From a GDExtension perspective, documented or not doesn't really matter. The getter/setter methods are currently exposed (i.e. advertised in extension_api.json), plus the property is mentioned in docs. As such it's rightful for code to depend on them.

The problem with straight removing them is that any extension that potentially calls them will run into UB under newer Godot versions. Compatibility methods, even if implemented as no-ops, prevent this.

[Akosmo]

What is it that I should be doing then? This sort of situation is not given as example in the contributing docs. For context, the variable was only used in a single place, that was removed to fix the problem. Should I add every getter/setter back, even though they don't do anything, or try to add compatibility methods, as demonstrated here?

If I'm doing the latter, my approach would be:

1. Add compatibility methods to header, while keeping the original getter/setter removed from both header and .cpp
2. Create .compat.inc file, where the getter returns the original default value of tap_back_pos, the removed variable, and a void setter (as in the original) that doesn't do anything (empty)
3. Include new file to .cpp
4. Keep tap_back_pos removed from the class documentation
5. Change .expected comment to something like: "Removed property that worsed the problem of jittered values in Spectrum Analzyzer.\nCompatibility methods registered."

My questions for this approach are:

• Should the original getter/setter stay removed in .h and .cpp, or does the compat methods only work if they exist? If they have to exist, doesn't that create an unnecessary property in the inspector?
• In .compat.inc, should the getter return the original value, or 0.0? And if I can't create an empty setter, what should be added inside?
• Should the property be removed or kept in the docs?

[Bromeon]

I'll try to answer one-by-one, hoping to clarify how exactly compatibility breakage can happen and what you can do about it 🙂

---

For context, the variable was only used in a single place, that was removed to fix the problem.

That applies to the engine-internal C++ code, however once a method is exposed (via the "bind methods" mechanism in a class), it can be used from GDScript, C# and GDExtension. Once it becomes part of a stable release, we generally (conservatively) assume that someone, somewhere, may use such a method.

Removing from GDScript is less problematic, as it will be hit by a parse-time or runtime-error, but for often-used APIs (not here), it should still be done carefully, as it adds a lot of churn to users and makes writing plugins across versions very hard.

Removing from GDExtension however means that an older extension (compiled against 4.5) now fetches a function pointer that is no longer existing in a newer Godot version. Depending on whether the binding checks validity of that pointer or not (and Release builds may skip that), it's possible to introduce undefined behavior.

---

Should I add every getter/setter back, even though they don't do anything, or try to add compatibility methods, as demonstrated here?

Should the original getter/setter stay removed in .h and .cpp, or does the compat methods only work if they exist? If they have to exist, doesn't that create an unnecessary property in the inspector?

Only compatibility methods need to be added (i.e. the part that's facing GDExtension). The C++ internal API can be refactored much more freely.

Properties are not directly accessible from GDExtension, instead they are exposed as a pair of setter/getter methods. So those setters/getters would need a compat method, that is then registered. Their original implementation can be removed.

So from the API perspective, these APIs are still removed:

• They're no longer accessible in GDScript.
• They don't appear in the docs.
• They aren't advertised in extension_api.json (the class specification for GDExtension).

However if an old extension fetches a method with a specific class name, method name and hash, it will still be found.

---

In .compat.inc, should the getter return the original value, or 0.0? And if I can't create an empty setter, what should be added inside?

You should stay as close as possible to the previous behavior, with reasonable effort. If you are confident that the previous implementation was flawed and thus not really usable, or if maintaining that would need to store complex extra state, it may not be worth it. It depends a bit how niche an API is. Here it's probably OK to keep things simple.

---

Should the property be removed or kept in the docs?

The property itself can be removed, as long as setter/getter are registered via compat methods.

The setter/getter don't need to be registered as regular methods any longer.

[Akosmo]

Attempted to squash all commits, but couldn't squash the first one, because I was afraid of messing something up if I resetted too far back. Also rebased.

I'm trying to get the text for the validation file, but no matter what I do, and despite getting help, the text won't show up. So just like the last time, I'll get the report from the automatic checks to use for the validation file, as expected. I'd keep the same one I had before, but I'm not sure if the errors are the same. Really sorry about that.

I tried my best to keep the behavior related to tap_back_pos as close to original as possible. In order to do that, I created a variable in the compat include file. I did not use access modifiers, and I don't know how to test the file, since I've never worked with GDExtension.

[Akosmo]

Changes addressed (thank you so much Bromeon), fix still works, validation file added, most commits squashed, and rebased. Ready for another review.

[dsnopek]

I feel like this should either be a class variable, or if we really want it here, it should at least be static like:

Suggested change

	float tap_back_pos = 0.01;
	static float tap_back_pos = 0.01;

[Akosmo]

Thanks, I felt like something was missing. Added 'static' to keep it simple

[Bromeon]

Should we actually still store this value? Now it looks like this works, but it has no effect.

Or should we rather emit a warning when calling these methods? And return a hardcoded value every time?

[Akosmo]

Should we actually still store this value? Now it looks like this works, but it has no effect.

Or should we rather emit a warning when calling these methods and return a hardcoded value every time?

It having no effect was expected. My thought process was that if anyone had code depending on what the value of tap_back_pos is, this would keep the intended behavior, not considering that it has no effect on the spectrum. Emitting a warning seems ideal, but I have no idea how to implement that, neither where to find reference for it. Seems like the choice is either adding an unnecessary variable to keep intended behavior for GDExtension, or clean up the code but risk breaking something for someone. That's what I see, at least.

[dsnopek]

Should we actually still store this value? Now it looks like this works, but it has no effect.

Or should we rather emit a warning when calling these methods? And return a hardcoded value every time?

Yeah, actually, maybe just not storing anything is best. So, it'd be something like:

void AudioEffectSpectrumAnalyzer::_set_tap_back_pos_bind_compat_114355(float p_seconds) {
	WARN_PRINT("AudioEffectSpectrumAnalyzer.set_tap_back_pos() is deprecated and the value will be discarded.");
}

float AudioEffectSpectrumAnalyzer::_get_tap_back_pos_bind_compat_114355() {
	return 0.01;
}

[Akosmo]

Done. Thanks!

[Bromeon]

Looks good from GDExtension perspective (I can't judge the jitter issue).
Thanks for the update!

[Akosmo]

PR had 20 successful checks, then I pushed one commit (with amend) that just added the keyword static to a new variable I had created, and it's now failing Android tests in places that don't seem related. Someone in Rocket Chat also complained about "flaky" Android tests. Not sure if it's this PR's issue, or the test itself. I'm unsure how to proceed.

Edit: apparently the issue is another repository

[Akosmo]

Updated description to try to help reviewers. I did my best to explain the issue, and my PR. I also provided videos, showing the issue, and the result of this PR.