Hanning window applied to input data (fftInput, not fftOutput) #491
Conversation
A Hanning window is a taper that "tapers" the edges of data segments before performing fft, so the Fourier output does not contain artifacts from the sharp edges at the start and end of data segments. Here, it was incorrectly applied to the output data (the Fourier data). The result was streak artifacts along the frequency domain of the "Freq. vs Throttle" plots in BBE. Here, I simple moved the Hanning window portion of the code before fft was computed and applied it to Input data ('fftInput') instead of output data ('fftOutput').
Hanning window was defaulted as "false", but should be on by default.
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -60,7 +60,7 @@ function UserSettingsDialog(dialog, onLoad, onSave) { | |||
| graphExpoOverride : true, // Ability to toggle Expo off=normal/ on=force 100% | |||
| graphGridOverride : true, // Ability to toggle Grid off=normal/ on=force disabled | |||
| analyserSampleRate : 2000/*Hz*/, // the loop time for the log | |||
| analyserHanning : false, // use a hanning window on the analyser sample data | |||
| analyserHanning : true, // use a hanning window on the analyser sample data | |||
There was a problem hiding this comment.
General remark: This line uses tabs instead of the 4 spaces normally used in this project. Yes, this is not your change, but as in most projects, it is seen as polite if code is left 'at least as good as, if not better' than how it was found, so this could have been fixed as part of the pull request.
|
I know I arrive late to this, but I was out. Only want to say that I'm not too sure to apply the hanning by default. This hanning is applied to all the spectrum types and the results are very different for some of them (I think remember the standard spectrum). I don't know nothing about the maths that produce this, so maybe the results are more correct with them applied, but the spectrum will be very different between versions and between the people who upgraded (it will remain disabled) and the people who installed it from zero (it will be enabled). As I say maybe this is the correct move, but is good that we know the implications. @bw1129 have you verified the other spectrum maths to confirm that the hanning is applied correctly? |
|
The Hanning window is just a window function something like a Gaussian which just removes the edge artifact of the data going into fft (because the start and end edges are rarely settled at 0). That’s not so much a problem with a large piece of data like is used with the main 2d spectral plots because the fft will be dominated by the good input so-to-speak. But when you select a short piece of data to analyze fft it will matter (so when you select out only a small portion of the logfile). This is why it really matters for the 3D freqxthrottle plots because these are created from a bunch of short data segments. Now, the Hanning window does tend to bring the overall scale down a negligible amount and if this is a concern, I’m happy to remove the Hanning calc from the 2D spec distributions altogether. That’s easy to do, and it’s probably unnecessary there anyway. Would that be ok?
Wrt the Hanning button under the settings window, I understand what you mean. BBE seems to have some code that holds the button state even when you restart the program or switch to different versions. So if you had it off in a previous version seems it will remain off in the next. I’m pretty sure though that whatever it’s state is under the settings tab on you version is what it is. That said, it was defaulted to off in code. Either way, the change I made was necessary for the 3D heatmap plots because even with the button on there, it was previously not implemented on the input data so it wouldn’t have worked right if people had it on anyway. It was previously applied to the output (fft) dat which basically acted to cut off the lower portion of the fft output in the plot. Try it on BBE3.5. You’ll see the plot is missing the freqs below 50hz (copter motion freqs). So that had to be fixed. Again, I’m happy to remove the hanning from the 2d spec plots though.
|
|
@McGiverGim: I see your concern about changing the default for new installations, but not for existing ones. But on the other hand I think having the Hanning window applied correctly is a fix, and that it makes sense to have it on by default, as there are some plots that can benefit from this. |
|
It's perfect for me. As you know my maths level is very basic, so if you think this is an improvement, it's ok for me. |
|
I agree with this as well. The hanning button is there if people wish to turn it off, and I think providing info about it would be perfect as I think the little settings tab goes unnoticed by most people anyway.
|
|
I found this little discussion about tapers very relevant and easy to understand. http://qingkaikong.blogspot.com/2016/10/signal-processing-why-do-we-need-taper.html?m=1
|
This PR is a minor improvement of the Spectrum Graph (PR #322)
A Hanning window is a taper that "tapers" (i.e., smooths) the edges of data segments used for fft, so that the Fourier output does not contain artifacts from the sharp edges at the start and end of data segments. In PR #322, it was incorrectly applied to the output data (processed Fourier data), but in fact was never called, and should have instead been applied to the input data segments ('fftInput'). The result was occasional streak artifacts along the frequency domain (x-axis) of the 'Freq. vs Throttle' plots (see below).
For this PR, I simply moved the Hanning window portion of the code before fft was computed and applied it to Input data ('fftInput') instead of output data ('fftOutput'). 'fftChunkLength * 2' was changed to just 'fftChunkLength' because it is the correct width for the Hanning window and did not need a multiplier. I also turned on the "use Hanning Window" in the user settings, which was previously off ('false') by default. The improvement of these changes to the spectrogram (using the exact same log file as above) can be seen below.
This is my first PR so my apologies if I've done something wrong here. Please let me know. thx -Brian