From 6e001b7534ddc031a9f177bac124fcea83f898a7 Mon Sep 17 00:00:00 2001 From: Ade Oshineye Date: Wed, 17 Dec 2025 11:58:34 +0000 Subject: [PATCH] docs: Reorganize and update documentation structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documentation reorganization: - Move app/docs/* to docs/ (developer workflow docs) - Move app/VOLUME-VERIFICATION-ANALYSIS.md to specs/research/ - Move docs/SYNTHESIS-ENGINE-ARCHITECTURE.md to specs/ - Move docs/research/MUSICAL-COVERAGE-ANALYSIS.md to specs/research/ - Rename files to UPPERCASE for consistency (lessons-learned.md, instrument-research.md, development-tools.md) Content updates to match MAX_STEPS=128 and STEP_COUNT_OPTIONS changes: - Update CHANGELOG.md with recent additions - Update specs/STATUS.md Phase 4 section - Update specs/ROADMAP.md Phase 18 text - Update specs/MUSICAL-FOUNDATIONS-SUMMARY.md step count options - Fix app/src/types.ts ParameterLock comment (Β±24 semitones) - Update LESSONS-LEARNED.md invariant section (64β†’128) Document cleanup: - Delete stale app/PHASE-21A-AUDIT-REPORT.md (824 lines, bugs now fixed) - Merge valuable patterns from app/docs/implementation-comparison.md into LESSONS-LEARNED.md, then delete - Rewrite specs/research/INSTRUMENT-RESEARCH.md with current inventory (44β†’68 instruments) πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 13 +- app/PHASE-21A-AUDIT-REPORT.md | 823 ------------------ app/docs/implementation-comparison.md | 598 ------------- app/src/types.ts | 2 +- {app/docs => docs}/AUDIO-CONTENT-TOOLS.md | 0 .../DEBUGGING-LESSONS-LEARNED.md | 0 {app/docs => docs}/DEBUGGING-WORKFLOW.md | 0 .../DEVELOPMENT-TOOLS.md | 0 .../LESSONS-LEARNED.md | 203 ++++- specs/MUSICAL-FOUNDATIONS-SUMMARY.md | 10 +- specs/ROADMAP.md | 6 +- specs/STATUS.md | 18 +- .../SYNTHESIS-ENGINE-ARCHITECTURE.md | 2 +- .../research/INSTRUMENT-RESEARCH.md | 300 +++++-- .../research/MUSICAL-COVERAGE-ANALYSIS.md | 11 +- .../research}/VOLUME-VERIFICATION-ANALYSIS.md | 0 16 files changed, 473 insertions(+), 1513 deletions(-) delete mode 100644 app/PHASE-21A-AUDIT-REPORT.md delete mode 100644 app/docs/implementation-comparison.md rename {app/docs => docs}/AUDIO-CONTENT-TOOLS.md (100%) rename {app/docs => docs}/DEBUGGING-LESSONS-LEARNED.md (100%) rename {app/docs => docs}/DEBUGGING-WORKFLOW.md (100%) rename app/docs/development-tools.md => docs/DEVELOPMENT-TOOLS.md (100%) rename app/docs/lessons-learned.md => docs/LESSONS-LEARNED.md (94%) rename {docs => specs}/SYNTHESIS-ENGINE-ARCHITECTURE.md (99%) rename app/docs/instrument-research.md => specs/research/INSTRUMENT-RESEARCH.md (57%) rename {docs => specs}/research/MUSICAL-COVERAGE-ANALYSIS.md (99%) rename {app => specs/research}/VOLUME-VERIFICATION-ANALYSIS.md (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1c4fea6..ed69ba18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,14 +8,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Planned -- Phase 16: Authentication & session ownership -- Phase 17: Advanced synthesis engine -- Phase 26: Performance optimization +- Phase 24: Authentication & session ownership +- Phase 26: Shared sample recording +- Performance optimization - Named Tone.js imports (tree-shaking) - Code splitting for audio modules - Lazy audio engine loading with feature flag - Future: Euclidean rhythms, per-track swing, conditional triggers +### Recently Added (since 0.2.0) +- Extended MAX_STEPS from 64 to 128 (8 bars at 16th note resolution) +- Added step count options: 96, 128 for full verse/chorus sections +- 6 new demo sessions showcasing all instrument categories +- LRU sample cache with reference counting for memory management +- Effects master bypass and XY pad controls + ## [0.2.0] - 2025-12-11 ### Added diff --git a/app/PHASE-21A-AUDIT-REPORT.md b/app/PHASE-21A-AUDIT-REPORT.md deleted file mode 100644 index 425a0669..00000000 --- a/app/PHASE-21A-AUDIT-REPORT.md +++ /dev/null @@ -1,823 +0,0 @@ -# Phase 21A Implementation Audit Report - -**Date:** 2025-12-15 -**Auditor:** Claude (Automated Code Audit) -**Scope:** Phase 21A eager piano loading, dependency injection, and observable state pattern - ---- - -## Executive Summary - -**Overall Status:** ⚠️ **CRITICAL ISSUES FOUND - REQUIRES IMMEDIATE ATTENTION** - -The Phase 21A implementation introduces **2 critical issues** and **3 medium-severity issues** that could break the app in production, particularly on mobile devices and slow networks. - -### Critical Issues (Must Fix): -1. **UI Blocking During Piano Load** - 1-2 second freeze on 3G connections -2. **Race Condition in Session Loading** - Piano may not be ready when user hits play - -### Medium Issues (Should Fix): -3. Error handling gaps in piano loading -4. Missing loading indicator for user feedback -5. No retry mechanism for failed piano loads - ---- - -## 1. Initialization Order Issues - -### πŸ”΄ CRITICAL: UI Blocking During Eager Piano Load - -**File:** `/Users/ade/Documents/projects/tmp/keyboardia/app/src/audio/engine.ts` -**Lines:** 105-110 - -#### The Problem - -Piano samples are loaded **synchronously during `initialize()`** which is triggered by user interaction: - -```typescript -// Line 105-110 in engine.ts -logger.audio.log('[PRELOAD] Eagerly loading piano samples...'); -await this._sampledInstrumentRegistry.load('piano'); // ⚠️ BLOCKS HERE -``` - -**Impact:** -- On 3G: ~1.2 second UI freeze (per spec: "C4 loads in ~1.2s on 3G") -- Total piano size: 484KB across 4 files -- **User sees:** Unresponsive button after clicking Play - -**Call Chain:** -``` -User clicks Play - β†’ StepSequencer.handlePlayPause() - β†’ initAudio() - β†’ audioEngine.initialize() - β†’ sampledInstrumentRegistry.load('piano') // ← BLOCKS HERE for 1.2s - β†’ [UI frozen until samples loaded] -``` - -#### Why This Is Critical - -1. **First-time user experience:** The very first interaction freezes the app -2. **Mobile Chrome/Safari:** Browsers may interpret this as "unresponsive" and show warning -3. **Spec violation:** Spec says "progressive loading ensures C4 is ready quickly" but this blocks until ALL initialization completes - -#### Recommended Fix - -**Option A: Make piano load non-blocking** -```typescript -// Start piano load but don't await it -this._sampledInstrumentRegistry.load('piano'); // Fire and forget -logger.audio.log('[PRELOAD] Piano loading started (non-blocking)...'); -``` - -**Option B: Show loading state** -```typescript -// In StepSequencer.tsx -const [audioLoading, setAudioLoading] = useState(false); - -const initAudio = useCallback(async () => { - if (!audioEngine.isInitialized()) { - setAudioLoading(true); - await audioEngine.initialize(); - setAudioLoading(false); - } -}, []); - -// Render loading spinner while audioLoading === true -``` - -**Option C: Defer piano load to idle time** -```typescript -// Load piano after a short delay to avoid blocking first interaction -setTimeout(() => { - this._sampledInstrumentRegistry.load('piano'); -}, 100); -``` - ---- - -### πŸ”΄ CRITICAL: Race Condition in Session Loading - -**File:** `/Users/ade/Documents/projects/tmp/keyboardia/app/src/hooks/useSession.ts` -**Lines:** 109-113 - -#### The Problem - -Session loads tracks and then starts piano preloading, but **does not wait for it to complete**: - -```typescript -// Line 109-113 in useSession.ts -loadState(gridState.tracks, gridState.tempo, gridState.swing); - -// Preload any sampled instruments used by tracks (e.g., piano) -// This ensures they're ready before user hits play -audioEngine.preloadInstrumentsForTracks(gridState.tracks); // ⚠️ NOT AWAITED! -``` - -**Race Condition Timeline:** -``` -T+0ms: loadState() called (synchronous) -T+0ms: preloadInstrumentsForTracks() called (async, not awaited) -T+50ms: User sees session loaded, clicks Play -T+100ms: Piano still loading... -T+150ms: scheduler.start() begins -T+200ms: First piano note scheduled -T+250ms: ❌ Piano not ready β†’ note skipped (silent) -T+1200ms: Piano finishes loading (too late) -``` - -**User Impact:** -- Session loads from URL -- User immediately clicks Play -- **First 1-2 seconds of piano notes are SILENT** -- No error message, just silent playback - -#### Why This Is Critical - -1. **Breaks "everyone hears the same music"** - Different users get different results based on network speed -2. **Silent failures** - User doesn't know why piano isn't playing -3. **Inconsistent with spec** - Spec says "piano should be preloaded during init, so this should always succeed" - -#### Recommended Fix - -```typescript -// Option A: Await the preload -await audioEngine.preloadInstrumentsForTracks(gridState.tracks); -loadingStateRef.current = 'ready'; -setStatus('ready'); - -// Option B: Check ready state before allowing play -// In StepSequencer.tsx -const handlePlayPause = useCallback(async () => { - await initAudio(); - - // NEW: Ensure piano is ready if used - const hasPiano = state.tracks.some(t => t.sampleId === 'synth:piano'); - if (hasPiano) { - const instrument = sampledInstrumentRegistry.get('piano'); - if (instrument && !instrument.isReady()) { - logger.audio.warn('Piano not ready yet, waiting...'); - await sampledInstrumentRegistry.load('piano'); - } - } - - // ... rest of playback code -}, [state.tracks, ...]); -``` - ---- - -## 2. Error Handling Gaps - -### 🟑 MEDIUM: No Error Recovery for Piano Load Failure - -**File:** `/Users/ade/Documents/projects/tmp/keyboardia/app/src/audio/engine.ts` -**Lines:** 110 - -#### The Problem - -If piano samples fail to load during initialization, the error is caught but the app continues silently: - -```typescript -// In SampledInstrumentRegistry.load() -try { - const success = await instrument.ensureLoaded(); - if (success) { - this.setState(instrumentId, 'ready'); - } else { - this.setState(instrumentId, 'error', new Error('Failed to load instrument')); - } - return success; -} catch (error) { - const err = error instanceof Error ? error : new Error(String(error)); - this.setState(instrumentId, 'error', err); - return false; // ⚠️ Error logged, app continues -} -``` - -**What Happens:** -1. Piano samples fail to load (network error, 404, etc.) -2. `load('piano')` returns `false` -3. No UI feedback -4. User clicks Play -5. Piano notes are silently skipped -6. **User has no idea piano is broken** - -#### Impact - -- Network failures on mobile -- CDN/R2 outages -- Corrupted manifest files -- 404s if samples are missing - -All result in **silent failure** with no user feedback. - -#### Recommended Fix - -```typescript -// In engine.ts initialize() -logger.audio.log('[PRELOAD] Eagerly loading piano samples...'); -const pianoLoaded = await this._sampledInstrumentRegistry.load('piano'); - -if (!pianoLoaded) { - logger.audio.error('[PRELOAD] Piano failed to load - falling back to synth'); - // Could show toast notification to user - // Or set a flag to show warning in UI -} -``` - -**Better: Add retry logic** -```typescript -// In SampledInstrumentRegistry -async loadWithRetry(instrumentId: string, maxRetries = 3): Promise { - for (let attempt = 0; attempt < maxRetries; attempt++) { - const success = await this.load(instrumentId); - if (success) return true; - - if (attempt < maxRetries - 1) { - const delay = Math.min(1000 * Math.pow(2, attempt), 5000); - logger.audio.log(`Retry ${attempt + 1}/${maxRetries} after ${delay}ms...`); - await new Promise(resolve => setTimeout(resolve, delay)); - } - } - return false; -} -``` - ---- - -### 🟑 MEDIUM: Missing Loading Indicator - -**Files:** All components that call `audioEngine.initialize()` - -#### The Problem - -There's **no visual feedback** during the 1-2 second piano load: - -**Current UX:** -``` -User: *clicks Play button* -App: *freezes for 1.2 seconds* -App: *starts playing* -``` - -**Expected UX:** -``` -User: *clicks Play button* -App: "Loading sounds..." (spinner) -App: *starts playing* -``` - -#### Locations Without Feedback - -1. **StepSequencer.tsx** - Play button -2. **SamplePicker.tsx** - Sample preview -3. **Recorder.tsx** - Recording initialization - -#### Recommended Fix - -**Add loading state to Transport component:** - -```typescript -// In Transport.tsx -interface TransportProps { - // ... existing props - audioLoading?: boolean; -} - -// Render - -``` - ---- - -## 3. Untested Code Paths - -### 🟑 MEDIUM: AudioContext Suspended During Initialize - -**Missing Test:** What happens if `audioContext.state === 'suspended'` when piano samples start loading? - -```typescript -// Current code in engine.ts:54-67 -this.audioContext = new AudioContextClass(); - -// Resume if suspended -if (this.audioContext.state === 'suspended' || ...) { - await this.audioContext.resume(); -} - -// ... later at line 110 -await this._sampledInstrumentRegistry.load('piano'); -// ⚠️ What if AudioContext is STILL suspended? -``` - -**Potential Issue:** -- `resume()` is asynchronous and may take time on iOS -- Piano samples could start loading while AudioContext is still "suspended" -- Sample decoding might fail or behave unexpectedly - -#### Recommended Test - -```typescript -// In sampled-instrument-integration.test.ts -describe('AudioContext state during load', () => { - it('should handle suspended AudioContext during piano load', async () => { - const mockContext = { - state: 'suspended', // ← Start suspended - resume: vi.fn(() => { - mockContext.state = 'running'; - return Promise.resolve(); - }), - // ... other mocks - }; - - const instrument = new SampledInstrument('piano', '/instruments'); - instrument.initialize(mockContext, mockGainNode); - - const loaded = await instrument.ensureLoaded(); - - expect(loaded).toBe(true); - expect(mockContext.state).toBe('running'); - }); -}); -``` - ---- - -### Testing Gap: Concurrent initialize() Calls - -**Missing Test:** What happens if multiple components call `audioEngine.initialize()` simultaneously? - -```typescript -// Current protection in engine.ts:54-55 -async initialize(): Promise { - if (this.initialized) return; // ⚠️ Check before async work - - this.audioContext = new AudioContextClass(); - // ... 50 lines of async operations ... - this.initialized = true; // Set flag at end -} -``` - -**Race Condition:** -``` -Call A: Check initialized (false) βœ“ -Call B: Check initialized (false) βœ“ // ⚠️ A hasn't set flag yet -Call A: Start creating AudioContext -Call B: Start creating AudioContext // ⚠️ Duplicate! -Call A: Load piano samples -Call B: Load piano samples // ⚠️ Duplicate! -``` - -#### Recommended Fix - -```typescript -private initializePromise: Promise | null = null; - -async initialize(): Promise { - if (this.initialized) return; - if (this.initializePromise) return this.initializePromise; - - this.initializePromise = this._initialize(); - await this.initializePromise; - this.initializePromise = null; -} - -private async _initialize(): Promise { - // ... actual initialization logic -} -``` - ---- - -## 4. Spec Compliance Review - -### βœ… PASS: No Synth Fallback for Sampled Instruments - -**Spec Requirement:** -> "Sampled instruments NEVER fall back to synth (was: synth fallback while loading)" - -**Implementation:** `engine.ts:236-250` - -```typescript -if (instrument) { - if (instrument.isReady()) { - instrument.playNote(noteId, midiNote, time, duration); - } else { - // If somehow not ready, silently skip rather than play wrong sound - logger.audio.warn(`[SKIP] ${presetName} not ready...`); - } - return; // Always return for sampled instruments - never use synth fallback -} -``` - -βœ… **Verified:** No code path leads to synth fallback when piano is selected. - ---- - -### ⚠️ PARTIAL PASS: Eager Loading Consistency - -**Spec Statement:** -> "Piano samples now load EAGERLY during AudioEngine.initialize() (was lazy)" - -**Implementation:** -- βœ… Piano loads during `initialize()` -- ⚠️ But `preloadInstrumentsForTracks()` also loads piano (redundant?) -- ⚠️ Comments in `sampled-instrument.ts:8` still say "Lazy loading" - -**Documentation Inconsistency:** - -```typescript -// Line 8 in sampled-instrument.ts -/** - * Key design decisions: - * - Lazy loading: samples load on first use, not at startup // ⚠️ WRONG - */ -``` - -Should be: -```typescript -/** - * Key design decisions: - * - Progressive loading: C4 loads first, rest load in background - * - Piano preloads during initialize(), other instruments load on demand - */ -``` - ---- - -### βœ… PASS: Dependency Injection - -**Spec Requirement:** -> "Added dependency injection to AudioEngine constructor" - -**Implementation:** `engine.ts:45-52` - -```typescript -constructor(deps?: AudioEngineDependencies) { - this._sampledInstrumentRegistry = deps?.sampledInstrumentRegistry ?? sampledInstrumentRegistry; - this._synthEngine = deps?.synthEngine ?? synthEngine; -} -``` - -βœ… **Verified:** Tests use this for mocking (`note-player.test.ts:114-130`) - ---- - -### βœ… PASS: Observable State Pattern - -**Spec Requirement:** -> "Added observable state pattern to SampledInstrumentRegistry" - -**Implementation:** `sampled-instrument.ts:373-523` - -```typescript -export type InstrumentState = 'idle' | 'loading' | 'ready' | 'error'; - -export class SampledInstrumentRegistry { - private states: Map = new Map(); - private errors: Map = new Map(); - private listeners: Set = new Set(); - - getState(instrumentId: string): InstrumentState { ... } - getError(instrumentId: string): Error | null { ... } - onStateChange(callback: StateChangeCallback): () => void { ... } - retry(id: string): Promise { ... } -} -``` - -βœ… **Verified:** Full state machine implementation with listeners. - -⚠️ **BUT:** No UI component currently uses this! The observable state pattern is implemented but not integrated with the UI. - ---- - -## 5. Start/Stop Button Functionality - -### βœ… VERIFIED: Button Still Works - -**Test Flow:** -``` -StepSequencer.handlePlayPause() - β†’ initAudio() - β†’ audioEngine.initialize() (only once, cached) - β†’ audioEngine.ensureAudioReady() - β†’ scheduler.start() or scheduler.stop() -``` - -**Potential Issues:** -- ⚠️ First click may freeze UI for 1.2s (piano loading) -- βœ… Subsequent clicks work normally (cached) -- βœ… Audio context resume handled correctly - ---- - -## 6. Check Past Debugging Lessons - -### βœ… No Regression: Memory Leaks - -**Lesson:** "Memory Leaks in Web Audio" (docs/lessons-learned.md:225-272) - -**Check:** Do sampled instruments clean up properly? - -```typescript -// In sampled-instrument.ts:304-307 -source.onended = () => { - source.disconnect(); - gainNode.disconnect(); -}; -``` - -βœ… **PASS:** Cleanup implemented correctly. - ---- - -### βœ… No Regression: Audio Chain Immutability - -**Lesson:** "The master audio chain must be immutable after initialization" (SYNTHESIS-ENHANCEMENT.md:514-526) - -**Check:** Is masterGain reference stable? - -```typescript -// In sampled-instrument.ts:77-89 -initialize(audioContext: AudioContext, destination: AudioNode): void { - this.audioContext = audioContext; - this.destination = destination; // βœ… Stored once, never changed - - logger.audio.log(`SampledInstrument initialized with destination:`, { ... }); -} - -// In playNote() at line 285 -gainNode.connect(this.destination!); // βœ… Uses stored reference -``` - -βœ… **PASS:** No reconnection, stable reference. - ---- - -### ⚠️ POTENTIAL ISSUE: iOS Audio Unlock - -**Lesson:** "iOS Audio: No Sound Despite Animation" (lessons-learned.md:624-740) - -**Current Implementation:** - -```typescript -// In engine.ts:160-179 -async ensureAudioReady(): Promise { - if (!this.audioContext) return false; - - const state = this.audioContext.state as string; - if (state === 'suspended' || state === 'interrupted') { - await this.audioContext.resume(); - } - - return this.audioContext.state === 'running'; -} -``` - -**Concern:** Piano loads during `initialize()` which happens BEFORE `ensureAudioReady()` is called. - -**Timeline on iOS:** -``` -1. User clicks Play -2. initialize() called - β†’ AudioContext created (state: suspended) - β†’ Resume attempted (may take 100ms) - β†’ Piano loading starts IMMEDIATELY - β†’ State might still be 'suspended' during decode -3. ensureAudioReady() called (too late?) -``` - -**Recommendation:** Ensure AudioContext is fully running before loading piano: - -```typescript -async initialize(): Promise { - if (this.initialized) return; - - this.audioContext = new AudioContextClass(); - - // Wait for running state before proceeding - if (this.audioContext.state !== 'running') { - await this.audioContext.resume(); - // Give iOS time to fully resume - await new Promise(resolve => setTimeout(resolve, 50)); - } - - // Now safe to load samples - await this._sampledInstrumentRegistry.load('piano'); - // ... -} -``` - ---- - -## 7. Verification Script (Recommended) - -### Proposed Test Script - -Create `/Users/ade/Documents/projects/tmp/keyboardia/app/scripts/verify-phase-21a.ts`: - -```typescript -/** - * Phase 21A Verification Script - * - * Verifies: - * 1. Piano is ready before first note can play - * 2. Start/stop button still works - * 3. Session playback works correctly - * 4. Error handling for piano load failure - */ - -import { audioEngine } from '../src/audio/engine'; -import { sampledInstrumentRegistry } from '../src/audio/sampled-instrument'; -import { logger } from '../src/utils/logger'; - -async function verifyPhase21A() { - console.log('=== Phase 21A Verification ===\n'); - - // Test 1: Initialize and check piano - console.log('Test 1: Piano preloading...'); - const startTime = Date.now(); - await audioEngine.initialize(); - const loadTime = Date.now() - startTime; - - const piano = sampledInstrumentRegistry.get('piano'); - if (!piano) { - console.error('❌ FAIL: Piano not registered'); - return false; - } - - if (!piano.isReady()) { - console.error('❌ FAIL: Piano not ready after initialize()'); - return false; - } - - console.log(`βœ… PASS: Piano ready in ${loadTime}ms`); - - // Test 2: Verify piano plays (not synth) - console.log('\nTest 2: Piano playback...'); - const noteSource = piano.playNote('test', 60, 0, 0.5, 1); - if (!noteSource) { - console.error('❌ FAIL: Piano playNote returned null'); - return false; - } - console.log('βœ… PASS: Piano plays correctly'); - - // Test 3: Audio chain verification - console.log('\nTest 3: Audio chain integrity...'); - const chainStatus = audioEngine.verifyAudioChain(); - if (!chainStatus.valid) { - console.error('❌ FAIL: Audio chain broken:', chainStatus.issues); - return false; - } - console.log('βœ… PASS: Audio chain intact'); - - // Test 4: Observable state - console.log('\nTest 4: Observable state pattern...'); - const state = sampledInstrumentRegistry.getState('piano'); - if (state !== 'ready') { - console.error(`❌ FAIL: Piano state is '${state}', expected 'ready'`); - return false; - } - console.log('βœ… PASS: Observable state working'); - - console.log('\n=== All Tests Passed βœ… ==='); - return true; -} - -verifyPhase21A().catch(console.error); -``` - ---- - -## Summary of Findings - -### Critical (Must Fix Before Production) - -| Issue | Severity | Impact | File | Fix Priority | -|-------|----------|--------|------|--------------| -| UI blocking during piano load | πŸ”΄ Critical | 1-2s freeze on first Play click | `engine.ts:110` | **P0 - Immediate** | -| Race condition in session load | πŸ”΄ Critical | Silent piano notes on slow networks | `useSession.ts:113` | **P0 - Immediate** | - -### Medium (Should Fix Soon) - -| Issue | Severity | Impact | File | Fix Priority | -|-------|----------|--------|------|--------------| -| No error recovery | 🟑 Medium | Silent failure if piano doesn't load | `sampled-instrument.ts:453-473` | **P1 - High** | -| Missing loading indicator | 🟑 Medium | Poor UX, user confusion | Multiple components | **P1 - High** | -| AudioContext state during load | 🟑 Medium | Potential iOS issues | `engine.ts:110` | **P2 - Medium** | - -### Low (Technical Debt) - -| Issue | Severity | Impact | File | Fix Priority | -|-------|----------|--------|------|--------------| -| Documentation inconsistency | 🟒 Low | Comments say "lazy", code is eager | `sampled-instrument.ts:8` | **P3 - Low** | -| Unused observable state | 🟒 Low | Code implemented but not integrated | Registry not used by UI | **P3 - Low** | -| Missing concurrent init test | 🟒 Low | Edge case not tested | Test suite | **P3 - Low** | - ---- - -## Recommended Action Plan - -### Phase 1: Critical Fixes (Do Today) - -1. **Make piano load non-blocking** - - Move `await` to after UI is ready - - Add loading state management - - Ensure play button waits for piano if needed - -2. **Fix session load race condition** - - Await `preloadInstrumentsForTracks()` - - Or add ready check before play - -### Phase 2: Error Handling (Do This Week) - -3. **Add error recovery** - - Retry logic for piano load failures - - User-facing error messages - - Graceful degradation - -4. **Add loading indicators** - - Spinner on first Play click - - Status text "Loading piano..." - - Disable button during load - -### Phase 3: Testing & Documentation (Next Sprint) - -5. **Add missing tests** - - Suspended AudioContext during load - - Concurrent initialize() calls - - Piano load failure scenarios - -6. **Update documentation** - - Fix "lazy loading" comments - - Document observable state pattern usage - - Add verification script to CI - ---- - -## Test Coverage Analysis - -**Current Test Stats:** -- 869 tests passing βœ… -- Sampled instrument tests: 23 tests -- Integration tests: 8 tests -- Engine tests: 10 tests - -**Missing Coverage:** -- ❌ UI blocking scenario -- ❌ Race condition in session load -- ❌ Error recovery flows -- ❌ Concurrent initialization -- ❌ AudioContext state transitions - -**Recommendation:** Add 15-20 tests covering edge cases before marking Phase 21A as complete. - ---- - -## Conclusion - -Phase 21A successfully implements the core features (eager loading, dependency injection, observable state), but has **2 critical production-blocking issues** that must be fixed: - -1. **UI freezes for 1-2 seconds** on first interaction (bad first impression) -2. **Piano notes silently fail** when loading sessions (breaks user expectations) - -Both issues are fixable with small code changes, but they represent significant UX problems that could damage user trust in the product. - -**Recommendation:** Fix critical issues before deploying to production. Consider the medium-priority fixes as "nice to have" for the next release. - -**Estimated Fix Time:** -- Critical fixes: 2-4 hours -- Medium priority: 4-8 hours -- Testing & documentation: 4-6 hours -- **Total: 10-18 hours** to complete Phase 21A properly - ---- - -## Appendix: Related Files - -### Modified in Phase 21A -- `src/audio/engine.ts` - Eager piano loading, dependency injection -- `src/audio/sampled-instrument.ts` - Observable state pattern -- `src/hooks/useSession.ts` - Session preloading -- `src/components/SamplePicker.tsx` - Sample selection preloading - -### Requires Changes (Based on Findings) -- `src/components/Transport.tsx` - Add loading indicator -- `src/components/StepSequencer.tsx` - Add audio loading state -- `src/audio/sampled-instrument.ts` - Add retry logic -- Test files - Add missing coverage - -### Reference Documentation -- `/Users/ade/Documents/projects/tmp/keyboardia/specs/SYNTHESIS-ENHANCEMENT.md` -- `/Users/ade/Documents/projects/tmp/keyboardia/app/docs/lessons-learned.md` diff --git a/app/docs/implementation-comparison.md b/app/docs/implementation-comparison.md deleted file mode 100644 index cb78dd65..00000000 --- a/app/docs/implementation-comparison.md +++ /dev/null @@ -1,598 +0,0 @@ -# Implementation Comparison: Old vs New Audio Architecture - -## Executive Summary - -This document compares the pre-Phase 21A "known good" implementation with the post-Phase 21A implementation after bug fixes. It extracts lessons learned and considers how they would inform a Tone.js-based alternative. - -**Key Insight:** The transition from in-memory synthesis to network-loaded samples fundamentally changed timing assumptions, exposing latent bugs that "worked by accident" in the old code. - ---- - -## Part 1: Architecture Comparison - -### Old Implementation (Pre-Phase 21A) - -``` -β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” -β”‚ AudioEngine β”‚ -β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ -β”‚ initialize() β”‚ -β”‚ β”œβ”€ Create AudioContext β”‚ -β”‚ β”œβ”€ Create masterGain β†’ compressor β†’ destination β”‚ -β”‚ β”œβ”€ synthEngine.initialize() β”‚ -β”‚ β”œβ”€ createSynthesizedSamples() ← ~50ms (in-memory) β”‚ -β”‚ β”œβ”€ this.initialized = true β”‚ -β”‚ └─ attachUnlockListeners() β”‚ -β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ -β”‚ playSynthNote(noteId, preset, semitone, time, dur) β”‚ -β”‚ └─ synthEngine.playNote() ← Always uses synth β”‚ -β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ - -Total init time: ~50ms (all in-memory generation) -``` - -**Characteristics:** -- Simple, linear initialization -- No network requests -- All sounds generated from oscillators -- No routing logic in playSynthNote() -- ~200 lines total - -### New Implementation (Post-Phase 21A) - -``` -β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” -β”‚ AudioEngine β”‚ -β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ -β”‚ constructor(deps?) β”‚ -β”‚ β”œβ”€ Dependency injection for testability β”‚ -β”‚ └─ _pendingPreloads = Set β”‚ -β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ -β”‚ initialize() β”‚ -β”‚ β”œβ”€ Guard: if (_initializePromise) return it β”‚ -β”‚ └─ _doInitialize() β”‚ -β”‚ β”œβ”€ Create AudioContext β”‚ -β”‚ β”œβ”€ Create masterGain β†’ compressor β†’ destination β”‚ -β”‚ β”œβ”€ synthEngine.initialize() β”‚ -β”‚ β”œβ”€ sampledInstrumentRegistry.initialize() β”‚ -β”‚ β”œβ”€ Register sampled instruments (piano, etc.) β”‚ -β”‚ β”œβ”€ createSynthesizedSamples() ← ~50ms β”‚ -β”‚ β”œβ”€ Load piano (await C4) ← ~300-500ms (NETWORK) β”‚ -β”‚ β”œβ”€ this.initialized = true β”‚ -β”‚ β”œβ”€ attachUnlockListeners() β”‚ -β”‚ └─ Load pending preloads (background) β”‚ -β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ -β”‚ preloadInstrumentsForTracks(tracks) β”‚ -β”‚ β”œβ”€ If not initialized: store in _pendingPreloads β”‚ -β”‚ └─ If initialized: _loadSampledInstruments() β”‚ -β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ -β”‚ playSynthNote(noteId, preset, semitone, time, dur) β”‚ -β”‚ β”œβ”€ Check: is this a sampled instrument? β”‚ -β”‚ β”‚ β”œβ”€ YES: use sampledInstrumentRegistry β”‚ -β”‚ β”‚ β”‚ └─ NEVER fall back to synth β”‚ -β”‚ β”‚ └─ NO: use synthEngine β”‚ -β”‚ └─ Route to appropriate player β”‚ -β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ - -Total init time: ~350-550ms (network-dependent) -``` - -**Characteristics:** -- Complex initialization with guards -- Network requests for samples -- Mix of synthesized and sampled sounds -- Routing logic in playSynthNote() -- Dependency injection for testing -- Observable state pattern for loading -- ~740 lines total - ---- - -## Part 2: What Changed and Why - -### Change 1: Concurrent Initialization Guard - -**Old Code:** -```typescript -async initialize(): Promise { - if (this.initialized) return; - // ... rest of init -} -``` - -**New Code:** -```typescript -async initialize(): Promise { - if (this.initialized) return; - if (this._initializePromise) { - return this._initializePromise; // Wait for in-progress init - } - this._initializePromise = this._doInitialize(); - // ... -} -``` - -**Why:** Multiple callers (Play button, preload, preview) could call initialize() simultaneously. Without the guard, each would create a new AudioContext, causing resource leaks and race conditions. - -### Change 2: Pending Preloads Queue - -**Old Code:** No preloading concept - everything was in-memory. - -**New Code:** -```typescript -preloadInstrumentsForTracks(tracks): Promise { - if (!this.initialized) { - // Store for later - can't load until user gesture - this._pendingPreloads.add(presetName); - return; - } - // Actually load -} -``` - -**Why:** useSession calls preloadInstrumentsForTracks() during page load, BEFORE user clicks anything. Web Audio requires user gesture to create AudioContext. Queue requests and fulfill after initialize(). - -### Change 3: Piano Awaited During Init - -**Old Code:** N/A - -**New Code:** -```typescript -// CRITICAL INVARIANT: After initialize() returns, piano must be ready. -this._pianoLoadPromise = this._sampledInstrumentRegistry.load('piano'); -await this._pianoLoadPromise; -``` - -**Why:** If scheduler starts before piano loads, piano notes are silent. Must guarantee piano ready before returning from initialize(). - -### Change 4: No Synth Fallback for Sampled Instruments - -**Old Code:** N/A (all synth) - -**New Code:** -```typescript -if (instrument.isReady()) { - instrument.playNote(...); -} else { - // INVARIANT VIOLATION - log error, skip note - // DO NOT fall back to synth -} -return; // Always return for sampled instruments -``` - -**Why:** If piano plays synth sound as "fallback," users hear wrong sound. Better to be silent (with error log) than to confuse users. - ---- - -## Part 3: The Timing Bug Deep Dive - -### The Latent Bug - -Both old and new code had this bug in SamplePicker.tsx: - -```typescript -const handlePreview = useCallback(async (sampleId: string) => { - if (!audioEngine.isInitialized()) { - await audioEngine.initialize(); // BUG: Called from mouseenter! - } - audioEngine.playNow(sampleId); -}, []); -``` - -`mouseenter` is NOT a valid user gesture for AudioContext creation. - -### Why Old Code Worked Anyway - -``` -OLD CODE TIMELINE: -0ms: mouseenter fires β†’ initialize() starts -0ms: AudioContext created (suspended) -50ms: createSynthesizedSamples() completes (FAST) -50ms: attachUnlockListeners() adds click handler -500ms: User clicks Play -500ms: Document click handler fires FIRST -500ms: resume() SUCCEEDS (within user gesture) -500ms: handlePlayPause runs with running context -RESULT: WORKS -``` - -### Why New Code Broke - -``` -NEW CODE TIMELINE: -0ms: mouseenter fires β†’ initialize() starts -0ms: AudioContext created (suspended) -50ms: createSynthesizedSamples() completes -50ms: Piano loading starts (network fetch) -300ms: User clicks Play BEFORE piano loads -300ms: handlePlayPause awaits initialize() (blocked on piano) -500ms: Piano loads, attachUnlockListeners() called (TOO LATE) -500ms: handlePlayPause continues -500ms: User gesture expired (~100-300ms timeout) -500ms: resume() fails or context stays suspended -RESULT: SILENT -``` - -### Why Second Load Worked - -Browser HTTP cache serves piano samples in ~10ms instead of ~450ms: -- Total init time: ~60ms (not ~500ms) -- Unlock listeners ready before user clicks -- Same timing as old code - -### The Fix - -Don't call initialize() from non-gesture contexts: - -```typescript -const handlePreview = useCallback((sampleId: string) => { - if (!audioEngine.isInitialized()) { - return; // Skip preview - must click first - } - audioEngine.playNow(sampleId); -}, []); -``` - ---- - -## Part 4: Lessons Learned - -### Lesson 1: Network Loading Changes Everything - -**Old assumption:** Initialization is fast (~50ms) -**New reality:** Network requests can take 500ms+ - -**Impact:** -- User gesture tokens expire during long async operations -- First load β‰  second load (cache effects) -- Mobile networks are slower and less predictable - -**Heuristic:** Any init that touches network should be treated as fundamentally different from in-memory init. - -### Lesson 2: User Gesture Tokens Expire - -**Constraint:** Browsers give ~100-300ms for async work within a gesture - -**Implications:** -- Can't do long network fetches after click -- Must either: - a) Preload before gesture - b) Create context in gesture, load after - c) Use progressive loading (load minimum first) - -**Our Solution:** Progressive loading - load C4 first (~300ms), background load rest. - -### Lesson 3: Test First Loads Specifically - -**The trap:** Developer refreshes β†’ cache warm β†’ works -**The reality:** New user β†’ cache cold β†’ broken - -**Testing strategy:** -- Incognito window testing -- Clear cache before test -- Network throttling to simulate slow connections -- Automated tests that mock network timing - -### Lesson 4: Concurrent Initialization Guards Are Essential - -**Problem:** Multiple callers (UI, preload, preview) can all trigger init -**Solution:** Promise-based guard that returns existing promise if in progress - -```typescript -if (this._initializePromise) { - return this._initializePromise; -} -this._initializePromise = this._doInitialize(); -``` - -### Lesson 5: Queue Pre-Init Requests - -**Problem:** Code wants to preload before user gesture -**Solution:** Queue requests, fulfill after init - -```typescript -if (!this.initialized) { - this._pendingPreloads.add(instrumentId); - return; // Will load when initialize() called -} -``` - -### Lesson 6: Never Silently Substitute Sounds - -**Problem:** What if piano samples fail to load? -**Bad solution:** Fall back to synth piano preset -**Why bad:** User selected "piano," hears sine waves -**Good solution:** Log error, skip note, be silent - -Silence is better than wrong sound. - -### Lesson 7: Dependency Injection Enables Testing - -**Old code:** Singleton instances, hard to mock -**New code:** Constructor injection with defaults - -```typescript -constructor(deps?: AudioEngineDependencies) { - this._sampledInstrumentRegistry = deps?.sampledInstrumentRegistry ?? sampledInstrumentRegistry; -} -``` - -Enables unit tests without real audio hardware. - ---- - -## Part 5: How Tone.js Would Help - -If implementing from scratch, Tone.js handles many of these concerns. - -### User Gesture Handling - -**Our implementation:** -```typescript -// Manual unlock listeners -const events = ['touchstart', 'touchend', 'click', 'keydown']; -events.forEach(event => { - document.addEventListener(event, unlock, { passive: true }); -}); -``` - -**Tone.js:** -```typescript -// Single call, handles all edge cases -await Tone.start(); -``` - -Tone.js internally: -- Tracks AudioContext state -- Automatically resumes on user gesture -- Handles iOS quirks (interrupted state) -- Provides `Tone.context.state` observable - -### Sample Loading - -**Our implementation:** -```typescript -class SampledInstrument { - async loadInstrument() { /* 200+ lines */ } - findNearestSample(midiNote) { /* pitch shifting logic */ } -} -``` - -**Tone.js:** -```typescript -const piano = new Tone.Sampler({ - urls: { C2: 'C2.mp3', C3: 'C3.mp3', C4: 'C4.mp3', C5: 'C5.mp3' }, - baseUrl: '/instruments/piano/', - onload: () => console.log('Piano ready'), -}).toDestination(); - -// Automatic pitch shifting to nearest sample -piano.triggerAttackRelease('F#4', '8n'); -``` - -Tone.js Sampler: -- Automatic nearest-sample selection -- Built-in pitch shifting -- Attack/release with duration syntax -- Lazy loading support - -### Scheduling - -**Our implementation:** -```typescript -class Scheduler { - private scheduleLoop() { - const lookahead = 0.1; - // Manual timing calculations - } -} -``` - -**Tone.js:** -```typescript -Tone.Transport.scheduleRepeat((time) => { - sampler.triggerAttackRelease('C4', '16n', time); -}, '16n'); - -Tone.Transport.start(); -``` - -Tone.js Transport: -- Built-in BPM control -- Swing support -- Loop scheduling -- Time signature support - -### Effects - -**Our implementation:** Rolled back (too complex to sync) - -**Tone.js:** -```typescript -const reverb = new Tone.Reverb(2).toDestination(); -const delay = new Tone.FeedbackDelay('8n', 0.5).connect(reverb); -piano.connect(delay); - -// Easy to serialize for sync -const effectState = { reverbDecay: 2, delayTime: '8n', feedback: 0.5 }; -``` - -### Synth Presets - -**Our implementation:** -```typescript -const SYNTH_PRESETS = { - supersaw: { - waveform: 'sawtooth', - filterCutoff: 4000, - osc2: { waveform: 'sawtooth', detune: 25, mix: 0.5 }, - // ... 20 more parameters - } -}; -``` - -**Tone.js:** -```typescript -const synth = new Tone.PolySynth(Tone.Synth, { - oscillator: { type: 'fatsawtooth', count: 3, spread: 30 }, - envelope: { attack: 0.01, decay: 0.2, sustain: 0.8, release: 0.3 }, -}); -``` - -Built-in synth types: -- `Tone.Synth` - basic -- `Tone.FMSynth` - FM synthesis -- `Tone.AMSynth` - AM synthesis -- `Tone.PolySynth` - polyphonic wrapper -- `Tone.Sampler` - sample playback - ---- - -## Part 6: Tone.js Implementation Considerations - -### What Tone.js Would Simplify - -| Concern | Our Code | Tone.js | -|---------|----------|---------| -| User gesture handling | Manual unlock listeners | `Tone.start()` | -| Sample loading | Custom SampledInstrument class | `Tone.Sampler` | -| Pitch shifting | Manual `findNearestSample()` | Automatic | -| Scheduling | Custom Scheduler class | `Tone.Transport` | -| Effects | Rolled back (too complex) | Built-in, chainable | -| Envelope/LFO | Custom SynthVoice class | Built-in modulators | -| Voice limiting | Manual implementation | `Tone.PolySynth` | - -### What We'd Still Need to Build - -| Concern | Why Tone.js Doesn't Help | -|---------|--------------------------| -| Multiplayer sync | Application-level concern | -| Session state | Application-level concern | -| Step sequencer UI | React components | -| WebSocket messages | Application-level concern | -| Preset management | Our preset format β‰  Tone.js format | - -### Migration Path - -If migrating to Tone.js: - -1. **Replace AudioEngine with Tone.js context** - ```typescript - // Old - this.audioContext = new AudioContext(); - - // New - // Tone.js manages context internally - await Tone.start(); - ``` - -2. **Replace SampledInstrument with Tone.Sampler** - ```typescript - // Old - const piano = new SampledInstrument('piano'); - await piano.ensureLoaded(); - piano.playNote(noteId, midiNote, time, duration); - - // New - const piano = new Tone.Sampler({ /* config */ }); - await Tone.loaded(); - piano.triggerAttackRelease(Tone.Frequency(midiNote, 'midi'), duration, time); - ``` - -3. **Replace SynthEngine with Tone.PolySynth** - ```typescript - // Old - synthEngine.playNote(noteId, frequency, params, time, duration); - - // New - synth.triggerAttackRelease(frequency, duration, time); - ``` - -4. **Replace Scheduler with Tone.Transport** - ```typescript - // Old - scheduler.start(); - scheduler.setBPM(120); - - // New - Tone.Transport.bpm.value = 120; - Tone.Transport.start(); - ``` - -### Bundle Size Consideration - -| Approach | Size | -|----------|------| -| Current (custom) | ~50KB (audio code only) | -| Tone.js (full) | ~200KB minified | -| Tone.js (tree-shaken) | ~80-120KB | - -Tone.js adds ~50-70KB over our custom implementation, but saves ~3000 lines of code. - ---- - -## Part 7: Recommendations - -### For Current Implementation - -1. **Keep current architecture** - It works, is tested, and is well-documented -2. **Add integration tests for first-load scenario** - Mock network delays -3. **Consider extracting timing constants** - Make gesture timeout explicit -4. **Monitor for iOS edge cases** - "interrupted" state handling - -### For Future Implementations - -1. **Start with Tone.js** if building from scratch -2. **Use `Tone.loaded()` pattern** for sample loading -3. **Use `Tone.Transport` for scheduling** - Don't reinvent -4. **Keep multiplayer sync separate** from audio engine -5. **Test on real mobile devices** - Emulators miss gesture bugs - -### Key Invariants to Preserve - -Regardless of implementation: - -1. **AudioContext must be created in user gesture** -2. **Samples must be loaded before playback** -3. **Never substitute sounds silently** -4. **First load must work (not just cached loads)** -5. **Concurrent init calls must be safe** - ---- - -## Appendix: Code Metrics - -### Lines of Code Comparison - -| Component | Old | New | Delta | -|-----------|-----|-----|-------| -| engine.ts | 350 | 740 | +390 | -| synth.ts | 400 | 1000 | +600 | -| samples.ts | 465 | 275 | -190 | -| sampled-instrument.ts | 0 | 553 | +553 | -| **Total audio code** | **1215** | **2568** | **+1353** | - -### Test Coverage - -| Component | Old Tests | New Tests | -|-----------|-----------|-----------| -| engine.ts | 24 | 48 | -| synth.ts | 85 | 150 | -| sampled-instrument.ts | 0 | 70 | -| SamplePicker.tsx | 0 | 12 | -| **Total** | **109** | **280** | - -### Complexity Added - -- Concurrent initialization guard -- Pending preloads queue -- Observable loading state -- Strategy pattern for note playback -- Dependency injection -- Progressive sample loading - -### Complexity Removed (via cleanup) - -- 6 redundant sample generation functions -- 2 redundant UI categories -- Duplicate instrument concepts diff --git a/app/src/types.ts b/app/src/types.ts index 1f8335a9..525c6db5 100644 --- a/app/src/types.ts +++ b/app/src/types.ts @@ -28,7 +28,7 @@ export type PlaybackMode = 'oneshot' | 'gate'; * Only non-undefined values override the track default. */ export interface ParameterLock { - pitch?: number; // Semitones offset from original (-12 to +12) + pitch?: number; // Semitones offset from original (-24 to +24) volume?: number; // 0-1, multiplier on track volume } diff --git a/app/docs/AUDIO-CONTENT-TOOLS.md b/docs/AUDIO-CONTENT-TOOLS.md similarity index 100% rename from app/docs/AUDIO-CONTENT-TOOLS.md rename to docs/AUDIO-CONTENT-TOOLS.md diff --git a/app/docs/DEBUGGING-LESSONS-LEARNED.md b/docs/DEBUGGING-LESSONS-LEARNED.md similarity index 100% rename from app/docs/DEBUGGING-LESSONS-LEARNED.md rename to docs/DEBUGGING-LESSONS-LEARNED.md diff --git a/app/docs/DEBUGGING-WORKFLOW.md b/docs/DEBUGGING-WORKFLOW.md similarity index 100% rename from app/docs/DEBUGGING-WORKFLOW.md rename to docs/DEBUGGING-WORKFLOW.md diff --git a/app/docs/development-tools.md b/docs/DEVELOPMENT-TOOLS.md similarity index 100% rename from app/docs/development-tools.md rename to docs/DEVELOPMENT-TOOLS.md diff --git a/app/docs/lessons-learned.md b/docs/LESSONS-LEARNED.md similarity index 94% rename from app/docs/lessons-learned.md rename to docs/LESSONS-LEARNED.md index 9d2cb3fa..02e06301 100644 --- a/app/docs/lessons-learned.md +++ b/docs/LESSONS-LEARNED.md @@ -14,6 +14,9 @@ Debugging war stories and insights from building Keyboardia. - [Exponential vs Linear Envelopes](#exponential-vs-linear-envelopes) - [Duplicate Bug = Missing Abstraction: Tone.js Time Conversion](#duplicate-bug--missing-abstraction-tonejs-time-conversion) - [Sampled Instrument Race Condition: Preload at Init](#sampled-instrument-race-condition-preload-at-init) +- [Concurrent Initialization Guards](#concurrent-initialization-guards) +- [Never Silently Substitute Sounds](#never-silently-substitute-sounds) +- [Dependency Injection for Audio Testing](#dependency-injection-for-audio-testing) ### Frontend / Mobile - [The Ghost Click Bug (Mobile Toggle Revert)](#2024-12-11-the-ghost-click-bug-mobile-toggle-revert) @@ -1337,6 +1340,200 @@ But the implementation didn't match the design! The lesson: **verify your implem --- +## Concurrent Initialization Guards + +**Date:** 2024-12 (Phase 21A: Piano Integration) + +### The Problem + +Multiple callers (Play button, preload requests, sample preview) can call `audioEngine.initialize()` simultaneously: + +```typescript +// BUGGY CODE +async initialize(): Promise { + if (this.initialized) return; // Check before async work + + this.audioContext = new AudioContext(); + // ... 50 lines of async operations ... + this.initialized = true; // Set flag at end +} +``` + +**Race Condition:** +``` +Call A: Check initialized (false) βœ“ +Call B: Check initialized (false) βœ“ // A hasn't set flag yet! +Call A: Start creating AudioContext +Call B: Start creating AudioContext // Duplicate! +Call A: Load piano samples +Call B: Load piano samples // Duplicate! +``` + +Result: Multiple AudioContexts, wasted memory, potential resource leaks. + +### The Fix + +Use a promise-based guard that returns the existing promise if initialization is in progress: + +```typescript +private _initializePromise: Promise | null = null; + +async initialize(): Promise { + if (this.initialized) return; + if (this._initializePromise) { + return this._initializePromise; // Wait for in-progress init + } + + this._initializePromise = this._doInitialize(); + await this._initializePromise; + this._initializePromise = null; +} +``` + +### The Pattern + +**Any async initialization that can be called from multiple places needs a promise guard.** + +This applies to: +- Audio engine initialization +- WebSocket connection setup +- Database connection pools +- Service workers +- Any singleton with async setup + +--- + +## Never Silently Substitute Sounds + +**Date:** 2024-12 (Phase 21A: Piano Integration) + +### The Temptation + +When piano samples fail to load (network error, slow connection), it's tempting to fall back to a synth sound: + +```typescript +// WRONG - tempting but harmful +if (pianoInstrument.isReady()) { + pianoInstrument.playNote(...); +} else { + synthEngine.playNote('piano', ...); // Synth fallback +} +``` + +### Why It's Wrong + +1. **User selected "piano"** β€” they expect piano sound, not sine waves +2. **Breaks expectations** β€” experienced users notice the wrong timbre immediately +3. **Hides the bug** β€” you don't know piano failed, just sounds "wrong" +4. **Multiplayer divergence** β€” Player A hears piano, Player B hears synth (if one has cached samples) + +### The Correct Approach + +**Silence is better than the wrong sound.** + +```typescript +// CORRECT - fail visibly +if (pianoInstrument.isReady()) { + pianoInstrument.playNote(...); +} else { + // Log error, skip note - user hears silence + logger.audio.warn(`Piano not ready, skipping note at step ${step}`); + // Optionally: show UI indicator that piano is still loading +} +return; // NEVER fall back to synth for sampled instruments +``` + +### The Rule + +**For audio substitution:** +- Wrong sound = always bad (confuses users) +- Silence = sometimes acceptable (debugging clue) +- Error message = best (tells user what's wrong) + +--- + +## Dependency Injection for Audio Testing + +**Date:** 2024-12 (Phase 21A: Piano Integration) + +### The Problem + +Audio code often uses singletons, making it hard to test: + +```typescript +// Hard to test - uses global singleton +class AudioEngine { + playNote() { + sampledInstrumentRegistry.get('piano').playNote(...); + // How do you mock sampledInstrumentRegistry? + } +} +``` + +### The Solution + +Constructor injection with defaults: + +```typescript +interface AudioEngineDependencies { + sampledInstrumentRegistry?: SampledInstrumentRegistry; + synthEngine?: SynthEngine; +} + +class AudioEngine { + private _sampledInstrumentRegistry: SampledInstrumentRegistry; + private _synthEngine: SynthEngine; + + constructor(deps?: AudioEngineDependencies) { + // Use provided dependencies or default to real singletons + this._sampledInstrumentRegistry = deps?.sampledInstrumentRegistry ?? sampledInstrumentRegistry; + this._synthEngine = deps?.synthEngine ?? synthEngine; + } +} +``` + +### Usage in Tests + +```typescript +describe('AudioEngine', () => { + it('should skip notes when piano not ready', () => { + const mockRegistry = { + get: vi.fn().mockReturnValue({ + isReady: () => false, + playNote: vi.fn(), + }), + }; + + const engine = new AudioEngine({ + sampledInstrumentRegistry: mockRegistry as any, + }); + + engine.playSynthNote('piano', 60, 0, 1); + + // Verify piano.playNote was NOT called (because not ready) + expect(mockRegistry.get('piano').playNote).not.toHaveBeenCalled(); + }); +}); +``` + +### The Pattern + +**Default to production, accept test doubles:** + +```typescript +constructor(deps?: Dependencies) { + this.dep = deps?.dependency ?? realSingleton; +} +``` + +Benefits: +- Production code unchanged (uses defaults) +- Tests can inject mocks +- No global state manipulation in tests +- Clear dependency graph + +--- + ## Audio Engineering Checklist ### Pre-Implementation @@ -2540,10 +2737,10 @@ assert(state.tracks.length <= 16); // Tempo within bounds assert(state.tempo >= 30 && state.tempo <= 300); -// All tracks have correct array sizes +// All tracks have valid step arrays (up to MAX_STEPS = 128) state.tracks.forEach(t => { - assert(t.steps.length === 64); - assert(t.parameterLocks.length === 64); + assert(t.steps.length <= 128); + assert(t.parameterLocks.length <= 128); }); ``` diff --git a/specs/MUSICAL-FOUNDATIONS-SUMMARY.md b/specs/MUSICAL-FOUNDATIONS-SUMMARY.md index df66ce31..572b1bba 100644 --- a/specs/MUSICAL-FOUNDATIONS-SUMMARY.md +++ b/specs/MUSICAL-FOUNDATIONS-SUMMARY.md @@ -8,16 +8,18 @@ Two foundational features that unlock additional musical genres. ## Features Delivered -### 1. Triplet Grids -**Implementation:** Added 12 and 24 to step count options -**Unlocks:** Jazz, Gospel, Blues, Waltz, Afro-Cuban, Trap (hi-hat rolls) +### 1. Triplet Grids & Extended Lengths +**Implementation:** Added 12, 24, 96, and 128 to step count options +**Unlocks:** Jazz, Gospel, Blues, Waltz, Afro-Cuban, Trap (hi-hat rolls), full verse/chorus sections ```typescript -export const STEP_COUNT_OPTIONS = [4, 8, 12, 16, 24, 32, 64] as const; +export const STEP_COUNT_OPTIONS = [4, 8, 12, 16, 24, 32, 64, 96, 128] as const; ``` - **12 steps**: Triplet feel, shuffle rhythms, jazz/gospel swing - **24 steps**: High-resolution triplets for trap hi-hat rolls +- **96 steps**: 6 bars β€” extended triplet patterns +- **128 steps**: 8 bars β€” full verse/chorus sections, cinematic builds ### 2. Extended Pitch Range **Implementation:** Β±24 semitones (4 octaves total) diff --git a/specs/ROADMAP.md b/specs/ROADMAP.md index ddbee03b..56302462 100644 --- a/specs/ROADMAP.md +++ b/specs/ROADMAP.md @@ -1189,14 +1189,16 @@ Add a distinctive favicon representing Keyboardia's step sequencer. Extend rhythmic and melodic capabilities with triplet grids and expanded pitch range. -#### Triplet Grids (12 and 24 steps) +#### Triplet Grids and Extended Lengths -Added to `STEP_COUNT_OPTIONS`: `[4, 8, 12, 16, 24, 32, 64]` +Updated `STEP_COUNT_OPTIONS` to: `[4, 8, 12, 16, 24, 32, 64, 96, 128]` | Step Count | Musical Use | |------------|-------------| | **12** | Triplet feel, jazz/gospel shuffle, waltz (3/4 time) | | **24** | Triplet hi-hats (trap), Afro-Cuban rhythms, swing patterns | +| **96** | Extended triplet patterns, 6-bar phrases | +| **128** | Full verse/chorus sections, cinematic builds | **Polyrhythmic possibilities:** - 8 vs 12 (LCM=24): Afrobeat / West African clave diff --git a/specs/STATUS.md b/specs/STATUS.md index 5ff9e2b4..5576f44c 100644 --- a/specs/STATUS.md +++ b/specs/STATUS.md @@ -145,8 +145,8 @@ ### Completed -- βœ… Extend MAX_STEPS to 64 -- βœ… Per-track stepCount property (4, 8, 16, 32, or 64) +- βœ… Extend MAX_STEPS to 128 +- βœ… Per-track stepCount property (4, 8, 12, 16, 24, 32, 64, 96, or 128) - βœ… Step count dropdown in track controls (replaced buttons) - βœ… Polyrhythmic looping β€” each track loops at its own length - βœ… Solo button β€” per-track solo with yellow (#f1c40f) active state @@ -164,11 +164,15 @@ | Steps | Bars | Loops/Bar | Use Case | |-------|------|-----------|----------| -| **4** | 0.25 | 4Γ— | Four-on-the-floor kick, pulse patterns, motorik beat | -| **8** | 0.5 | 2Γ— | Half-bar phrases, 8th-note arpeggios, call-response | -| 16 | 1 | 1Γ— | Standard patterns (drums, bass) | -| 32 | 2 | 0.5Γ— | Basslines with variation, 2-bar melodies | -| 64 | 4 | 0.25Γ— | Long melodies, chord progressions, evolving patterns | +| **4** | 0.25 | 8Γ— | Four-on-the-floor kick, pulse patterns, motorik beat | +| **8** | 0.5 | 4Γ— | Half-bar phrases, 8th-note arpeggios, call-response | +| **12** | 0.75 | ~2.67Γ— | Triplet feel, jazz/gospel shuffle, waltz | +| 16 | 1 | 2Γ— | Standard patterns (drums, bass) | +| **24** | 1.5 | ~1.33Γ— | Triplet hi-hats (trap), Afro-Cuban rhythms | +| 32 | 2 | 1Γ— | Basslines with variation, 2-bar melodies | +| 64 | 4 | 0.5Γ— | Long melodies, chord progressions, evolving patterns | +| **96** | 6 | ~0.33Γ— | Extended triplet patterns, 6-bar phrases | +| **128** | 8 | 0.25Γ— | Full verse/chorus sections, cinematic builds | ### Polyrhythmic Combinations diff --git a/docs/SYNTHESIS-ENGINE-ARCHITECTURE.md b/specs/SYNTHESIS-ENGINE-ARCHITECTURE.md similarity index 99% rename from docs/SYNTHESIS-ENGINE-ARCHITECTURE.md rename to specs/SYNTHESIS-ENGINE-ARCHITECTURE.md index 9a7078fe..a0c68e83 100644 --- a/docs/SYNTHESIS-ENGINE-ARCHITECTURE.md +++ b/specs/SYNTHESIS-ENGINE-ARCHITECTURE.md @@ -319,7 +319,7 @@ interface SessionState { | Constraint | Value | |------------|-------| | MAX_TRACKS | 16 | -| MAX_STEPS | 64 | +| MAX_STEPS | 128 | | MIN_TEMPO | 60 BPM | | MAX_TEMPO | 180 BPM | | MIN_TRANSPOSE | -24 semitones | diff --git a/app/docs/instrument-research.md b/specs/research/INSTRUMENT-RESEARCH.md similarity index 57% rename from app/docs/instrument-research.md rename to specs/research/INSTRUMENT-RESEARCH.md index ba451ac2..4ce22629 100644 --- a/app/docs/instrument-research.md +++ b/specs/research/INSTRUMENT-RESEARCH.md @@ -4,7 +4,7 @@ This document catalogs all instruments currently available in Keyboardia and provides research on recommended future additions. All instruments are designed to be deterministic and multiplayer-compatible. -**Last Updated:** Phase 21A +**Last Updated:** Phase 22 --- @@ -14,16 +14,27 @@ This document catalogs all instruments currently available in Keyboardia and pro | Category | Count | Type | |----------|-------|------| -| Synthesized Samples | 10 | Runtime-generated AudioBuffers (drums + FX) | -| Synth Presets | 33 | Real-time synthesis (oscillators + filters) | +| Synthesized Samples | 16 | Runtime-generated AudioBuffers (drums, bass, synth, FX) | +| Web Audio Synth Presets | 32 | Real-time synthesis (oscillators + filters) | +| Tone.js Synth Presets | 11 | FM/AM synthesis, membrane/metal drums (Tone.js) | +| Advanced Synth Presets | 8 | Dual-oscillator + LFO + filter envelope (Tone.js) | | Sampled Instruments | 1 | Pre-recorded audio files (piano) | -| **Total** | **44** | | +| **Total** | **68** | | -**Note:** Melodic one-shot samples (bass, subbass, lead, pluck, chord, pad) were removed as redundant β€” the real-time synth presets (`synth:bass`, `synth:lead`, etc.) provide the same sounds with better pitch control and envelope features. +### UI Organization (by musical function) + +| Category | Instruments | Count | +|----------|-------------|-------| +| Drums | kick, snare, hihat, clap, tom, rim, cowbell, openhat, synth-kick, synth-tom, cymbal, metal-hat | 12 | +| Bass | bass, sub, synth, acid, deep-sub, funk, disco, reese, hoover, fm-bass, sub-bass, wobble, acid-303 | 13 | +| Keys | piano, rhodes, wurli, e-piano, fm-piano, organ, phaser, clav, vibes | 9 | +| Leads | lead, pluck, classic, synth-pluck, supersaw, hypersaw, string, duo, fat-saw, thick, vibrato | 11 | +| Pads | pad, chord, soft, warm, strings, shimmer, dream, glass, jangle, evolve, sweep, lush, tremolo | 13 | +| FX | zap, noise, bell, stab, brass, wobble, growl, fm-bell, am-bell, tremolo | 10 | --- -## 1.1 Synthesized Samples (10 total) +## 1.1 Synthesized Samples (16 total) These are one-shot sounds generated at runtime using Web Audio API oscillators. Zero external files. @@ -40,6 +51,22 @@ These are one-shot sounds generated at runtime using Web Audio API oscillators. | `cowbell` | Cowbell | Square + bandpass | Metallic, cutting | Disco, Latin, funk | | `openhat` | Open Hat | Long noise decay | Sizzle, sustain | Offbeats, transitions | +### Bass (2) + +| ID | Name | Waveform | Character | Use Case | +|----|------|----------|-----------|----------| +| `bass` | Bass | Sawtooth harmonics | Warm, fundamental | Bass lines | +| `subbass` | Sub Bass | Pure sine | Deep, minimal | Sub layers | + +### Melodic (4) + +| ID | Name | Waveform | Character | Use Case | +|----|------|----------|-----------|----------| +| `lead` | Lead | Square (odd harmonics) | Cutting, present | Melodies | +| `pluck` | Pluck | Harmonic decay | Percussive, bright | Arpeggios | +| `chord` | Chord | Minor triad (A, C, E) | Soft, layered | Chord stabs | +| `pad` | Pad | Detuned sines | Slow attack, lush | Atmosphere | + ### FX (2) | ID | Name | Waveform | Character | Use Case | @@ -49,9 +76,9 @@ These are one-shot sounds generated at runtime using Web Audio API oscillators. --- -## 1.2 Synth Presets (33 total) +## 1.2 Web Audio Synth Presets (32 total) -Real-time synthesis using oscillators, filters, envelopes, and LFOs. All parameters are preset-locked (no user adjustment) to ensure multiplayer sync. +Real-time synthesis using Web Audio API oscillators, filters, envelopes, and LFOs. All parameters are preset-locked (no user adjustment) to ensure multiplayer sync. Accessed via `synth:{id}` prefix. ### Core (5 presets) @@ -63,48 +90,40 @@ Real-time synthesis using oscillators, filters, envelopes, and LFOs. All paramet | `pluck` | Pluck | Triangle | 3500Hz, Q10 | 0.005/0.4/0.15/0.25 | Percussive, bright attack | | `acid` | Acid | Sawtooth | 600Hz, Q16 | 0.01/0.15/0.35/0.1 | TB-303 style, squelchy | -### Keys (8 presets) +### Funk / Soul (2 presets) + +| ID | Name | Waveform | Special Features | Character | +|----|------|----------|------------------|-----------| +| `funkbass` | Funk | Square | Short release (0.05s) | Punchy, Bootsy Collins | +| `clavinet` | Clav | Sawtooth | High cutoff (4000Hz) | Bright, percussive (Stevie Wonder) | + +### Keys (6 presets) | ID | Name | Waveform | Special Features | Character | |----|------|----------|------------------|-----------| -| `piano` | Piano | Triangle | Osc2: sine +2Β’, filterEnv | Hammer attack, natural decay | | `rhodes` | Rhodes | Sine | β€” | Mellow, bell-like (Herbie Hancock) | | `organ` | Organ | Square | β€” | Sustained, churchy (Hammond B3) | | `wurlitzer` | Wurli | Triangle | β€” | Warmer than Rhodes, more bark | -| `clavinet` | Clav | Sawtooth | β€” | Bright, percussive (Stevie Wonder) | | `epiano` | E.Piano | Triangle | Osc2: sine +5Β’ | Electric piano, layered | | `vibes` | Vibes | Sine | LFOβ†’amplitude 5Hz | Vibraphone tremolo | | `organphase` | Phase | Square | Osc2: -12st, LFOβ†’pitch 0.8Hz | Rotary speaker effect | -### Electronic (6 presets) - -| ID | Name | Waveform | Special Features | Character | -|----|------|----------|------------------|-----------| -| `supersaw` | Super | Sawtooth | Osc2: saw +25Β’ | Classic trance, thick | -| `hypersaw` | Hyper | Sawtooth | Osc2: saw +50Β’, filterEnv | Massive, even thicker | -| `wobble` | Wobble | Sawtooth | LFOβ†’filter 2Hz | Dubstep bass wobble | -| `growl` | Growl | Square | LFOβ†’filter 4Hz (square), filterEnv | Aggressive modulation | -| `stab` | Stab | Sawtooth | Q10 | Classic house chord stab | -| `sub` | Sub | Sine | 200Hz cutoff, Q0 | Deep sub bass, minimal | - -### Bass (4 presets) +### Disco (3 presets) | ID | Name | Waveform | Special Features | Character | |----|------|----------|------------------|-----------| -| `funkbass` | Funk | Square | Short release (0.05s) | Punchy, Bootsy Collins | | `discobass` | Disco | Sawtooth | β€” | Octave-jumping groove | -| `reese` | Reese | Sawtooth | Osc2: saw +15Β’, LFOβ†’filter 0.5Hz | Jungle/DnB, phasing | -| `hoover` | Hoover | Sawtooth | Osc2: saw -12st +40Β’, filterEnv(-) | Mentasm, downward sweep | +| `strings` | Strings | Sawtooth | Long release (0.8s) | Philly strings, lush | +| `brass` | Brass | Sawtooth | Q3, attack 0.05s | Punchy horn stabs | -### Strings (3 presets) +### House / Techno (2 presets) | ID | Name | Waveform | Special Features | Character | |----|------|----------|------------------|-----------| -| `strings` | Strings | Sawtooth | Long release (0.8s) | Philly strings, lush | -| `brass` | Brass | Sawtooth | Q3, attack 0.05s | Punchy horn stabs | -| `warmpad` | Warm | Sawtooth | Osc2: sine +7Β’ | Subtle chorus, full pad | +| `stab` | Stab | Sawtooth | Q10 | Classic house chord stab | +| `sub` | Sub | Sine | 200Hz cutoff, Q0 | Deep sub bass, minimal | -### Ambient (7 presets) +### Atmospheric (8 presets) | ID | Name | Waveform | Special Features | Character | |----|------|----------|------------------|-----------| @@ -114,13 +133,89 @@ Real-time synthesis using oscillators, filters, envelopes, and LFOs. All paramet | `bell` | Bell | Sine | Decay 0.5s, release 1.0s | Pure tone, vibraphone-like | | `evolving` | Evolve | Sawtooth | FilterEnv 2s attack, LFO 0.2Hz | Slow organic movement | | `sweep` | Sweep | Sawtooth | Osc2: +15Β’, filterEnv 1s attack | Build/transition sound | +| `warmpad` | Warm | Sawtooth | Osc2: sine +7Β’ | Subtle chorus, full pad | | `glass` | Glass | Sine | Osc2: triangle +12st, filterEnv | Crystalline, bright attack | +### Electronic (4 presets) + +| ID | Name | Waveform | Special Features | Character | +|----|------|----------|------------------|-----------| +| `supersaw` | Super | Sawtooth | Osc2: saw +25Β’ | Classic trance, thick | +| `hypersaw` | Hyper | Sawtooth | Osc2: saw +50Β’, filterEnv | Massive, even thicker | +| `wobble` | Wobble | Sawtooth | LFOβ†’filter 2Hz | Dubstep bass wobble | +| `growl` | Growl | Square | LFOβ†’filter 4Hz (square), filterEnv | Aggressive modulation | + +### Bass Enhancement (2 presets) + +| ID | Name | Waveform | Special Features | Character | +|----|------|----------|------------------|-----------| +| `reese` | Reese | Sawtooth | Osc2: saw +15Β’, LFOβ†’filter 0.5Hz | Jungle/DnB, phasing | +| `hoover` | Hoover | Sawtooth | Osc2: saw -12st +40Β’, filterEnv(-) | Mentasm, downward sweep | + +--- + +## 1.3 Tone.js Synth Presets (11 total) + +Advanced synthesis using Tone.js library for sounds that require more complex algorithms (FM synthesis, physical modeling). Accessed via `tone:{id}` prefix. + +### FM Synths (3 presets) + +| ID | Name | Algorithm | Character | +|----|------|-----------|-----------| +| `fm-epiano` | FM E-Piano | 2-op FM, mod index 14 | DX7-style electric piano | +| `fm-bass` | FM Bass | 2-op FM, mod index 8 | Deep, harmonically rich | +| `fm-bell` | FM Bell | 2-op FM, mod index 20 | Crystalline, long decay | + +### AM Synths (2 presets) + +| ID | Name | Algorithm | Character | +|----|------|-----------|-----------| +| `am-bell` | AM Bell | AM, harmonicity 3.5 | Ring-mod bell tones | +| `am-tremolo` | Tremolo | AM, harmonicity 1 | Classic tremolo effect | + +### Membrane Synths (2 presets) + +| ID | Name | Algorithm | Character | +|----|------|-----------|-----------| +| `membrane-kick` | Synth Kick | Membrane, 8 octaves pitch decay | 808-style kick drum | +| `membrane-tom` | Synth Tom | Membrane, 4 octaves pitch decay | Electronic tom | + +### Metal Synths (2 presets) + +| ID | Name | Algorithm | Character | +|----|------|-----------|-----------| +| `metal-cymbal` | Cymbal | Metal, resonance 4000Hz | Crash/ride cymbal | +| `metal-hihat` | Metal Hat | Metal, resonance 5000Hz | Electronic hi-hat | + +### Other (2 presets) + +| ID | Name | Algorithm | Character | +|----|------|-----------|-----------| +| `pluck-string` | String | Karplus-Strong | Plucked string sound | +| `duo-lead` | Duo Lead | 2 parallel synths | Rich, vibrato lead | + +--- + +## 1.4 Advanced Synth Presets (8 total) + +Full-featured synthesis with dual oscillators, filter envelope, and LFO modulation using Tone.js. Accessed via `advanced:{id}` prefix. + +| ID | Name | Osc1 | Osc2 | LFO | Character | +|----|------|------|------|-----|-----------| +| `supersaw` | Fat Saw | Saw -15Β’ | Saw +15Β’ | Filter 0.5Hz | Trance/EDM lead | +| `sub-bass` | Sub Bass | Sine | Square -12st | β€” | Deep sub with octave | +| `wobble-bass` | Wobble | Saw | Square +5Β’ | Filter 2Hz | Dubstep bass | +| `warm-pad` | Lush Pad | Saw -10Β’ | Tri +10Β’ +12st | Filter 0.3Hz | Warm, evolving pad | +| `vibrato-lead` | Vibrato | Square | Saw -7Β’ | Pitch 6Hz | Expressive lead | +| `tremolo-strings` | Tremolo | Saw -5Β’ | Saw +5Β’ | Amp 5Hz | String tremolo | +| `acid-bass` | Acid 303 | Saw | β€” | β€” | TB-303 acid line | +| `thick-lead` | Thick | Square -25Β’ | Square +25Β’ | Pitch 4Hz | PWM-style lead | + --- -## 1.3 Sampled Instruments (1 total) +## 1.5 Sampled Instruments (1 total) -Pre-recorded audio samples loaded from files. Used when synthesis cannot convincingly replicate the sound. +Pre-recorded audio samples loaded from files. Used when synthesis cannot convincingly replicate the sound. Accessed via `sampled:{id}` prefix. ### Piano @@ -141,7 +236,7 @@ Pre-recorded audio samples loaded from files. Used when synthesis cannot convinc ## Part 2: Synthesis Engine Capabilities -### Core Parameters (all presets) +### Web Audio Engine Parameters | Parameter | Range | Description | |-----------|-------|-------------| @@ -153,7 +248,7 @@ Pre-recorded audio samples loaded from files. Used when synthesis cannot convinc | `sustain` | 0-1 | Volume while held | | `release` | 0-2s | Fade time after release | -### Enhanced Parameters (Phase 21A) +### Enhanced Parameters (Phase 22) | Feature | Parameters | Use Case | |---------|------------|----------| @@ -161,13 +256,32 @@ Pre-recorded audio samples loaded from files. Used when synthesis cannot convinc | **Filter Envelope** | amount (Β±1), attack, decay, sustain | Filter movement over time | | **LFO** | waveform, rate (0.1-20Hz), depth, destination | Wobble, vibrato, tremolo | +### Tone.js Engine Parameters + +| Feature | Parameters | Use Case | +|---------|------------|----------| +| **FM Synthesis** | harmonicity, modulationIndex, modulation envelope | DX7-style sounds | +| **AM Synthesis** | harmonicity, amplitude envelope | Tremolo, bell tones | +| **Membrane** | pitchDecay, octaves | Drum synthesis | +| **Metal** | frequency, harmonicity, resonance | Cymbal synthesis | +| **Pluck** | attackNoise, dampening, resonance | Karplus-Strong strings | + +### Advanced Engine Parameters + +| Feature | Parameters | Use Case | +|---------|------------|----------| +| **Dual Oscillator** | 2Γ— (waveform, level, detune, coarseDetune) | Thick, detuned sounds | +| **Filter Envelope** | ADSR + envelopeAmount | Filter sweeps | +| **LFO** | frequency, waveform, destination, amount, sync | Modulation routing | +| **Noise Layer** | noiseLevel (0-1) | Texture, breath | + ### Audio Engineering Constants | Constant | Value | Purpose | |----------|-------|---------| -| `MAX_VOICES` | 16 | Prevent CPU overload | -| `ENVELOPE_PEAK` | 0.85 | Full, rich sound (was 0.1 before gain fix) | -| `MAX_FILTER_RESONANCE` | 20 | Prevent self-oscillation | +| `MAX_VOICES` | 16 (Web Audio), 8 (Advanced) | Prevent CPU overload | +| `ENVELOPE_PEAK` | 0.85 | Full, rich sound | +| `MAX_FILTER_RESONANCE` | 20-30 | Prevent self-oscillation | --- @@ -236,14 +350,14 @@ Pre-recorded audio samples loaded from files. Used when synthesis cannot convinc ### Synth Presets (No Samples Needed) -These can be added as new presets using existing synthesis engine: +These can be added as new presets using existing synthesis engines: -| Sound | Approach | Effort | -|-------|----------|--------| -| **FM Bass** (DX7) | FM synthesis with 2 oscillators | Medium | -| **Chip/8-bit** | Simple square waves, low-fi | Low | -| **PWM Pad** | Pulse width modulation (needs engine work) | High | -| **Sync Lead** | Oscillator sync (needs engine work) | High | +| Sound | Approach | Engine | Effort | +|-------|----------|--------|--------| +| **Chip/8-bit** | Simple square waves, low-fi | Web Audio | Low | +| **PWM Pad** | Pulse width modulation | Advanced | Medium | +| **Sync Lead** | Oscillator sync | Advanced | High | +| **Formant Vowels** | Multiple bandpass filters | Advanced | High | ### What to Avoid (For Now) @@ -265,7 +379,9 @@ These can be added as new presets using existing synthesis engine: |-------|----------|------------------| | House/Techno | 95% | βœ“ Solved | | Disco | 90% | βœ“ Solved | -| Synth-pop | 75% | Minor gaps | +| Synthwave | 90% | βœ“ Solved (new supersaw/hypersaw presets) | +| Synth-pop | 85% | βœ“ Nearly solved | +| Ambient/Atmospheric | 80% | βœ“ Good coverage (shimmer, evolving, warm-pad) | | Lo-fi Hip-hop | 50% | No vinyl crackle, limited keys | | Funk/Soul | 45% | Need real bass, better brass | | Jazz | 25% | Need upright bass, brushes | @@ -276,19 +392,19 @@ These can be added as new presets using existing synthesis engine: | Addition | Cumulative Coverage | |----------|-------------------| -| Current | ~35% of all music | -| + Electric Bass | ~50% | -| + Vinyl Crackle | ~58% | -| + Clean Guitar | ~68% | -| + Brass Stabs | ~73% | -| + Choir | ~78% | +| Current | ~45% of all music (↑ from 35%) | +| + Electric Bass | ~60% | +| + Vinyl Crackle | ~68% | +| + Clean Guitar | ~78% | +| + Brass Stabs | ~83% | +| + Choir | ~88% | ### Architectural Limits (Cannot Overcome) | Blocker | Affected Genres | Why | |---------|-----------------|-----| | **12-TET only** | Maqam, Indian classical | Microtonal pitch system is fundamental | -| **Power-of-2 steps** | Progressive rock (5/4, 7/8) | Step counts limited by design | +| **Step counts** | Progressive rock (5/4, 7/8) | Step counts limited to powers of 2 + triplets | | **Grid quantization** | Jazz phrasing, classical rubato | Required for multiplayer sync | | **Web Audio latency** | Live monitoring | 30-100ms minimum latency | | **Discrete steps** | Blues, slide guitar | No pitch bends between steps | @@ -334,8 +450,7 @@ sampledInstrumentRegistry.register('bass'); // 4. Add synth fallback preset (optional but recommended) // In synth.ts SYNTH_PRESETS -// 5. Add to SamplePicker categories -// In SamplePicker.tsx SYNTH_CATEGORIES +// 5. Add to sample-constants.ts INSTRUMENT_CATEGORIES ``` ### For New Synth Presets @@ -356,7 +471,42 @@ newpreset: { lfo: { waveform: 'sine', rate: 2, depth: 0.5, destination: 'filter' }, }, -// Then add to SYNTH_CATEGORIES and SYNTH_NAMES in SamplePicker.tsx +// Then add to sample-constants.ts INSTRUMENT_CATEGORIES +``` + +### For New Tone.js Presets + +```typescript +// Add to TONE_SYNTH_PRESETS in toneSynths.ts +'new-sound': { + type: 'fm', + config: { + harmonicity: 3, + modulationIndex: 10, + envelope: { attack: 0.01, decay: 0.2, sustain: 0.5, release: 0.5 }, + modulation: { type: 'square' }, + }, +}, + +// Then add to sample-constants.ts INSTRUMENT_CATEGORIES +``` + +### For New Advanced Presets + +```typescript +// Add to ADVANCED_SYNTH_PRESETS in advancedSynth.ts +'new-preset': { + name: 'New Preset', + oscillator1: { waveform: 'sawtooth', level: 0.5, detune: -10, coarseDetune: 0 }, + oscillator2: { waveform: 'triangle', level: 0.5, detune: 10, coarseDetune: 12 }, + amplitudeEnvelope: { attack: 0.1, decay: 0.3, sustain: 0.6, release: 0.8 }, + filter: { frequency: 2000, resonance: 4, type: 'lowpass', envelopeAmount: 0.4 }, + filterEnvelope: { attack: 0.2, decay: 0.3, sustain: 0.5, release: 0.5 }, + lfo: { frequency: 0.5, waveform: 'sine', destination: 'filter', amount: 0.3, sync: false }, + noiseLevel: 0, +}, + +// Then add to sample-constants.ts INSTRUMENT_CATEGORIES ``` --- @@ -398,23 +548,43 @@ All instruments must produce identical audio across all players. This rules out: ### All Instrument IDs -**One-Shot Samples (10):** +**Synthesized Samples (16):** ``` Drums: kick, snare, hihat, clap, tom, rim, cowbell, openhat +Bass: bass, subbass +Melodic: lead, pluck, chord, pad FX: zap, noise ``` -**Synth Presets (33):** +**Web Audio Synth Presets (32):** +``` +Core: synth:bass, synth:lead, synth:pad, synth:pluck, synth:acid +Funk/Soul: synth:funkbass, synth:clavinet +Keys: synth:rhodes, synth:organ, synth:wurlitzer, synth:epiano, synth:vibes, synth:organphase +Disco: synth:discobass, synth:strings, synth:brass +House/Techno: synth:stab, synth:sub +Atmospheric: synth:shimmer, synth:jangle, synth:dreampop, synth:bell, synth:evolving, synth:sweep, synth:warmpad, synth:glass +Electronic: synth:supersaw, synth:hypersaw, synth:wobble, synth:growl +Bass: synth:reese, synth:hoover +``` + +**Tone.js Synth Presets (11):** +``` +FM: tone:fm-epiano, tone:fm-bass, tone:fm-bell +AM: tone:am-bell, tone:am-tremolo +Membrane: tone:membrane-kick, tone:membrane-tom +Metal: tone:metal-cymbal, tone:metal-hihat +Other: tone:pluck-string, tone:duo-lead +``` + +**Advanced Synth Presets (8):** ``` -Core: bass, lead, pad, pluck, acid -Keys: piano, rhodes, organ, wurlitzer, clavinet, epiano, vibes, organphase -Electronic: supersaw, hypersaw, wobble, growl, stab, sub -Bass: funkbass, discobass, reese, hoover -Strings: strings, brass, warmpad -Ambient: shimmer, jangle, dreampop, bell, evolving, sweep, glass +Leads: advanced:supersaw, advanced:thick-lead, advanced:vibrato-lead +Bass: advanced:sub-bass, advanced:wobble-bass, advanced:acid-bass +Pads: advanced:warm-pad, advanced:tremolo-strings ``` **Sampled Instruments (1):** ``` -piano +sampled:piano ``` diff --git a/docs/research/MUSICAL-COVERAGE-ANALYSIS.md b/specs/research/MUSICAL-COVERAGE-ANALYSIS.md similarity index 99% rename from docs/research/MUSICAL-COVERAGE-ANALYSIS.md rename to specs/research/MUSICAL-COVERAGE-ANALYSIS.md index 6b6504fe..586cce51 100644 --- a/docs/research/MUSICAL-COVERAGE-ANALYSIS.md +++ b/specs/research/MUSICAL-COVERAGE-ANALYSIS.md @@ -85,8 +85,8 @@ CUSTOM: Mic recording (unlimited, in-memory only) | Capability | Range | Musical Use | |------------|-------|-------------| -| **Pitch** | Β±12 semitones (2 octaves) | Melodies, bass lines | -| **Steps** | 4, 8, 16, 32, 64 per track | Polyrhythms, long phrases | +| **Pitch** | Β±24 semitones (4 octaves) | Melodies, bass lines, sub-bass, high leads | +| **Steps** | 4, 8, 12, 16, 24, 32, 64, 96, 128 per track | Polyrhythms, triplets, full verses | | **Tempo** | 60-180 BPM | Hip-hop through drum & bass | | **Swing** | 0-100% | Straight to heavy shuffle | | **Tracks** | Up to 16 | Full arrangements | @@ -591,12 +591,11 @@ Add triplet grid options for swing-based genres. #### Implementation -Add 12 and 24 to step count options: +βœ… **Implemented** β€” Triplet grids (12, 24) plus extended lengths (96, 128): ```typescript -// types.ts -export const STEP_COUNT_OPTIONS = [4, 8, 12, 16, 24, 32, 64] as const; -// ^^ ^^ NEW +// types.ts (current) +export const STEP_COUNT_OPTIONS = [4, 8, 12, 16, 24, 32, 64, 96, 128] as const; ``` | Steps | Resolution | Musical Use | diff --git a/app/VOLUME-VERIFICATION-ANALYSIS.md b/specs/research/VOLUME-VERIFICATION-ANALYSIS.md similarity index 100% rename from app/VOLUME-VERIFICATION-ANALYSIS.md rename to specs/research/VOLUME-VERIFICATION-ANALYSIS.md