From c1a939002462f707f7c08d0744aa5fee915bac29 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 16 Aug 2026 13:44:13 -0700 Subject: [PATCH] fix(core): stop the pitch shift delaying audio it is not shifting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 7, in the two parts the worklet can actually fix. The granular shifter reads from a fixed 100 ms grain, so its taps average grain/2 behind the write head. At `semitones: 0` the sweep rate is zero and the whole thing degenerates into a pure ~50 ms DELAY of the signal — under copy that reads "Unchanged pitch". It bypasses now, as does `mix: 0`. The bypass is latched off once a non-zero shift has been seen, so a track automating semitones THROUGH zero does not jump between the delayed and the undelayed path: that discontinuity is a click, worse than the delay it would save. The ring keeps filling either way, so a later shift does not start cold. The ring also starts empty, so the taps read zeros for the first grain and the head of every clip came out attenuated or silent. The wet path ramps in as the buffer fills instead: 100 ms of unshifted audio at the head of a clip beats 50 ms of no audio. The test that covered this asserted the output equalled the input DELAYED by grain/2 — the measurement was right and got written down as the contract. What this does NOT fix: the ~50 ms group delay for an actual shift. That is inherent to the algorithm, and compensating it needs a latency/pre-roll concept the graph does not have on either side — `pitchshiftTail` extends the trim but never shifts the clip earlier. It is stated in the effect's description rather than left as a trap, and it is a design decision, not a bug fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/src/audio/audioFxWorklets.test.ts | 66 ++++++++++++++++--- packages/core/src/audio/audioFxWorklets.ts | 45 +++++++++++-- packages/core/src/audioFx.ts | 7 +- 3 files changed, 104 insertions(+), 14 deletions(-) diff --git a/packages/core/src/audio/audioFxWorklets.test.ts b/packages/core/src/audio/audioFxWorklets.test.ts index e7089d7ce..cc9e89d9a 100644 --- a/packages/core/src/audio/audioFxWorklets.test.ts +++ b/packages/core/src/audio/audioFxWorklets.test.ts @@ -136,25 +136,73 @@ describe("the worklet processors themselves", () => { return crossings / ((s.length - start) / SR); } - it("at semitones: 0, mix: 1 reproduces the input, delayed by exactly one grain/2", async () => { + // This assertion used to be that the output equalled the input DELAYED by + // grain/2 — the measurement was right and was written down as the contract. + // But the grain delay is there to shift pitch, and at semitones: 0 nothing + // is being shifted: the node degenerated into a pure 50 ms delay of the + // signal, plus a head of silence while the ring filled, under a label that + // reads "Unchanged pitch". + it("at semitones: 0, mix: 1 passes the input through untouched", async () => { const HfPitchshift = (await loadProcessors()).get("hf-pitchshift"); if (!HfPitchshift) throw new Error("hf-pitchshift not registered"); const p = new HfPitchshift({ processorOptions: { semitones: 0, mix: 1 } }); const input = sine(440, 0.5); const output = run(p, input); - const grain = Math.round(SR * 0.1); - // readTap reads from `write - 1`, i.e. one sample behind the one just - // written in this same iteration — so the effective delay is one sample - // more than the nominal grain/2. - const delay = grain / 2 + 1; - // Skip the first grain while the ring buffer is still filling. let maxErr = 0; - for (let i = grain * 2; i < input.length; i++) { - maxErr = Math.max(maxErr, Math.abs((output[i] ?? 0) - (input[i - delay] ?? 0))); + for (let i = 0; i < input.length; i++) { + maxErr = Math.max(maxErr, Math.abs((output[i] ?? 0) - (input[i] ?? 0))); } expect(maxErr).toBeLessThan(1e-6); }); + it("mix: 0 passes the input through untouched too", async () => { + const HfPitchshift = (await loadProcessors()).get("hf-pitchshift"); + if (!HfPitchshift) throw new Error("hf-pitchshift not registered"); + const p = new HfPitchshift({ processorOptions: { semitones: 7, mix: 0 } }); + const input = sine(440, 0.25); + const output = run(p, input); + let maxErr = 0; + for (let i = 0; i < input.length; i++) { + maxErr = Math.max(maxErr, Math.abs((output[i] ?? 0) - (input[i] ?? 0))); + } + expect(maxErr).toBeLessThan(1e-6); + }); + + // A track whose semitones are automated THROUGH zero must not jump between + // the delayed and the undelayed path — that discontinuity is a click, which + // is worse than the delay the bypass would save. + it("keeps processing at zero once it has shifted, rather than clicking to dry", async () => { + const HfPitchshift = (await loadProcessors()).get("hf-pitchshift"); + if (!HfPitchshift) throw new Error("hf-pitchshift not registered"); + const p = new HfPitchshift({ processorOptions: { semitones: 7, mix: 1 } }); + const input = sine(440, 0.4); + run(p, input); + p.p = { ...p.p, semitones: 0 }; + const output = run(p, sine(440, 0.4)); + // Still the wet path (grain-delayed), so it does NOT equal the input. + let maxErr = 0; + for (let i = 0; i < 4000; i++) { + maxErr = Math.max(maxErr, Math.abs((output[i] ?? 0) - (input[i] ?? 0))); + } + expect(maxErr).toBeGreaterThan(1e-3); + }); + + // The ring starts empty, so the taps read zeros for the first grain. That + // used to come out of the head of every clip as silence; it ramps the wet + // path in instead, which is unshifted audio rather than no audio. + it("does not open with silence while the grain buffer fills", async () => { + const HfPitchshift = (await loadProcessors()).get("hf-pitchshift"); + if (!HfPitchshift) throw new Error("hf-pitchshift not registered"); + const p = new HfPitchshift({ processorOptions: { semitones: 7, mix: 1 } }); + const input = sine(440, 0.5); + const output = run(p, input); + // Peak over the first 20 ms — well inside the old dead zone. + let peak = 0; + for (let i = 0; i < Math.round(SR * 0.02); i++) + peak = Math.max(peak, Math.abs(output[i] ?? 0)); + expect(peak).toBeGreaterThan(0.5); + }); + it("at semitones: 12, doubles the fundamental (one octave up)", async () => { const HfPitchshift = (await loadProcessors()).get("hf-pitchshift"); if (!HfPitchshift) throw new Error("hf-pitchshift not registered"); diff --git a/packages/core/src/audio/audioFxWorklets.ts b/packages/core/src/audio/audioFxWorklets.ts index d0dd8dded..673dba725 100644 --- a/packages/core/src/audio/audioFxWorklets.ts +++ b/packages/core/src/audio/audioFxWorklets.ts @@ -253,6 +253,15 @@ class HfPitchshift extends AudioWorkletProcessor { this.buf = []; this.write = 0; this.phase = 0; + // Samples written so far, capped at one grain. The taps read up to a grain + // behind the write head, so until this fills they would read the ring's + // zeros — the head of every clip came out attenuated or silent. + this.filled = 0; + // Latched the first time a non-zero shift is asked for. The no-op bypass + // below must not engage for a track whose semitones are AUTOMATED through + // zero: switching between a delayed and an undelayed path mid-signal is a + // click, which is worse than the delay it would save. + this.everShifted = false; this.port.onmessage = (e) => { if (e.data && e.data.__hfDispose) { this.dead = true; return; } this.p = { ...this.p, ...e.data }; @@ -265,20 +274,46 @@ class HfPitchshift extends AudioWorkletProcessor { const p = this.p; const semitones = Math.max(-12, Math.min(12, p.semitones ?? 0)); const mix = Math.max(0, Math.min(1, p.mix ?? 1)); - const ratio = Math.pow(2, semitones / 12); + if (semitones !== 0) this.everShifted = true; const grain = this.grain; const ringLen = grain * 2; - const inc = (1 - ratio) / grain; const n = i[0] ? i[0].length : 0; for (let ch = 0; ch < i.length; ch++) { if (!this.buf[ch]) this.buf[ch] = new Float32Array(ringLen); } - let write = this.write, phase = this.phase; + + // A node asking for no shift at all, or mixed fully out, is transparent. + // The grain delay is ~grain/2 whatever the ratio, so at semitones=0 this + // used to degrade into a pure 50 ms delay of the signal — while the copy + // for that exact setting reads "Unchanged pitch". The ring keeps filling + // so a later shift does not start cold. + if (mix === 0 || (semitones === 0 && !this.everShifted)) { + let w = this.write; + for (let s = 0; s < n; s++) { + for (let ch = 0; ch < i.length; ch++) { + const x = i[ch][s]; + this.buf[ch][w] = x; + o[ch][s] = x; + } + w = (w + 1) % ringLen; + } + this.write = w; + this.filled = Math.min(grain, this.filled + n); + return true; + } + + const ratio = Math.pow(2, semitones / 12); + const inc = (1 - ratio) / grain; + let write = this.write, phase = this.phase, filled = this.filled; for (let s = 0; s < n; s++) { phase += inc; phase -= Math.floor(phase); const phaseB = (phase + 0.5) % 1; const gA = xfade(phase), gB = xfade(phaseB); + // Ramp the wet path in as the ring fills rather than reading zeros: + // 100 ms of unshifted audio at the head of a clip beats 50 ms of silence. + const warm = filled >= grain ? 1 : filled / grain; + const wetMix = mix * warm; for (let ch = 0; ch < i.length; ch++) { const ring = this.buf[ch]; const inp = i[ch], out = o[ch]; @@ -286,12 +321,14 @@ class HfPitchshift extends AudioWorkletProcessor { ring[write] = x; const wet = readTap(ring, write, phase * grain) * gA + readTap(ring, write, phaseB * grain) * gB; - out[s] = x * (1 - mix) + wet * mix; + out[s] = x * (1 - wetMix) + wet * wetMix; } write = (write + 1) % ringLen; + if (filled < grain) filled++; } this.write = write; this.phase = phase; + this.filled = filled; return true; } } diff --git a/packages/core/src/audioFx.ts b/packages/core/src/audioFx.ts index fd31285b8..a89b318f4 100644 --- a/packages/core/src/audioFx.ts +++ b/packages/core/src/audioFx.ts @@ -509,7 +509,12 @@ export const HF_AUDIO_FX: readonly HfAudioFxDef[] = [ id: "pitchshift", label: "Pitch shift", group: "time", - description: "Shifts pitch up or down without changing playback speed.", + // The granular algorithm reads from a 100 ms grain, so its output runs a + // constant ~50 ms behind its input and nothing in the graph subtracts that + // — there is no latency/pre-roll concept here yet. It is inaudible on its + // own and audible against picture or against an unshifted track, so it is + // stated rather than hidden. `semitones: 0` bypasses the node entirely. + description: "Shifts pitch up or down without changing playback speed. Adds ~50 ms of latency.", params: [ { kind: "number",