Every Lfo starts from the same sample-and-hold seed #33

New issue

Closed

opened 2026-08-30 14:53:02 +00:00 by bcox · 0 comments

bcox commented

2026-08-30 14:53:02 +00:00

Owner

Every Lfo is value-initialized with the same sample-and-hold seed, so two S&H LFOs never have independent sequences.

Symptom

Two LFOs on one layer, both set to kSampleHold:

Same rate — bit-identical outputs. Two modulators, one signal.
Different rates — identical first value (both draw at note-on), then the same value list traversed at different speeds. They separate in time but never become independent; a listener hears one sequence in canon, not two sources.

This does not need a chord, a cold instrument, or polyphony. One voice, one layer, two LFOs is enough, which is why it is worth fixing rather than working around.

Cause

dsp/lfo.h:317 — std::uint32_t random_ = 0x2545F491u;

Nothing ever writes it. Every Lfo array is value-initialized, so every element takes that constant:

dsp/layer.h:489 — 2 per voice
dsp/fm_pair_layer.h:3380 — 2 per FM pair layer
dsp/va_layer.h:1622 — 3 per VA layer

With 8 layers per voice and 200 voices, every S&H generator in the instrument starts from one word.

Lfo::NoteOn deliberately does not reseed (dsp/lfo.h:97-99), and that is correct — it is what makes a second note a new draw rather than a replay. The defect is that there is no seed to preserve in the first place.

Second axis: across voices

The same constant also correlates voices. Two voices' generators differ only by their accumulated note-ons and rendered wraps (OnWrap at dsp/lfo.h:224-232, called from NoteOn at dsp/lfo.h:124), so ordinary playing separates them within a note or two — but a fresh instrument struck with a simultaneous chord gives every voice the identical S&H stream, and identical rates keep them locked.

This is the case that surfaced it: 30 voices of an authored pitch wander, all drawing the same numbers.

Precedent

6776866 "Give every voice its own random sequence" fixed exactly this shape for RandomA/RandomB, scoped to the sources the modulation matrix routes. The Lfo generator is the second copy of the same bug, one level in, and was never in that diff.

That commit's reasoning applies unchanged here, including the part that matters most: the seed must be a bit-mixing finalizer of the index, not an affine function of it. An LCG seeded affinely gives a first draw affine in the index — a chord comes out as a ramp.

Fix direction

Seed each Lfo from the voice seed mixed with the LFO's identity (layer slot and LFO index), using the same finalizer as VoiceAllocator::SeedFor (dsp/voice_allocator.h:49). Seed once, at the same point the voice is seeded; do not reseed on note-on, and do not reseed on Reset — a panic stops the sound, it does not rewind the dice.

Tests

dsp/lfo_test.cc has SampleAndHoldDoesNotReplayItsSequenceOnEveryNote, which covers one LFO across notes. Nothing covers independence between instances. Add:

Two S&H LFOs in one layer at equal rates — reject identical output sequences.
A cold N-voice chord with S&H routed to a lane — reject an arithmetic progression across voices, the shape assertion 6776866's test already established. A minimum-gap assertion would be backwards: a ramp has perfectly even gaps.

Verify both by injecting the shared seed and watching them fail.

Expected fallout

plugin/render_golden_test.cc renders DefaultFmPairPatch, which does not select kSampleHold, so the goldens should not move. Confirm rather than assume.

Three factory patches do use S&H — arp2600_burble, rattlesnake, kargyraa — and will change how they sound. That is the fix working, not a regression, but it is worth an audition pass.

Every `Lfo` is value-initialized with the same sample-and-hold seed, so two S&H LFOs never have independent sequences. ## Symptom Two LFOs on one layer, both set to `kSampleHold`: - **Same rate** — bit-identical outputs. Two modulators, one signal. - **Different rates** — identical first value (both draw at note-on), then the same value list traversed at different speeds. They separate in time but never become independent; a listener hears one sequence in canon, not two sources. This does not need a chord, a cold instrument, or polyphony. One voice, one layer, two LFOs is enough, which is why it is worth fixing rather than working around. ## Cause `dsp/lfo.h:317` — `std::uint32_t random_ = 0x2545F491u;` Nothing ever writes it. Every `Lfo` array is value-initialized, so every element takes that constant: - `dsp/layer.h:489` — 2 per voice - `dsp/fm_pair_layer.h:3380` — 2 per FM pair layer - `dsp/va_layer.h:1622` — 3 per VA layer With 8 layers per voice and 200 voices, every S&H generator in the instrument starts from one word. `Lfo::NoteOn` deliberately does not reseed (`dsp/lfo.h:97-99`), and that is correct — it is what makes a second note a new draw rather than a replay. The defect is that there is no seed to preserve in the first place. ## Second axis: across voices The same constant also correlates voices. Two voices' generators differ only by their accumulated note-ons and rendered wraps (`OnWrap` at `dsp/lfo.h:224-232`, called from `NoteOn` at `dsp/lfo.h:124`), so ordinary playing separates them within a note or two — but a fresh instrument struck with a simultaneous chord gives every voice the identical S&H stream, and identical rates keep them locked. This is the case that surfaced it: 30 voices of an authored pitch wander, all drawing the same numbers. ## Precedent `6776866` "Give every voice its own random sequence" fixed exactly this shape for `RandomA`/`RandomB`, scoped to the sources the modulation matrix routes. The `Lfo` generator is the second copy of the same bug, one level in, and was never in that diff. That commit's reasoning applies unchanged here, including the part that matters most: the seed must be a **bit-mixing finalizer** of the index, not an affine function of it. An LCG seeded affinely gives a first draw affine in the index — a chord comes out as a ramp. ## Fix direction Seed each `Lfo` from the voice seed mixed with the LFO's identity (layer slot and LFO index), using the same finalizer as `VoiceAllocator::SeedFor` (`dsp/voice_allocator.h:49`). Seed once, at the same point the voice is seeded; do not reseed on note-on, and do not reseed on `Reset` — a panic stops the sound, it does not rewind the dice. ## Tests `dsp/lfo_test.cc` has `SampleAndHoldDoesNotReplayItsSequenceOnEveryNote`, which covers one LFO across notes. Nothing covers independence *between* instances. Add: 1. Two S&H LFOs in one layer at equal rates — reject identical output sequences. 2. A cold N-voice chord with S&H routed to a lane — reject an arithmetic progression across voices, the shape assertion `6776866`'s test already established. A minimum-gap assertion would be backwards: a ramp has perfectly even gaps. Verify both by injecting the shared seed and watching them fail. ## Expected fallout `plugin/render_golden_test.cc` renders `DefaultFmPairPatch`, which does not select `kSampleHold`, so the goldens should not move. Confirm rather than assume. Three factory patches do use S&H — `arp2600_burble`, `rattlesnake`, `kargyraa` — and will change how they sound. That is the fix working, not a regression, but it is worth an audition pass.