From d2d20763ac61f59187d261bb7d6dedcab525bc54 Mon Sep 17 00:00:00 2001 From: skal Date: Sun, 8 Feb 2026 10:12:35 +0100 Subject: docs(audio): Update AUDIO_LIFECYCLE_REFACTOR.md and AUDIO_TIMING_ARCHITECTURE.md\n\nfeat(audio): Converted AUDIO_LIFECYCLE_REFACTOR.md from a design plan to a description of the implemented AudioEngine and SpectrogramResourceManager architecture, detailing the lazy-loading strategy and seeking capabilities.\n\ndocs(audio): Updated AUDIO_TIMING_ARCHITECTURE.md to reflect current code discrepancies in timing, BPM, and peak decay. Renamed sections to clarify current vs. proposed architecture and outlined a detailed implementation plan for Task #71.\n\nhandoff(Gemini): Finished updating audio documentation to reflect current state and future tasks. --- doc/AUDIO_TIMING_ARCHITECTURE.md | 484 ++++++++++----------------------------- 1 file changed, 122 insertions(+), 362 deletions(-) (limited to 'doc/AUDIO_TIMING_ARCHITECTURE.md') diff --git a/doc/AUDIO_TIMING_ARCHITECTURE.md b/doc/AUDIO_TIMING_ARCHITECTURE.md index 9ac3927..7b125d0 100644 --- a/doc/AUDIO_TIMING_ARCHITECTURE.md +++ b/doc/AUDIO_TIMING_ARCHITECTURE.md @@ -1,26 +1,53 @@ -# Audio Timing Architecture - Proper Solution (February 7, 2026) +# Audio Timing Architecture - Analysis and Proposed Solution (February 8, 2026) ## Problem Statement **Original Issue:** "demo is still flashing a lot" due to audio-visual timing mismatch. **Root Causes:** -1. Multiple time sources with no clear hierarchy -2. Hardcoded latency constants (400ms) in solution proposals -3. Beat calculation using wrong time source -4. Peak decay rate not matched to music tempo +1. Multiple time sources with no clear hierarchy. +2. Beat calculation for visuals uses the wrong time source (physical time instead of audio playback time). +3. Hardcoded peak decay rate does not adapt to the music's tempo. +4. Hardcoded BPM values in some places prevent data-driven tempo changes. --- -## Correct Architecture ✅ +## Current State Analysis (The Problem) -### Single Source of Truth: Physical Clock +The current implementation suffers from several discrepancies that lead to audio-visual desynchronization. + +### Discrepancy #1: Wrong Time Source for Beat Calculation +Both `main.cc` and `test_demo.cc` use the physical wall clock time to calculate the current musical beat for visual effects. However, the audio peak is measured at the moment the audio is actually played, which can be hundreds of milliseconds later due to audio buffering. + +**`main.cc` (Incorrect Logic):** ```cpp -platform_get_time() → ONE authoritative wall clock from OS +// current_time is derived from platform_state.time (physical clock) +const double current_time = platform_state.time + seek_time; +// Beat is calculated from physical time +const float beat_time = (float)current_time * g_tracker_score.bpm / 60.0f; +// Peak is from audio playback time (a different clock) +const float raw_peak = audio_get_realtime_peak(); + +// MISMATCH: `beat_time` and `raw_peak` are out of sync! ``` +This is the primary cause of the AV sync issue. + +### Discrepancy #2: Hardcoded Configuration -**Everything else derives from this:** +The system relies on several hardcoded values instead of being data-driven. + +- **Hardcoded BPM:** `src/test_demo.cc` hardcodes the BPM to `120.0f`, ignoring the value in the track file. +- **Hardcoded Peak Decay:** `src/audio/backend/miniaudio_backend.cc` uses a fixed decay rate of `0.5f`, which does not adapt to different tempos. A fast song will have its visual peak decay too slowly, and a slow song too quickly. +- **No `tracker_get_bpm()` API:** There is no formal function to get the BPM from the tracker; code accesses the global `g_tracker_score.bpm` directly. + +--- + +## Proposed Architecture (The Solution) ✅ + +### Single Source of Truth: Physical Clock + +The core principle remains: `platform_get_time()` is the one authoritative wall clock from the OS. All other time representations should derive from it in a managed way. ``` Physical Time (platform_get_time()) @@ -46,14 +73,16 @@ Physical Time (platform_get_time()) | Time Source | Purpose | How to Get | Use For | |-------------|---------|------------|---------| | **Physical Time** | Wall clock, frame deltas | `platform_get_time()` | dt calculations, physics | -| **Audio Playback Time** | What's being HEARD | `audio_get_playback_time()` | Audio-visual sync, beat display | +| **Audio Playback Time** | What's being HEARD | `audio_get_playback_time()` | **Audio-visual sync, beat display** | | **Music Time** | Tracker time (tempo-scaled) | `g_music_time` | Tracker event triggering | --- -## Implementation: test_demo.cc +## Correct Implementation Example + +This demonstrates how `test_demo.cc` and `main.cc` *should* be implemented to achieve proper synchronization. -### Before (Wrong ❌) +### Before (Current Incorrect State ❌) ```cpp const double current_time = platform_state.time; // Physical time @@ -61,19 +90,17 @@ const double current_time = platform_state.time; // Physical time // Beat calculation based on physical time const float beat_time = (float)current_time * 120.0f / 60.0f; -// But peak is measured at audio playback time (400ms behind!) +// But peak is measured at audio playback time (e.g. 400ms behind!) const float raw_peak = audio_get_realtime_peak(); // MISMATCH: beat and peak are from different time sources! ``` -**Problem:** Visual beat shows beat 2 (physical time), but peak shows beat 1 (audio playback time). +**Problem:** Visual beat shows beat 2 (from physical time), but peak is for beat 1 (from audio playback time). -### After (Correct ✅) +### After (Proposed Correct State ✅) ```cpp -const double physical_time = platform_state.time; // For dt calculations - // Audio playback time: what's being HEARD right now const float audio_time = audio_get_playback_time(); @@ -83,10 +110,10 @@ const float beat_time = audio_time * 120.0f / 60.0f; // Peak is measured at audio playback time const float raw_peak = audio_get_realtime_peak(); -// SYNCHRONIZED: beat and peak are from same time source! +// SYNCHRONIZED: beat and peak are from the same time source! ``` -**Result:** Visual beat shows beat 1, peak shows beat 1 → synchronized! ✅ +**Result:** Visual beat shows beat 1, peak is for beat 1 → synchronized! ✅ --- @@ -99,354 +126,87 @@ float audio_get_playback_time() { return (float)total_samples / (RING_BUFFER_SAMPLE_RATE * RING_BUFFER_CHANNELS); } ``` - **Key Points:** -1. **Tracks samples consumed** by audio callback (not samples rendered) -2. **Automatically accounts for ring buffer latency** (no hardcoded constants!) -3. **Self-consistent** with `audio_get_realtime_peak()` (measured at same moment) - -**Example Timeline:** -``` -T(physical) = 1.00s: - → Ring buffer has 400ms of lookahead - → Audio callback is playing samples rendered at T=0.60s (music time) - → total_read counter reflects 0.60s worth of samples - → audio_get_playback_time() returns 0.60s - → audio_get_realtime_peak() measured from samples at 0.60s - → Beat calculation: 0.60s * 2 = 1.2 → beat 1 - → SYNCHRONIZED! ✅ -``` - ---- - -## Remaining Issues: Data-Driven Configuration - -### Issue #1: Hardcoded Decay Rate - -**Current** (miniaudio_backend.cc:166): -```cpp -realtime_peak_ *= 0.5f; // Hardcoded: 50% per callback -``` - -**Problem:** Decay rate should match music tempo, not be hardcoded! - -**Proposed Solution:** -```cpp -// AudioBackend should query decay rate from audio system: -float decay_rate = audio_get_peak_decay_rate(); // Returns BPM-adjusted rate -realtime_peak_ *= decay_rate; -``` - -**How to calculate:** -```cpp -// In audio system (based on current BPM): -float audio_get_peak_decay_rate() { - const float bpm = tracker_get_bpm(); // e.g., 120 - const float beat_interval = 60.0f / bpm; // e.g., 0.5s - const float callback_interval = 0.128f; // Measured from device - - // Decay to 10% within one beat: - // decay_rate^(beat_interval / callback_interval) = 0.1 - // decay_rate = 0.1^(callback_interval / beat_interval) - - const float num_callbacks_per_beat = beat_interval / callback_interval; - return powf(0.1f, 1.0f / num_callbacks_per_beat); -} -``` - -**Result:** At 120 BPM, decay to 10% in 0.5s (1 beat). At 60 BPM, decay to 10% in 1.0s (1 beat). Adapts automatically! - ---- - -### Issue #2: Hardcoded BPM - -**Current** (test_demo.cc:305): -```cpp -const float beat_time = audio_time * 120.0f / 60.0f; // Hardcoded BPM -``` - -**Problem:** BPM should come from tracker/music data! - -**Proposed Solution:** -```cpp -// Tracker should expose BPM: -const float bpm = tracker_get_bpm(); // From TrackerScore -const float beat_time = audio_time * bpm / 60.0f; - -// Or even better, tracker calculates beat directly: -const float beat = tracker_get_current_beat(audio_time); -``` - -**Implementation:** -```cpp -// In tracker.h/cc: -float tracker_get_bpm() { - return g_tracker_score.bpm; // From parsed .track file -} - -float tracker_get_current_beat(float audio_time) { - return audio_time * (g_tracker_score.bpm / 60.0f); -} -``` - -**Result:** Change BPM in `.track` file → everything updates automatically! - ---- - -### Issue #3: No API for "What time is it in sequence world?" - -**User's Suggestion:** -> "ask the AudioSystem or demo system (MainSequence?) what 'time' it is in the sequence world" - -**Current Approach:** Each system tracks its own time independently -- test_demo.cc: Uses `audio_get_playback_time()` directly -- main.cc: Uses `platform_state.time + seek_time` -- MainSequence: Uses `global_time` parameter passed to `render_frame()` - -**Problem:** No central "what time should I use?" API - -**Proposed API:** -```cpp -// In MainSequence or AudioEngine: -class TimeProvider { - public: - // Returns: Current time in "sequence world" (accounting for all latencies) - float get_current_time() const { - return audio_get_playback_time(); // Use audio playback time - } - - // Returns: Current beat (fractional) - float get_current_beat() const { - return get_current_time() * (bpm_ / 60.0f); - } - - // Returns: Current peak (synchronized with current time) - float get_current_peak() const { - return audio_get_realtime_peak(); // Already synchronized - } -}; - -// Usage in test_demo.cc or main.cc: -const float time = g_time_provider.get_current_time(); -const float beat = g_time_provider.get_current_beat(); -const float peak = g_time_provider.get_current_peak(); - -// All guaranteed to be synchronized! -``` - -**Benefits:** -- Single point of query for all timing -- Hides implementation details (ring buffer, latency, etc.) -- Easy to change timing strategy globally -- Clear contract: "This is the time to use for audio-visual sync" - ---- - -## Next Steps - -### Completed ✅ -1. ✅ Use `audio_get_playback_time()` instead of physical time for beat calculation -2. ✅ Faster decay rate (0.5 instead of 0.7) to prevent constant flashing -3. ✅ Peak meter visualization to verify timing visually -4. ✅ No hardcoded latency constants (system queries its own state) - -### Future Work (Deferred) - -#### Task: Add tracker_get_bpm() API -**Purpose:** Read BPM from `.track` file instead of hardcoding in test_demo.cc/main.cc - -**Implementation:** -```cpp -// In tracker.h: -float tracker_get_bpm(); // Returns g_tracker_score.bpm - -// In tracker.cc: -float tracker_get_bpm() { - return g_tracker_score.bpm; -} - -// Usage in test_demo.cc/main.cc: -const float bpm = tracker_get_bpm(); // Instead of hardcoded 120.0f -const float beat_time = audio_time * (bpm / 60.0f); -``` - -**Benefits:** -- Change BPM in `.track` file → everything updates automatically -- No hardcoded BPM values in demo code -- Supports variable BPM (future enhancement) - ---- - -#### Task: BPM-Aware Peak Decay Rate -**Purpose:** Calculate decay rate based on current BPM to match beat interval - -**Implementation:** -```cpp -// In audio.h: -float audio_get_peak_decay_rate(); // BPM-adjusted decay - -// In audio.cc: -float audio_get_peak_decay_rate() { - const float bpm = tracker_get_bpm(); - const float beat_interval = 60.0f / bpm; // e.g., 0.5s at 120 BPM - const float callback_interval = 0.128f; // Measured from device - - // Decay to 10% within one beat: - const float n = beat_interval / callback_interval; - return powf(0.1f, 1.0f / n); -} - -// In miniaudio_backend.cc: -realtime_peak_ *= audio_get_peak_decay_rate(); // Instead of hardcoded 0.5f -``` - -**Benefits:** -- Peak decays in exactly 1 beat (regardless of BPM) -- At 120 BPM: decay = 0.5 (500ms fade) -- At 60 BPM: decay = 0.7 (1000ms fade) -- Adapts automatically to tempo changes - ---- - -#### Task: TimeProvider Class (Architectural) -**Purpose:** Centralize all timing queries with single source of truth - -**Design:** -```cpp -// In audio/time_provider.h: -class TimeProvider { - public: - TimeProvider(); - - // Returns: Current time in "sequence world" (what's being heard) - float get_current_time() const { - return audio_get_playback_time(); - } - - // Returns: Current beat (fractional, BPM-aware) - float get_current_beat() const { - const float bpm = tracker_get_bpm(); - return get_current_time() * (bpm / 60.0f); - } - - // Returns: Current peak (synchronized with current time) - float get_current_peak() const { - return audio_get_realtime_peak(); - } - - // Returns: Current BPM - float get_bpm() const { - return tracker_get_bpm(); - } -}; - -// Usage in test_demo.cc, main.cc, effects: -extern TimeProvider g_time_provider; // Global or MainSequence member - -const float time = g_time_provider.get_current_time(); -const float beat = g_time_provider.get_current_beat(); -const float peak = g_time_provider.get_current_peak(); - -// All guaranteed to be synchronized! -``` - -**Integration with MainSequence:** -```cpp -class MainSequence { - public: - TimeProvider time_provider; - - void render_frame(float global_time, float beat, float peak, - float aspect_ratio, WGPUSurface surface) { - // Effects can query: time_provider.get_current_time() etc. - } -}; -``` - -**Benefits:** -- Single point of query for all timing -- Hides implementation details (ring buffer, latency) -- Easy to change timing strategy globally -- Clear contract: "This is the time for audio-visual sync" -- No more passing time parameters everywhere - -**Migration Path:** -1. Create TimeProvider class -2. Expose as global or MainSequence member -3. Gradually migrate test_demo.cc, main.cc, effects to use it -4. Remove time/beat/peak parameters from render functions -5. Everything queries TimeProvider directly +1. **Tracks samples consumed** by the audio callback (what is being heard). +2. **Automatically accounts for ring buffer latency** without hardcoded constants. +3. **Is self-consistent** with `audio_get_realtime_peak()`, which is measured at the same moment. --- -### Design Principles Established - -1. ✅ **Single physical clock:** `platform_get_time()` is the only wall clock -2. ✅ **Systems expose their state:** `audio_get_playback_time()` knows its latency -3. ✅ **No hardcoded constants:** System queries its own state dynamically -4. ✅ **Data-driven configuration:** BPM from tracker, decay from BPM (future) -5. ✅ **Synchronized time sources:** Beat and peak from same moment +## Implementation Plan (Task #71) + +To fix the jitter and simplify the audio pipeline, the following steps should be taken. + +### Phase 1: Fix Core Timing Synchronization + +1. **Update `main.cc` and `test_demo.cc`:** Modify the main loop and test loop to use `audio_get_playback_time()` as the time source for all beat and visual calculations. This is the highest priority fix. + +### Phase 2: Implement Data-Driven Configuration + +1. **Create `tracker_get_bpm()` API:** + * **Purpose:** Provide a formal API to read the BPM from the active `.track` file, removing the need for hardcoded values. + * **Implementation:** + ```cpp + // In tracker.h: + float tracker_get_bpm(); + + // In tracker.cc: + float tracker_get_bpm() { return g_tracker_score.bpm; } + ``` + * **Adoption:** Update `main.cc` and `test_demo.cc` to call this function instead of using `g_tracker_score.bpm` directly or hardcoding `120.0f`. + +2. **Implement BPM-Aware Peak Decay Rate:** + * **Purpose:** Calculate the peak decay rate dynamically based on the current BPM, so that visual feedback feels consistent across different tempos. + * **Implementation:** + ```cpp + // In audio.h (or similar): + float audio_get_peak_decay_rate(); + + // In audio.cc: + float audio_get_peak_decay_rate() { + const float bpm = tracker_get_bpm(); + const float beat_interval = 60.0f / bpm; // e.g., 0.5s at 120 BPM + const float callback_interval = 0.128f; // Approx. from device + const float num_callbacks_per_beat = beat_interval / callback_interval; + // Decay to 10% within one beat + return powf(0.1f, 1.0f / num_callbacks_per_beat); + } + + // In miniaudio_backend.cc: + // Replace realtime_peak_ *= 0.5f; with: + realtime_peak_ *= audio_get_peak_decay_rate(); + ``` + +### Phase 3: Architectural Simplification + +1. **Implement a `TimeProvider` Class:** + * **Purpose:** Centralize all timing queries into a single, authoritative source to simplify the logic in the main loop and effects system. + * **Design:** + ```cpp + // In a new file, e.g., audio/time_provider.h: + class TimeProvider { + public: + // Returns current time for AV sync (what's being heard) + float get_current_time() const { return audio_get_playback_time(); } + + // Returns current beat (fractional, BPM-aware) + float get_current_beat() const; + + // Returns current peak (synchronized with current time) + float get_current_peak() const { return audio_get_realtime_peak(); } + + // Returns current BPM + float get_bpm() const { return tracker_get_bpm(); } + }; + ``` + * **Migration:** Gradually refactor `main.cc`, `test_demo.cc`, and visual effects to get all timing information from an instance of this class, removing the need to pass time, beat, and peak as parameters. --- -## Testing Verification - -### With Peak Meter Visualization - -Run `./build/test_demo` and observe: -- ✅ Red bar extends when kicks hit (beats 0, 2, 4, ...) -- ✅ Bar width matches FlashEffect intensity -- ✅ Bar decays before next beat (no constant red bar) -- ✅ Snares show narrower bar width (~50-70%) - -### With Peak Logging - -Run `./build/test_demo --log-peaks peaks.txt` and verify: -```bash -# Expected pattern (120 BPM, kicks every 1s): -Beat 0 (T=0.0s): High peak (kick) -Beat 1 (T=0.5s): Medium peak (snare) -Beat 2 (T=1.0s): High peak (kick) -Beat 3 (T=1.5s): Medium peak (snare) -... -``` - -### Console Output - -Should show: -``` -[AudioT=0.06, Beat=0, Frac=0.13, Peak=1.00] ← Kick -[AudioT=0.58, Beat=1, Frac=0.15, Peak=0.62] ← Snare (quieter) -[AudioT=1.09, Beat=2, Frac=0.18, Peak=0.16] ← Decayed (between beats) -[AudioT=2.62, Beat=5, Frac=0.25, Peak=1.00] ← Kick -``` - -**No more constant Peak=1.00 from beat 15 onward!** - ---- - -## Summary - -### What We Fixed -1. ✅ **Use audio playback time** instead of physical time for beat calculation -2. ✅ **Faster decay** (0.5 instead of 0.7) to match beat interval -3. ✅ **No hardcoded latency** - system queries its own state - -### What Still Needs Improvement -1. ⚠️ **BPM should come from tracker** (not hardcoded 120) -2. ⚠️ **Decay rate should be calculated from BPM** (not hardcoded 0.5) -3. ⚠️ **Centralized TimeProvider** for all timing queries - -### Key Insight (User's Contribution) -> "There should be a unique tick-source somewhere, that is the real physical_time. Then, we shouldn't hardcode the constants like 400ms, but really ask the AudioSystem or demo system (MainSequence?) what 'time' it is in the sequence world." - -**This is the correct architectural principle!** ✅ -- ONE physical clock (platform_get_time) -- Systems expose their own state (audio_get_playback_time) -- No hardcoded constants - query the system -- Data-driven configuration (BPM from tracker) - ---- +### Design Principles to Uphold -*Created: February 7, 2026* -*Architectural discussion and implementation complete* +1. ✅ **Single physical clock:** `platform_get_time()` is the only wall clock. +2. ✅ **Systems expose their state:** `audio_get_playback_time()` reports its true playback position. +3. ✅ **No hardcoded constants:** System queries its own state dynamically. +4. ✅ **Data-driven configuration:** BPM comes from the tracker, decay rate comes from BPM. +5. ✅ **Synchronized time sources:** Beat calculations and peak measurements must use the same clock (`audio_get_playback_time`). -- cgit v1.2.3