perf(editor): compute banded onsets in the background (chunked) - #250
Merged
Merged
Conversation
The pass-1 onset detector (_onsetTimesFromPeaksPure) was broadband RMS-rise on
the waveform envelope, blind to the events that matter most for charting: a note
entering a sustained/pedaled chord (no total-energy rise), a low B0/A0 bass
attack (a clean transient but no pitch), and it can't discriminate kick vs snare
vs hat (no frequency information). P2-2 — the first code PR of the "make the
automatic map good" 2nd pass; de-risks segment-first mapping (P2-3) and makes
Map Health (P2-4) meaningful.
New pure src/onsets.js, zero dependency: an iterative radix-2 FFT, an STFT
(Hann-windowed), half-wave-rectified spectral flux in THREE bands (low ≲150Hz =
kick/bass-attack, mid = snare, high = hats/subdivision), an adaptive local-median
threshold, local-max peak-pick, and parabolic sub-hop interpolation for ~ms
timing off a coarse hop. The FFT/STFT front-end is factored separably for a
future audio-to-MIDI lane.
Wired behind _ensureOnsets() (audio.js): the emitted shape is the pass-1 [{t,s}]
EXTENDED to [{t,s,bands:{lo,mid,hi}}], so every consumer is unchanged; the RMS
detector stays as a labelled fallback (_onsetDetectorLabel). _ensureOnsetsShifted
now carries the bands through the shift. Computed once per load (downsampled to
~22kHz, cached).
Tests: tests/onsets_spectral_flux.test.mjs (12) against SYNTHETIC signals with
known answers — FFT (cosine→its bin), Hann shape, band-bin math, decimation,
flux frames (silence→burst spike; 60Hz→lo band, 8kHz→hi band), peak-pick
(adaptive threshold / refractory / parabolic sub-hop), and the full pipeline on a
click train + junk-input degradation. 135 JS green, lint 0-err, routes.py
untouched. Live-verified on AC/DC: 675 banded onsets, one-time compute 147ms
(cached after), 3-way band split (113 lo / 285 mid / 277 hi) — real kick/snare/
cymbal separation.
FOLLOW-UP (noted, not in this PR): the ~150ms compute is synchronous + on-demand
(onset strip is off by default; RMS covers the pre-compute instant). Chunk across
rAF or a Worker for zero-jank on a strip-enabled load.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
Follow-up to #248. The banded spectral-flux analysis was synchronous (~150ms one-time), which froze a frame when the onset strip / snap first needed it. Now it runs in the background: _ensureOnsets returns the cheap RMS-envelope onsets IMMEDIATELY and kicks a chunked spectral-flux job that upgrades the cache + redraws when done. onsets.js: the STFT is split into a resumable plan/step (_spectralFluxPlan + _spectralFluxStep) so the frame loop can run in bounded batches; the one-shot _spectralFluxFramesPure / _spectralFluxOnsetsPure are unchanged wrappers over it (bit-identical output). audio.js: _startOnsetFluxJob downsamples once then steps ~1500 frames per requestAnimationFrame, swapping in the sharper onsets on completion; cancelled on new-audio (computeWaveform) and teardownAudio. Tests: tests/onset_chunk.test.mjs (4) — stepping in ANY budget split is bit-identical to the one-shot frames + onsets, the done-flag fires exactly at the last frame, and a zero-frame plan is empty-and-done. Existing onset suite still green. 136 JS green, lint 0-err, routes.py untouched. Live-verified on AC/DC: first _ensureOnsets returns in 23.5ms as 'rms' (907), then upgrades to 'spectral-flux' (675, banded) in the background — no synchronous freeze. Stacks on #248 (feat/editor-onset-spectral-flux); rebase onto main when it merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
… session Two lifecycle bugs in the chunked onset job. 1. A stale job wrote the OLD song's onsets into the NEW session's cache. Loading an audio-less song nulls S.audioBuffer directly (file-ops.js loadCDLC, create.js editorApplyCreateResult) — it never reaches computeWaveform()/teardownAudio(), so the job's `cancelled` flag was never set and its spectral-flux result landed in the fresh session's cache (and repainted). Re-check the precondition on every resume: the job snapshots its buffer and bails if S.audioBuffer is no longer it. Also only clear _onsetJob when the handle is still ours, so a cancelled job's last tick can't null out the job that replaced it. 2. The chunk driver was requestAnimationFrame: undefined under node (every module here has to stay node-importable — _ensureOnsets() threw ReferenceError the moment a decoded buffer was present) and frozen in a backgrounded window, which stalled the analysis until the editor was looked at again. Drive it on setTimeout instead — a chunk that runs BETWEEN frames also costs the paint less than one that runs in it. tests/onset_job.test.mjs pins both (plus a positive control that the upgrade does land, and finite onset times).
…sPlan _startOnsetFluxJob hand-copied _spectralFluxOnsetsPure's pipeline (the 22050 target, the factor formula, fftSize/hop 512) and audio.js stopped importing that function, so the SHIPPED onsets and the TESTED onsets were two separate copies of one tuning — retune the pure function and the editor would silently keep the old numbers. Split the setup out as _spectralFluxOnsetsPlan and have both paths build their plan with it; the chunked driver is now the only thing that differs between them. onset_chunk.test.mjs pins plan+step+pick == _spectralFluxOnsetsPure at 44.1k (so the internal downsample is in the comparison too).
byrongamatos
force-pushed
the
feat/editor-onset-async
branch
from
July 13, 2026 21:23
1c1fdf7 to
1f1eaf1
Compare
byrongamatos
changed the base branch from
feat/editor-onset-spectral-flux
to
main
July 13, 2026 21:29
The chunked driver split _spectralFluxOnsetsPure into a plan/step pair, while #248 fixed bugs INSIDE the frame loop it moved. Git merges those cleanly and silently keeps the old, broken loop on the chunked path. Carried across by hand: · hop = fftSize/2 (was fftSize at the _spectralFluxOnsetsPlan call site, which overrode the default, so the chunked path still ran zero-overlap frames and its sub-hop interpolation was interpolating noise) · frames carry centreSec, added at peak-pick (a frame reports the window START, so onsets read ~10ms early on every consumer) · sqrt over Math.hypot in the magnitude inner loop · _ensureOnsets keeps #248's identity-keyed cache (taking this branch's side wholesale would have reverted that BLOCKER: a new song inheriting the old song's onsets), now combined with the async upgrade + job cancellation Kept main's onset test file (a superset) and adapted its cache-key test to the async contract: flux now arrives via a background job, so the test drains it. Both of #248's guards — the click-train bias test and the cache-key test — pass against the chunked path, which is what proves the reconciliation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on #248 (base is
feat/editor-onset-spectral-flux; rebase ontomainwhen #248 merges).Follow-up noted in #248: the banded spectral-flux analysis was synchronous (~150 ms one-time), which froze a frame the first time the onset strip / snap needed it. Now it runs in the background.
Change
_ensureOnsets()returns the cheap RMS-envelope onsets immediately and kicks a chunked spectral-flux job that upgrades the cache + redraws when done. No decoded buffer ⇒ RMS is the final answer.onsets.js: the STFT is split into a resumable plan/step (_spectralFluxPlan+_spectralFluxStep); the one-shot_spectralFluxFramesPure/_spectralFluxOnsetsPureare now thin wrappers over it — bit-identical output.audio.js:_startOnsetFluxJobdownsamples once, then steps ~1500 frames perrequestAnimationFrame, swapping in the sharper onsets on completion; cancelled on new audio (computeWaveform) andteardownAudio.Tests
tests/onset_chunk.test.mjs(4) — stepping in any budget split (1, 3, 7, 50, all) is bit-identical to the one-shot frames and onsets; the done-flag fires exactly at the last frame; a zero-frame plan is empty-and-done. Existing onset suite still green.routes.pyuntouched._ensureOnsets()returns in 23.5 ms asrms(907 onsets), then upgrades tospectral-flux(675, banded) in the background — the ~150 ms synchronous freeze is gone.🤖 Generated with Claude Code