Correct a bug with dynamic updates on offed EGs

When offing a voice, the release time is cut to a minimum. Dynamic
updates would reset this to the original value. The commit adds a way to
disable dynamic EG updates and does so when offing the voice.
The engine will also steal the oldest offed voice as a last resort to
hopefully sound the newly played note every time.

src/sfizz/ADSREnvelope.cpp

@@ -34,7 +34,7 @@ Float ADSREnvelope::secondsToExpRate(Float timeInSeconds) const noexcept
     if (timeInSeconds <= 0)
         return Float(0.0);
 
-    timeInSeconds = std::max(Float(25e-3), timeInSeconds);
+    timeInSeconds = std::max(Float(Default::offTime), timeInSeconds);
     return std::exp(Float(-9.0) / (timeInSeconds * sampleRate));
 };
 
@@ -42,6 +42,7 @@ void ADSREnvelope::reset(const EGDescription& desc, const Region& region, int de
 {
     this->sampleRate = sampleRate;
     desc_ = &desc;
+    dynamic_ = desc.dynamic;
     triggerVelocity_ = velocity;
     currentState = State::Delay; // Has to be before the update
     updateValues(delay);
@@ -58,7 +59,6 @@ void ADSREnvelope::updateValues(int delay) noexcept
 {
     if (currentState == State::Delay)
         this->delay = delay + secondsToSamples(desc_->getDelay(midiState_, triggerVelocity_, delay));
-
     this->attackStep = secondsToLinRate(desc_->getAttack(midiState_, triggerVelocity_, delay));
     this->decayRate = secondsToExpRate(desc_->getDecay(midiState_, triggerVelocity_, delay));
     this->releaseRate = secondsToExpRate(desc_->getRelease(midiState_, triggerVelocity_, delay));
@@ -70,7 +70,7 @@ void ADSREnvelope::updateValues(int delay) noexcept
 
 void ADSREnvelope::getBlock(absl::Span<Float> output) noexcept
 {
-    if (desc_ && desc_->dynamic) {
+    if (dynamic_) {
         int processed = 0;
         int remaining = static_cast<int>(output.size());
         while(remaining > 0) {

src/sfizz/ADSREnvelope.h

@@ -76,6 +76,12 @@ public:
      */
     int getRemainingDelay() const noexcept { return delay; }
 
+    /**
+     * @brief Stop dynamic updates of the EG values.
+     *
+     */
+    void stopDynamicUpdates() { dynamic_ = false; }
+
 private:
     float sampleRate { config::defaultSampleRate };
     int secondsToSamples(Float timeInSeconds) const noexcept;
@@ -99,6 +105,7 @@ private:
     const EGDescription* desc_ { nullptr };
     const MidiState& midiState_;
     float triggerVelocity_ { 0.0f };
+    bool dynamic_ { false };
     int delay { 0 };
     Float attackStep { 0 };
     Float decayRate { 0 };

src/sfizz/Config.h

@@ -153,17 +153,12 @@ namespace config {
      */
     static constexpr float overflowVoiceMultiplier { 1.5f };
     static_assert(overflowVoiceMultiplier >= 1.0f, "This needs to add voices");
-
     /**
-     * @brief Calculate the effective voice number for the polyphony setting,
+     * @brief Minimum number of overflow voices to add
-     * accounting for the overflow factor.
+     *
      */
-    inline constexpr int calculateActualVoices(int polyphony)
+    static constexpr int minOverflowVoices { 4 };
-    {
+
-        return
-            (int(polyphony * config::overflowVoiceMultiplier) < int(config::maxVoices)) ?
-            int(polyphony * config::overflowVoiceMultiplier) : int(config::maxVoices);
-    }
     /**
      * @brief The smoothing time constant per "smooth" steps
      */

src/sfizz/Synth.cpp

@@ -1042,6 +1042,12 @@ int Synth::getNumActiveVoices() const noexcept
         std::min(impl.numVoices_, activeVoices) : activeVoices;
 }
 
+std::vector<const Voice*> Synth::getActiveVoices() const noexcept
+{
+    Impl& impl = *impl_;
+    return impl.voiceManager_.getActiveVoices();
+}
+
 void Synth::setSamplesPerBlock(int samplesPerBlock) noexcept
 {
     Impl& impl = *impl_;
@@ -1295,7 +1301,6 @@ void Synth::Impl::startVoice(Layer* layer, int delay, const TriggerEvent& trigge
     if (selectedVoice == nullptr)
         return;
 
-    ASSERT(selectedVoice->isFree());
     if (selectedVoice->startVoice(layer, delay, triggerEvent))
         ring.addVoiceToRing(selectedVoice);
 }

src/sfizz/Synth.h

@@ -555,6 +555,14 @@ public:
      * @return int
      */
     int getNumActiveVoices() const noexcept;
+
+    /**
+     * @brief Get the active voices as a view
+     *
+     * @return std::vector<const Voice*>
+     */
+    std::vector<const Voice*> getActiveVoices() const noexcept;
+
     /**
      * @brief Get the total number of voices in the synth (the polyphony)
      *

src/sfizz/Voice.cpp

@@ -607,6 +607,7 @@ void Voice::Impl::off(int delay, bool fast) noexcept
         } else if (region_->offMode == OffMode::time) {
             egAmplitude_.setReleaseTime(region_->offTime);
         }
+        egAmplitude_.stopDynamicUpdates();
     }
     else {
         // TODO(jpc): Flex AmpEG

src/sfizz/VoiceManager.cpp

@@ -104,6 +104,14 @@ const PolyphonyGroup* VoiceManager::getPolyphonyGroupView(int idx) noexcept
     return &polyphonyGroups_[idx];
 }
 
+std::vector<const Voice*> VoiceManager::getActiveVoices() const noexcept
+{
+    std::vector<const Voice*> returned;
+    std::copy(activeVoices_.begin(), activeVoices_.end(), std::back_inserter(returned));
+    return returned;
+}
+
+
 void VoiceManager::clear()
 {
     for (auto& pg : polyphonyGroups_)
@@ -147,12 +155,21 @@ void VoiceManager::checkPolyphony(const Region* region, int delay, const Trigger
 
 Voice* VoiceManager::findFreeVoice() noexcept
 {
-    auto freeVoice = absl::c_find_if(list_, [](const Voice& voice) {
+    Voice* freeVoice = nullptr;
-        return voice.isFree();
+    for (auto& v: list_) {
-    });
+        if (v.isFree()) {
+            freeVoice = &v;
+            break;
+        }
 
-    if (freeVoice != list_.end())
+        if (v.offedOrFree()) {
-        return &*freeVoice;
+            if (freeVoice == nullptr || v.getAge() > freeVoice->getAge())
+                freeVoice = &v;
+        }
+    };
+
+    if (freeVoice != nullptr)
+        return freeVoice;
 
     DBG("Engine hard polyphony reached");
     return {};
@@ -161,7 +178,8 @@ Voice* VoiceManager::findFreeVoice() noexcept
 void VoiceManager::requireNumVoices(int numVoices, Resources& resources)
 {
     numRequiredVoices_ = numVoices;
-    const int numEffectiveVoices = getNumEffectiveVoices();
+    const int numEffectiveVoices = std::min(int(config::maxVoices), numVoices +
+            std::max(int(numVoices * config::overflowVoiceMultiplier), config::minOverflowVoices));
 
     clear();
     list_.reserve(numEffectiveVoices);
@@ -249,7 +267,7 @@ void VoiceManager::checkEnginePolyphony(int delay) noexcept
 {
     Voice* candidate = stealer_->checkPolyphony(
         absl::MakeSpan(activeVoices_), numRequiredVoices_);
-    SisterVoiceRing::offAllSisters(candidate, delay);
+    SisterVoiceRing::offAllSisters(candidate, delay, true);
 }
 
 } // namespace sfz

src/sfizz/VoiceManager.h

@@ -80,6 +80,13 @@ struct VoiceManager final : public Voice::StateListener
      */
     const PolyphonyGroup* getPolyphonyGroupView(int idx) noexcept;
 
+    /**
+     * @brief Get the actives voices as a view
+     *
+     * @return std::vector<const Voice*>
+     */
+    std::vector<const Voice*> getActiveVoices() const noexcept;
+
     /**
      * @brief Clear all voices and polyphony groups.
      * Also resets the stealing algorithm to default.
@@ -134,7 +141,6 @@ struct VoiceManager final : public Voice::StateListener
 
 private:
     int numRequiredVoices_ { config::numVoices };
-    int getNumEffectiveVoices() const noexcept { return config::calculateActualVoices(numRequiredVoices_); }
     std::vector<Voice> list_;
     std::vector<Voice*> activeVoices_;
     std::vector<Voice*> temp_;

tests/SynthT.cpp

@@ -989,11 +989,11 @@ TEST_CASE("[Synth] Sustain threshold")
     REQUIRE( playingSamples(synth) == std::vector<std::string> { "*sine", "*sine" } );
     synth.noteOn(0, 62, 85);
     synth.renderBlock(buffer);
-    REQUIRE( playingSamples(synth) == std::vector<std::string> { "*silence", "*sine", "*sine" } );
+    REQUIRE( playingSamples(synth) == std::vector<std::string> { "*sine", "*sine", "*silence" } );
     synth.cc(0, 64, 64);
     synth.noteOff(0, 62, 85);
     synth.renderBlock(buffer);
-    REQUIRE( playingSamples(synth) == std::vector<std::string> { "*silence", "*sine", "*sine" } );
+    REQUIRE( playingSamples(synth) == std::vector<std::string> {"*sine", "*sine",  "*silence" } );
 }
 
 TEST_CASE("[Synth] Sustain")
@@ -2140,3 +2140,23 @@ TEST_CASE("[Synth] Reloading a file ignores the `set_ccN` opcodes")
 
     REQUIRE(messageList == expected);
 }
+
+TEST_CASE("[Synth] Reuse offed voices in the last case scenario for new notes")
+{
+    sfz::Synth synth;
+    synth.setNumVoices(2);
+    sfz::AudioBuffer<float> buffer { 2, static_cast<unsigned>(synth.getSamplesPerBlock()) };
+    synth.loadSfzString(fs::current_path() / "tests/TestFiles/polyphony.sfz", R"(
+        <region> sample=*sine ampeg_release=10
+    )");
+    synth.noteOn(0, 60, 63);
+    synth.renderBlock(buffer);
+    REQUIRE( playingNotes(synth) == std::vector<int> { 60 } );
+    for (int i = 61; i < 80; ++i) {
+        synth.noteOn(0, i, 63);
+        synth.renderBlock(buffer);
+        auto notes = playingNotes(synth);
+        std::sort(notes.begin(), notes.end());
+        REQUIRE( notes == std::vector<int> { i - 1, i } );
+    }
+}

tests/TestHelpers.cpp

@@ -92,8 +92,8 @@ unsigned numActiveVoices(const sfz::Synth& synth)
 const std::vector<std::string> playingSamples(const sfz::Synth& synth)
 {
     std::vector<std::string> samples;
-    for (int i = 0; i < synth.getNumVoices(); ++i) {
+    auto activeVoices = synth.getActiveVoices();
-        const auto* voice = synth.getVoiceView(i);
+    for (const auto* voice: activeVoices) {
         if (!voice->released()) {
             if (auto region = voice->getRegion())
                 samples.push_back(region->sampleId->filename());
@@ -105,19 +105,30 @@ const std::vector<std::string> playingSamples(const sfz::Synth& synth)
 const std::vector<float> playingVelocities(const sfz::Synth& synth)
 {
     std::vector<float> velocities;
-    for (int i = 0; i < synth.getNumVoices(); ++i) {
+    auto activeVoices = synth.getActiveVoices();
-        const auto* voice = synth.getVoiceView(i);
+    for (const auto* voice: activeVoices) {
         if (!voice->released())
             velocities.push_back(voice->getTriggerEvent().value);
     }
     return velocities;
 }
 
+const std::vector<int> playingNotes(const sfz::Synth& synth)
+{
+    std::vector<int> notes;
+    auto activeVoices = synth.getActiveVoices();
+    for (const auto* voice: activeVoices) {
+        if (!voice->released())
+            notes.push_back(voice->getTriggerEvent().number);
+    }
+    return notes;
+}
+
 const std::vector<std::string> activeSamples(const sfz::Synth& synth)
 {
     std::vector<std::string> samples;
-    for (int i = 0; i < synth.getNumVoices(); ++i) {
+    auto activeVoices = synth.getActiveVoices();
-        const auto* voice = synth.getVoiceView(i);
+    for (const auto* voice: activeVoices) {
         if (!voice->isFree()) {
             const sfz::Region* region = voice->getRegion();
             if (region)
@@ -130,14 +141,25 @@ const std::vector<std::string> activeSamples(const sfz::Synth& synth)
 const std::vector<float> activeVelocities(const sfz::Synth& synth)
 {
     std::vector<float> velocities;
-    for (int i = 0; i < synth.getNumVoices(); ++i) {
+    auto activeVoices = synth.getActiveVoices();
-        const auto* voice = synth.getVoiceView(i);
+    for (const auto* voice: activeVoices) {
         if (!voice->isFree())
             velocities.push_back(voice->getTriggerEvent().value);
     }
     return velocities;
 }
 
+const std::vector<int> activeNotes(const sfz::Synth& synth)
+{
+    std::vector<int> notes;
+    auto activeVoices = synth.getActiveVoices();
+    for (const auto* voice: activeVoices) {
+        if (!voice->isFree())
+            notes.push_back(voice->getTriggerEvent().number);
+    }
+    return notes;
+}
+
 std::string createDefaultGraph(std::vector<std::string> lines, int numRegions)
 {
     for (int regionIdx = 0; regionIdx < numRegions; ++regionIdx) {

tests/TestHelpers.h

@@ -90,7 +90,7 @@ unsigned numActiveVoices(const sfz::Synth& synth);
  * @brief Get the playing samples
  *
  * @param synth
- * @return unsigned
+ * @return std::vector<std::string>
  */
 const std::vector<std::string> playingSamples(const sfz::Synth& synth);
 
@@ -98,15 +98,23 @@ const std::vector<std::string> playingSamples(const sfz::Synth& synth);
  * @brief Get the playing notes velocities
  *
  * @param synth
- * @return unsigned
+ * @return std::vector<float>
  */
 const std::vector<float> playingVelocities(const sfz::Synth& synth);
 
+/**
+ * @brief Get the playing notes
+ *
+ * @param synth
+ * @return std::vector<int>
+ */
+const std::vector<int> playingNotes(const sfz::Synth& synth);
+
 /**
  * @brief Get the active samples
  *
  * @param synth
- * @return unsigned
+ * @return std::vector<std::string>
  */
 const std::vector<std::string> activeSamples(const sfz::Synth& synth);
 
@@ -114,10 +122,18 @@ const std::vector<std::string> activeSamples(const sfz::Synth& synth);
  * @brief Get the active notes velocities
  *
  * @param synth
- * @return unsigned
+ * @return std::vector<float>
  */
 const std::vector<float> activeVelocities(const sfz::Synth& synth);
 
+/**
+ * @brief Get the active notes
+ *
+ * @param synth
+ * @return std::vector<int>
+ */
+const std::vector<int> activeNotes(const sfz::Synth& synth);
+
 /**
  * @brief Create the default dot graph representation for standard regions
  *