projectM-visualizer / projectm

projectM - Cross-platform Music Visualization Library. Open-source and Milkdrop-compatible.
https://discord.gg/mMrxAqaa3W
GNU Lesser General Public License v2.1
3.22k stars 365 forks source link

Refactor waveform aligner and add test #766

Closed dpw13 closed 4 months ago

dpw13 commented 5 months ago

This PR adds a friend test for WaveformAligner that verifies a few internal implementation details as well as verifying proper alignment for waveforms with supported offsets.

This PR also refactors WaveformAligner for readability and fixes two bugs: 1) The code to calculate errorSum is supposed to calculate a windowed convolution but introduced an indexing bug that resulted in repeatedly calculating the product of the two waveforms instead. 2) The calculated "mip levels" or resampled waveforms were recalculated in the original Milkdrop code but was re-used as an optimization in the projectM code. However the re-used mip levels did not take the applied shift into account, resulting in the calculated offset being relative to the previous unshifted waveform. This bug was not present in the original Milkdrop code since the mip levels were recalculated including the applied shift.

dpw13 commented 4 months ago

I split the commit into several pieces that should be much easier to review. I've explained what's changing in each commit. The commit that splits Align into multiple methods is likely the more difficult to review; I suggest disabling viewing whitespace changes with that commit. I've ordered the methods so hopefully the diff looks cleaner than before (where ResampleOctaves was placed too low in the file). The last commit contains only the actual fixes so git blame should make it clear what's changed and why.