Skip to content

Commit fbb2116

Browse files
adewaleclaude
adewale
and
claude
committed
fix: Volume P-lock now works for all instrument types
Root cause: Volume P-lock was computed and LOGGED in scheduler but not actually PASSED to most play methods. Only playSampledInstrument was correctly receiving the volume parameter. Changes: - Add volume parameter to playSample (engine.ts:455) - Add volume parameter to playSynthNote (engine.ts:342) - Add volume parameter to playToneSynth (engine.ts:802) - Add volume parameter to playAdvancedSynth (engine.ts:857) - Update synthEngine.playNote and SynthVoice.start to accept volume - Update ToneSynthManager.playNote to pass volume as velocity - Update AdvancedSynthEngine chain to accept and apply volume - Update scheduler to pass volumeMultiplier to ALL play methods Implementation details: - playSample: Apply volume via envelope gain ramp - playSynthNote: Scale envelope peak by volume - playToneSynth: Pass volume as velocity to triggerAttackRelease - playAdvancedSynth: Pass volume to amplitude envelope Testing: - Added volume-plock.test.ts with contract tests - All 1817 unit tests pass - All 45 integration tests pass Documentation: - Updated BUG-PATTERNS.md with fix summary and status - Added Bug Pattern #3: "Computed Value Logged But Not Used" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 3f8441f commit fbb2116

File tree

7 files changed

+548
-46
lines changed

7 files changed

+548
-46
lines changed

‎app/src/audio/advancedSynth.ts‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -485,14 +485,15 @@ export class AdvancedSynthVoice {
485485

486486
/**
487487
* Trigger attack and release with duration
488+
* @param volume Volume multiplier from P-lock (0-1, default 1)
488489
*/
489-
triggerAttackRelease(frequency: number, duration: number | string, time?: number): void {
490+
triggerAttackRelease(frequency: number, duration: number | string, time?: number, volume: number = 1): void {
490491
if (!this.osc1 || !this.osc2 || !this.ampEnvelope || !this.filterEnvelope || !this.noise || !this.lfo) {
491492
logger.audio.warn('Voice triggerAttackRelease: nodes not initialized');
492493
return;
493494
}
494495

495-
logger.audio.log(`Voice triggering: freq=${frequency.toFixed(1)}Hz, wasActive=${this.active}, time=${time?.toFixed(3) ?? 'undefined'}`);
496+
logger.audio.log(`Voice triggering: freq=${frequency.toFixed(1)}Hz, vol=${volume}, wasActive=${this.active}, time=${time?.toFixed(3) ?? 'undefined'}`);
496497

497498
// Clear any pending release timeout
498499
if (this.releaseTimeoutId) {
@@ -517,8 +518,8 @@ export class AdvancedSynthVoice {
517518
this.active = true;
518519
}
519520

520-
// Trigger envelopes
521-
this.ampEnvelope.triggerAttackRelease(duration, time);
521+
// Trigger envelopes - pass volume as velocity to amplitude envelope
522+
this.ampEnvelope.triggerAttackRelease(duration, time, volume);
522523
this.filterEnvelope.triggerAttackRelease(duration, time);
523524

524525
// Schedule voice to become inactive after duration + release
@@ -724,23 +725,27 @@ export class AdvancedSynthEngine {
724725

725726
/**
726727
* Play a note by semitone offset from C4
728+
* @param volume Volume multiplier from P-lock (0-1, default 1)
727729
*/
728730
playNoteSemitone(
729731
semitone: number,
730732
duration: number | string,
731-
time?: number
733+
time?: number,
734+
volume: number = 1
732735
): void {
733736
const frequency = semitoneToFrequency(semitone);
734-
this.playNoteFrequency(frequency, duration, time);
737+
this.playNoteFrequency(frequency, duration, time, volume);
735738
}
736739

737740
/**
738741
* Play a note by frequency
742+
* @param volume Volume multiplier from P-lock (0-1, default 1)
739743
*/
740744
playNoteFrequency(
741745
frequency: number,
742746
duration: number | string,
743-
time?: number
747+
time?: number,
748+
volume: number = 1
744749
): void {
745750
if (!this.ready) {
746751
logger.audio.warn('AdvancedSynthEngine not ready');
@@ -771,19 +776,20 @@ export class AdvancedSynthEngine {
771776
}
772777
this.lastScheduledTime = startTime;
773778

774-
logger.audio.log(`AdvancedSynth playing: freq=${frequency.toFixed(1)}Hz, duration=${duration}, time=${startTime.toFixed(3)}, preset=${this.currentPreset?.name}`);
779+
logger.audio.log(`AdvancedSynth playing: freq=${frequency.toFixed(1)}Hz, duration=${duration}, time=${startTime.toFixed(3)}, vol=${volume}, preset=${this.currentPreset?.name}`);
775780

776781
// Use try-catch to handle cases where Tone.js internal state rejects the time
777782
// This can happen during rapid BPM changes
783+
// Volume P-lock is passed to voice for amplitude scaling
778784
try {
779-
voice.triggerAttackRelease(frequency, duration, startTime);
785+
voice.triggerAttackRelease(frequency, duration, startTime, volume);
780786
} catch (_err) {
781787
// If Tone.js rejects the time, retry with current time + buffer
782788
const retryTime = Tone.now() + 0.01;
783789
this.lastScheduledTime = retryTime;
784790
logger.audio.warn(`AdvancedSynth timing retry: original=${startTime.toFixed(3)}, retry=${retryTime.toFixed(3)}`);
785791
try {
786-
voice.triggerAttackRelease(frequency, duration, retryTime);
792+
voice.triggerAttackRelease(frequency, duration, retryTime, volume);
787793
} catch (retryErr) {
788794
logger.audio.error('AdvancedSynth timing error - note skipped:', retryErr);
789795
}

‎app/src/audio/engine.ts‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -331,13 +331,15 @@ export class AudioEngine {
331331

332332
/**
333333
* Play a synthesizer note (real-time synthesis, not sample-based)
334+
* @param volume - Volume multiplier from P-lock (0-1, default 1)
334335
*/
335336
playSynthNote(
336337
noteId: string,
337338
presetName: string,
338339
semitone: number,
339340
time: number,
340-
duration?: number
341+
duration?: number,
342+
volume: number = 1
341343
): void {
342344
const preset = SYNTH_PRESETS[presetName];
343345
if (!preset) {
@@ -346,9 +348,9 @@ export class AudioEngine {
346348
const actualPreset = preset || SYNTH_PRESETS.lead;
347349
const frequency = semitoneToFrequency(semitone);
348350

349-
logger.audio.log(`playSynthNote: noteId=${noteId}, preset=${presetName}, freq=${frequency.toFixed(1)}Hz, time=${time.toFixed(3)}`);
351+
logger.audio.log(`playSynthNote: noteId=${noteId}, preset=${presetName}, freq=${frequency.toFixed(1)}Hz, time=${time.toFixed(3)}, vol=${volume}`);
350352

351-
synthEngine.playNote(noteId, frequency, actualPreset, time, duration);
353+
synthEngine.playNote(noteId, frequency, actualPreset, time, duration, volume);
352354
}
353355

354356
/**
@@ -451,7 +453,8 @@ export class AudioEngine {
451453
time: number,
452454
duration?: number,
453455
playbackMode: 'oneshot' | 'gate' = 'oneshot',
454-
pitchSemitones: number = 0
456+
pitchSemitones: number = 0,
457+
volume: number = 1
455458
): void {
456459
if (!this.audioContext || !this.masterGain) {
457460
logger.audio.warn('AudioContext not initialized');
@@ -498,11 +501,12 @@ export class AudioEngine {
498501
logger.audio.log(`Created new track gain for ${trackId}, connected to master`);
499502
}
500503

501-
// Create envelope gain for click prevention (micro-fades)
504+
// Create envelope gain for click prevention (micro-fades) and P-lock volume
502505
// Without this, abrupt starts/stops cause audible clicks
506+
// Volume P-lock is applied here (ramp to volume instead of 1)
503507
const envGain = this.audioContext.createGain();
504508
envGain.gain.setValueAtTime(0, time);
505-
envGain.gain.linearRampToValueAtTime(1, time + FADE_TIME);
509+
envGain.gain.linearRampToValueAtTime(volume, time + FADE_TIME);
506510

507511
// Connect: source -> envGain -> trackGain
508512
source.connect(envGain);
@@ -523,8 +527,8 @@ export class AudioEngine {
523527
// One-shot is industry standard for drums and recordings
524528
if (playbackMode === 'gate' && duration !== undefined) {
525529
const stopTime = actualStartTime + duration;
526-
// Fade out before stopping to prevent click
527-
envGain.gain.setValueAtTime(1, stopTime - FADE_TIME);
530+
// Fade out before stopping to prevent click (from current volume to 0)
531+
envGain.gain.setValueAtTime(volume, stopTime - FADE_TIME);
528532
envGain.gain.linearRampToValueAtTime(0, stopTime);
529533
source.stop(stopTime);
530534
}
@@ -788,12 +792,14 @@ export class AudioEngine {
788792
* @param semitone Semitone offset from C4 (0 = C4, 12 = C5, -12 = C3)
789793
* @param time Absolute Web Audio context time to start playback
790794
* @param duration Note duration (e.g., "8n", "4n", or seconds)
795+
* @param volume Volume multiplier from P-lock (0-1, default 1)
791796
*/
792797
playToneSynth(
793798
presetName: ToneSynthType,
794799
semitone: number,
795800
time: number,
796-
duration: string | number = '8n'
801+
duration: string | number = '8n',
802+
volume: number = 1
797803
): void {
798804
if (!this.toneSynths) {
799805
logger.audio.warn('Cannot play Tone.js synth: not initialized');
@@ -803,7 +809,7 @@ export class AudioEngine {
803809
const noteName = this.toneSynths.semitoneToNoteName(semitone);
804810
// Convert absolute Web Audio time to safe Tone.js relative time
805811
const toneTime = this.toToneRelativeTime(time);
806-
this.toneSynths.playNote(presetName, noteName, duration, toneTime);
812+
this.toneSynths.playNote(presetName, noteName, duration, toneTime, volume);
807813
}
808814

809815
/**
@@ -841,12 +847,14 @@ export class AudioEngine {
841847
* @param semitone Semitone offset from C4 (0 = C4, 12 = C5, -12 = C3)
842848
* @param time Absolute Web Audio context time to start playback
843849
* @param duration Note duration in seconds
850+
* @param volume Volume multiplier from P-lock (0-1, default 1)
844851
*/
845852
playAdvancedSynth(
846853
presetName: string,
847854
semitone: number,
848855
time: number,
849-
duration: number = 0.3
856+
duration: number = 0.3,
857+
volume: number = 1
850858
): void {
851859
if (!this.advancedSynth) {
852860
logger.audio.warn('Cannot play advanced synth: not initialized');
@@ -863,7 +871,7 @@ export class AudioEngine {
863871
this.advancedSynth.setPreset(presetName);
864872
// Convert absolute Web Audio time to safe Tone.js relative time
865873
const toneTime = this.toToneRelativeTime(time);
866-
this.advancedSynth.playNoteSemitone(semitone, duration, toneTime);
874+
this.advancedSynth.playNoteSemitone(semitone, duration, toneTime, volume);
867875
}
868876

869877
/**

‎app/src/audio/scheduler.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -291,13 +291,13 @@ export class Scheduler {
291291
} else {
292292
// Convert semitone offset to MIDI note (C4 = 60 is our reference)
293293
const midiNote = 60 + pitchSemitones;
294-
logger.audio.log(`Playing sampled ${preset} at step ${trackStep}, time ${time.toFixed(3)}, midiNote=${midiNote}`);
295-
audioEngine.playSampledInstrument(preset, noteId, midiNote, time, duration * 0.9);
294+
logger.audio.log(`Playing sampled ${preset} at step ${trackStep}, time ${time.toFixed(3)}, midiNote=${midiNote}, vol=${volumeMultiplier}`);
295+
audioEngine.playSampledInstrument(preset, noteId, midiNote, time, duration * 0.9, volumeMultiplier);
296296
}
297297
} else {
298-
// Basic Web Audio synth
299-
logger.audio.log(`Playing synth ${preset} at step ${trackStep}, time ${time.toFixed(3)}, pitch=${pitchSemitones}`);
300-
audioEngine.playSynthNote(noteId, preset, pitchSemitones, time, duration * 0.9);
298+
// Basic Web Audio synth - pass volume P-lock
299+
logger.audio.log(`Playing synth ${preset} at step ${trackStep}, time ${time.toFixed(3)}, pitch=${pitchSemitones}, vol=${volumeMultiplier}`);
300+
audioEngine.playSynthNote(noteId, preset, pitchSemitones, time, duration * 0.9, volumeMultiplier);
301301
}
302302
} else if (track.sampleId.startsWith('tone:')) {
303303
// Tone.js synth (FM, AM, Membrane, Metal, etc.)
@@ -306,9 +306,10 @@ export class Scheduler {
306306
logger.audio.warn(`Tone.js not ready, skipping ${track.sampleId} at step ${trackStep}`);
307307
} else {
308308
const preset = track.sampleId.replace('tone:', '');
309-
logger.audio.log(`Playing Tone.js ${preset} at step ${trackStep}, time ${time.toFixed(3)}, pitch=${pitchSemitones}`);
309+
logger.audio.log(`Playing Tone.js ${preset} at step ${trackStep}, time ${time.toFixed(3)}, pitch=${pitchSemitones}, vol=${volumeMultiplier}`);
310310
// Phase 22: Pass absolute time - audioEngine handles Tone.js conversion internally
311-
audioEngine.playToneSynth(preset as Parameters<typeof audioEngine.playToneSynth>[0], pitchSemitones, time, duration * 0.9);
311+
// Phase 25: Pass volume P-lock
312+
audioEngine.playToneSynth(preset as Parameters<typeof audioEngine.playToneSynth>[0], pitchSemitones, time, duration * 0.9, volumeMultiplier);
312313
}
313314
} else if (track.sampleId.startsWith('advanced:')) {
314315
// Advanced dual-oscillator synth
@@ -317,9 +318,10 @@ export class Scheduler {
317318
logger.audio.warn(`Advanced synth not ready, skipping ${track.sampleId} at step ${trackStep}`);
318319
} else {
319320
const preset = track.sampleId.replace('advanced:', '');
320-
logger.audio.log(`Playing Advanced ${preset} at step ${trackStep}, time ${time.toFixed(3)}, pitch=${pitchSemitones}`);
321+
logger.audio.log(`Playing Advanced ${preset} at step ${trackStep}, time ${time.toFixed(3)}, pitch=${pitchSemitones}, vol=${volumeMultiplier}`);
321322
// Phase 22: Pass absolute time - audioEngine handles Tone.js conversion internally
322-
audioEngine.playAdvancedSynth(preset, pitchSemitones, time, duration * 0.9);
323+
// Phase 25: Pass volume P-lock
324+
audioEngine.playAdvancedSynth(preset, pitchSemitones, time, duration * 0.9, volumeMultiplier);
323325
}
324326
} else if (track.sampleId.startsWith('sampled:')) {
325327
// Sampled instrument (e.g., piano with real audio samples)
@@ -335,9 +337,9 @@ export class Scheduler {
335337
audioEngine.playSampledInstrument(instrumentId, noteId, midiNote, time, duration * 0.9, volumeMultiplier);
336338
}
337339
} else {
338-
// Sample-based playback
340+
// Sample-based playback - pass volume P-lock (Phase 25 fix)
339341
logger.audio.log(`Playing ${track.sampleId} at step ${trackStep}, time ${time.toFixed(3)}, pitch=${pitchSemitones}, vol=${volumeMultiplier}`);
340-
audioEngine.playSample(track.sampleId, track.id, time, duration * 0.9, track.playbackMode, pitchSemitones);
342+
audioEngine.playSample(track.sampleId, track.id, time, duration * 0.9, track.playbackMode, pitchSemitones, volumeMultiplier);
341343
}
342344

343345
// Reset volume after a short delay (hacky but works for now)

‎app/src/audio/synth.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -625,16 +625,18 @@ export class SynthEngine {
625625
* @param params - Synth parameters
626626
* @param time - AudioContext time to start
627627
* @param duration - Optional duration (for sequenced notes)
628+
* @param volume - Volume multiplier from P-lock (0-1, default 1)
628629
*/
629630
playNote(
630631
noteId: string,
631632
frequency: number,
632633
params: SynthParams,
633634
time: number,
634-
duration?: number
635+
duration?: number,
636+
volume: number = 1
635637
): void {
636638
// DEBUG: Log entry to verify method is being called
637-
logger.audio.log(`SynthEngine.playNote: noteId=${noteId}, freq=${frequency.toFixed(1)}Hz, time=${time.toFixed(3)}, duration=${duration}`);
639+
logger.audio.log(`SynthEngine.playNote: noteId=${noteId}, freq=${frequency.toFixed(1)}Hz, time=${time.toFixed(3)}, duration=${duration}, vol=${volume}`);
638640

639641
if (!this.audioContext || !this.masterGain) {
640642
logger.audio.error('SynthEngine.playNote: AudioContext or masterGain not initialized!', {
@@ -662,8 +664,8 @@ export class SynthEngine {
662664
}
663665

664666
const voice = new SynthVoice(this.audioContext, this.masterGain, params);
665-
voice.start(frequency, time);
666-
logger.audio.log(`SynthEngine voice created and started: noteId=${noteId}, preset=${params.waveform}, activeVoices=${this.activeVoices.size + 1}`);
667+
voice.start(frequency, time, volume);
668+
logger.audio.log(`SynthEngine voice created and started: noteId=${noteId}, preset=${params.waveform}, vol=${volume}, activeVoices=${this.activeVoices.size + 1}`);
667669

668670
if (duration !== undefined) {
669671
voice.stop(time + duration);
@@ -840,7 +842,7 @@ class SynthVoice {
840842
}
841843
}
842844

843-
start(frequency: number, time: number): void {
845+
start(frequency: number, time: number, volume: number = 1): void {
844846
// Set oscillator 1 frequency
845847
this.oscillator1.frequency.setValueAtTime(frequency, time);
846848

@@ -856,16 +858,18 @@ class SynthVoice {
856858

857859
// === Amplitude Envelope (ADSR) ===
858860
// Using exponential ramps for natural sound (human hearing is logarithmic)
861+
// Volume P-lock scales the envelope peak and sustain levels
859862

860-
// Attack phase
863+
// Attack phase (peak scaled by volume)
864+
const scaledPeak = ENVELOPE_PEAK * volume;
861865
this.gainNode.gain.setValueAtTime(MIN_GAIN_VALUE, time);
862866
this.gainNode.gain.exponentialRampToValueAtTime(
863-
ENVELOPE_PEAK,
867+
Math.max(scaledPeak, MIN_GAIN_VALUE), // Ensure we don't go below min
864868
time + Math.max(this.params.attack, 0.001)
865869
);
866870

867-
// Decay to sustain
868-
const sustainLevel = Math.max(ENVELOPE_PEAK * this.params.sustain, MIN_GAIN_VALUE);
871+
// Decay to sustain (sustain also scaled by volume)
872+
const sustainLevel = Math.max(scaledPeak * this.params.sustain, MIN_GAIN_VALUE);
869873
this.gainNode.gain.exponentialRampToValueAtTime(
870874
sustainLevel,
871875
time + this.params.attack + this.params.decay

‎app/src/audio/toneSynths.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,12 +333,14 @@ export class ToneSynthManager {
333333

334334
/**
335335
* Play a note with the specified preset
336+
* @param volume Volume multiplier from P-lock (0-1, default 1)
336337
*/
337338
playNote(
338339
presetName: ToneSynthType,
339340
note: string | number,
340341
duration: string | number,
341-
time: number
342+
time: number,
343+
volume: number = 1
342344
): void {
343345
if (!this.ready) {
344346
logger.audio.warn('ToneSynthManager not ready');
@@ -378,12 +380,13 @@ export class ToneSynthManager {
378380
// Use try-catch to handle cases where Tone.js internal state rejects the time
379381
// This can happen during rapid BPM changes where Tone.js's StateTimeline
380382
// has events scheduled at later times from previous notes' release phases
383+
// Volume P-lock is passed as velocity (4th param of triggerAttackRelease)
381384
try {
382385
if (preset.type === 'pluck') {
383386
(synth as Tone.PluckSynth).triggerAttack(noteValue, startTime);
384387
} else {
385388
(synth as Tone.FMSynth | Tone.AMSynth | Tone.MembraneSynth | Tone.MetalSynth | Tone.DuoSynth)
386-
.triggerAttackRelease(noteValue, duration, startTime);
389+
.triggerAttackRelease(noteValue, duration, startTime, volume);
387390
}
388391
} catch (_err) {
389392
// If Tone.js rejects the time, retry with current time + buffer
@@ -396,7 +399,7 @@ export class ToneSynthManager {
396399
(synth as Tone.PluckSynth).triggerAttack(noteValue, retryTime);
397400
} else {
398401
(synth as Tone.FMSynth | Tone.AMSynth | Tone.MembraneSynth | Tone.MetalSynth | Tone.DuoSynth)
399-
.triggerAttackRelease(noteValue, duration, retryTime);
402+
.triggerAttackRelease(noteValue, duration, retryTime, volume);
400403
}
401404
} catch (retryErr) {
402405
// If retry also fails, log and skip this note

0 commit comments

Comments
 (0)