From 26f72b110ceb07bc837eec5450084c6f5421ebb7 Mon Sep 17 00:00:00 2001
From: Paul Licameli <paul.licameli@audacityteam.org>
Date: Wed, 15 Aug 2018 23:10:25 -0400
Subject: [PATCH] Fix the hiccup at the start of scrub or play at speed...

... We need to start the polling of mouse state before starting the audio
stream, and not "nudge" AudioThread, so that AudioThread primes the ring
buffer correctly, not inserting some silence.

This requires yields to timer events in AudioIO::StartStream.
---
 src/AudioIO.cpp             |  7 +++-
 src/tracks/ui/Scrubbing.cpp | 66 +++++++++++++++++++++++--------------
 src/tracks/ui/Scrubbing.h   |  2 ++
 3 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/src/AudioIO.cpp b/src/AudioIO.cpp
index 8eb94eefc..102a1ba88 100644
--- a/src/AudioIO.cpp
+++ b/src/AudioIO.cpp
@@ -2277,7 +2277,12 @@ int AudioIO::StartStream(const TransportTracks &tracks,
 
    while( mAudioThreadShouldCallFillBuffersOnce ) {
       if (mScrubQueue)
-         mScrubQueue->Nudge();
+         // If not using a scrub thread, then we need to let the mouse polling
+         // in the main thread deliver its messages to ScrubQueue, so that
+         // FillBuffers() can progress in priming the RingBuffer.  Else we would
+         // deadlock here.  If not using the scrub thread, this should be
+         // harmless.
+         wxEventLoopBase::GetActive()->YieldFor( wxEVT_CATEGORY_TIMER );
       wxMilliSleep( 50 );
    }
 
diff --git a/src/tracks/ui/Scrubbing.cpp b/src/tracks/ui/Scrubbing.cpp
index bcd3c7d26..216ef0e01 100644
--- a/src/tracks/ui/Scrubbing.cpp
+++ b/src/tracks/ui/Scrubbing.cpp
@@ -336,8 +336,7 @@ bool Scrubber::MaybeStartScrubbing(wxCoord xx)
          double time1 = std::min(maxTime,
             viewInfo.PositionToTime(position, leftOffset)
          );
-         if (time1 != time0)
-         {
+         if (time1 != time0) {
             if (busy) {
                auto position = mScrubStartPosition;
                ctb->StopPlaying();
@@ -404,6 +403,15 @@ bool Scrubber::MaybeStartScrubbing(wxCoord xx)
             );
 #endif
             mScrubSpeedDisplayCountdown = 0;
+
+            // Must start the thread and poller first or else PlayPlayRegion
+            // will deadlock
+            StartPolling();
+            auto cleanup = finally([this]{
+               if (mScrubToken < 0)
+                  StopPolling();
+            });
+
             mScrubToken =
                ctb->PlayPlayRegion(SelectedRegion(time0, time1), options,
                                    PlayMode::normalPlay, appearance, backwards);
@@ -422,17 +430,7 @@ bool Scrubber::MaybeStartScrubbing(wxCoord xx)
          mOptions.startClockTimeMillis = ::wxGetLocalTimeMillis();
 
       if (IsScrubbing()) {
-         mPaused = false;
          mLastScrubPosition = xx;
-
-#ifdef USE_SCRUB_THREAD
-         // Detached thread is self-deleting, after it receives the Delete() message
-         mpThread = safenew ScrubPollerThread{ *this };
-         mpThread->Create(4096);
-         mpThread->Run();
-#endif
-
-         mPoller->Start(ScrubPollInterval_ms);
       }
 
       // Return true whether we started scrub, or are still waiting to decide.
@@ -492,6 +490,14 @@ bool Scrubber::StartSpeedPlay(double speed, double time0, double time1)
    );
 #endif
 
+   // Must start the thread and poller first or else PlayPlayRegion
+   // will deadlock
+   StartPolling();
+   auto cleanup = finally([this]{
+      if (mScrubToken < 0)
+         StopPolling();
+   });
+   
    mScrubSpeedDisplayCountdown = 0;
    // last buffer should not be any bigger than this.
    double lastBuffer = (2 * ScrubPollInterval_ms) / 1000.0;
@@ -501,18 +507,9 @@ bool Scrubber::StartSpeedPlay(double speed, double time0, double time1)
          PlayMode::normalPlay, appearance, backwards);
 
    if (mScrubToken >= 0) {
-      mPaused = false;
       mLastScrubPosition = 0;
-
-#ifdef USE_SCRUB_THREAD
-      // Detached thread is self-deleting, after it receives the Delete() message
-      mpThread = safenew ScrubPollerThread{ *this };
-      mpThread->Create(4096);
-      mpThread->Run();
-#endif
-
-      mPoller->Start(ScrubPollInterval_ms);
    }
+
    return true;
 }
 
@@ -631,16 +628,37 @@ void Scrubber::ContinueScrubbingUI()
    }
 }
 
-void Scrubber::StopScrubbing()
+void Scrubber::StartPolling()
 {
+   mPaused = false;
+   
+#ifdef USE_SCRUB_THREAD
+   // Detached thread is self-deleting, after it receives the Delete() message
+   mpThread = safenew ScrubPollerThread{ *this };
+   mpThread->Create(4096);
+   mpThread->Run();
+#endif
+   
+   mPoller->Start(ScrubPollInterval_ms);
+}
+
+void Scrubber::StopPolling()
+{
+   mPaused = true;
+
 #ifdef USE_SCRUB_THREAD
    if (mpThread) {
       mpThread->Delete();
       mpThread = nullptr;
    }
 #endif
-
+   
    mPoller->Stop();
+}
+
+void Scrubber::StopScrubbing()
+{
+   StopPolling();
 
    if (HasMark() && !mCancelled) {
       const wxMouseState state(::wxGetMouseState());
diff --git a/src/tracks/ui/Scrubbing.h b/src/tracks/ui/Scrubbing.h
index 30fcf9db9..37f73e75d 100644
--- a/src/tracks/ui/Scrubbing.h
+++ b/src/tracks/ui/Scrubbing.h
@@ -154,6 +154,8 @@ public:
    void CheckMenuItems();
 
 private:
+   void StartPolling();
+   void StopPolling();
    void DoScrub(bool seek);
    void OnActivateOrDeactivateApp(wxActivateEvent & event);