From 6086284efec6ad240cedf11a6dc51f687d149d78 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sun, 19 Jul 2020 13:18:30 +0200 Subject: [PATCH] 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 0a19625c..e03e50a7 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 772ee399..366efd1f 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 @@ private: 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 6081ba2b..0ab5664b 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 ecfea499..d98b6c7a 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 ccd92b94..3a07437a 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() ); +}