From 1a3991db45bf95329b223277b27cdc1466396e88 Mon Sep 17 00:00:00 2001 From: David Madl Date: Mon, 23 Mar 2026 01:07:05 +0100 Subject: [PATCH] fix: fixes for buffer level handling, player lifecycle handling --- TODO.md | 2 + app/src/main/cpp/AudioCallback.h | 7 +- app/src/main/cpp/MixingPlayer.h | 9 ++- app/src/main/cpp/PlaybackEngine.cpp | 81 ++++++++++++++++--- app/src/main/cpp/PlaybackEngine.h | 8 +- .../java/at/lockstep/app/MainActivity.java | 1 + 6 files changed, 95 insertions(+), 13 deletions(-) diff --git a/TODO.md b/TODO.md index 1b20f40..5a8b80e 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,7 @@ ## TODO +* do not open oboe upon app startup, only if we are actually recording + * PlaybackEngine - Buffer overrun on output for channel (1.000000) - we are feeding too much data into 'stretcher' diff --git a/app/src/main/cpp/AudioCallback.h b/app/src/main/cpp/AudioCallback.h index 00dcda3..23045a3 100644 --- a/app/src/main/cpp/AudioCallback.h +++ b/app/src/main/cpp/AudioCallback.h @@ -13,7 +13,12 @@ class AudioCallbackProvider { public: virtual ~AudioCallbackProvider() {} - /** in current impl, this passes a buffer where data may already live. the provider may add to it and re-normalize to [-1.0, 1.0]. */ + /** + * in current impl, this passes a buffer where data may already live. + * the provider may add to it and re-normalize to [-1.0, 1.0]. + * + * upon is_finished=true it will not do anything - the caller is responsible for 0-ing the data buffer. + * */ virtual void onAudioReady(float *data, int32_t frames) {} }; diff --git a/app/src/main/cpp/MixingPlayer.h b/app/src/main/cpp/MixingPlayer.h index 56c8b87..c6e684d 100644 --- a/app/src/main/cpp/MixingPlayer.h +++ b/app/src/main/cpp/MixingPlayer.h @@ -22,8 +22,9 @@ protected: std::vector beatIdx; std::atomic startBeat; int numBeatsPlaying; + bool mIsPlaying; public: - explicit MixingPlayer(std::vector beatSound) : beatSound(beatSound), startBeat(0), numBeatsPlaying(0), mHaveMusic(false) {} + explicit MixingPlayer(std::vector beatSound) : beatSound(beatSound), startBeat(0), numBeatsPlaying(0), mIsPlaying(false), mHaveMusic(false) {} virtual ~MixingPlayer() = default; @@ -52,6 +53,7 @@ public: // Typically, start the stream after querying some stream information, as well as some input from the user result = mStream->requestStart(); + mIsPlaying = (result == Result::OK); return (int32_t) result; } @@ -67,8 +69,11 @@ public: mStream->close(); mStream.reset(); } + mIsPlaying = false; } + bool isPlaying() { return mIsPlaying; } + void setMusic(std::shared_ptr cb) { std::lock_guard lock(mLock); mMusic = std::move(cb); @@ -115,6 +120,8 @@ public: } if(mHaveMusic.load()) { + // note: the contract for onAudioReady() upon is_finished=true implies it will not do anything + // (the buffer must be set to all-0 here in the caller) mMusic->onAudioReady(floatData, numFrames); } diff --git a/app/src/main/cpp/PlaybackEngine.cpp b/app/src/main/cpp/PlaybackEngine.cpp index fe45486..877d095 100644 --- a/app/src/main/cpp/PlaybackEngine.cpp +++ b/app/src/main/cpp/PlaybackEngine.cpp @@ -79,6 +79,7 @@ PlaybackEngine::PlaybackEngine(std::string filesDir, int resid): mFilesDir(filesDir), haveMusicFile(false), exitMusicFeedThread(false), + isSetMusic(false), android_fd(0), haveTimeRatio(false), timeRatio(1.0), @@ -86,7 +87,8 @@ PlaybackEngine::PlaybackEngine(std::string filesDir, int resid): // these 3 values are preliminary -- will be set from MixingPlayer defaults in the ctor body below playbackRate(48000), numOutChannels(2), - numInChannels(2) + numInChannels(2), + back_pressure(0) { LOGI("PlaybackEngine()"); LOGI("NDK LOG_LEVEL=%d", LOG_LEVEL); @@ -98,10 +100,6 @@ PlaybackEngine::PlaybackEngine(std::string filesDir, int resid): LOGI("read_mp3() for bump effect, is_ok=%d", is_ok); mPlayer = new MixingPlayer(samples); - int32_t res = mPlayer->startAudio(); - playbackRate.store(mPlayer->getRate()); - numOutChannels.store(mPlayer->getNumChannels()); - LOGI("startAudio() = %d rate=%d channels=%d", res, playbackRate.load(), numOutChannels.load()); // configure stretcher and start musicFeedThread() initRubberBand(); @@ -109,6 +107,7 @@ PlaybackEngine::PlaybackEngine(std::string filesDir, int resid): void PlaybackEngine::initRubberBand() { // we do not yet have a music file with actual sampling rate, so set the default ratio + stretcher.reset(); stretcher.setTimeRatio(1.0); stretcher.setPitchScale(1.0); @@ -159,6 +158,8 @@ void PlaybackEngine::closeMusicFile() { close(android_fd); android_fd = 0; } + isSetMusic.store(false); + mPlayer->setMusic(nullptr); } void PlaybackEngine::mapChannels(int *channel_map, int num_ch_in, int num_ch_out) { @@ -217,6 +218,18 @@ void PlaybackEngine::musicFeedThread() { // thread 2: polling for decoding more mp3 -> process() -- getSamplesRequired() while(!exitMusicFeedThread.load()) { + if(!haveMusicFile.load()) { + // while no MusicProvider is connected, no samples will be read from 'stretcher' + // therefore, we do not write any samples into it! + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + continue; + } + + if(!isSetMusic.load()) { + mPlayer->setMusic(std::make_shared(&stretcher, buf_size_samples, numOutChannels.load(), &back_pressure)); + isSetMusic.store(true); + } + if(haveTimeRatio.load()) { stretcher.setTimeRatio(timeRatio.load()); haveTimeRatio.store(false); @@ -235,6 +248,22 @@ void PlaybackEngine::musicFeedThread() { mapChannels(channel_map, num_ch_in, num_ch_out); // do work ... + + // note: getSamplesRequired() itself only gives us how many samples to create another + // output buffer increment, not if the output buffer has been emptied. + // We need to manage the buffer sizes ourselves. + + // this draft should always keep the output buffers filled at 50-100 ms + int target_output_buffer_frames = 100 * playbackRate.load() / 1000; // 100 ms worth of audio + if(idebug < 10) { + LOGI("back_pressure available=%d target=%d", back_pressure.load(), target_output_buffer_frames); + } + if(back_pressure.load() >= target_output_buffer_frames) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + + // at 48000 Hz playbackRate, the 512-1024 frames returned here give us additional (10-21 ms) output buffer + // (this is somewhat approximate, but the above control loop should keep us within a reasonable buffer size) size_t num_samples = stretcher.getSamplesRequired(); // note: how much to sleep until output has played x samples...? @@ -242,6 +271,7 @@ void PlaybackEngine::musicFeedThread() { // (is it like double-buffering implemented in 'stretcher'?) if (num_samples == 0) { + // this was never the case in actual testing -- see note above. LOGD("waiting %d us for getSamplesRequired()", loop_delay_us); std::this_thread::sleep_for(std::chrono::microseconds(loop_delay_us)); continue; @@ -283,6 +313,8 @@ void PlaybackEngine::musicFeedThread() { // next iteration will play silence closeMusicFile(); stretcher.setTimeRatio(1.0); // buffer size for playing silence is computed from 'playbackRate', so reset timeRatio + stretcher.process(buf_ptr, 0, true); // set end of playback + mPlayer->stopAudio(); continue; } if(err == MPG123_DONE) { @@ -290,6 +322,8 @@ void PlaybackEngine::musicFeedThread() { LOGI("finished reading mp3 file (MPG123_DONE)"); closeMusicFile(); stretcher.setTimeRatio(1.0); // buffer size for playing silence is computed from 'playbackRate', so reset timeRatio + stretcher.process(buf_ptr, 0, true); // set end of playback + mPlayer->stopAudio(); continue; } @@ -341,14 +375,31 @@ void PlaybackEngine::playMusic(int fd) { numInChannels.store(musicFile->channels); haveMusicFile.store(true); } - mPlayer->setMusic(std::make_shared(&stretcher, buf_size_samples, numOutChannels.load())); + + bool is_finished = (stretcher.available() == -1); + if(is_finished) { + // so that we may play again after "final chunk" + closeRubberBand(); + initRubberBand(); + } + + if(!mPlayer->isPlaying()) { + int32_t res = mPlayer->startAudio(); + playbackRate.store(mPlayer->getRate()); + numOutChannels.store(mPlayer->getNumChannels()); + LOGI("startAudio() = %d rate=%d channels=%d", res, playbackRate.load(), numOutChannels.load()); + } + + // to wait the 50 ms that the musicFeedThread() is idling when it first receives a file + // we don't call mPlayer->setMusic() here, but in the musicFeedThread() } -MusicProvider::MusicProvider(RubberBand::RubberBandStretcher *stretcher, size_t buf_size_samples, int num_ch_out) : +MusicProvider::MusicProvider(RubberBand::RubberBandStretcher *stretcher, size_t buf_size_samples, int num_ch_out, std::atomic *back_pressure) : stretcher(stretcher), idebug(0), buf_size_samples(buf_size_samples), - num_ch_out(num_ch_out) + num_ch_out(num_ch_out), + back_pressure(back_pressure) { buf = (float*) malloc(buf_size_samples*num_ch_out*sizeof(float)); buf_ptr = (float**) malloc(num_ch_out * sizeof(float*)); @@ -373,8 +424,18 @@ void MusicProvider::onAudioReady(float *data, int32_t frames) { } // 1. read from oboe into our temp de-interleaved buffer 'buf' - size_t num_frames_requested = std::min((size_t) frames, buf_size_samples); - size_t num_frames_available = stretcher->available(); + int num_frames_requested = std::min((int) frames, (int) buf_size_samples); + int num_frames_available = stretcher->available(); + bool is_finished = (num_frames_available == -1); + (*back_pressure).store((int) num_frames_available); + + if(is_finished) { + return; + } + + if(idebug < 10) { + LOGI("onAudioReady() available=%d", num_frames_available); + } if(num_frames_available < num_frames_requested) { // this is an audio glitch // TODO: bubble info upwards, in a counter (so we can collect device-specific glitch stats) diff --git a/app/src/main/cpp/PlaybackEngine.h b/app/src/main/cpp/PlaybackEngine.h index 39daeb9..c9241a1 100644 --- a/app/src/main/cpp/PlaybackEngine.h +++ b/app/src/main/cpp/PlaybackEngine.h @@ -18,7 +18,7 @@ /** Provides music through a regular callback to oboe. Called from separate oboe thread. */ class MusicProvider : public AudioCallbackProvider { public: - explicit MusicProvider(RubberBand::RubberBandStretcher *stretcher, size_t buf_size_samples, int num_ch_out); + explicit MusicProvider(RubberBand::RubberBandStretcher *stretcher, size_t buf_size_samples, int num_ch_out, std::atomic *back_pressure); ~MusicProvider() override; /** Called from separate oboe thread. */ @@ -30,6 +30,8 @@ private: int idebug; size_t buf_size_samples; int num_ch_out; + /** contains the current available() frames from 'stretcher' in the audio callback thread 2 (oboe) */ + std::atomic *back_pressure; }; class PlaybackEngine : public StepListener { @@ -47,12 +49,16 @@ private: std::atomic haveMusicFile; std::unique_ptr musicFeed; std::atomic exitMusicFeedThread; + /** where musicFeedThread() keeps track of the fact that we have music set -- will start the audio cb */ + std::atomic isSetMusic; int android_fd; std::atomic haveTimeRatio; std::atomic timeRatio; std::atomic playbackRate; std::atomic numOutChannels; std::atomic numInChannels; + /** contains the current available() frames from 'stretcher' in the audio callback thread 2 (oboe) */ + std::atomic back_pressure; /** this is actually in frames, not samples */ static size_t constexpr buf_size_samples = 1024; void initRubberBand(); diff --git a/app/src/main/java/at/lockstep/app/MainActivity.java b/app/src/main/java/at/lockstep/app/MainActivity.java index 85503cd..2d1a8ee 100644 --- a/app/src/main/java/at/lockstep/app/MainActivity.java +++ b/app/src/main/java/at/lockstep/app/MainActivity.java @@ -128,6 +128,7 @@ public class MainActivity extends AppCompatActivity implements LstForegroundServ // TODO: since the Service keeps running, we must signal oboe to stop playing // TODO: signal the pause to the C++ lib + startService(LstForegroundService.stopIntent(MainActivity.this)); } private boolean isForeground = false;