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/tests/test_assets.cc | 62 +++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 25 deletions(-) (limited to 'src/tests/test_assets.cc') diff --git a/src/tests/test_assets.cc b/src/tests/test_assets.cc index 5ae266e..86b4ba4 100644 --- a/src/tests/test_assets.cc +++ b/src/tests/test_assets.cc @@ -55,7 +55,7 @@ int main() { GetAsset(AssetId::ASSET_PROC_NOISE_256, &proc_size); assert(proc_data_1 != nullptr); // Expect 256x256 RGBA8 + 8 byte header - assert(proc_size == 256 * 256 * 4 + 8); + assert(proc_size == 256 * 256 * 4 + 8); // Verify first few bytes of DATA (skip header) // Header is 8 bytes @@ -97,24 +97,30 @@ int main() { assert(tex.pixels != nullptr); assert(tex.width == 2); assert(tex.height == 2); - + // Verify pixels (Expected RGBA) // Pixel 0: Red (255, 0, 0, 255) - assert(tex.pixels[0] == 255 && tex.pixels[1] == 0 && tex.pixels[2] == 0 && tex.pixels[3] == 255); + assert(tex.pixels[0] == 255 && tex.pixels[1] == 0 && tex.pixels[2] == 0 && + tex.pixels[3] == 255); // Pixel 1: Green (0, 255, 0, 255) - assert(tex.pixels[4] == 0 && tex.pixels[5] == 255 && tex.pixels[6] == 0 && tex.pixels[7] == 255); + assert(tex.pixels[4] == 0 && tex.pixels[5] == 255 && tex.pixels[6] == 0 && + tex.pixels[7] == 255); // Pixel 2: Blue (0, 0, 255, 255) - assert(tex.pixels[8] == 0 && tex.pixels[9] == 0 && tex.pixels[10] == 255 && tex.pixels[11] == 255); + assert(tex.pixels[8] == 0 && tex.pixels[9] == 0 && tex.pixels[10] == 255 && + tex.pixels[11] == 255); // Pixel 3: White (255, 255, 255, 255) - assert(tex.pixels[12] == 255 && tex.pixels[13] == 255 && tex.pixels[14] == 255 && tex.pixels[15] == 255); - + assert(tex.pixels[12] == 255 && tex.pixels[13] == 255 && + tex.pixels[14] == 255 && tex.pixels[15] == 255); + printf("Texture Asset content verification: SUCCESS\n"); // Test Unknown Procedural Function printf("\nRunning Unknown Procedural Function test...\n"); size_t unknown_size = 0; - // This should print an error to stderr: "Error: Unknown procedural function..." - const uint8_t* unknown_data = GetAsset(AssetId::ASSET_PROC_UNKNOWN, &unknown_size); + // This should print an error to stderr: "Error: Unknown procedural + // function..." + const uint8_t* unknown_data = + GetAsset(AssetId::ASSET_PROC_UNKNOWN, &unknown_size); assert(unknown_data == nullptr); assert(unknown_size == 0); printf("Unknown Procedural Function test: SUCCESS\n"); @@ -122,17 +128,20 @@ int main() { // Test Failing Procedural Function printf("\nRunning Failing Procedural Function test...\n"); size_t fail_size = 0; - // This should print an error to stderr: "Error: Procedural generation failed..." + // This should print an error to stderr: "Error: Procedural generation + // failed..." const uint8_t* fail_data = GetAsset(AssetId::ASSET_PROC_FAIL, &fail_size); assert(fail_data == nullptr); assert(fail_size == 0); printf("Failing Procedural Function test: SUCCESS\n"); // Test Out-of-Bounds ID (beyond ASSET_LAST_ID) - // Casting to AssetId to suppress compiler warnings if checking strict enum types + // Casting to AssetId to suppress compiler warnings if checking strict enum + // types printf("\nRunning Out-of-Bounds ID test...\n"); size_t oob_size = 0; - const uint8_t* oob_data = GetAsset((AssetId)((int)AssetId::ASSET_LAST_ID + 1), &oob_size); + const uint8_t* oob_data = + GetAsset((AssetId)((int)AssetId::ASSET_LAST_ID + 1), &oob_size); assert(oob_data == nullptr); assert(oob_size == 0); printf("Out-of-Bounds ID test: SUCCESS\n"); @@ -141,25 +150,28 @@ int main() { printf("\nRunning DropAsset edge cases test...\n"); // Invalid ID DropAsset((AssetId)((int)AssetId::ASSET_LAST_ID + 1), nullptr); - + // Mismatched pointer (should do nothing) - // We use proc_data_2 which is valid, but pass a different ID (e.g. ASSET_TEST_ASSET_1 which is static) + // We use proc_data_2 which is valid, but pass a different ID (e.g. + // ASSET_TEST_ASSET_1 which is static) DropAsset(AssetId::ASSET_TEST_ASSET_1, proc_data_2); - // Verify proc_data_2 is still valid (by checking it's in cache). - // Note: GetAsset will just return the cached pointer. If DropAsset worked, it would have been cleared. - // But wait, DropAsset clears it from cache. - // The correct test for "mismatched pointer" is: pass the correct ID but WRONG pointer. - // This ensures we don't clear the cache if the user passes a stale/wrong pointer. - + // Verify proc_data_2 is still valid (by checking it's in cache). + // Note: GetAsset will just return the cached pointer. If DropAsset worked, it + // would have been cleared. But wait, DropAsset clears it from cache. The + // correct test for "mismatched pointer" is: pass the correct ID but WRONG + // pointer. This ensures we don't clear the cache if the user passes a + // stale/wrong pointer. + // Let's try to drop ASSET_PROC_NOISE_256 with a dummy pointer. uint8_t dummy_ptr; DropAsset(AssetId::ASSET_PROC_NOISE_256, &dummy_ptr); - // Check if asset is still in cache (should be, as we didn't drop the real one) - // We can't peek into g_asset_cache directly from here (it's static). - // But GetAsset should return the SAME pointer as proc_data_2 without re-generation. - // If it was dropped, GetAsset would re-generate and likely return a NEW pointer (new allocation). + // Check if asset is still in cache (should be, as we didn't drop the real + // one) We can't peek into g_asset_cache directly from here (it's static). But + // GetAsset should return the SAME pointer as proc_data_2 without + // re-generation. If it was dropped, GetAsset would re-generate and likely + // return a NEW pointer (new allocation). const uint8_t* proc_data_3 = GetAsset(AssetId::ASSET_PROC_NOISE_256, nullptr); - assert(proc_data_3 == proc_data_2); + assert(proc_data_3 == proc_data_2); printf("DropAsset edge cases test: SUCCESS\n"); printf("Procedural Asset test PASSED\n"); -- cgit v1.2.3