From 0c1687b5fad03c5c6c9353edec7cc3a570b08ba3 Mon Sep 17 00:00:00 2001
From: Paul Fd <paul@ferrand.cc>
Date: Sun, 4 Apr 2021 22:24:43 +0200
Subject: [PATCH] Voices can also be offed by note-off events Also add a check
 to see if a voice is already released in checkOffGroup, preventing and
 endless recursion

---
 src/sfizz/Synth.cpp      | 26 +++++++++++++-------------
 src/sfizz/SynthPrivate.h | 10 ++++++++++
 src/sfizz/Voice.cpp      |  3 +++
 tests/SynthT.cpp         | 22 ++++++++++++++++++++++
 4 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp
index 7b5a7369..1febb49c 100644
--- a/src/sfizz/Synth.cpp
+++ b/src/sfizz/Synth.cpp
@@ -1088,6 +1088,16 @@ void Synth::Impl::startVoice(Layer* layer, int delay, const TriggerEvent& trigge
         ring.addVoiceToRing(selectedVoice);
 }
 
+void Synth::Impl::checkOffGroups(const Region* region, int delay, int number)
+{
+    for (auto& voice : voiceManager_) {
+        if (voice.checkOffGroup(region, delay, number)) {
+            const TriggerEvent& event = voice.getTriggerEvent();
+            noteOffDispatch(delay, event.number, event.value);
+        }
+    }
+}
+
 void Synth::Impl::noteOffDispatch(int delay, int noteNumber, float velocity) noexcept
 {
     const auto randValue = randNoteDistribution_(Random::randomGenerator);
@@ -1106,6 +1116,7 @@ void Synth::Impl::noteOffDispatch(int delay, int noteNumber, float velocity) noe
             if (region.trigger == Trigger::release && !region.rtDead && !voiceManager_.playingAttackVoice(&region))
                 continue;
 
+            checkOffGroups(&region, delay, noteNumber);
             startVoice(layer, delay, triggerEvent, ring);
         }
     }
@@ -1136,13 +1147,7 @@ void Synth::Impl::noteOnDispatch(int delay, int noteNumber, float velocity) noex
     for (Layer* layer : noteActivationLists_[noteNumber]) {
         if (layer->registerNoteOn(noteNumber, velocity, randValue)) {
             const Region& region = layer->getRegion();
-            for (auto& voice : voiceManager_) {
-                if (voice.checkOffGroup(&region, delay, noteNumber)) {
-                    const TriggerEvent& event = voice.getTriggerEvent();
-                    noteOffDispatch(delay, event.number, event.value);
-                }
-            }
-
+            checkOffGroups(&region, delay, noteNumber);
             TriggerEvent triggerEvent { TriggerEventType::NoteOn, noteNumber, velocity };
             startVoice(layer, delay, triggerEvent, ring);
         }
@@ -1215,12 +1220,7 @@ void Synth::Impl::ccDispatch(int delay, int ccNumber, float value) noexcept
         }
 
         if (layer->registerCC(ccNumber, value)) {
-            for (auto& voice : voiceManager_) {
-                if (voice.checkOffGroup(&region, delay, ccNumber)) {
-                    const TriggerEvent& event = voice.getTriggerEvent();
-                    noteOffDispatch(delay, event.number, event.value);
-                }
-            }
+            checkOffGroups(&region, delay, ccNumber);
             startVoice(layer, delay, triggerEvent, ring);
         }
     }
diff --git a/src/sfizz/SynthPrivate.h b/src/sfizz/SynthPrivate.h
index 3b8835a6..1fafde39 100644
--- a/src/sfizz/SynthPrivate.h
+++ b/src/sfizz/SynthPrivate.h
@@ -219,6 +219,16 @@ struct Synth::Impl final: public Parser::Listener {
      */
     void setDefaultHdcc(int ccNumber, float value);
 
+    /**
+     * @brief Check if we have to kill any voice when starting a new one
+     *      on the specified region with the specified note/cc number
+     *
+     * @param region
+     * @param delay
+     * @param number
+     */
+    void checkOffGroups(const Region* region, int delay, int number);
+
     int numGroups_ { 0 };
     int numMasters_ { 0 };
 
diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp
index 836f1075..0a3f8c74 100644
--- a/src/sfizz/Voice.cpp
+++ b/src/sfizz/Voice.cpp
@@ -1571,6 +1571,9 @@ bool Voice::checkOffGroup(const Region* other, int delay, int noteNumber) noexce
     if (region == nullptr || other == nullptr)
         return false;
 
+    if (impl.released())
+        return false;
+
     if (impl.triggerEvent_.type == TriggerEventType::NoteOn
         && region->offBy && *region->offBy == other->group
         && (region->group != other->group || noteNumber != impl.triggerEvent_.number)) {
diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp
index 538fd606..cb1a2c9e 100644
--- a/tests/SynthT.cpp
+++ b/tests/SynthT.cpp
@@ -1626,6 +1626,28 @@ TEST_CASE("[Synth] Off by a CC event")
     REQUIRE( numPlayingVoices(synth) == 1 );
 }
 
+TEST_CASE("[Synth] Off by a note-off event")
+{
+    sfz::Synth synth;
+    sfz::AudioBuffer<float> buffer { 2, static_cast<unsigned>(synth.getSamplesPerBlock()) };
+
+    synth.loadSfzString(fs::current_path(), R"(
+        <region> key=60 group=1 off_by=2 sample=*saw
+        <region> key=62 sample=*silence
+        <region> key=62 trigger=release group=2 sample=*silence
+    )");
+    synth.noteOn(0, 60, 85);
+    synth.renderBlock(buffer);
+    REQUIRE( numPlayingVoices(synth) == 1 );
+    synth.noteOn(0, 62, 85);
+    synth.renderBlock(buffer);
+    REQUIRE( numPlayingVoices(synth) == 2 );
+    synth.noteOff(10, 62, 85);
+    synth.renderBlock(buffer);
+    REQUIRE( numPlayingVoices(synth) == 1 );
+    // TODO: check the samples; the last one should be *silence
+}
+
 TEST_CASE("[Synth] Initial values of CC")
 {
     sfz::Synth synth;