Changes to the starting and stopping of scrub/seek/at-speed...

... Necessary but not sufficient for fixing the "bounce" at start of scrub, and
for simplifying thread synchronization

src/AudioIO.cpp

@@ -549,15 +549,6 @@ struct AudioIO::ScrubState
       }
    }
 
-   // This is for avoiding deadlocks while starting a scrub:
-   // Audio stream needs to be unblocked
-   void Nudge()
-   {
-      wxMutexLocker locker(mUpdating);
-      mNudged = true;
-      mAvailable.Signal();
-   }
-
    bool Update(double end, const ScrubbingOptions &options)
    {
       // Main thread indicates a scrubbing interval
@@ -617,14 +608,14 @@ struct AudioIO::ScrubState
       if (!cleanup) {
          cleanup.create(mUpdating);
       }
-      while(!mNudged && mMiddleIdx == mLeadingIdx)
+      while(! mStopped.load( std::memory_order_relaxed )&&
+           mMiddleIdx == mLeadingIdx)
          mAvailable.Wait();
 
-      mNudged = false;
-
       auto now = ::wxGetLocalTimeMillis();
 
-      if (mMiddleIdx != mLeadingIdx) {
+      if ( ! mStopped.load( std::memory_order_relaxed )  &&
+           mMiddleIdx != mLeadingIdx ) {
          Data &entry = mEntries[mMiddleIdx];
          if (entry.mDuration > 0) {
             // First use of the entry
@@ -646,11 +637,18 @@ struct AudioIO::ScrubState
          }
       }
       else {
-         // We got the shut-down signal, or we got nudged, or we discarded all the work.
+         // We got the shut-down signal, or we discarded all the work.
          startSample = endSample = duration = -1L;
       }
    }
 
+   void Stop()
+   {
+      mStopped.store( true, std::memory_order_relaxed );
+      wxMutexLocker locker(mUpdating);
+      mAvailable.Signal();
+   }
+
    double LastTrackTime() const
    {
       // Needed by the main thread sometimes
@@ -832,12 +830,12 @@ private:
    unsigned mTrailingIdx;
    unsigned mMiddleIdx;
    unsigned mLeadingIdx;
+   std::atomic<bool> mStopped { false };
    const double mRate;
    wxLongLong mLastScrubTimeMillis;
 
    mutable wxMutex mUpdating;
    mutable wxCondition mAvailable { mUpdating };
-   bool mNudged { false };
 };
 #endif
 
@@ -2006,9 +2004,17 @@ int AudioIO::StartStream(const TransportTracks &tracks,
    mAudioThreadShouldCallFillBuffersOnce = true;
 
    while( mAudioThreadShouldCallFillBuffersOnce ) {
-      if (mScrubState)
-         mScrubState->Nudge();
-      wxMilliSleep( 50 );
+#ifndef USE_SCRUB_THREAD
+      // Yuck, we either have to poll "by hand" when scrub polling doesn't
+      // work with a thread, or else yield to timer messages, but that would
+      // execute too much else
+      if (mScrubState) {
+         mOwningProject->GetScrubber().ContinueScrubbingPoll();
+         wxMilliSleep( Scrubber::ScrubPollInterval_ms );
+      }
+      else
+#endif
+        wxMilliSleep( 50 );
    }
 
    if(mNumPlaybackChannels > 0 || mNumCaptureChannels > 0) {
@@ -2513,8 +2519,6 @@ void AudioIO::StopStream()
    //
 
    mAudioThreadFillBuffersLoopRunning = false;
-   if (mScrubState)
-      mScrubState->Nudge();
 
    // Audacity can deadlock if it tries to update meters while
    // we're stopping PortAudio (because the meter updating code
@@ -2622,8 +2626,6 @@ void AudioIO::StopStream()
       {
          // LLL:  Experienced recursive yield here...once.
          wxGetApp().Yield(true); // Pass true for onlyIfNeeded to avoid recursive call error.
-         if (mScrubState)
-            mScrubState->Nudge();
          wxMilliSleep( 50 );
       }
 
@@ -2768,6 +2770,12 @@ bool AudioIO::UpdateScrub
       return false;
 }
 
+void AudioIO::StopScrub()
+{
+   if (mScrubState)
+      mScrubState->Stop();
+}
+
 double AudioIO::GetLastScrubTime() const
 {
    if (mScrubState)

src/AudioIO.h

@@ -261,6 +261,8 @@ class AUDACITY_DLL_API AudioIO final {
    */
    bool UpdateScrub(double endTimeOrSpeed, const ScrubbingOptions &options);
 
+   void StopScrub();
+
    /** \brief return the ending time of the last scrub interval.
    */
    double GetLastScrubTime() const;

src/tracks/ui/Scrubbing.cpp

@@ -51,9 +51,7 @@ enum {
    ScrubSpeedStepsPerOctave = 4,
 #endif
 
-   ScrubPollInterval_ms = 50,
-
-   kOneSecondCountdown = 1000 / ScrubPollInterval_ms,
+   kOneSecondCountdown = 1000 / Scrubber::ScrubPollInterval_ms,
 };
 
 static const double MinStutter = 0.2;
@@ -336,8 +334,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();
@@ -403,6 +400,15 @@ bool Scrubber::MaybeStartScrubbing(wxCoord xx)
             );
 #endif
             mScrubSpeedDisplayCountdown = 0;
+
+            // Must start the thread and poller first or else PlayPlayRegion
+            // will insert some silence
+            StartPolling();
+            auto cleanup = finally([this]{
+               if (mScrubToken < 0)
+                  StopPolling();
+            });
+
             mScrubToken =
                ctb->PlayPlayRegion(SelectedRegion(time0, time1), options,
                                    PlayMode::normalPlay, appearance, backwards);
@@ -421,17 +427,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.
@@ -491,6 +487,14 @@ bool Scrubber::StartSpeedPlay(double speed, double time0, double time1)
    );
 #endif
 
+   // Must start the thread and poller first or else PlayPlayRegion
+   // will insert some silence
+   StartPolling();
+   auto cleanup = finally([this]{
+      if (mScrubToken < 0)
+         StopPolling();
+   });
+   
    mScrubSpeedDisplayCountdown = 0;
    // Aim to stop within 20 samples of correct position.
    double stopTolerance = 20.0 / options.rate;
@@ -500,18 +504,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;
 }
 
@@ -630,16 +625,38 @@ 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()
+{
+   gAudioIO->StopScrub();
+   StopPolling();
 
    if (HasMark() && !mCancelled) {
       const wxMouseState state(::wxGetMouseState());

src/tracks/ui/Scrubbing.h

@@ -72,6 +72,8 @@ struct ScrubbingOptions {
 class Scrubber : public wxEvtHandler
 {
 public:
+   static constexpr unsigned ScrubPollInterval_ms = 50;
+   
    Scrubber(AudacityProject *project);
    ~Scrubber();
 
@@ -154,6 +156,8 @@ public:
    void CheckMenuItems();
 
 private:
+   void StartPolling();
+   void StopPolling();
    void DoScrub(bool seek);
    void OnActivateOrDeactivateApp(wxActivateEvent & event);