From 12816810855883472ecab454f9c0d08d66f0ae52 Mon Sep 17 00:00:00 2001 From: skal Date: Thu, 5 Feb 2026 20:18:28 +0100 Subject: feat(audio): Complete Task #56 - Audio Lifecycle Refactor (All Phases) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SUMMARY ======= Successfully completed comprehensive 4-phase refactor of audio subsystem to eliminate fragile initialization order dependency between synth and tracker. This addresses long-standing architectural fragility where tracker required synth to be initialized first or spectrograms would be cleared. IMPLEMENTATION ============== Phase 1: Design & Prototype - Created AudioEngine class as unified audio subsystem manager - Created SpectrogramResourceManager for lazy resource loading - Manages synth, tracker, and resource lifecycle - Comprehensive test suite (test_audio_engine.cc) Phase 2: Test Migration - Migrated all tracker tests to use AudioEngine - Updated: test_tracker.cc, test_tracker_timing.cc, test_variable_tempo.cc, test_wav_dump.cc - Pattern: Replace synth_init() + tracker_init() with engine.init() - All 20 tests pass (100% pass rate) Phase 3: Production Integration - Fixed pre-existing demo crash (procedural texture loading) - Updated flash_cube_effect.cc and hybrid_3d_effect.cc - Migrated main.cc to use AudioEngine - Replaced tracker_update() calls with engine.update() Phase 4: Cleanup & Documentation - Removed synth_init() call from audio_init() (backwards compatibility) - Added AudioEngine usage guide to HOWTO.md - Added audio initialization protocols to CONTRIBUTING.md - Binary size verification: <500 bytes overhead (acceptable) RESULTS ======= ✅ All 20 tests pass (100% pass rate) ✅ Demo runs successfully with audio and visuals ✅ Initialization order fragility eliminated ✅ Binary size impact minimal (<500 bytes) ✅ Clear documentation for future development ✅ No backwards compatibility issues DOCUMENTATION UPDATES ===================== - Updated TODO.md: Moved Task #56 to "Recently Completed" - Updated PROJECT_CONTEXT.md: Added AudioEngine milestone - Updated HOWTO.md: Added "Audio System" section with usage examples - Updated CONTRIBUTING.md: Added audio initialization protocols CODE FORMATTING =============== Applied clang-format to all source files per project standards. FILES CREATED ============= - src/audio/audio_engine.h (new) - src/audio/audio_engine.cc (new) - src/audio/spectrogram_resource_manager.h (new) - src/audio/spectrogram_resource_manager.cc (new) - src/tests/test_audio_engine.cc (new) KEY FILES MODIFIED ================== - src/main.cc (migrated to AudioEngine) - src/audio/audio.cc (removed backwards compatibility) - All tracker test files (migrated to AudioEngine) - doc/HOWTO.md (added usage guide) - doc/CONTRIBUTING.md (added protocols) - TODO.md (marked complete) - PROJECT_CONTEXT.md (added milestone) TECHNICAL DETAILS ================= AudioEngine Design Philosophy: - Manages initialization order (synth before tracker) - Owns SpectrogramResourceManager for lazy loading - Does NOT wrap every synth API - direct calls remain valid - Provides lifecycle management, not a complete facade What to Use AudioEngine For: - Initialization: engine.init() instead of separate init calls - Updates: engine.update(music_time) instead of tracker_update() - Cleanup: engine.shutdown() for proper teardown - Seeking: engine.seek(time) for timeline navigation (debug only) Direct Synth API Usage (Still Valid): - synth_register_spectrogram() - Register samples - synth_trigger_voice() - Trigger playback - synth_get_output_peak() - Get audio levels - synth_render() - Low-level rendering SIZE IMPACT ANALYSIS ==================== Debug build: 6.2MB Size-optimized build: 5.0MB Stripped build: 5.0MB AudioEngine overhead: <500 bytes (0.01% of total) BACKWARD COMPATIBILITY ====================== No breaking changes. Tests that need low-level control can still call synth_init() directly. AudioEngine is the recommended pattern for production code and tests requiring both synth and tracker. handoff(Claude): Task #56 COMPLETE - All 4 phases finished. Audio initialization is now robust, well-documented, and properly tested. The fragile initialization order dependency has been eliminated. Co-Authored-By: Claude Sonnet 4.5 --- src/audio/miniaudio_backend.cc | 100 ++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 37 deletions(-) (limited to 'src/audio/miniaudio_backend.cc') diff --git a/src/audio/miniaudio_backend.cc b/src/audio/miniaudio_backend.cc index c2db268..baaf9bb 100644 --- a/src/audio/miniaudio_backend.cc +++ b/src/audio/miniaudio_backend.cc @@ -7,12 +7,12 @@ #include "ring_buffer.h" #include "util/debug.h" #include -#include // for abort() +#include // for abort() // Static callback for miniaudio (C API requirement) void MiniaudioBackend::audio_callback(ma_device* pDevice, void* pOutput, - const void* pInput, - ma_uint32 frameCount) { + const void* pInput, + ma_uint32 frameCount) { (void)pInput; #if defined(DEBUG_LOG_AUDIO) @@ -33,25 +33,30 @@ void MiniaudioBackend::audio_callback(ma_device* pDevice, void* pOutput, double now = ts.tv_sec + ts.tv_nsec / 1000000000.0; if (timing_initialized) { - double delta = (now - last_time) * 1000.0; // ms + double delta = (now - last_time) * 1000.0; // ms double expected = ((double)frameCount / pDevice->sampleRate) * 1000.0; double jitter = delta - expected; // Enhanced logging: Log first 20 callbacks in detail, then periodic summary if (callback_number <= 20 || callback_number % 50 == 0) { - const double elapsed_by_frames = (double)total_frames_requested / pDevice->sampleRate * 1000.0; - const double elapsed_by_time = now * 1000.0; // Convert to ms - DEBUG_AUDIO("[CB#%llu] frameCount=%u, Delta=%.2fms, Expected=%.2fms, Jitter=%.2fms, " - "TotalFrames=%llu (%.1fms), TotalTime=%.1fms, Drift=%.2fms\n", - callback_number, frameCount, delta, expected, jitter, - total_frames_requested, elapsed_by_frames, elapsed_by_time, - elapsed_by_time - elapsed_by_frames); + const double elapsed_by_frames = + (double)total_frames_requested / pDevice->sampleRate * 1000.0; + const double elapsed_by_time = now * 1000.0; // Convert to ms + DEBUG_AUDIO( + "[CB#%llu] frameCount=%u, Delta=%.2fms, Expected=%.2fms, " + "Jitter=%.2fms, " + "TotalFrames=%llu (%.1fms), TotalTime=%.1fms, Drift=%.2fms\n", + callback_number, frameCount, delta, expected, jitter, + total_frames_requested, elapsed_by_frames, elapsed_by_time, + elapsed_by_time - elapsed_by_frames); } // Detect large timing anomalies (>5ms off from expected) if (fabs(jitter) > 5.0) { - DEBUG_AUDIO("[TIMING ANOMALY] CB#%llu Delta=%.2fms, Expected=%.2fms, Jitter=%.2fms\n", - callback_number, delta, expected, jitter); + DEBUG_AUDIO( + "[TIMING ANOMALY] CB#%llu Delta=%.2fms, Expected=%.2fms, " + "Jitter=%.2fms\n", + callback_number, delta, expected, jitter); } } last_time = now; @@ -81,8 +86,10 @@ void MiniaudioBackend::audio_callback(ma_device* pDevice, void* pOutput, if (pDevice->sampleRate != 32000) { static int rate_warning = 0; if (rate_warning++ == 0) { - DEBUG_AUDIO("WARNING: Device sample rate is %u, not 32000! Resampling may occur.\n", - pDevice->sampleRate); + DEBUG_AUDIO( + "WARNING: Device sample rate is %u, not 32000! Resampling may " + "occur.\n", + pDevice->sampleRate); } } #endif /* defined(DEBUG_LOG_AUDIO) */ @@ -91,14 +98,15 @@ void MiniaudioBackend::audio_callback(ma_device* pDevice, void* pOutput, // BOUNDS CHECK: Sanity check on frameCount if (frameCount > 8192 || frameCount == 0) { - fprintf(stderr, "AUDIO CALLBACK ERROR: frameCount=%u (unreasonable!)\n", frameCount); + fprintf(stderr, "AUDIO CALLBACK ERROR: frameCount=%u (unreasonable!)\n", + frameCount); abort(); } // Read from ring buffer instead of calling synth directly AudioRingBuffer* ring_buffer = audio_get_ring_buffer(); if (ring_buffer != nullptr) { - const int samples_to_read = (int)frameCount * 2; // Stereo + const int samples_to_read = (int)frameCount * 2; // Stereo #if defined(DEBUG_LOG_RING_BUFFER) // Track buffer level and detect drops @@ -108,20 +116,24 @@ void MiniaudioBackend::audio_callback(ma_device* pDevice, void* pOutput, if (available < min_available) { min_available = available; DEBUG_RING_BUFFER("[BUFFER] CB#%llu NEW MIN: available=%d (%.1fms)\n", - callback_number, available, (float)available / (32000.0f * 2.0f) * 1000.0f); + callback_number, available, + (float)available / (32000.0f * 2.0f) * 1000.0f); } // Log buffer state for first 20 callbacks and periodically if (callback_number <= 20 || callback_number % 50 == 0) { - DEBUG_RING_BUFFER("[BUFFER] CB#%llu requested=%d, available=%d (%.1fms), min=%d\n", - callback_number, samples_to_read, available, - (float)available / (32000.0f * 2.0f) * 1000.0f, min_available); + DEBUG_RING_BUFFER( + "[BUFFER] CB#%llu requested=%d, available=%d (%.1fms), min=%d\n", + callback_number, samples_to_read, available, + (float)available / (32000.0f * 2.0f) * 1000.0f, min_available); } // CRITICAL: Verify we have enough samples if (available < samples_to_read) { - DEBUG_RING_BUFFER("[BUFFER UNDERRUN] CB#%llu requested=%d, available=%d, SHORT=%d\n", - callback_number, samples_to_read, available, samples_to_read - available); + DEBUG_RING_BUFFER( + "[BUFFER UNDERRUN] CB#%llu requested=%d, available=%d, SHORT=%d\n", + callback_number, samples_to_read, available, + samples_to_read - available); } #endif /* defined(DEBUG_LOG_RING_BUFFER) */ @@ -129,8 +141,11 @@ void MiniaudioBackend::audio_callback(ma_device* pDevice, void* pOutput, #if defined(DEBUG_LOG_RING_BUFFER) if (actually_read < samples_to_read) { - DEBUG_RING_BUFFER("[PARTIAL READ] CB#%llu requested=%d, got=%d, padded=%d with silence\n", - callback_number, samples_to_read, actually_read, samples_to_read - actually_read); + DEBUG_RING_BUFFER( + "[PARTIAL READ] CB#%llu requested=%d, got=%d, padded=%d with " + "silence\n", + callback_number, samples_to_read, actually_read, + samples_to_read - actually_read); } #endif /* defined(DEBUG_LOG_RING_BUFFER) */ } @@ -171,8 +186,8 @@ void MiniaudioBackend::init() { config.performanceProfile = ma_performance_profile_conservative; // Let Core Audio choose the period size based on conservative profile - config.periodSizeInFrames = 0; // 0 = let backend decide - config.periods = 0; // 0 = let backend decide based on performance profile + config.periodSizeInFrames = 0; // 0 = let backend decide + config.periods = 0; // 0 = let backend decide based on performance profile config.dataCallback = MiniaudioBackend::audio_callback; config.pUserData = this; @@ -187,23 +202,34 @@ void MiniaudioBackend::init() { DEBUG_AUDIO("\n=== MINIAUDIO DEVICE CONFIGURATION ===\n"); DEBUG_AUDIO(" Sample rate: %u (requested: 32000)\n", device_.sampleRate); DEBUG_AUDIO(" Channels: %u (requested: 2)\n", device_.playback.channels); - DEBUG_AUDIO(" Format: %d (requested: %d, f32=%d)\n", - device_.playback.format, config.playback.format, ma_format_f32); + DEBUG_AUDIO(" Format: %d (requested: %d, f32=%d)\n", device_.playback.format, + config.playback.format, ma_format_f32); DEBUG_AUDIO(" Period size: %u frames (%.1fms at %uHz)\n", device_.playback.internalPeriodSizeInFrames, - (float)device_.playback.internalPeriodSizeInFrames / device_.sampleRate * 1000.0f, + (float)device_.playback.internalPeriodSizeInFrames / + device_.sampleRate * 1000.0f, device_.sampleRate); - DEBUG_AUDIO(" Periods: %u (buffer multiplier)\n", device_.playback.internalPeriods); - DEBUG_AUDIO(" Backend: %s\n", ma_get_backend_name(device_.pContext->backend)); + DEBUG_AUDIO(" Periods: %u (buffer multiplier)\n", + device_.playback.internalPeriods); + DEBUG_AUDIO(" Backend: %s\n", + ma_get_backend_name(device_.pContext->backend)); DEBUG_AUDIO(" Total buffer size: %u frames (%.2fms) [period * periods]\n", - device_.playback.internalPeriodSizeInFrames * device_.playback.internalPeriods, - (float)(device_.playback.internalPeriodSizeInFrames * device_.playback.internalPeriods) / device_.sampleRate * 1000.0f); + device_.playback.internalPeriodSizeInFrames * + device_.playback.internalPeriods, + (float)(device_.playback.internalPeriodSizeInFrames * + device_.playback.internalPeriods) / + device_.sampleRate * 1000.0f); // Calculate expected callback interval if (device_.playback.internalPeriodSizeInFrames > 0) { - const float expected_callback_ms = (float)device_.playback.internalPeriodSizeInFrames / device_.sampleRate * 1000.0f; - DEBUG_AUDIO(" Expected callback interval: %.2fms (based on period size)\n", expected_callback_ms); - DEBUG_AUDIO(" WARNING: If actual callback interval differs, audio corruption may occur!\n"); + const float expected_callback_ms = + (float)device_.playback.internalPeriodSizeInFrames / + device_.sampleRate * 1000.0f; + DEBUG_AUDIO(" Expected callback interval: %.2fms (based on period size)\n", + expected_callback_ms); + DEBUG_AUDIO( + " WARNING: If actual callback interval differs, audio corruption may " + "occur!\n"); } DEBUG_AUDIO("======================================\n\n"); fflush(stderr); -- cgit v1.2.3