Fix(Spectrogram): Prevent misalignment of Spectrogram at start and end of sample #3904
Conversation
Add 0 padding to the end of spectrogram if some data was left out due to window and file length mismatch
WalkthroughThe changes enhance the Changes
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ValeHaas
changed the title
ValeHaas
changed the title
|
I can't seem to figure out, why the first two sample need to be added to the visualization. I'd appreciate any input on why they seem to be missing. This is all that's holding me back, since I don't want to push this and breaking the visualization. |
|
Tbh I don’t know either. The plugin was contributed a long time ago by someone else and I barely touched it since. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/plugins/spectrogram.ts (1)
622-644: LGTM! Consider refactoring duplicated dB conversion logic.The zero-padding implementation correctly handles the case where the audio length isn't a multiple of the FFT window size. This is a standard approach in signal processing.
Consider extracting the dB conversion logic into a separate method to avoid code duplication. Here's a suggested refactor:
+ private convertToDB(spectrum: Float32Array, array: Uint8Array): void { + for (let j = 0; j < this.fftSamples / 2; j++) { + let valueDB = 20 * Math.log10(spectrum[j]) + if (valueDB < -this.rangeDB) { + array[j] = 0 + } else if (valueDB > -this.gainDB) { + array[j] = 255 + } else { + array[j] = (valueDB + this.gainDB) / this.rangeDB * 255 + 256 + } + } + } // Then replace both occurrences with: - for (let j = 0; j < fftSamples / 2; j++) { - let valueDB = 20 * Math.log10(spectrum[j]) - if (valueDB < -this.rangeDB) { - array[j] = 0 - } else if (valueDB > -this.gainDB) { - array[j] = 255 - } else { - array[j] = (valueDB + this.gainDB) / this.rangeDB * 255 + 256 - } - } + this.convertToDB(spectrum, array)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/plugins/spectrogram.ts(2 hunks)
🔇 Additional comments (1)
src/plugins/spectrogram.ts (1)
595-597: Investigate the root cause for requiring exactly two empty frequencies.While adding two empty frequency arrays fixes the alignment issue, the choice seems arbitrary. Understanding why exactly two frequencies are needed would help ensure this is the correct solution.
Consider:
- Analyzing the FFT window size and overlap to determine if the padding requirement is related to these parameters.
- Investigating if this is related to the phase alignment of the FFT windows.
- Documenting the findings to justify this specific implementation.
katspaugh
changed the title
Short description
For Short audio files the spectrogram is only aligned towards the beginning and end of a file. This becomes especially apparent when zooming into the audio/spectrogram.
Resolves #3535
Implementation details
Recommended improvements:
How to test it
Take a short audio file (<20 sec, even better <10s) with pronounced beats / cuts in the the audio, e.g. this heartbeat: heartbeat.zip
Load it into the examples/audio folder
Change the spectrogram example to this code: spectrogram.js.txt
Zoom in to the ends of the spectrogram
So far, the beats do not align, with this pull request they do.
Screenshots
Before
Spectrogram aligns in center
Spectrogram is "late" towards the end of the file.
Spectrogram is "early" towards the beginning of the file.
After
Spectrogram is aligned in the end
Spectrogram is aligned in the beginning
Checklist
Summary by CodeRabbit