From de56cd114a678003dfef17abbd74ebd9203964eb Mon Sep 17 00:00:00 2001
From: Chris Cannam <cannam@all-day-breakfast.com>
Date: Thu, 29 Sep 2022 11:19:21 +0100
Subject: [PATCH] Update the resampler logic to follow that of R2 more - ignore
 the pitch hq/hs setting in offline mode entirely. The alternative is too
 tricky to handle elegantly in the command line tool, which I think means too
 complicated

---
 rubberband/RubberBandStretcher.h |  8 ++---
 src/finer/R3Stretcher.cpp        |  8 +----
 src/finer/R3Stretcher.h          | 50 ++++++++++++++++++++++++++------
 3 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/rubberband/RubberBandStretcher.h b/rubberband/RubberBandStretcher.h
index 756b636..657156a 100644
--- a/rubberband/RubberBandStretcher.h
+++ b/rubberband/RubberBandStretcher.h
@@ -328,10 +328,10 @@ public:
      *   perceived pitch profile of the voice or instrument.
      *
      * 10. Flags prefixed \c OptionPitch control the method used for
-     * pitch shifting. In the R2 engine they may be changed at any
-     * time but affect only realtime mode (in offline mode the method
-     * cannot be changed). In the R3 engine they affect both realtime
-     * and offline modes but are fixed on construction.
+     * pitch shifting. These options affect only realtime mode. In
+     * offline mode the method is not adjustable. In the R2 engine
+     * these options may be changed at any time; in the R3 engine they
+     * may be set only on construction.
      *
      *   \li \c OptionPitchHighSpeed - Favour CPU cost over sound
      *   quality. This is the default. Use this when time-stretching
diff --git a/src/finer/R3Stretcher.cpp b/src/finer/R3Stretcher.cpp
index d2db24f..d81a240 100644
--- a/src/finer/R3Stretcher.cpp
+++ b/src/finer/R3Stretcher.cpp
@@ -274,13 +274,7 @@ R3Stretcher::createResampler()
     Profiler profiler("R3Stretcher::createResampler");
     
     Resampler::Parameters resamplerParameters;
-
-    if (m_parameters.options & RubberBandStretcher::OptionPitchHighQuality) {
-        resamplerParameters.quality = Resampler::Best;
-    } else {
-        resamplerParameters.quality = Resampler::FastestTolerable;
-    }
-    
+    resamplerParameters.quality = Resampler::FastestTolerable;
     resamplerParameters.initialSampleRate = m_parameters.sampleRate;
     resamplerParameters.maxBufferSize = m_guideConfiguration.longestFftSize;
 
diff --git a/src/finer/R3Stretcher.h b/src/finer/R3Stretcher.h
index 889e216..1d4bb66 100644
--- a/src/finer/R3Stretcher.h
+++ b/src/finer/R3Stretcher.h
@@ -418,21 +418,53 @@ protected:
 
         if (before) *before = false;
         if (after) *after = false;
+
+        // Some of the logic about when/whether to resample may have
+        // already been decided, because we might not have a
+        // resampler. This call is for the processing path, we don't
+        // make big decisions here and can't be saying we should
+        // resample if there is no resampler
         if (!m_resampler) return;
 
-        if (m_parameters.options &
-            RubberBandStretcher::OptionPitchHighConsistency) {
+        // In offline mode we always ignore the OptionPitch setting
+        // and resample afterwards, like OptionPitchHighConsistency
+        // (except that we don't resample for ratio of 1.0). This is
+        // because (a) the initial algorithm was developed and tested
+        // that way, (b) R2 works that way and nobody has ever
+        // complained, and (c) otherwise for the command-line tool
+        // (which is probably the most common way to use RB offline)
+        // we would have to choose between defaulting to lower-quality
+        // OptionPitchHighSpeed or offering yet another setting, both
+        // of which are undesirable.
+        
+        if (!isRealTime()) {
+            if (m_pitchScale != 1.0) {
+                // Offline, any pitch scale
+                if (after) *after = true;
+            }
+        } else if (m_parameters.options &
+                   RubberBandStretcher::OptionPitchHighConsistency) {
+            // RT, any pitch scale, HC
             if (after) *after = true;
             
         } else if (m_pitchScale != 1.0) {
-            if (m_pitchScale > 1.0 &&
-                (m_parameters.options &
-                 RubberBandStretcher::OptionPitchHighQuality)) {
-                if (after) *after = true;
+
+            bool hq = (m_parameters.options &
+                       RubberBandStretcher::OptionPitchHighQuality);
+            // We already handled OptionPitchHighConsistency, so if
+            // !hq then we must be HighSpeed
+            if (m_pitchScale > 1.0) {
+                if (hq) {
+                    if (after) *after = true;
+                } else {
+                    if (before) *before = true;
+                }
             } else if (m_pitchScale < 1.0) {
-                if (after) *after = true;
-            } else {
-                if (before) *before = true;
+                if (hq) {
+                    if (before) *before = true;
+                } else {
+                    if (after) *after = true;
+                }
             }
         }
     }