Repository navigation
feat(core): signal rhythm is rate-independent, refuses grids in the tracker, follows chords - #5644
Conversation
… grid in the tracker The tracker resamples every channel to 22.05 kHz and averages their band power, so the CLI's native-rate decode and Studio's AudioContext decode get the same beats, and a mono file and the same sound on two channels get identical ones. The no-steady-beat floor moves from analyzeMusicFromSignal into the tracker, so hyperframes audio, grep_audio and beats share one floor. It is 0.15: above every one of 66 synthetic ambient, speech, scattered-effect and noise clips (max 0.122), where the wrapper's 0.2 also refused a third of GTZAN's produced music.
…harmony The tracker now keeps a 10 fps chroma beside its band energies. Downbeats weigh where the chords change, over one beat and over two, with the low-end hit, instead of the low end alone. Sections compare timbre and harmony equally. Tuned on GTZAN's dev half: the tempo prior narrows to 0.8 octave, the local tempo stays within 10% of the global one, and beats sit 15 ms after the onset peak, where annotators put them.
Drops doc comments whose function or field name already says the same thing.
miguel-heygen
left a comment
There was a problem hiding this comment.
Independent review of live successor 11792b8b1bf0414300565a35ef2e52f27b972c91 (the extra commit after requested 59b4c9bb changes comments only).
The resampling and channel-power path checks out: 26 focused tests pass. My separate 12s known-time pulse probe returns the same 24 beat times across 16/22.05/44.1/48/96 kHz × mono/duplicate/inverted/right-only channels; actual signalProvider and wrapper results agree. A 1 kHz tone retains RMS .70694, while 15 kHz attenuates to .000349 on 48→22.05 kHz conversion. Removing the low-pass scaling makes the actual alias regression fail (RMS .70636); source restored afterward.
audioRhythm.ts:461 now applies the same .15 floor before beats/downbeats reach provider or wrapper; CLI beats and Studio URL/buffer analysis use the wrapper, and audio uses signalProvider. The ambient/noise/silence tests and the separate silence/tone probes pass. Chord-change/downbeat and returning-section tests also pass.
Quality judgment: the disclosed drum-only downbeat loss (.903→.686) and drums-under-voice loss (.668→.539) are material, not noise. A produced-music improvement does not establish an improvement for those inputs. The existing confidence: "low" remains appropriate for downbeats. I am not independently validating the GTZAN/R1/S scores, split hygiene or threshold generalization: scripts/per-case outputs are absent. Retaining reproducible benchmark scripts and per-case results would make future threshold changes reviewable, especially since positives and negatives use different synthetic distributions.
Cost: fresh-process 120s mono/48k pulse analysis was ~.62s on base and ~.73s on head on this machine; RSS rose ~81 vs ~91 MiB. This is one bounded fixture, not a statistical benchmark or hour-long/Studio responsiveness certification. New resampled per-channel buffers and cached chroma increase peak/retained work; keep the disclosed long-file limit.
Compatibility note: audioRhythm.ts makes previously exported bandFrames/rhythmOf private; audioAnalysis.ts re-exports this module through the public audio-analysis entry. Repo consumers are migrated, but external named imports would break. Preserve aliases or explicitly record that API removal before publishing; this review does not certify unknown external callers.
Current-head Comments passes, but many other hosted checks are still running with no failures reported at this read. I am withholding the approval stamp pending those gates; no confirmed algorithm blocker found in this bounded review. No live Studio, native/model providers, paid API, independent corpus or hour-long run. No merge/release/deploy.
— Magi (desktop-review)
Verdict: COMMENT
Reasoning: Independent rate/channel/anti-alias/floor probes support the implementation; the accuracy tradeoff stays explicit and author benchmarks remain unverified. Current-head CI must complete before approval.
Edit accuracy: accurate 271 (base branch 271), smooth 165 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
jrusso1020
left a comment
There was a problem hiding this comment.
I reviewed this at 90741e4 and re-checked at 59b4c9b and 11792b8. Both later commits only move or remove comments in audioRhythm.ts and audioSections.ts; no code changed, so everything below holds at this head. No blockers.
What I verified
-
No cancellation between channels. Band power is computed per channel and then averaged (
audioRhythm.ts:102). Chroma takes per-channel magnitudes (audioRhythm.ts:140). Neither path mixes the raw samples first. I ran a 20 s click-plus-chords track as mono, L = -R, the same sound on both channels, and at 48 kHz. All four gave the same beats, downbeats, meter and section starts. -
The resampler low-pass filters before decimating. It uses a Hann-windowed sinc with 8 zero crossings and a cutoff at 0.95 of the lower Nyquist. Measured results:
- A 10 kHz tone resampled from 44.1k to 16k is about -60 dB (RMS 0.0007). It is filtered out, not folded down to about 6 kHz.
- From 48k to 22.05k, attenuation is -16 dB at 11.5 kHz, -25 dB at 12 kHz, -44 dB at 13 kHz and -63 dB at 14 kHz and above.
- The leaky part of that range folds to about 10–10.5 kHz. That is above the top band (9 kHz) and the 2 kHz chroma ceiling, so the analysis never sees it.
-
There is one confidence floor, in one place (
STEADY_BEAT_CONFIDENCE,audioRhythm.ts:447), and it is reached only throughrhythm(). Every consumer goes through that function:- the
beats/downbeatscapabilities analyzeMusicFromSignal(thebeatsCLI and Studio)- section snapping (
audioSections.ts:142) - API-answer snapping (
cli/.../providers/api.ts:134)
One side effect: clips that score between 0.05 and 0.15 no longer get signal-beat snapping in the sections and API paths. That is consistent behaviour, but worth a line in the PR description.
- the
-
Cost. In my measurements, resampling takes about 145 ms per channel-minute at 44.1k. Full
rhythm()takes about 740 ms on 60 s of 44.1k stereo (including JIT warm-up) and about 245 ms on 22.05k mono. Chroma adds about 600 FFTs of 4096 points per channel-minute. There are no quadratic paths. -
Benchmark. The dev/test split is by clip index within each genre. Tuning was done on dev only, and the fitted model was left out even though it scored higher on test. All 66 negatives still get no grid, and the highest-scoring negative is 0.122, under the 0.15 floor.
-
The known regressions are acceptable trades:
- Downbeats on drum-only grids drop from 0.903 to 0.686 F.
- Drums under voice drop from 0.668 to 0.539 beat F. Those clips are refused, not given wrong grids.
- In return, GTZAN test downbeat F rises from 0.342 to 0.504, and the synthetic R1 set rises from 0.934 to 0.999 unfloored.
- Drum-only material with no harmony is a minority of what people beat-sync to.
Should fix (non-blocking)
-
Public API change, call it out in the release notes.
audioAnalysis.tsdoesexport * from "./audioRhythm"and@hyperframes/core/audio-analysisis a published subpath, so this removesbandFrames(now private), changesrhythmOf(frames, sampleRate)to a private one-argument function, and raises the exportedSTEADY_BEAT_CONFIDENCEfrom 0.05 to 0.15. Nothing else in the repo calls the removed pair, so I'm not blocking on it. -
Chroma scales with the number of channels.
audioRhythm.ts:140sums magnitudes across channels, whilebandPowerdivides bychannels.length. I measured the same sound on two channels at exactly 2.000× the mono chroma. Downbeats are unaffected becausechromaBetweennormalizes. Sections applylog1pbefore normalizing (audioSections.ts:56), and that is not scale-invariant, so "mono equals the same sound on two channels" holds only approximately for sections. Dividing bychannels.lengthwould make it exact. -
The parity test (
audioRhythm.test.ts:94) covers two identical channels but not L = -R. One more case would lock in the no-cancellation property.
Nits / follow-ups
bandFramesOf(signal);atbeatDetection.ts:63is a bare call that exists only to warm the cache before apause. A short note there, or a clearer name, would help readers.- To win back drum-only downbeats, the two chord cues (
audioRhythm.ts:392) could be gated on chroma tonality, falling back to the low-end cue when there is no harmony. bandFrameskeeps a resampled copy of every channel alive at the same time (audioRhythm.ts:149). For an hour of 48k stereo that is about 635 MB extra. Processing one channel at a time would cap it. This only matters for the long-file case the PR says it has not profiled.- At 4096 points and 22.05 kHz, chroma bins are about 5.4 Hz wide. Below about 90 Hz that is coarser than a semitone, so the lowest pitch classes blur a little.
What I ran: bun install --frozen-lockfile, then built packages/parsers. Core vitest run src/audio src/beats passed 440/440 across 22 files, matching the PR description. A scratch script measured resampler attenuation, timing, and channel and rate parity.
— Rames
miguel-heygen
left a comment
There was a problem hiding this comment.
Independent review of live successor 11792b8b1bf0414300565a35ef2e52f27b972c91 (the extra commit after requested 59b4c9bb changes comments only).
The resampling and channel-power path checks out: 26 focused tests pass. My separate 12s known-time pulse probe returns the same 24 beat times across 16/22.05/44.1/48/96 kHz × mono/duplicate/inverted/right-only channels; actual signalProvider and wrapper results agree. A 1 kHz tone retains RMS .70694, while 15 kHz attenuates to .000349 on 48→22.05 kHz conversion. Removing the low-pass scaling makes the actual alias regression fail (RMS .70636); source restored afterward.
audioRhythm.ts:461 now applies the same .15 floor before beats/downbeats reach provider or wrapper; CLI beats and Studio URL/buffer analysis use the wrapper, and audio uses signalProvider. The ambient/noise/silence tests and the separate silence/tone probes pass. Chord-change/downbeat and returning-section tests also pass.
Quality judgment: the disclosed drum-only downbeat loss (.903→.686) and drums-under-voice loss (.668→.539) are material, not noise. A produced-music improvement does not establish an improvement for those inputs. The existing confidence: "low" remains appropriate for downbeats. I am not independently validating the GTZAN/R1/S scores, split hygiene or threshold generalization: scripts/per-case outputs are absent. Retaining reproducible benchmark scripts and per-case results would make future threshold changes reviewable, especially since positives and negatives use different synthetic distributions.
Cost: fresh-process 120s mono/48k pulse analysis was ~.62s on base and ~.73s on head on this machine; RSS rose ~81 vs ~91 MiB. This is one bounded fixture, not a statistical benchmark or hour-long/Studio responsiveness certification. New resampled per-channel buffers and cached chroma increase peak/retained work; keep the disclosed long-file limit.
Compatibility note: audioRhythm.ts makes previously exported bandFrames/rhythmOf private; audioAnalysis.ts re-exports this module through the public audio-analysis entry. Repo consumers are migrated, but external named imports would break. Preserve aliases or explicitly record that API removal before publishing; this review does not certify unknown external callers.
Current-head CI has completed: all reported checks are successful or skipped, including regression and Windows jobs; GitHub reports OPEN/CLEAN on this exact head. This supersedes the CI-pending comment5529452944. No confirmed algorithm blocker found in this bounded review. No live Studio, native/model providers, paid API, independent corpus or hour-long run. No merge/release/deploy.
— Magi (desktop-review)
Verdict: APPROVE
Reasoning: Independent rate/channel/anti-alias/floor probes support the implementation; the accuracy tradeoff stays explicit and author benchmarks remain unverified. Current-head CI is now complete without a failure; the documented quality and public-export compatibility notes remain non-blocking release considerations.
What
The
signalrung is what most users get: no Python, no macOS 27, no API. This PR makes it better and measures every change against a fixed benchmark. Everything is inpackages/core/src; no CLI or provider code changes. This covers the floor recalibration and the sections work; coarse section role labels are not in this PR (see rejected).fix). The tracker resamples every channel to 22.05 kHz (newaudioResample.ts, a windowed-sinc resampler, pure TS) and averages the channels' band power. The CLI decodes at the file's native rate, Studio at theAudioContextrate; both now get the same beats. Mono and the same sound on two channels give identical results.fix). The floor that refuses a grid ("no steady beat") moves out ofanalyzeMusicFromSignalintorhythm().hyperframes beats,hyperframes audioandgrep_audionow share it. It is recalibrated from 0.2 (wrapper) and 0.05 (tracker) to 0.15, just above every synthetic negative.feat). The tracker now also computes a 10 fps chroma (fromfft.ts, 4096-point). The bar phase is chosen from three cues: the low-end hit, the chord change across one beat, and the chord change across two beats. Before, it used the low end only.feat). Foote novelty runs over timbre (band energies) and chroma, weighted equally.Benchmark
00000–00049of each genre are dev (tuning only),00050–00099are test.CPJKU/beat_this_annotations.Metrics come from mir_eval 0.8.2: F at ±70 ms, CMLt, AMLt, downbeat F; mir_eval trims the first 5 s. Tempo accuracy uses ±4%: Acc1 is the right tempo, Acc2 also accepts ×2, ×½, ×3 and ×⅓. Beat This small0 is the reference ceiling (the
beat-thisrung).What users get: beats after the floor (
hyperframes beats, Studio, and nowaudio)"before" is
mainat 1544b51:beatswith the wrapper's 0.2 floor. A refused clip scores 0.R1 does not regress: 0.887 → 0.900. The two refused R1 tracks (297b03, 3333be) are the same two #5634 refused; refusing them is still right, because without the floor their tempo is halved.
The "before" downbeat column comes from the tracker, which had no 0.2 floor, so on that column "before" is scored on more grids than "after".
Tracker quality without the floor
This isolates the tracker from the refusal decision. "before" is the tracker on
mainwith its own 0.05 floor, which is whathyperframes audioused to return. "after" uses floor 0.beat_track(DSP reference)Per GTZAN genre, F before → after (test and dev together; Beat This in brackets):
Downbeat F on GTZAN, all clips: 0.327 → 0.494. The biggest gains are disco 0.24 → 0.67, reggae 0.27 → 0.61 and rock 0.40 → 0.62.
Refusing a grid
beats(0.2)audio(0.05)The highest-scoring negative reaches 0.122, so 0.15 leaves a margin. Lowering the floor further mostly adds wrong grids; at 0.15 the dev set still keeps 42 grids with F < 0.5. Because
audioused to give a grid at 0.05, it now refuses more often. That is the intended effect of fix 2.Why the floor lowers GTZAN beat F from 0.776 to 0.654
The tracker itself is unchanged by the floor. The drop is entirely refused clips, which score 0.
Sections: boundary F at ±3 s (F3) and ±0.5 s (F0.5), before → after
Beat This has no sections rung, so this table has no reference column. These are joins of different recordings, which is an easier task than finding sections within one song.
Speed (bun, one process, analysis only)
Targets, honestly
Tried and rejected (GTZAN dev F unless noted; reference 0.743 at the end, 0.735 after commit 1)
Known costs
"Fails without the fix" mutations (all run)
bandFramesusessignal.channelswithout resamplinggives the same beats at any sample rate and for mono or the same sound on two channels:expected 0.25 to be less than or equal to 0.011refuses an ambient pad in the tracker itself, so every caller of the signal tracker agrees, andreports no beats and no tempo for audio without a steady pulseputs the bar where the chords change when every beat hits the same: recall 0 < 0.9scale = CUTOFF, so it folds)removes what the lower rate cannot hold instead of folding it down: RMS 0.706, expected < 0.01Runs here:
src/audio*andsrc/beats: 440 passed.src/audio,src/commands/beats.test.tsandsrc/commands/audio: 47 passed.src/hooksandsrc/utils/beat*: 1567 passed.origin/main: no issues.hyperframes audio --only beats,downbeats,bpm,sectionsruns end to end from the built CLI.GTZAN (not committed)
https://huggingface.co/datasets/marsyas/gtzan/resolve/main/data/genres.tar.gz.http://opihi.cs.uvic.ca/sound/genres.tar.gz, in mirdata ≤ 0.3.8) went offline. Current mirdata ships onlygtzan_mini.b28cc067ff6199bd826f5d1a6931458586d64acae7f44b2e882b7a97af057531, md54c13f740bb9d223a1d74d4a7f78ca336.5b3d6dddb579ab49814ab86dba69e7c7: this tarball holds.wavinstead of.au, plus a.DS_Store.CPJKU/beat_this_annotationsatfd9fcc0896cb78730bae735102c8753ef3a3badd, foldergtzan, MIT.Not exercised
fft.tsis still the only FFT), and Studio's tests and typecheck pass, but it has not been run in a browser.