AudioEffectSpectrumAnalyzerInstance.get_magnitude_for_frequency_range reports jittered values

[Zylann]

Godot version

3.5 stable
4.0 beta3
4.4.1 stable

System information

Windows 10 64 bits NVIDIA GeForce GTX 1060

Issue description

I started playing with rendering a spectrum, and even after some smoothing and peaking, noticed it was kinda low quality. Then I noticed that the reported frequencies were actually jittering along the frequency axis.

It happens for all FFT sizes, but can be mistaken for a "normal precision issue" with the default 1024. It is a lot more obvious with larger FFT sizes and a chirp sound. The frequency peak "wiggles" constantly around the frequency it is supposed to be "following".

2022-10-19.22-58-06.mp4

After seeing this, it becomes clear other FFT sizes also have this problem, but around a shorter width.
It seems the issue is temporal: reported values alternate between some time close to now and some time in the past. To the point when the chirp restarts, we can see values from the previous chirp still flicker back and forth for a brief moment.

Such discrepancy is not expected. It really looks like something is wrong in the way values are reported. Maybe the data from the ringbuffer is read in the wrong way?

Steps to reproduce

Test project:

3.5:
AudioAnalyzerFFTBug.zip
4.0 beta3:
AudioAnalyzerFFTBug_GD4.zip

Minimal reproduction project

No response

[Calinou]

Related to #67611, #20198 and #55090. See #57985, which tried to improve on this issue but had to be reverted.

[EspeuteClement]

I can also confirm this bug. The vaues produced get_magnitude_for_frequency_range produce values that appear correct at first glance but are far too noisy when comparing to other FFT implementations done in other software. I also don't think that is bug is new, as if I recall correcly even the demo project for the feature when it was announced back in godot 3.2 (see here : https://godotengine.org/article/godot-32-will-get-new-audio-features ) did suffer from the same issue.

As the reporter suggested it looks like the problem is time related, as the fft calculations looks correct, but they seem scrambled accros time, and thus produce jittery values.

I tried to dig out in the c++ implementation to try to find out where the bug came from but I didn't have enough experience in audio alalysis to find where whe bug could have come from.

[jcommander]

Still happening on 4.2,
I wish this got more attention especially since there's an official demo for an audio spectrum.
Lerping the results to midigate this issue doesn't produce acceptable results for any audio spectrum project imo.
I'll try naively poking at the code.

[20kdc]

Also happening on 3.5.3, especially with larger FFT sizes

To me, it seems reasonably clear that past FFT frames are repeating themselves.
It is also worth noting that I have attempted locking/unlocking AudioServer, as the code very clearly does not perform any such locking itself.
Looking at the code, I'm not sure what to do except to simply remove the part that tries to compensate for latency/etc. (It isn't functioning, and the current state of things is worse)

[jcommander]

Looking at the code, I'm not sure what to do except to simply remove the part that tries to compensate for latency/etc. (It isn't functioning, and the current state of things is worse)

i tried changing the code a bit but to no avail...for anyone interested in researching this further maybe I can save you some time by pointing you to the code snippet that causes this.
If calculation is as should be, AudioServer::get_singleton()->get_output_latency(); is probably the biggest suspect here. (see #38215)
The problem should be caused by the while loop or rather, the diff var it depends on in the snippet below:

Vector2 AudioEffectSpectrumAnalyzerInstance::get_magnitude_for_frequency_range(float p_begin, float p_end, MagnitudeMode p_mode) const {
	if (last_fft_time == 0) {
		return Vector2();
	}
	uint64_t time = OS::get_singleton()->get_ticks_usec();
	float diff = double(time - last_fft_time) / 1000000.0 + base->get_tap_back_pos();
	diff -= AudioServer::get_singleton()->get_output_latency();
	float fft_time_size = float(fft_size) / mix_rate;

	int fft_index = fft_pos;

	while (diff > fft_time_size) {
		diff -= fft_time_size;
		fft_index -= 1;
		if (fft_index < 0) {
			fft_index = fft_count - 1;
		}
	}

Link to Code

[Akosmo]

What I find weird is that you don't only notice this with frequency change. Even with a sound that's static in frequency AND amplitude, you still notice jittered values, at least from what I've noticed in my own projects. You can try that by playing a sine wave and printing the return of the method.
I tried my best to read through the source, despite having 0 knowledge of C++. I think what jcommander mentioned is correct, but if you can still see the problem with a static audio, couldn't the FFT calculation or capture of spectrum be broken somewhere, especially if it involves the magnitude of the signal?

As someone working with real-time audio visualization in Godot, I would really love to see this fixed. I wish I could fix it myself, but this is way beyond my skillset, and all I can do it bring more attention to it.

[Zylann]

Still happening in Godot 4.4.1, updated project:
AudioAnalyzerFFTBug_GD4.zip

Looking at the code, I'm not sure what to do except to simply remove the part that tries to compensate for latency/etc. (It isn't functioning, and the current state of things is worse)

Indeed removing all that part seens to fix the issue. I'm not sure why that is here.

AudioServer::get_singleton()->get_output_latency(); is probably the biggest suspect here

When I log this value, it is constant all the way. It does contribute to diff (and so moving back fft_index) but not in a variable way (and so isn't causing the issue, I think). The problem here is jittering because diff naturally increases over time, until it goes above fft_time_size, which causes fft_index to go backwards, potentially more than once, until a new FFT gets calculated causing diff to go down again, and so on.
The fact the code decides to suddenly read an OLDER FFT about half of the time makes no sense to me, I wonder what the expected behavior was supposed to be. Like, if we extrapolate and calculate a new FFT at even longer intervals, it would keep going back in time giving older and older result, instead of say, the same "to be considered last" result.

Some guesses:

• My only guess about what this code should be doing, is to eventually NOT return the very last FFT in order to account for latencies. So if "now" is T, the code should return the FFT for T-latency, which is a little further into the past. But as time progresses, we should not go fetch an even older FFT later on, only advance until no more FFT is available (unless latency suddenly increases, but in my testing, it did not). But that's just a guess, without the author or comments it's not easy to guess the intention.

• diff is the amount of time between "now" and the last time an FFT was calculated (+tap_back_pos, which is a constant, undocumented). Then the code subtracts get_output_latency from it... so the higher latency is... the smaller that gap is? Does that make sense?

• Something curious: in the example, a new FFT gets calculated every N seconds. When I plot fft_index and fft_pos, the "going back in time" always occur at about N/2 seconds, and occasionally occurs again the frame before a new FFT gets pushed. It's like fft_time_size is half of what it should be? (but might only be coincidence). If there had to be an offset between these two graphs, it should look like the same graph "offset" in time, not a different jittered graph.
  
  (blue bars: fft_index after the "adjustment" the code does with diff, you can see it wiggle back and forth; red bars: fft_pos, as it would be when removing the code, it only goes up as one would expect; Sampled every frame)

• Instead of removing the latency code, maybe it could be rewritten, but backwards:

		while (diff < 0.f) { // flipped
			diff += fft_time_size; // flipped
			fft_index -= 1;
			if (fft_index < 0) {
				fft_index = fft_count - 1;
			}
		}

All positive values of diff mean that the last FFT we got is now, or in the past (and no new FFT since), so there is no reason to fetch an older one. Since we subtract latency from diff, I can imagine it could be negative, if we got high latency. Which THEN means we look into the previous FFTs, hence the rewriting backwards. Maybe tap_back_pos should be flipped too, not sure.

2025/10/11 Edit: I made a 22min long video with a spectrogram made in a custom-compiled Godot with that change: https://www.youtube.com/watch?v=25vPAjD-Q_0

[Akosmo]

Sorry for my ignorance, but I had these thoughts last time I went through that file:

• I also did notice that get_output_latency stayed constant to me. I had the assumption that it'd keep changing over time, and that that change would mess with the while loop, causing undesired behavior, but isn't it possible that the issue could be happening because the while loop expects a non-constant output latency? If the last FFT calculation took too long, maybe what would be expected would be for get_output_latency to account for that and subtract a larger number from diff, keeping it under fft_time_size
• Related to the point above, the only non-constant value in that part of the function is time. Not sure if it's worth looking into, I think the first point makes more sense.
• What really is tap_back_pos? It's used in diff so it seems relevant. What if it's removed?

As a response, does removing the while loop solve the problem completely? What if instead of going back 1 fft_index until diff <= fft_time_size, we "return" the previously "returned" fft_index (the fft_index of the previous frame)? In better words, an if statement in that case, where if diff > fft_time_size, fft_index -= 1 and move on (do diff -= fft_time_size; if needed)

Your approach seems interesting, I wonder what it looks like in a spectrum.
I'd be testing all these things but I haven't learned about how to fork and build Godot yet.

[Zylann]

isn't it possible that the issue could be happening because the while loop expects a non-constant output latency?

That sounds unlikely to me. While I wouldn't be surprised if latency fluctuates a bit, I wouldn't expect it to vary so wildly to the point it would "perfectly correct" the saw-tooth pattern it would have to match to "fix" the issue. The fact it remains constant sounds ideal actually. Yet the problem occurs.

What really is tap_back_pos? It's used in diff so it seems relevant. What if it's removed?

It's a user setting. It's undocumented, so I don't know what it's for.

As a response, does removing the while loop solve the problem completely

It appears to fix the issue in my use case. Whether it actually does what was intended by the author, I don't know.
The approach where I reverse the loop feels most conservative, and still appears to fix the issue, but again without insight from the author (or whoever knows how that function should work) it's not easy to tell what's right.