From 2917d33866f052eca1e9daea175d95382a9087f2 Mon Sep 17 00:00:00 2001 From: NeilClack Date: Fri, 28 Aug 2026 13:55:20 -0500 Subject: [PATCH] fixed (bug) Audio not looping after ending. --- src/audio/audio_manager.cpp | 100 +++++++++++++++++++++++++------- src/audio/audio_manager.h | 11 ++++ tests/test_audio.cpp | 11 ++++ tests/test_scroller_streams.cpp | 56 ++++++++++++++++++ 4 files changed, 156 insertions(+), 22 deletions(-) diff --git a/src/audio/audio_manager.cpp b/src/audio/audio_manager.cpp index c72b8ef..b5e59a9 100644 --- a/src/audio/audio_manager.cpp +++ b/src/audio/audio_manager.cpp @@ -157,6 +157,40 @@ namespace radiantwave::audio return enabled_ ? (volume_ / 100.0f) : 0.0f; } + SDL_PropertiesID AudioManager::createInfiniteLoopOptions() + { + // MIX_PlayTrack()'s options default to a single pass; requesting + // infinite loops is what keeps a track going until it is explicitly + // faded out or the app shuts down. + SDL_PropertiesID props = SDL_CreateProperties(); + if (props) + { + SDL_SetNumberProperty(props, MIX_PROP_PLAY_LOOPS_NUMBER, -1); + } + return props; + } + + bool AudioManager::playTrackLooping(int slotIndex) + { + if (!initialized_ || !mixer_ || slotIndex < 0 || slotIndex >= 2 || + !slots_[slotIndex].track) + { + return false; + } + + SDL_PropertiesID props = createInfiniteLoopOptions(); + if (!props) + { + logger::Logger::instance().error( + "Audio", "Failed to create playback options for " + + displayNameFor(currentTrack_)); + return false; + } + bool ok = MIX_PlayTrack(slots_[slotIndex].track, props); + SDL_DestroyProperties(props); + return ok; + } + void AudioManager::shutdown() { std::lock_guard lock(mutex_); @@ -495,7 +529,7 @@ namespace radiantwave::audio else { MIX_SetTrackGain(slots_[activeSlot_].track, 1.0f); - MIX_PlayTrack(slots_[activeSlot_].track, 0); // Loop playback + playTrackLooping(activeSlot_); // Loop playback } } } @@ -553,7 +587,7 @@ namespace radiantwave::audio slot.fadeInDuration = seconds; slot.fadeInElapsed = 0.0f; MIX_SetTrackGain(slot.track, 0.0f); - MIX_PlayTrack(slot.track, 0); // Loop playback + playTrackLooping(slotIndex); // Loop playback logger::Logger::instance().info("Audio", "Started playback of " + displayNameFor(currentTrack_) + " (fade-in)"); } @@ -567,7 +601,7 @@ namespace radiantwave::audio return false; MIX_SetTrackGain(slots_[activeSlot_].track, 1.0f); - MIX_PlayTrack(slots_[activeSlot_].track, 0); // Loop playback + playTrackLooping(activeSlot_); // Loop playback // Explicit user selection: persist the manual track so it survives a // restart and is used whenever dynamic audio is turned off. Dynamic @@ -604,10 +638,11 @@ namespace radiantwave::audio slot.fadeInElapsed = 0.0f; slot.fadeOutRemaining = 0.0f; MIX_SetTrackGain(slot.track, 1.0f); - if (!MIX_TrackPlaying(slot.track)) - { - MIX_PlayTrack(slot.track, 0); // Loop playback - } + // Always restart: MIX_PlayTrack() re-starts a playing track too, so + // this cancels any in-flight stop-fade inherited from a quick + // pop->push within the fade window. Gating on MIX_TrackPlaying() + // left a fading track to finish its fade and go silently stopped. + playTrackLooping(activeSlot_); // Loop playback return true; } @@ -642,10 +677,14 @@ namespace radiantwave::audio if (!loadTrackInto(otherSlot, wav_filename)) return false; - // Fade out whatever is currently playing on the active slot. + // Fade out whatever is currently playing on the active slot. Clear any + // in-flight fade-in bookkeeping first so update() stops ramping this + // slot's gain up while the mixer fades it down. TrackSlot &active = slots_[activeSlot_]; if (active.audio) { + active.fadeInDuration = 0.0f; + active.fadeInElapsed = 0.0f; MIX_StopTrack(active.track, MIX_TrackMSToFrames(active.track, static_cast(fade * 1000.0))); active.fadeOutRemaining = fade; } @@ -673,6 +712,10 @@ namespace radiantwave::audio { continue; } + // The mixer owns the ramp now; drop any pending fade-in so the + // per-frame update() cannot fight the fade-out. + slot.fadeInDuration = 0.0f; + slot.fadeInElapsed = 0.0f; MIX_StopTrack(slot.track, MIX_TrackMSToFrames(slot.track, static_cast(fade * 1000.0))); slot.fadeOutRemaining = fade; faded = true; @@ -707,7 +750,29 @@ namespace radiantwave::audio { TrackSlot &slot = slots_[i]; if (!slot.track) + { continue; + } + + // A slot mid stop-fade belongs to the mixer: skip both the fade-in + // ramp and the active-gain pin so nothing drives its gain up while + // the mixer ramps it down (this also keeps a quit/pop fade clean). + if (slot.fadeOutRemaining > 0.0f) + { + slot.fadeOutRemaining -= dt; + if (slot.fadeOutRemaining <= 0.0f) + { + slot.fadeOutRemaining = 0.0f; + // Free the audio once the fade-out has finished; the track + // object itself is reused. + if (slot.audio) + { + MIX_DestroyAudio(slot.audio); + slot.audio = nullptr; + } + } + continue; + } if (slot.fadeInDuration > 0.0f) { @@ -724,21 +789,12 @@ namespace radiantwave::audio // Keep the active track at full per-track gain; the mixer gain // carries the user's volume / mute state. MIX_SetTrackGain(slot.track, 1.0f); - } - - if (slot.fadeOutRemaining > 0.0f) - { - slot.fadeOutRemaining -= dt; - if (slot.fadeOutRemaining <= 0.0f) + // Self-heal: playback must continue indefinitely, so restart a + // track that stopped unexpectedly (there is no pause state and + // nothing else should legitimately stop the active track). + if (slot.audio && enabled_ && !MIX_TrackPlaying(slot.track)) { - slot.fadeOutRemaining = 0.0f; - // Free the audio once the fade-out has finished; the track - // object itself is reused. - if (slot.audio) - { - MIX_DestroyAudio(slot.audio); - slot.audio = nullptr; - } + playTrackLooping(i); } } } diff --git a/src/audio/audio_manager.h b/src/audio/audio_manager.h index 404280d..082a693 100644 --- a/src/audio/audio_manager.h +++ b/src/audio/audio_manager.h @@ -66,6 +66,12 @@ public: // for tests; the real lookup is through getPlaybackDevices(). static std::string resolvePlaybackDeviceName(SDL_AudioDeviceID id); + // Builds the SDL_PropertiesID passed to MIX_PlayTrack() for tracks that + // loop forever (MIX_PROP_PLAY_LOOPS_NUMBER = -1). Exposed for tests; no + // mixer, track, or audio device is required. The caller owns the property + // set and must release it with SDL_DestroyProperties(). + static SDL_PropertiesID createInfiniteLoopOptions(); + // Playback control bool playTrack(const std::string& wav_filename); void stopTrack(); @@ -97,6 +103,11 @@ private: void startFadeIn(int slotIndex, float seconds); std::string displayNameFor(const std::string& wav_filename) const; + // Starts the track on the given slot with infinite looping. The track must + // already be loaded on the slot. Calling this on a playing/stopped/fading + // track restarts it (cancelling any in-flight stop-fade). + bool playTrackLooping(int slotIndex); + // Recreates the mixer on the given playback device id (0 = system default). // Called with mutex_ held. Preserves playback state: whichever track was // playing on the active slot is reloaded and resumed at full gain, an diff --git a/tests/test_audio.cpp b/tests/test_audio.cpp index 04c2828..6e319e5 100644 --- a/tests/test_audio.cpp +++ b/tests/test_audio.cpp @@ -59,3 +59,14 @@ TEST(AudioTest, AudioManagerVolumeClamping) { am.shutdown(); std::filesystem::remove(temp_path); } + +TEST(AudioTest, PlayOptionsLoopInfinite) { + // Regression: MIX_PlayTrack() must not default to a single pass. Tracks + // have to loop forever so playback never runs out mid-scroll; a track that + // stops (rather than attenuates) also can't be revived by raising app + // volume. Assert the options request infinite looping (-1). + SDL_PropertiesID props = AudioManager::createInfiniteLoopOptions(); + ASSERT_NE(props, 0u); + EXPECT_EQ(SDL_GetNumberProperty(props, MIX_PROP_PLAY_LOOPS_NUMBER, 0), -1); + SDL_DestroyProperties(props); +} diff --git a/tests/test_scroller_streams.cpp b/tests/test_scroller_streams.cpp index 8156aa5..f0a9736 100644 --- a/tests/test_scroller_streams.cpp +++ b/tests/test_scroller_streams.cpp @@ -94,6 +94,62 @@ TEST(ScrollerStreamsTest, InterleavesSelectedWithDefault) EXPECT_EQ(streams[3].audioFilename, "OriginalAudio.wav"); } +TEST(ScrollerStreamsTest, InterleavedListCyclesBackToFirstSelected) +{ + // Three selected proclamations: the built list must be + // [S1, Default, S2, Default, S3, Default]. ScrollerPage advances through + // this list with a modulo, so once the trailing default finishes the cycle + // restarts at the FIRST selected proclamation — the "always on" default + // must never accumulate or take over the queue. + ProclamationRecord b; + b.id = "B"; + b.title = "B"; + b.content = "bbbb"; + b.selected = true; + b.solfeggio_frequency_id = 2; + + ProclamationRecord c; + c.id = "C"; + c.title = "C"; + c.content = "cccc"; + c.selected = true; + c.solfeggio_frequency_id = 3; + + ProclamationRecord d; + d.id = "D"; + d.title = "D"; + d.content = "dddd"; + d.selected = true; + d.solfeggio_frequency_id = 4; + + ProclamationRecord def; + def.id = kDefaultId; + def.title = "Default"; + def.content = "dddddddddd"; + def.selected = true; + def.solfeggio_frequency_id = 5; + + auto streams = buildStreams({b, def, c, d}, {}, palette(), {}, kDefaultId); + + ASSERT_EQ(streams.size(), 6u); + EXPECT_EQ(streams[0].name, "B"); + EXPECT_EQ(streams[1].name, "Default"); + EXPECT_EQ(streams[2].name, "C"); + EXPECT_EQ(streams[3].name, "Default"); + EXPECT_EQ(streams[4].name, "D"); + EXPECT_EQ(streams[5].name, "Default"); + + // Every default copy is independent playback state (fresh counters), so a + // copy finishing its passes advances to the next stream rather than + // restarting "the default" forever. + for (size_t i = 1; i < streams.size(); i += 2) { + EXPECT_EQ(streams[i].name, "Default"); + EXPECT_EQ(streams[i].completed, 0) << "default copy " << i; + EXPECT_EQ(streams[i].wordsPlaced, 0) << "default copy " << i; + EXPECT_EQ(streams[i].repeat, kDefaultRepeatCount) << "default copy " << i; + } +} + TEST(ScrollerStreamsTest, DefaultOnlyWhenNothingSelected) { ProclamationRecord p1;