Repository navigation
perf: per-frame pitch stepping and shared chorus sin/cos - #7
Merged
Merged
Conversation
renderVoice evaluated 2 ** and the glide/envelope/ratio/Hz ramps once per oversampled substep while pitch was moving. The pitch exponent is piecewise linear, so it is now evaluated at both ends of the output frame and stepped multiplicatively (two pows per frame); ramped Hz uses the parabola through x = 0, 1/2, 1, which is exact for both the linear and the quadratic (Hz ramp chasing a ratio ramp) cases. Output is bit-identical for ratio, fixed-Hz and pitch-envelope runs and within one Float32 ULP for glides. The chorus derived its eight LFO taps from one sin/cos pair (voices are pi/2 apart) and skips voices whose weight is exactly zero; its output is bit-identical. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014NSFwhZm5KbGGm7vXyfCtS
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
Requested by 小語 · project thread
Summary
Dynamic pitch (
src/core/synth.ts,renderVoice). While a voice glides, follows a pitch envelope or ramps ratios/fixed Hz, the old code ran2 **,pitchAt,pitchEnvelopeAtand tworampAtper operator on every oversampled substep. Now:e = glide/12 + envelope/1200is piecewise linear, so it is evaluated at both ends of the output frame and stepped multiplicatively:2^e(n + s/O) = 2^e(n) · r^s,r = 2^((e(n+1) − e(n))/O). That is two pows per frame (one when the exponent is flat).hz = h0 + x(b + cx)) is exact in both cases. A first version with linear interpolation failed the Fourier phase-reference test inengine-v6.test.ts(2e-6 error), so the quadratic is required.rampedFrequencyhelper.Chorus (
src/core/fx.ts). The four voices are π/2 apart and the right tap leads by π/2, so onesin/cospair gives all eight taps: left[s, c, −s, −c], right[c, −s, −c, s]. Voices whose weight is exactly 0 skip their delay reads, and their contribution is exactly zero anyway.The LFO was already evaluated once per output frame, so it is unchanged.
Output equivalence (vs
main, 8 voices, 2000 blocks × 128 frames, 48 kHz)Benchmark (5 interleaved base/new process pairs, median of per-block medians)
Checks
npm run typecheck: passnpm test: 443/443 passnpm run sound-quality: passednpm run security: passeddist/rebuilt withnpm run buildGenerated by Claude Code