Merge pull request audacity#6176 from saintmatthieu/6158-buzz-when-to…

…ggling-formant-preservation

6158 buzz when toggling formant preservation

libraries/lib-stretching-sequence/ClipSegment.cpp

@@ -12,7 +12,6 @@
 #include "ClipInterface.h"
 #include "SampleFormat.h"
 #include "StaffPadTimeAndPitch.h"
-
 #include <cassert>
 #include <cmath>
 #include <functional>
@@ -44,28 +43,47 @@ ClipSegment::ClipSegment(
     : mTotalNumSamplesToProduce { GetTotalNumSamplesToProduce(
          clip, durationToDiscard) }
     , mSource { clip, durationToDiscard, direction }
+    , mPreserveFormants { clip.GetPitchAndSpeedPreset() ==
+                          PitchAndSpeedPreset::OptimizeForVoice }
+    , mCentShift { clip.GetCentShift() }
     , mStretcher { std::make_unique<StaffPadTimeAndPitch>(
          clip.GetRate(), clip.GetWidth(), mSource,
          GetStretchingParameters(clip)) }
     , mOnSemitoneShiftChangeSubscription { clip.SubscribeToCentShiftChange(
          [this](int cents) {
-            std::lock_guard<std::mutex> lock(mStretcherMutex);
-            mStretcher->OnCentShiftChange(cents);
+            mCentShift = cents;
+            mUpdateCentShift = true;
          }) }
     , mOnFormantPreservationChangeSubscription {
        clip.SubscribeToPitchAndSpeedPresetChange(
           [this](PitchAndSpeedPreset preset) {
-             std::lock_guard<std::mutex> lock(mStretcherMutex);
-             mStretcher->OnFormantPreservationChange(
-                preset == PitchAndSpeedPreset::OptimizeForVoice);
+             mPreserveFormants =
+                preset == PitchAndSpeedPreset::OptimizeForVoice;
+             mUpdateFormantPreservation = true;
           })
     }
 {
 }
 
+ClipSegment::~ClipSegment()
+{
+   mOnSemitoneShiftChangeSubscription.Reset();
+   mOnFormantPreservationChangeSubscription.Reset();
+}
+
 size_t ClipSegment::GetFloats(float* const* buffers, size_t numSamples)
 {
-   std::lock_guard<std::mutex> lock(mStretcherMutex);
+   // Check if formant preservation of pitch shift needs to be updated.
+   // This approach is not immune to a race condition, but it is unlikely and
+   // not critical, as it would only affect one playback pass, during which the
+   // user could easily correct the mistake if needed. On the other hand, we
+   // cannot trust that the observer subscriptions do not get called after
+   // destruction of this object, so better not do anything too sophisticated
+   // there.
+   if (mUpdateFormantPreservation.exchange(false))
+      mStretcher->OnFormantPreservationChange(mPreserveFormants);
+   if (mUpdateCentShift.exchange(false))
+      mStretcher->OnCentShiftChange(mCentShift);
    const auto numSamplesToProduce = limitSampleBufferSize(
       numSamples, mTotalNumSamplesToProduce - mTotalNumSamplesProduced);
    mStretcher->GetSamples(buffers, numSamplesToProduce);

libraries/lib-stretching-sequence/ClipSegment.h

@@ -14,8 +14,8 @@
 #include "ClipTimeAndPitchSource.h"
 #include "Observer.h"
 #include "PlaybackDirection.h"
+#include <atomic>
 #include <memory>
-#include <mutex>
 
 class ClipInterface;
 class TimeAndPitchInterface;
@@ -31,6 +31,7 @@ class STRETCHING_SEQUENCE_API ClipSegment final : public AudioSegment
 {
 public:
    ClipSegment(ClipInterface&, double durationToDiscard, PlaybackDirection);
+   ~ClipSegment() override;
 
    // AudioSegment
    size_t GetFloats(float* const* buffers, size_t numSamples) override;
@@ -41,7 +42,10 @@ class STRETCHING_SEQUENCE_API ClipSegment final : public AudioSegment
    const sampleCount mTotalNumSamplesToProduce;
    sampleCount mTotalNumSamplesProduced = 0;
    ClipTimeAndPitchSource mSource;
-   std::mutex mStretcherMutex;
+   bool mPreserveFormants;
+   int mCentShift;
+   std::atomic<bool> mUpdateFormantPreservation = false;
+   std::atomic<bool> mUpdateCentShift = false;
    // Careful that this guy is constructed after `mSource`, which it refers to
    // in its ctor.
    // todo(mhodgkinson) make this safe.

libraries/lib-stretching-sequence/ClipTimeAndPitchSource.cpp

@@ -80,6 +80,11 @@ void ClipTimeAndPitchSource::Pull(
             // become a reasonably small value again :)
             -sampleCount { numSamplesToRead };
    }
+   else
+   {
+      for (auto i = 0u; i < mClip.GetWidth(); ++i)
+         std::fill(buffers[i], buffers[i] + samplesPerChannel, 0.f);
+   }
 }
 
 size_t ClipTimeAndPitchSource::GetWidth() const