Speed up kaiser_hq resampler and fix onset/beat tracker parity#4
Speed up kaiser_hq resampler and fix onset/beat tracker parity#4olilarkin wants to merge 4 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21c82eedcd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Eigen::Index k_stop = std::min<Eigen::Index>( | ||
| i + half_window, window_size); |
There was a problem hiding this comment.
When the convolution window overlaps frame 0, this exclusive upper bound omits k == i + half_window, which is exactly the tap for onset_envelope(0). For any onset envelope with energy at the first frame, the optimized loop now drops that contribution from every affected local score (at i == 0 it also drops the center tap), whereas the previous validity-checked loop included it; the bound should allow one more element.
Useful? React with 👍 / 👎.
| Eigen::Index n = 0; | ||
| while (n < localscore.size() && localscore(n) <= threshold) { | ||
| trimmed[static_cast<size_t>(n)] = false; | ||
| ++n; |
There was a problem hiding this comment.
This leading trim now scans every frame's localscore and stops on the first above-threshold frame, even if that frame is not a selected beat. If a strong non-beat onset occurs before a weak first beat, the loop exits early and leaves that weak leading beat untrimmed (with the analogous issue at the tail); trimming should be driven by the ordered selected beat scores/indices rather than arbitrary intervening frames.
Useful? React with 👍 / 👎.
1 participant
35x speed up.
kaiser_hq is still not matching soxr_hq and is the source of some disparities