From b97595681242def1972b7452a2906a1a39d5e5d6 Mon Sep 17 00:00:00 2001
From: James Crook <james.k.crook@gmail.com>
Date: Sat, 24 Feb 2018 10:31:50 +0000
Subject: [PATCH] Add re-entrancy checks to Nyquist

These changes below are paving the way for aud-do:

Nyquist Effect now detects self reentrancy and skips reentry if so.
Nyquist Effect now detects if another effect has run during effect processing, and does not send Nyquist
output if so.

Neither of these preparatory changes have an effect in absence of AUD-DO (or other function
that allows Nyquist effect to call out to Audacity functions).
---
 src/commands/SelectCommand.cpp  |  4 ++++
 src/effects/Effect.cpp          | 11 ++++++++---
 src/effects/Effect.h            |  6 +++++-
 src/effects/nyquist/Nyquist.cpp | 23 +++++++++++++++++++++--
 src/effects/nyquist/Nyquist.h   |  1 +
 5 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/src/commands/SelectCommand.cpp b/src/commands/SelectCommand.cpp
index c44a49c20..4a4144c6d 100644
--- a/src/commands/SelectCommand.cpp
+++ b/src/commands/SelectCommand.cpp
@@ -207,6 +207,10 @@ void SelectTracksCommand::PopulateOrExchange(ShuttleGui & S)
 
 bool SelectTracksCommand::Apply(const CommandContext &context)
 {
+
+   // Count selection as a do-nothing effect.
+   // Used to invalidate cached selection and tracks.
+   Effect::IncEffectCounter();
    int index = 0;
    TrackList *tracks = context.GetProject()->GetTracks();
 
diff --git a/src/effects/Effect.cpp b/src/effects/Effect.cpp
index d0af0ac52..974a1c9ba 100644
--- a/src/effects/Effect.cpp
+++ b/src/effects/Effect.cpp
@@ -70,6 +70,9 @@ greater use in future.
 
 #include <unordered_map>
 
+// Effect application counter
+int Effect::nEffectsDone=0;
+
 static const int kDummyID = 20000;
 static const int kSaveAsID = 20001;
 static const int kImportID = 20002;
@@ -1074,8 +1077,9 @@ bool Effect::SetAutomationParameters(const wxString & parms)
             preset
          )
       );
-
-      return false;
+      // We are using defualt settings and we still wish to continue.
+      return true;
+      //return false;
    }
 
    if (!mUIDialog)
@@ -1246,7 +1250,7 @@ bool Effect::DoEffect(wxWindow *parent,
       returnVal = Process();
    }
 
-   if (returnVal)
+   if (returnVal && (mT1 >= mT0 ))
    {
       selectedRegion->setTimes(mT0, mT1);
    }
@@ -2246,6 +2250,7 @@ void Effect::ReplaceProcessedTracks(const bool bGoodResult)
    // The output list is no longer needed
    mOutputTracks.reset();
    mOutputTracksType = Track::None;
+   nEffectsDone++;
 }
 
 void Effect::CountWaveTracks()
diff --git a/src/effects/Effect.h b/src/effects/Effect.h
index de3ddbd02..19b051b9c 100644
--- a/src/effects/Effect.h
+++ b/src/effects/Effect.h
@@ -270,6 +270,8 @@ class AUDACITY_DLL_API Effect /* not final */ : public wxEvtHandler,
                   long style = DefaultMessageBoxStyle,
                   const wxString& titleStr = wxString{});
 
+   static void IncEffectCounter(){ nEffectsDone++;};
+
 //
 // protected virtual methods
 //
@@ -341,7 +343,6 @@ protected:
    bool TrackGroupProgress(int whichGroup, double frac, const wxString & = wxEmptyString);
 
    int GetNumWaveTracks() { return mNumTracks; }
-
    int GetNumWaveGroups() { return mNumGroups; }
 
    // Calculates the start time and selection length in samples
@@ -370,6 +371,9 @@ protected:
    void CopyInputTracks(); // trackType = Track::Wave
    void CopyInputTracks(int trackType);
 
+   // A global counter of all the successful Effect invocations.
+   static int nEffectsDone;
+
    // For the use of analyzers, which don't need to make output wave tracks,
    // but may need to add label tracks.
    class AddedAnalysisTrack {
diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp
index 851f8effb..fc7740254 100644
--- a/src/effects/nyquist/Nyquist.cpp
+++ b/src/effects/nyquist/Nyquist.cpp
@@ -75,6 +75,8 @@ effects from this one class.
 
 #include "../../Experimental.h"
 
+int NyquistEffect::mReentryCount = 0;
+
 enum
 {
    ID_Editor = 10000,
@@ -584,10 +586,21 @@ static void RegisterFunctions();
 
 bool NyquistEffect::Process()
 {
+   // Check for reentrant Nyquist commands.
+   // I'm choosing to mark skipped Nyquist commands as successful even though
+   // they are skipped.  The reason is that when Nyquist calls out to a chain,
+   // and that chain contains Nyquist,  it will be clearer if the chain completes 
+   // skipping Nyquist, rather than doing nothing at all.
+   if( mReentryCount > 0 )
+      return true;
+
+   // Restore the reentry counter (to zero) when we exit.
+   auto cleanup = valueRestorer( mReentryCount);
+   mReentryCount++;
    RegisterFunctions();
 
    bool success = true;
-
+   int nEffectsSoFar = nEffectsDone;
    mProjectChanged = false;
    EffectManager & em = EffectManager::Get();
    em.SetSkipStateFlag(false);
@@ -894,7 +907,13 @@ finish:
       dlog.ShowModal();
    }
 
-   ReplaceProcessedTracks(success);
+   // Has rug been pulled from under us by some effect done within Nyquist??
+   if( nEffectsSoFar == nEffectsDone )
+      ReplaceProcessedTracks(success);
+   else{
+      ReplaceProcessedTracks(false); // Do not use the results.
+      mT1 = mT0 - 1.0;// And don't use the times either, in resetting the selection  (make them bogus).
+   }
 
    if (!mProjectChanged)
       em.SetSkipStateFlag(true);
diff --git a/src/effects/nyquist/Nyquist.h b/src/effects/nyquist/Nyquist.h
index 103d6fb7b..97869dd9f 100644
--- a/src/effects/nyquist/Nyquist.h
+++ b/src/effects/nyquist/Nyquist.h
@@ -129,6 +129,7 @@ public:
    void Stop();
 
 private:
+   static int mReentryCount;
    // NyquistEffect implementation
 
    bool ProcessOne();