From 6086284efec6ad240cedf11a6dc51f687d149d78 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sun, 19 Jul 2020 13:18:30 +0200 Subject: [PATCH 1/8] If the region spans multiple keys they can all fire on pedal up --- src/sfizz/Region.cpp | 38 ++++++++++++++++++++++--------- src/sfizz/Region.h | 4 +++- src/sfizz/Synth.cpp | 29 +++++++++++++++++------- tests/RegionT.cpp | 47 ++++++++++++++++++++++++++++++++------ tests/SynthT.cpp | 54 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 145 insertions(+), 27 deletions(-) diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index 0a19625c5..e03e50a74 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -1031,24 +1031,45 @@ bool sfz::Region::registerNoteOff(int noteNumber, float velocity, float randValu keySwitched = true; } - const bool keyOk = keyRange.containsWithEnd(noteNumber); - if (!isSwitchedOn()) return false; if (!triggerOnNote) return false; + // Prerequisites + + const bool keyOk = keyRange.containsWithEnd(noteNumber); const bool velOk = velocityRange.containsWithEnd(velocity); const bool randOk = randRange.contains(randValue); - bool releaseTrigger = (trigger == SfzTrigger::release_key); + + if (!(velOk && keyOk && randOk)) + return false; + + // Release logic + + if (trigger == SfzTrigger::release_key) + return true; + if (trigger == SfzTrigger::release) { if (midiState.getCCValue(sustainCC) < sustainThreshold) - releaseTrigger = true; + return true; + + // If we reach this part, we're storing the notes to delay their release on CC up + // This is handled by the Synth object + + const auto sameNoteTest = [noteNumber](const std::pair& noteAndValue) { + return noteAndValue.first == noteNumber; + }; + + auto it = absl::c_find_if(delayedReleases, sameNoteTest); + if (it == delayedReleases.end()) + delayedReleases.emplace_back(noteNumber, midiState.getNoteVelocity(noteNumber)); else - noteIsOff = true; + it->second = velocity; } - return keyOk && velOk && randOk && releaseTrigger; + + return false; } bool sfz::Region::registerCC(int ccNumber, float ccValue) noexcept @@ -1062,11 +1083,6 @@ bool sfz::Region::registerCC(int ccNumber, float ccValue) noexcept if (!isSwitchedOn()) return false; - if (sustainCC == ccNumber && ccValue < sustainThreshold && noteIsOff) { - noteIsOff = false; - return true; - } - if (!triggerOnCC) return false; diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index 772ee3999..366efd1fd 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -376,6 +376,9 @@ struct Region { // Parent RegionSet* parent { nullptr }; + + // Started notes + std::vector> delayedReleases; private: const MidiState& midiState; bool keySwitched { true }; @@ -384,7 +387,6 @@ struct Region { bool pitchSwitched { true }; bool bpmSwitched { true }; bool aftertouchSwitched { true }; - bool noteIsOff { false }; std::bitset ccSwitched; absl::string_view defaultPath { "" }; diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 6081ba2b3..0ab5664ba 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -161,6 +161,9 @@ void sfz::Synth::buildRegion(const std::vector& regionOpcodes) lastRegion->parent = currentSet; currentSet->addRegion(lastRegion.get()); + // Adapt the size of the delayed releases to avoid allocating later on + lastRegion->delayedReleases.reserve(lastRegion->keyRange.length()); + regions.push_back(std::move(lastRegion)); } @@ -998,18 +1001,28 @@ void sfz::Synth::hdcc(int delay, int ccNumber, float normValue) noexcept SisterVoiceRingBuilder ring; for (auto& region : ccActivationLists[ccNumber]) { - if (region->registerCC(ccNumber, normValue)) { + if (ccNumber == region->sustainCC) { + for (auto& note: region->delayedReleases) { + // FIXME: we really need to have some form of common method to find and start voices... + auto voice = findFreeVoice(); + if (voice == nullptr) + continue; + + voice->startVoice(region, delay, note.first, note.second, Voice::TriggerType::NoteOff); + + ring.addVoiceToRing(voice); + RegionSet::registerVoiceInHierarchy(region, voice); + polyphonyGroups[region->group].registerVoice(voice); + } + + region->delayedReleases.clear(); + } else if (region->registerCC(ccNumber, normValue)) { auto voice = findFreeVoice(); if (voice == nullptr) continue; - if (!region->triggerOnCC) { - // This is a sustain trigger - const auto replacedVelocity = resources.midiState.getNoteVelocity(region->pitchKeycenter); - voice->startVoice(region, delay, region->pitchKeycenter, replacedVelocity, Voice::TriggerType::NoteOff); - } else { - voice->startVoice(region, delay, ccNumber, normValue, Voice::TriggerType::CC); - } + + voice->startVoice(region, delay, ccNumber, normValue, Voice::TriggerType::CC); ring.addVoiceToRing(voice); RegionSet::registerVoiceInHierarchy(region, voice); diff --git a/tests/RegionT.cpp b/tests/RegionT.cpp index ecfea499a..d98b6c7a8 100644 --- a/tests/RegionT.cpp +++ b/tests/RegionT.cpp @@ -1749,7 +1749,8 @@ TEST_CASE("[Region] Release and release key") { MidiState midiState; Region region { 0, midiState }; - region.parseOpcode({ "key", "63" }); + region.parseOpcode({ "lokey", "63" }); + region.parseOpcode({ "hikey", "65" }); region.parseOpcode({ "sample", "*sine" }); SECTION("Release key without sustain") { @@ -1765,9 +1766,8 @@ TEST_CASE("[Region] Release and release key") REQUIRE( !region.registerCC(64, 1.0f) ); REQUIRE( !region.registerNoteOn(63, 0.5f, 0.0f) ); REQUIRE( region.registerNoteOff(63, 0.5f, 0.0f) ); - midiState.ccEvent(0, 64, 0.0f); - REQUIRE( !region.registerCC(64, 0.0f) ); } + SECTION("Release without sustain") { region.parseOpcode({ "trigger", "release" }); @@ -1775,20 +1775,53 @@ TEST_CASE("[Region] Release and release key") REQUIRE( !region.registerNoteOn(63, 0.5f, 0.0f) ); REQUIRE( region.registerNoteOff(63, 0.5f, 0.0f) ); } + SECTION("Release with sustain") { region.parseOpcode({ "trigger", "release" }); midiState.ccEvent(0, 64, 1.0f); + midiState.noteOnEvent(0, 63, 0.5f); REQUIRE( !region.registerNoteOn(63, 0.5f, 0.0f) ); REQUIRE( !region.registerNoteOff(63, 0.5f, 0.0f) ); + REQUIRE( region.delayedReleases.size() == 1 ); + std::vector> expected = { + { 63, 0.5f } + }; + REQUIRE( region.delayedReleases == expected ); } - SECTION("Release with sustain") + + SECTION("Release with sustain and 2 notes") { region.parseOpcode({ "trigger", "release" }); midiState.ccEvent(0, 64, 1.0f); + midiState.noteOnEvent(0, 63, 0.5f); REQUIRE( !region.registerNoteOn(63, 0.5f, 0.0f) ); - REQUIRE( !region.registerNoteOff(63, 0.5f, 0.0f) ); - midiState.ccEvent(0, 64, 0.0f); - REQUIRE( region.registerCC(64, 0.0f) ); + midiState.noteOnEvent(0, 64, 0.6f); + REQUIRE( !region.registerNoteOn(64, 0.6f, 0.0f) ); + REQUIRE( !region.registerNoteOff(63, 0.0f, 0.0f) ); + REQUIRE( !region.registerNoteOff(64, 0.2f, 0.0f) ); + REQUIRE( region.delayedReleases.size() == 2 ); + std::vector> expected = { + { 63, 0.5f }, + { 64, 0.6f } + }; + REQUIRE( region.delayedReleases == expected ); + } + + SECTION("Release with sustain and 2 notes but 1 outside") + { + region.parseOpcode({ "trigger", "release" }); + midiState.ccEvent(0, 64, 1.0f); + midiState.noteOnEvent(0, 63, 0.5f); + REQUIRE( !region.registerNoteOn(63, 0.5f, 0.0f) ); + midiState.noteOnEvent(0, 66, 0.6f); + REQUIRE( !region.registerNoteOn(66, 0.6f, 0.0f) ); + REQUIRE( !region.registerNoteOff(63, 0.0f, 0.0f) ); + REQUIRE( !region.registerNoteOff(66, 0.2f, 0.0f) ); + REQUIRE( region.delayedReleases.size() == 1 ); + std::vector> expected = { + { 63, 0.5f } + }; + REQUIRE( region.delayedReleases == expected ); } } diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index ccd92b943..3a07437af 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -695,3 +695,57 @@ TEST_CASE("[Synth] Sustain threshold") synth.noteOff(0, 62, 85); REQUIRE( synth.getNumActiveVoices(true) == 2 ); } + +TEST_CASE("[Synth] Release (Multiple notes)") +{ + sfz::Synth synth; + synth.loadSfzString(fs::current_path(), R"( + lokey=62 hikey=64 sample=*sine trigger=release + )"); + synth.noteOn(0, 62, 85); + synth.noteOn(0, 63, 78); + synth.noteOn(0, 64, 34); + synth.cc(0, 64, 127); + synth.noteOff(0, 64, 0); + synth.noteOff(0, 63, 2); + synth.noteOff(0, 62, 85); + REQUIRE( synth.getNumActiveVoices() == 0 ); + synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices() == 3 ); +} + +TEST_CASE("[Synth] Release (Multiple notes, release_key ignores the pedal)") +{ + sfz::Synth synth; + synth.loadSfzString(fs::current_path(), R"( + lokey=62 hikey=64 sample=*sine trigger=release_key + )"); + synth.noteOn(0, 62, 85); + synth.noteOn(0, 63, 78); + synth.noteOn(0, 64, 34); + synth.cc(0, 64, 127); + synth.noteOff(0, 64, 0); + synth.noteOff(0, 63, 2); + synth.noteOff(0, 62, 85); + REQUIRE( synth.getNumActiveVoices() == 3 ); +} + +TEST_CASE("[Synth] Release (Multiple notes, cleared the delayed voices after)") +{ + sfz::Synth synth; + synth.loadSfzString(fs::current_path(), R"( + lokey=62 hikey=64 sample=*sine trigger=release + loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 + )"); + synth.noteOn(0, 62, 85); + synth.noteOn(0, 63, 78); + synth.noteOn(0, 64, 34); + synth.cc(0, 64, 127); + synth.noteOff(0, 64, 0); + synth.noteOff(0, 63, 2); + synth.noteOff(0, 62, 85); + REQUIRE( synth.getNumActiveVoices() == 0 ); + synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices() == 3 ); + REQUIRE( synth.getRegionView(0)->delayedReleases.empty() ); +} From f631f7031d393600cd61cce68e1abeaccdb8ca20 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Fri, 7 Aug 2020 20:36:04 +0200 Subject: [PATCH 2/8] Update the tests --- tests/SynthT.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index 3a07437af..17f2ee50c 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -709,9 +709,9 @@ TEST_CASE("[Synth] Release (Multiple notes)") synth.noteOff(0, 64, 0); synth.noteOff(0, 63, 2); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices() == 0 ); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); synth.cc(0, 64, 0); - REQUIRE( synth.getNumActiveVoices() == 3 ); + REQUIRE( synth.getNumActiveVoices(true) == 3 ); } TEST_CASE("[Synth] Release (Multiple notes, release_key ignores the pedal)") @@ -727,7 +727,7 @@ TEST_CASE("[Synth] Release (Multiple notes, release_key ignores the pedal)") synth.noteOff(0, 64, 0); synth.noteOff(0, 63, 2); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices() == 3 ); + REQUIRE( synth.getNumActiveVoices(true) == 3 ); } TEST_CASE("[Synth] Release (Multiple notes, cleared the delayed voices after)") @@ -744,8 +744,8 @@ TEST_CASE("[Synth] Release (Multiple notes, cleared the delayed voices after)") synth.noteOff(0, 64, 0); synth.noteOff(0, 63, 2); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices() == 0 ); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); synth.cc(0, 64, 0); - REQUIRE( synth.getNumActiveVoices() == 3 ); + REQUIRE( synth.getNumActiveVoices(true) == 3 ); REQUIRE( synth.getRegionView(0)->delayedReleases.empty() ); } From a3943abf79ae8acf7be3468a9dcd760ca1313c26 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sat, 8 Aug 2020 23:49:36 +0200 Subject: [PATCH 3/8] Added tests --- tests/SynthT.cpp | 229 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 208 insertions(+), 21 deletions(-) diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index 17f2ee50c..43ce0cf85 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -8,6 +8,7 @@ #include "sfizz/SisterVoiceRing.h" #include "sfizz/SfzHelpers.h" #include "sfizz/NumericId.h" +#include #include "catch2/catch.hpp" using namespace Catch::literals; using namespace sfz::literals; @@ -626,14 +627,48 @@ TEST_CASE("[Synth] Release") { sfz::Synth synth; synth.loadSfzString(fs::current_path(), R"( + key=62 sample=*silence key=62 sample=*sine trigger=release )"); synth.noteOn(0, 62, 85); synth.cc(0, 64, 127); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 0 ); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices(true) == 2 ); +} + +TEST_CASE("[Synth] Release (pedal was already down)") +{ + sfz::Synth synth; + synth.loadSfzString(fs::current_path(), R"( + key=62 sample=*silence + key=62 sample=*sine trigger=release + )"); + synth.cc(0, 64, 127); + synth.noteOn(0, 62, 85); + synth.noteOff(0, 62, 85); REQUIRE( synth.getNumActiveVoices(true) == 1 ); + synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices(true) == 2 ); +} + + + +TEST_CASE("[Synth] Release samples don't play unless there is another playing region that matches") +{ + sfz::Synth synth; + synth.loadSfzString(fs::current_path(), R"( + key=62 sample=*sine trigger=release + )"); + synth.noteOn(0, 62, 85); + synth.noteOff(0, 62, 0); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.cc(0, 64, 127); + synth.noteOn(0, 62, 85); + synth.noteOff(0, 62, 0); + synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); } TEST_CASE("[Synth] Release key (Different sustain CC)") @@ -646,7 +681,7 @@ TEST_CASE("[Synth] Release key (Different sustain CC)") synth.noteOn(0, 62, 85); synth.cc(0, 54, 127); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 1 ); + REQUIRE( synth.getNumActiveVoices(true) == 2 ); } TEST_CASE("[Synth] Release (Different sustain CC)") @@ -654,14 +689,15 @@ TEST_CASE("[Synth] Release (Different sustain CC)") sfz::Synth synth; synth.loadSfzString(fs::current_path(), R"( sustain_cc=54 + key=62 sample=*silence key=62 sample=*sine trigger=release )"); synth.noteOn(0, 62, 85); synth.cc(0, 54, 127); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 0 ); - synth.cc(0, 54, 0); REQUIRE( synth.getNumActiveVoices(true) == 1 ); + synth.cc(0, 54, 0); + REQUIRE( synth.getNumActiveVoices(true) == 2 ); } TEST_CASE("[Synth] Sustain threshold default") @@ -681,26 +717,52 @@ TEST_CASE("[Synth] Sustain threshold") sfz::Synth synth; synth.loadSfzString(fs::current_path(), R"( sustain_lo=63 + key=62 sample=*silence key=62 sample=*sine trigger=release )"); synth.noteOn(0, 62, 85); synth.cc(0, 64, 1); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 1 ); + REQUIRE( synth.getNumActiveVoices(true) == 2 ); synth.noteOn(0, 62, 85); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 2 ); + REQUIRE( synth.getNumActiveVoices(true) == 4 ); synth.noteOn(0, 62, 85); + REQUIRE( synth.getNumActiveVoices(true) == 5 ); synth.cc(0, 64, 64); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 2 ); + REQUIRE( synth.getNumActiveVoices(true) == 5 ); +} + +template +void sortAll(C& container) +{ + std::sort(container.begin(), container.end()); +} + +template +void sortAll(C& container, Args&... others) +{ + std::sort(container.begin(), container.end()); + sortAll(others...); } -TEST_CASE("[Synth] Release (Multiple notes)") +const std::vector getActiveVoices(const sfz::Synth& synth) +{ + std::vector activeVoices; + for (int i = 0; i < synth.getNumVoices(); ++i) { + const auto* voice = synth.getVoiceView(i); + if (!voice->isFree()) + activeVoices.push_back(voice); + } + return activeVoices; +} + +TEST_CASE("[Synth] Release (Multiple notes, release_key ignores the pedal)") { sfz::Synth synth; synth.loadSfzString(fs::current_path(), R"( - lokey=62 hikey=64 sample=*sine trigger=release + lokey=62 hikey=64 sample=*sine trigger=release_key )"); synth.noteOn(0, 62, 85); synth.noteOn(0, 63, 78); @@ -709,16 +771,24 @@ TEST_CASE("[Synth] Release (Multiple notes)") synth.noteOff(0, 64, 0); synth.noteOff(0, 63, 2); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 0 ); - synth.cc(0, 64, 0); REQUIRE( synth.getNumActiveVoices(true) == 3 ); + + std::vector requiredVelocities { 34_norm, 78_norm, 85_norm}; + std::vector actualVelocities; + for (auto* v: getActiveVoices(synth)) { + actualVelocities.push_back(v->getTriggerValue()); + } + sortAll(requiredVelocities, actualVelocities); + REQUIRE( requiredVelocities == actualVelocities ); } -TEST_CASE("[Synth] Release (Multiple notes, release_key ignores the pedal)") +TEST_CASE("[Synth] Release (Multiple notes, release, cleared the delayed voices after)") { sfz::Synth synth; synth.loadSfzString(fs::current_path(), R"( - lokey=62 hikey=64 sample=*sine trigger=release_key + lokey=62 hikey=64 sample=*silence + lokey=62 hikey=64 sample=*sine trigger=release + loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 )"); synth.noteOn(0, 62, 85); synth.noteOn(0, 63, 78); @@ -728,24 +798,141 @@ TEST_CASE("[Synth] Release (Multiple notes, release_key ignores the pedal)") synth.noteOff(0, 63, 2); synth.noteOff(0, 62, 85); REQUIRE( synth.getNumActiveVoices(true) == 3 ); + synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices(true) == 6 ); + + std::vector requiredVelocities { 34_norm, 78_norm, 85_norm, 34_norm, 78_norm, 85_norm }; + std::vector actualVelocities; + for (auto* v: getActiveVoices(synth)) { + actualVelocities.push_back(v->getTriggerValue()); + } + sortAll(requiredVelocities, actualVelocities); + REQUIRE( requiredVelocities == actualVelocities ); + + REQUIRE( synth.getRegionView(1)->delayedReleases.empty() ); +} + +TEST_CASE("[Synth] Release (Multiple notes after pedal is down, release, cleared the delayed voices after)") +{ + sfz::Synth synth; + synth.loadSfzString(fs::current_path(), R"( + lokey=62 hikey=64 sample=*silence + lokey=62 hikey=64 sample=*sine trigger=release + loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 + )"); + synth.cc(0, 64, 127); + synth.noteOn(1, 62, 85); + synth.noteOn(1, 63, 78); + synth.noteOn(1, 64, 34); + synth.noteOff(2, 64, 0); + synth.noteOff(2, 63, 2); + synth.noteOff(2, 62, 3); + REQUIRE( synth.getNumActiveVoices(true) == 3 ); + synth.cc(3, 64, 0); + REQUIRE( synth.getNumActiveVoices(true) == 6 ); + + std::vector requiredVelocities { 34_norm, 78_norm, 85_norm, 34_norm, 78_norm, 85_norm }; + std::vector actualVelocities; + for (auto* v: getActiveVoices(synth)) { + actualVelocities.push_back(v->getTriggerValue()); + } + sortAll(requiredVelocities, actualVelocities); + REQUIRE( requiredVelocities == actualVelocities ); + + REQUIRE( synth.getRegionView(1)->delayedReleases.empty() ); } -TEST_CASE("[Synth] Release (Multiple notes, cleared the delayed voices after)") +TEST_CASE("[Synth] Release (Multiple note ons during pedal down)") { sfz::Synth synth; synth.loadSfzString(fs::current_path(), R"( + lokey=62 hikey=64 sample=*silence lokey=62 hikey=64 sample=*sine trigger=release loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 )"); synth.noteOn(0, 62, 85); - synth.noteOn(0, 63, 78); - synth.noteOn(0, 64, 34); synth.cc(0, 64, 127); - synth.noteOff(0, 64, 0); - synth.noteOff(0, 63, 2); - synth.noteOff(0, 62, 85); + synth.noteOff(0, 62, 0); + synth.noteOn(0, 62, 78); + synth.noteOff(0, 62, 2); + REQUIRE( synth.getNumActiveVoices(true) == 2 ); + synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices(true) == 4 ); + + std::vector requiredVelocities { 78_norm, 85_norm, 78_norm, 85_norm }; + std::vector actualVelocities; + for (auto* v: getActiveVoices(synth)) { + actualVelocities.push_back(v->getTriggerValue()); + } + sortAll(requiredVelocities, actualVelocities); + REQUIRE( requiredVelocities == actualVelocities ); + REQUIRE( synth.getRegionView(1)->delayedReleases.empty() ); +} + +TEST_CASE("[Synth] No release sample after the main sample stopped sounding by default") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 4096 }; + + synth.loadSfzString(fs::current_path(), R"( + lokey=62 hikey=64 sample=TestFiles/closedhat.wav loop_mode=one_shot + lokey=62 hikey=64 sample=*sine trigger=release + loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 + )"); + synth.noteOn(0, 62, 85); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + for (unsigned i = 0; i < 100; ++i) { + synth.renderBlock(buffer); + } + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOff(0, 62, 0); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + + synth.noteOn(0, 62, 85); + synth.cc(0, 64, 127); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + for (unsigned i = 0; i < 100; ++i) { + synth.renderBlock(buffer); + } + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOff(0, 62, 0); REQUIRE( synth.getNumActiveVoices(true) == 0 ); synth.cc(0, 64, 0); - REQUIRE( synth.getNumActiveVoices(true) == 3 ); - REQUIRE( synth.getRegionView(0)->delayedReleases.empty() ); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + + REQUIRE( synth.getRegionView(1)->delayedReleases.empty() ); +} + +TEST_CASE("[Synth] If rt_dead is active the release sample can sound after the attack sample died") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 4096 }; + + synth.loadSfzString(fs::current_path(), R"( + lokey=62 hikey=64 sample=TestFiles/closedhat.wav loop_mode=one_shot + lokey=62 hikey=64 sample=*sine trigger=release + loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 + )"); + synth.noteOn(0, 62, 85); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + for (unsigned i = 0; i < 100; ++i) { + synth.renderBlock(buffer); + } + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOff(0, 62, 0); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + + synth.noteOn(0, 62, 85); + synth.cc(0, 64, 127); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + for (unsigned i = 0; i < 100; ++i) { + synth.renderBlock(buffer); + } + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOff(0, 62, 0); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.cc(0, 64, 0); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + + REQUIRE( synth.getRegionView(1)->delayedReleases.empty() ); } From dc328cb9a7d7267a3f9a74f4d02a819cfda97cd3 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sun, 9 Aug 2020 11:06:10 +0200 Subject: [PATCH 4/8] Multiple release samples can play on pedal up --- src/sfizz/Region.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index e03e50a74..ca2c15b8b 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -1058,15 +1058,7 @@ bool sfz::Region::registerNoteOff(int noteNumber, float velocity, float randValu // If we reach this part, we're storing the notes to delay their release on CC up // This is handled by the Synth object - const auto sameNoteTest = [noteNumber](const std::pair& noteAndValue) { - return noteAndValue.first == noteNumber; - }; - - auto it = absl::c_find_if(delayedReleases, sameNoteTest); - if (it == delayedReleases.end()) - delayedReleases.emplace_back(noteNumber, midiState.getNoteVelocity(noteNumber)); - else - it->second = velocity; + delayedReleases.emplace_back(noteNumber, midiState.getNoteVelocity(noteNumber)); } return false; From 3dc98e7fd2f8d6c5631dd9d5799a945255b04ce7 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sun, 9 Aug 2020 16:01:58 +0200 Subject: [PATCH 5/8] parse rt_dead --- src/sfizz/Defaults.h | 1 + src/sfizz/Region.cpp | 29 +++++++++++++++++++---------- src/sfizz/Region.h | 1 + tests/RegionT.cpp | 12 ++++++++++++ 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/sfizz/Defaults.h b/src/sfizz/Defaults.h index 8be2fc96e..cc0ac0b31 100644 --- a/src/sfizz/Defaults.h +++ b/src/sfizz/Defaults.h @@ -143,6 +143,7 @@ namespace Default constexpr SfzCrossfadeCurve crossfadeVelCurve { SfzCrossfadeCurve::power }; constexpr SfzCrossfadeCurve crossfadeCCCurve { SfzCrossfadeCurve::power }; constexpr float rtDecay { 0.0f }; + constexpr bool rtDead { false }; constexpr Range rtDecayRange { 0.0f, 200.0f }; // Performance parameters: Filters diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index ca2c15b8b..93bc89960 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -111,7 +111,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) loopMode = SfzLoopMode::loop_sustain; break; default: - DBG("Unkown loop mode:" << std::string(opcode.value)); + DBG("Unkown loop mode:" << opcode.value); } break; case hash("loop_end"): // also loopend @@ -161,7 +161,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) offMode = SfzOffMode::normal; break; default: - DBG("Unkown off mode:" << std::string(opcode.value)); + DBG("Unkown off mode:" << opcode.value); } break; case hash("polyphony"): @@ -181,7 +181,16 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) selfMask = SfzSelfMask::dontMask; break; default: - DBG("Unkown self mask value:" << std::string(opcode.value)); + DBG("Unkown self mask value:" << opcode.value); + } + break; + case hash("rt_dead"): + if (opcode.value == "on") { + rtDead = true; + } else if (opcode.value == "off") { + rtDead = false; + } else { + DBG("Unkown rt_dead value:" << opcode.value); } break; // Region logic: key mapping @@ -274,7 +283,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) velocityOverride = SfzVelocityOverride::previous; break; default: - DBG("Unknown velocity mode: " << std::string(opcode.value)); + DBG("Unknown velocity mode: " << opcode.value); } break; @@ -337,7 +346,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) trigger = SfzTrigger::release_key; break; default: - DBG("Unknown trigger mode: " << std::string(opcode.value)); + DBG("Unknown trigger mode: " << opcode.value); } break; case hash("start_locc&"): // also on_locc& @@ -468,7 +477,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) crossfadeKeyCurve = SfzCrossfadeCurve::gain; break; default: - DBG("Unknown crossfade power curve: " << std::string(opcode.value)); + DBG("Unknown crossfade power curve: " << opcode.value); } break; case hash("xf_velcurve"): @@ -480,7 +489,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) crossfadeVelCurve = SfzCrossfadeCurve::gain; break; default: - DBG("Unknown crossfade power curve: " << std::string(opcode.value)); + DBG("Unknown crossfade power curve: " << opcode.value); } break; case hash("xfin_locc&"): @@ -516,7 +525,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) crossfadeCCCurve = SfzCrossfadeCurve::gain; break; default: - DBG("Unknown crossfade power curve: " << std::string(opcode.value)); + DBG("Unknown crossfade power curve: " << opcode.value); } break; case hash("rt_decay"): @@ -636,7 +645,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) filters[filterIndex].type = *ftype; else { filters[filterIndex].type = FilterType::kFilterNone; - DBG("Unknown filter type: " << std::string(opcode.value)); + DBG("Unknown filter type: " << opcode.value); } } break; @@ -745,7 +754,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) equalizers[eqNumber - 1].type = *ftype; else { equalizers[eqNumber - 1].type = EqType::kEqNone; - DBG("Unknown EQ type: " << std::string(opcode.value)); + DBG("Unknown EQ type: " << opcode.value); } } break; diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index 366efd1fd..3a1752524 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -288,6 +288,7 @@ struct Region { absl::optional notePolyphony {}; // note_polyphony unsigned polyphony { config::maxVoices }; // polyphony SfzSelfMask selfMask { Default::selfMask }; + bool rtDead { Default::rtDead }; // Region logic: key mapping Range keyRange { Default::keyRange }; //lokey, hikey and key diff --git a/tests/RegionT.cpp b/tests/RegionT.cpp index d98b6c7a8..e1d692c5e 100644 --- a/tests/RegionT.cpp +++ b/tests/RegionT.cpp @@ -1628,6 +1628,18 @@ TEST_CASE("[Region] Parsing opcodes") REQUIRE(region.selfMask == SfzSelfMask::dontMask); } + SECTION("Release dead") + { + REQUIRE(region.rtDead == false); + region.parseOpcode({ "rt_dead", "on" }); + REQUIRE(region.rtDead == true); + region.parseOpcode({ "rt_dead", "off" }); + REQUIRE(region.rtDead == false); + region.parseOpcode({ "rt_dead", "on" }); + region.parseOpcode({ "rt_dead", "garbage" }); + REQUIRE(region.rtDead == true); + } + SECTION("amplitude") { REQUIRE(region.amplitude == 1.0_a); From bbf22b058e9c8b61ed658a35452c1dd4ec7ae110 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sun, 9 Aug 2020 16:02:14 +0200 Subject: [PATCH 6/8] Small test cleanups --- tests/SynthT.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index 43ce0cf85..a71cb91d1 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -201,6 +201,7 @@ TEST_CASE("[Synth] Trigger=release and an envelope properly kills the voice at t synth.setNumVoices(1); synth.loadSfzString(fs::current_path() / "tests/TestFiles/envelope_trigger_release.sfz", R"( lovel=0 hivel=127 + sample=*silence trigger=release sample=*noise loop_mode=one_shot ampeg_attack=0.02 ampeg_decay=0.02 ampeg_release=0.1 ampeg_sustain=0 )"); @@ -681,7 +682,7 @@ TEST_CASE("[Synth] Release key (Different sustain CC)") synth.noteOn(0, 62, 85); synth.cc(0, 54, 127); synth.noteOff(0, 62, 85); - REQUIRE( synth.getNumActiveVoices(true) == 2 ); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); } TEST_CASE("[Synth] Release (Different sustain CC)") @@ -872,10 +873,11 @@ TEST_CASE("[Synth] Release (Multiple note ons during pedal down)") TEST_CASE("[Synth] No release sample after the main sample stopped sounding by default") { sfz::Synth synth; + synth.setSamplesPerBlock(4096); sfz::AudioBuffer buffer { 2, 4096 }; synth.loadSfzString(fs::current_path(), R"( - lokey=62 hikey=64 sample=TestFiles/closedhat.wav loop_mode=one_shot + lokey=62 hikey=64 sample=tests/TestFiles/closedhat.wav loop_mode=one_shot lokey=62 hikey=64 sample=*sine trigger=release loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 )"); @@ -906,10 +908,11 @@ TEST_CASE("[Synth] No release sample after the main sample stopped sounding by d TEST_CASE("[Synth] If rt_dead is active the release sample can sound after the attack sample died") { sfz::Synth synth; + synth.setSamplesPerBlock(4096); sfz::AudioBuffer buffer { 2, 4096 }; synth.loadSfzString(fs::current_path(), R"( - lokey=62 hikey=64 sample=TestFiles/closedhat.wav loop_mode=one_shot + lokey=62 hikey=64 sample=tests/TestFiles/closedhat.wav loop_mode=one_shot lokey=62 hikey=64 sample=*sine trigger=release loopmode=one_shot ampeg_attack=0.02 ampeg_release=0.1 )"); From 39abff2ccacbee9343f1a5ee9da539571470831c Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sun, 9 Aug 2020 16:02:41 +0200 Subject: [PATCH 7/8] Check that there is a voice playing for release samples --- src/sfizz/Synth.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 0ab5664ba..31928c2cf 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -844,6 +844,25 @@ void sfz::Synth::noteOffDispatch(int delay, int noteNumber, float velocity) noex for (auto& region : noteActivationLists[noteNumber]) { if (region->registerNoteOff(noteNumber, velocity, randValue)) { + if (region->triggerOnNote && region->trigger == SfzTrigger::release && !region->rtDead) { + // check that a voice with compatible trigger is playing + // FIXME: we're going twice over the voices, when the synth + // handles the regions completely these dispatch functions + // should be overhauled, also to include voice stealing on + // all events + const auto compatibleVoice = [region](const VoicePtr& v) -> bool { + return ( + !v->isFree() + && v->getTriggerType() == Voice::TriggerType::NoteOn + && region->keyRange.containsWithEnd(v->getTriggerNumber()) + && region->velocityRange.containsWithEnd(v->getTriggerValue()) + ); + }; + + if (absl::c_find_if(voices, compatibleVoice) == voices.end()) + continue; + } + auto voice = findFreeVoice(); if (voice == nullptr) continue; @@ -1002,6 +1021,27 @@ void sfz::Synth::hdcc(int delay, int ccNumber, float normValue) noexcept for (auto& region : ccActivationLists[ccNumber]) { if (ccNumber == region->sustainCC) { + if (!region->rtDead) { + // check that a voice with compatible trigger is playing + // FIXME: we're going twice over the voices, when the synth + // handles the regions completely these dispatch functions + // should be overhauled, also to include voice stealing on + // all events + const auto compatibleVoice = [region](const VoicePtr& v) -> bool { + return ( + !v->isFree() + && v->getTriggerType() == Voice::TriggerType::NoteOn + && region->keyRange.containsWithEnd(v->getTriggerNumber()) + && region->velocityRange.containsWithEnd(v->getTriggerValue()) + ); + }; + + if (absl::c_find_if(voices, compatibleVoice) == voices.end()) { + region->delayedReleases.clear(); + continue; + } + } + for (auto& note: region->delayedReleases) { // FIXME: we really need to have some form of common method to find and start voices... auto voice = findFreeVoice(); From cb367d385fbb8ad79870eabffae4fcfb5b905684 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sun, 9 Aug 2020 16:41:47 +0200 Subject: [PATCH 8/8] Make the matching function a free one --- src/sfizz/Synth.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 31928c2cf..c2f6114f9 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -837,6 +837,15 @@ void sfz::Synth::noteOff(int delay, int noteNumber, uint8_t velocity) noexcept noteOffDispatch(delay, noteNumber, replacedVelocity); } +bool matchReleaseRegionAndVoice(const sfz::Region& region, const sfz::Voice& voice) { + return ( + !voice.isFree() + && voice.getTriggerType() == sfz::Voice::TriggerType::NoteOn + && region.keyRange.containsWithEnd(voice.getTriggerNumber()) + && region.velocityRange.containsWithEnd(voice.getTriggerValue()) + ); +} + void sfz::Synth::noteOffDispatch(int delay, int noteNumber, float velocity) noexcept { const auto randValue = randNoteDistribution(Random::randomGenerator); @@ -851,12 +860,7 @@ void sfz::Synth::noteOffDispatch(int delay, int noteNumber, float velocity) noex // should be overhauled, also to include voice stealing on // all events const auto compatibleVoice = [region](const VoicePtr& v) -> bool { - return ( - !v->isFree() - && v->getTriggerType() == Voice::TriggerType::NoteOn - && region->keyRange.containsWithEnd(v->getTriggerNumber()) - && region->velocityRange.containsWithEnd(v->getTriggerValue()) - ); + return matchReleaseRegionAndVoice(*region, *v); }; if (absl::c_find_if(voices, compatibleVoice) == voices.end()) @@ -1028,12 +1032,7 @@ void sfz::Synth::hdcc(int delay, int ccNumber, float normValue) noexcept // should be overhauled, also to include voice stealing on // all events const auto compatibleVoice = [region](const VoicePtr& v) -> bool { - return ( - !v->isFree() - && v->getTriggerType() == Voice::TriggerType::NoteOn - && region->keyRange.containsWithEnd(v->getTriggerNumber()) - && region->velocityRange.containsWithEnd(v->getTriggerValue()) - ); + return matchReleaseRegionAndVoice(*region, *v); }; if (absl::c_find_if(voices, compatibleVoice) == voices.end()) {