fix: fixes for buffer level handling, player lifecycle handling

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'
 

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) {}
 };
 

app/src/main/cpp/MixingPlayer.h

@@ -22,8 +22,9 @@ protected:
     std::vector<int> beatIdx;
     std::atomic<int> startBeat;
     int numBeatsPlaying;
+    bool mIsPlaying;
 public:
-    explicit MixingPlayer(std::vector<float> beatSound) : beatSound(beatSound), startBeat(0), numBeatsPlaying(0), mHaveMusic(false) {}
+    explicit MixingPlayer(std::vector<float> 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<AudioCallbackProvider> cb) {
         std::lock_guard<std::mutex> 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);
         }
 

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<MusicProvider>(&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<MusicProvider>(&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<int> *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);
+    int num_frames_requested = std::min((int) frames, (int) buf_size_samples);
-    size_t num_frames_available = stretcher->available();
+    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)

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<int> *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<int> *back_pressure;
 };
 
 class PlaybackEngine : public StepListener {
@@ -47,12 +49,16 @@ private:
     std::atomic<bool> haveMusicFile;
     std::unique_ptr<std::thread> musicFeed;
     std::atomic<bool> exitMusicFeedThread;
+    /** where musicFeedThread() keeps track of the fact that we have music set -- will start the audio cb */
+    std::atomic<bool> isSetMusic;
     int android_fd;
     std::atomic<bool> haveTimeRatio;
     std::atomic<double> timeRatio;
     std::atomic<int> playbackRate;
     std::atomic<int> numOutChannels;
     std::atomic<int> numInChannels;
+    /** contains the current available() frames from 'stretcher' in the audio callback thread 2 (oboe) */
+    std::atomic<int> back_pressure;
     /** this is actually in frames, not samples */
     static size_t constexpr buf_size_samples = 1024;
     void initRubberBand();

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;