Poller thread leaves messages, Audio interprets; a buffer not a queue

src/AudioIO.cpp

@@ -527,108 +527,71 @@ struct AudioIO::ScrubState
    ScrubState(double t0, wxLongLong startClockMillis,
               double rate,
               const ScrubbingOptions &options)
-      : mTrailingIdx(0)
+      : mRate(rate)
-      , mMiddleIdx(1)
-      , mLeadingIdx(1)
-      , mRate(rate)
       , mLastScrubTimeMillis(startClockMillis)
-      , mUpdating()
       , mStartTime( t0 )
    {
+      const double t1 = options.bySpeed ? 1.0 : t0;
+      Update( t1, options );
    }
 
-   bool Update(double end, const ScrubbingOptions &options)
+   void Update(double end, const ScrubbingOptions &options)
    {
-      Duration dd { *this };
+      // Called by another thread
-      if (dd.duration <= 0)
+      mMessage.Write({ end, options });
-         return false;
-
-      wxMutexLocker locker(mUpdating);
-
-      if ( !mStarted ) {
-         const sampleCount s0 { llrint( mRate *
-            std::max( options.minTime,
-               std::min( options.maxTime, mStartTime ) ) ) };
-         const sampleCount s1 ( options.bySpeed
-            ? s0.as_double() +
-               llrint(dd.duration.as_double() * end) // end is a speed
-            : llrint(end * mRate)            // end is a time
-         );
-         auto actualDuration = std::max(sampleCount{1}, dd.duration);
-         auto success = mEntries[mMiddleIdx].Init(nullptr,
-            s0, s1, actualDuration, options, mRate);
-         if (success)
-            ++mLeadingIdx;
-         else {
-            // If not, we can wait to enqueue again later
-            dd.Cancel();
-            return false;
-         }
-
-         mStarted = true;
-         return true;
-      }
-
-      // MAY ADVANCE mLeadingIdx, BUT IT NEVER CATCHES UP TO mTrailingIdx.
-
-      bool result = true;
-      unsigned next = (mLeadingIdx + 1) % Size;
-      if (next != mTrailingIdx)
-      {
-         auto current = &mEntries[mLeadingIdx];
-         auto previous = &mEntries[(mLeadingIdx + Size - 1) % Size];
-
-         // Use the previous end as NEW start.
-         const auto s0 = previous->mS1;
-
-         const sampleCount s1 ( options.bySpeed
-            ? s0.as_double() +
-               lrint(dd.duration.as_double() * end) // end is a speed
-            : lrint(end * mRate)            // end is a time
-         );
-         auto success =
-            current->Init(previous, s0, s1, dd.duration, options, mRate);
-         if (success)
-            mLeadingIdx = next;
-         else {
-            dd.Cancel();
-            return false;
-         }
-
-         mAvailable.Signal();
-         return result;
-      }
-      else
-      {
-         // ??
-         // Queue wasn't long enough.  Write side (UI thread)
-         // has overtaken the trailing read side (Audio thread), despite
-         // my comments above!  We lose some work requests then.
-         // wxASSERT(false);
-         return false;
-      }
    }
 
    void Get(sampleCount &startSample, sampleCount &endSample,
-            sampleCount &duration,
+         sampleCount &duration)
-            Maybe<wxMutexLocker> &cleanup)
    {
-      // Audio thread is ready for the next interval.
+      // Called by the thread that calls AudioIO::FillBuffers
+      startSample = endSample = duration = -1LL;
+      Duration dd { *this };
+      if (dd.duration <= 0)
+         return;
 
-      // MAY ADVANCE mMiddleIdx, WHICH MAY EQUAL mLeadingIdx, BUT DOES NOT PASS IT.
+      Message message{ mMessage.Read() };
-
+      if ( !mStarted ) {
-      if (!cleanup) {
+         const sampleCount s0 { llrint( mRate *
-         cleanup.create(mUpdating);
+            std::max( message.options.minTime,
+               std::min( message.options.maxTime, mStartTime ) ) ) };
+         const sampleCount s1 ( message.options.bySpeed
+            ? s0.as_double() +
+               llrint(dd.duration.as_double() * message.end) // end is a speed
+            : llrint(message.end * mRate)            // end is a time
+         );
+         auto actualDuration = std::max(sampleCount{1}, dd.duration);
+         auto success = mData.Init(nullptr,
+            s0, s1, actualDuration, message.options, mRate);
+         if ( !success ) {
+            // If not, we can wait to enqueue again later
+            dd.Cancel();
+            return;
          }
-      while(! mStopped.load( std::memory_order_relaxed )&&
+         mStarted = true;
-           mMiddleIdx == mLeadingIdx)
+      }
-         mAvailable.Wait();
+      else {
+         Data newData;
+         auto previous = &mData;
 
-      auto now = ::wxGetLocalTimeMillis();
+         // Use the previous end as NEW start.
+         const auto s0 = previous->mS1;
+         const sampleCount s1 ( message.options.bySpeed
+            ? s0.as_double() +
+               lrint(dd.duration.as_double() * message.end) // end is a speed
+            : lrint(message.end * mRate)            // end is a time
+         );
+         auto success =
+            newData.Init(previous, s0, s1, dd.duration, message.options, mRate);
+         if ( !success ) {
+            dd.Cancel();
+            return;
+         }
+         mData = newData;
+      }
 
-      if ( ! mStopped.load( std::memory_order_relaxed )  &&
+      if ( ! mStopped.load( std::memory_order_relaxed ) ) {
-           mMiddleIdx != mLeadingIdx ) {
+         Data &entry = mData;
-         Data &entry = mEntries[mMiddleIdx];
          if (entry.mDuration > 0) {
             // First use of the entry
             startSample = entry.mS0;
@@ -642,32 +605,27 @@ struct AudioIO::ScrubState
             duration = entry.mSilence;
             entry.mSilence = 0;
          }
-         if (entry.mSilence == 0) {
-            // Entry is used up
-            mTrailingIdx = mMiddleIdx;
-            mMiddleIdx = (mMiddleIdx + 1) % Size;
-         }
       }
       else {
          // We got the shut-down signal, or we discarded all the work.
-         startSample = endSample = duration = -1L;
+         // Output the -1 values.
       }
    }
 
    void Stop()
    {
       mStopped.store( true, std::memory_order_relaxed );
-      wxMutexLocker locker(mUpdating);
-      mAvailable.Signal();
    }
 
+#if 0
+   // Needed only for the DRAG_SCRUB experiment
+   // Should make mS1 atomic then?
    double LastTrackTime() const
    {
       // Needed by the main thread sometimes
-      wxMutexLocker locker(mUpdating);
+      return mData.mS1.as_double() / mRate;
-      const Data &previous = mEntries[(mLeadingIdx + Size - 1) % Size];
-      return previous.mS1.as_double() / mRate;
    }
+#endif
 
    ~ScrubState() {}
 
@@ -837,19 +795,17 @@ private:
       bool cancelled { false };
    };
 
-   enum { Size = 10 };
-   Data mEntries[Size];
-   unsigned mTrailingIdx;
-   unsigned mMiddleIdx;
-   unsigned mLeadingIdx;
    double mStartTime;
    bool mStarted{ false };
    std::atomic<bool> mStopped { false };
+   Data mData;
    const double mRate;
    wxLongLong mLastScrubTimeMillis;
-
+   struct Message {
-   mutable wxMutex mUpdating;
+      double end;
-   mutable wxCondition mAvailable { mUpdating };
+      ScrubbingOptions options;
+   };
+   MessageBuffer<Message> mMessage;
 };
 #endif
 
@@ -2776,13 +2732,11 @@ bool AudioIO::IsPaused() const
 }
 
 #ifdef EXPERIMENTAL_SCRUBBING_SUPPORT
-bool AudioIO::UpdateScrub
+void AudioIO::UpdateScrub
    (double endTimeOrSpeed, const ScrubbingOptions &options)
 {
    if (mScrubState)
-      return mScrubState->Update(endTimeOrSpeed, options);
+      mScrubState->Update(endTimeOrSpeed, options);
-   else
-      return false;
 }
 
 void AudioIO::StopScrub()
@@ -2791,6 +2745,8 @@ void AudioIO::StopScrub()
       mScrubState->Stop();
 }
 
+#if 0
+// Only for DRAG_SCRUB
 double AudioIO::GetLastScrubTime() const
 {
    if (mScrubState)
@@ -2798,6 +2754,7 @@ double AudioIO::GetLastScrubTime() const
    else
       return -1.0;
 }
+#endif
 
 #endif
 
@@ -3862,7 +3819,6 @@ void AudioIO::FillBuffers()
          // PRL: or, when scrubbing, we may get work repeatedly from the
          // user interface.
          bool done = false;
-         Maybe<wxMutexLocker> cleanup;
          do {
             // How many samples to produce for each channel.
             auto frames = available;
@@ -3948,7 +3904,7 @@ void AudioIO::FillBuffers()
                {
                   sampleCount startSample, endSample;
                   mScrubState->Get(
-                     startSample, endSample, mScrubDuration, cleanup);
+                     startSample, endSample, mScrubDuration);
                   if (mScrubDuration < 0)
                   {
                      // Can't play anything

src/AudioIO.h

@@ -362,7 +362,7 @@ class AUDACITY_DLL_API AudioIO final {
    * scrub speed, adjust the beginning of the scrub interval rather than the
    * end, so that the scrub skips or "stutters" to stay near the cursor.
    */
-   bool UpdateScrub(double endTimeOrSpeed, const ScrubbingOptions &options);
+   void UpdateScrub(double endTimeOrSpeed, const ScrubbingOptions &options);
 
    void StopScrub();
 

src/tracks/ui/Scrubbing.cpp

@@ -522,14 +522,13 @@ void Scrubber::ContinueScrubbingPoll()
    // timer callback, to a left click event detected elsewhere.)
    const bool seek = TemporarilySeeks() || Seeks();
 
-   bool result = false;
    if (mPaused) {
       // When paused, make silent scrubs.
       mOptions.minSpeed = 0.0;
       mOptions.maxSpeed = mMaxSpeed;
       mOptions.adjustStart = false;
       mOptions.bySpeed = true;
-      result = gAudioIO->UpdateScrub(0, mOptions);
+      gAudioIO->UpdateScrub(0, mOptions);
    }
    else if (mSpeedPlaying) {
       // default speed of 1.3 set, so that we can hear there is a problem
@@ -543,7 +542,7 @@ void Scrubber::ContinueScrubbingPoll()
       mOptions.maxSpeed = speed +0.01;
       mOptions.adjustStart = false;
       mOptions.bySpeed = true;
-      result = gAudioIO->UpdateScrub(speed, mOptions);
+      gAudioIO->UpdateScrub(speed, mOptions);
    } else {
       const wxMouseState state(::wxGetMouseState());
       const auto trackPanel = mProject->GetTrackPanel();
@@ -558,7 +557,7 @@ void Scrubber::ContinueScrubbingPoll()
          mOptions.maxSpeed = mMaxSpeed;
          mOptions.adjustStart = true;
          mOptions.bySpeed = false;
-         result = gAudioIO->UpdateScrub(time, mOptions);
+         gAudioIO->UpdateScrub(time, mOptions);
          mLastScrubPosition = position.x;
       }
       else
@@ -572,16 +571,15 @@ void Scrubber::ContinueScrubbingPoll()
          if (mSmoothScrollingScrub) {
             const double speed = FindScrubSpeed(seek, time);
             mOptions.bySpeed = true;
-            result = gAudioIO->UpdateScrub(speed, mOptions);
+            gAudioIO->UpdateScrub(speed, mOptions);
          }
          else {
             mOptions.bySpeed = false;
-            result = gAudioIO->UpdateScrub(time, mOptions);
+            gAudioIO->UpdateScrub(time, mOptions);
          }
       }
    }
 
-   if (result)
    mScrubSeekPress = false;
 
    // else, if seek requested, try again at a later time when we might