From 80aee5a9cdaf37a895d811301af552769f27f543 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Wed, 19 Aug 2020 00:47:31 +0200 Subject: [PATCH 1/7] Add a helper for playing voices --- tests/TestHelpers.cpp | 11 +++++++++++ tests/TestHelpers.h | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/TestHelpers.cpp b/tests/TestHelpers.cpp index ca32de0b..f58eee73 100644 --- a/tests/TestHelpers.cpp +++ b/tests/TestHelpers.cpp @@ -51,6 +51,17 @@ const std::vector getActiveVoices(const sfz::Synth& synth) return activeVoices; } +const std::vector getPlayingVoices(const sfz::Synth& synth) +{ + std::vector playingVoices; + for (int i = 0; i < synth.getNumVoices(); ++i) { + const auto* voice = synth.getVoiceView(i); + if (!voice->releasedOrFree()) + playingVoices.push_back(voice); + } + return playingVoices; +} + unsigned numPlayingVoices(const sfz::Synth& synth) { return absl::c_count_if(getActiveVoices(synth), [](const sfz::Voice* v) { diff --git a/tests/TestHelpers.h b/tests/TestHelpers.h index ac8be3dc..e5e48671 100644 --- a/tests/TestHelpers.h +++ b/tests/TestHelpers.h @@ -49,6 +49,15 @@ void sortAll(C& container, Args&... others) */ const std::vector getActiveVoices(const sfz::Synth& synth); +/** + * @brief Get playing (unreleased) voices from the synth + * + * @param synth + * @return const std::vector + */ +const std::vector getPlayingVoices(const sfz::Synth& synth); + + /** * @brief Count the number of playing (unreleased) voices from the synth * From 5c61298f08f6eed9a1f754bd3bf2f3734e09e828 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Wed, 19 Aug 2020 00:48:20 +0200 Subject: [PATCH 2/7] A region is disabled by end=-1 --- src/sfizz/Region.cpp | 6 +++++- src/sfizz/Region.h | 1 + tests/RegionT.cpp | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index 2d96835a..cad035b8 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -92,7 +92,11 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) offsetCC[opcode.parameters.back()] = *value; break; case hash("end"): - setValueFromOpcode(opcode, sampleEnd, Default::sampleEndRange); + if (opcode.value == "-1") { + disabled = true; + } else { + setValueFromOpcode(opcode, sampleEnd, Default::sampleEndRange); + } break; case hash("count"): setValueFromOpcode(opcode, sampleCount, Default::sampleCountRange); diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index 4935a85e..4dc5cdef 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -272,6 +272,7 @@ struct Region { int64_t offsetRandom { Default::offsetRandom }; // offset_random CCMap offsetCC { Default::offset }; uint32_t sampleEnd { Default::sampleEndRange.getEnd() }; // end + bool disabled { false }; // end=-1 and other disabling events absl::optional sampleCount {}; // count absl::optional loopMode {}; // loopmode Range loopRange { Default::loopRange }; //loopstart and loopend diff --git a/tests/RegionT.cpp b/tests/RegionT.cpp index 9ea9b924..d4e68982 100644 --- a/tests/RegionT.cpp +++ b/tests/RegionT.cpp @@ -95,7 +95,7 @@ TEST_CASE("[Region] Parsing opcodes") region.parseOpcode({ "end", "184" }); REQUIRE(region.sampleEnd == 184); region.parseOpcode({ "end", "-1" }); - REQUIRE(region.sampleEnd == 0); + REQUIRE(region.disabled); } SECTION("count") From cef44cbdb67aba8ef448c1736d9a455d88f01bf1 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Wed, 19 Aug 2020 00:50:09 +0200 Subject: [PATCH 3/7] Disabled regions don't need to take up polyphony The tests were a bit flaky and the did not seem justified by any behavior in e.g. ARIA The new tests cover a similar situation which is validated in sforzando --- src/sfizz/Voice.cpp | 7 ++-- tests/FilesT.cpp | 50 ---------------------------- tests/SynthT.cpp | 80 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 53 deletions(-) diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index d7517a3d..86dd1cd4 100644 --- a/src/sfizz/Voice.cpp +++ b/src/sfizz/Voice.cpp @@ -51,6 +51,10 @@ void sfz::Voice::startVoice(Region* region, int delay, int number, float value, triggerValue = value; this->region = region; + + if (region->disabled) + return; + switchState(State::playing); ASSERT(delay >= 0); @@ -715,9 +719,6 @@ bool sfz::Voice::checkOffGroup(int delay, uint32_t group) noexcept if (region == nullptr) return false; - if (delay <= this->triggerDelay) - return false; - if (triggerType == TriggerType::NoteOn && region->offBy == group) { off(delay); return true; diff --git a/tests/FilesT.cpp b/tests/FilesT.cpp index f92f496c..fdc78691 100644 --- a/tests/FilesT.cpp +++ b/tests/FilesT.cpp @@ -471,56 +471,6 @@ TEST_CASE("[Files] Note and octave offsets") REQUIRE( synth.getRegionView(6)->pitchKeycenter == 50 ); } -TEST_CASE("[Files] Off by with different delays") -{ - Synth synth; - synth.setSamplesPerBlock(256); - AudioBuffer buffer(2, 256); - synth.loadSfzFile(fs::current_path() / "tests/TestFiles/off_by.sfz"); - REQUIRE( synth.getNumRegions() == 4 ); - synth.noteOn(0, 63, 63); - REQUIRE( synth.getNumActiveVoices(true) == 1 ); - auto group1Voice = synth.getVoiceView(0); - REQUIRE( group1Voice->getRegion()->group == 1ul ); - REQUIRE( group1Voice->getRegion()->offBy == 2ul ); - synth.noteOn(100, 64, 63); - synth.renderBlock(buffer); - REQUIRE(group1Voice->releasedOrFree()); -} - -TEST_CASE("[Files] Off by with the same delays") -{ - Synth synth; - synth.setSamplesPerBlock(256); - synth.loadSfzFile(fs::current_path() / "tests/TestFiles/off_by.sfz"); - REQUIRE( synth.getNumRegions() == 4 ); - synth.noteOn(0, 63, 63); - REQUIRE( synth.getNumActiveVoices(true) == 1 ); - auto group1Voice = synth.getVoiceView(0); - REQUIRE( group1Voice->getRegion()->group == 1ul ); - REQUIRE( group1Voice->getRegion()->offBy == 2ul ); - synth.noteOn(0, 64, 63); - REQUIRE(!group1Voice->releasedOrFree()); -} - -TEST_CASE("[Files] Off by with the same notes at the same time") -{ - Synth synth; - synth.setSamplesPerBlock(256); - synth.loadSfzFile(fs::current_path() / "tests/TestFiles/off_by.sfz"); - REQUIRE( synth.getNumRegions() == 4 ); - synth.noteOn(0, 65, 63); - REQUIRE( synth.getNumActiveVoices(true) == 2 ); - synth.noteOn(0, 65, 63); - REQUIRE( synth.getNumActiveVoices(true) == 4 ); - AudioBuffer buffer { 2, 256 }; - synth.renderBlock(buffer); - synth.noteOn(0, 65, 63); - synth.renderBlock(buffer); - REQUIRE( synth.getNumActiveVoices(true) == 6 ); - REQUIRE( numPlayingVoices(synth) == 2 ); -} - TEST_CASE("[Files] Off modes") { Synth synth; diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index cf933ead..5d167385 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -1143,3 +1143,83 @@ TEST_CASE("[Synth] Trigger also on the sustain CC") synth.cc(0, 64, 127); REQUIRE( synth.getNumActiveVoices(true) == 1 ); } + +TEST_CASE("[Synth] end=-1 voices are immediately killed after triggering but they kill other voices") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 256 }; + + synth.loadSfzString(fs::current_path(), R"( + key=60 end=-1 sample=*sine + key=61 end=-1 sample=*silence + key=62 sample=*sine off_by=2 + key=63 end=-1 sample=*saw group=2 + )"); + synth.noteOn(0, 60, 85); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOn(0, 61, 85); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOn(0, 62, 85); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + REQUIRE( numPlayingVoices(synth) == 1 ); + synth.noteOn(1, 63, 85); + synth.renderBlock(buffer); + REQUIRE( numPlayingVoices(synth) == 0 ); +} + +TEST_CASE("[Synth] Off by standard") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 256 }; + + synth.loadSfzString(fs::current_path(), R"( + group=1 off_by=2 sample=*saw transpose=12 key=60 + group=2 off_by=1 sample=*triangle key=62 + )"); + synth.noteOn(0, 60, 85); + REQUIRE( numPlayingVoices(synth) == 1 ); + synth.noteOn(10, 62, 85); + REQUIRE( numPlayingVoices(synth) == 1 ); + auto playingVoices = getPlayingVoices(synth); + REQUIRE( playingVoices.front()->getRegion()->keyRange.containsWithEnd(62) ); + synth.noteOn(10, 60, 85); + playingVoices = getPlayingVoices(synth); + REQUIRE( playingVoices.front()->getRegion()->keyRange.containsWithEnd(60) ); +} + +TEST_CASE("[Synth] Off by same group") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 256 }; + + synth.loadSfzString(fs::current_path(), R"( + group=1 off_by=1 sample=*saw transpose=12 key=60 + group=1 off_by=1 sample=*triangle key=62 + )"); + synth.noteOn(0, 60, 85); + REQUIRE( numPlayingVoices(synth) == 1 ); + synth.noteOn(10, 62, 85); + REQUIRE( numPlayingVoices(synth) == 1 ); + auto playingVoices = getPlayingVoices(synth); + REQUIRE( playingVoices.front()->getRegion()->keyRange.containsWithEnd(62) ); + synth.noteOn(10, 60, 85); + playingVoices = getPlayingVoices(synth); + REQUIRE( playingVoices.front()->getRegion()->keyRange.containsWithEnd(60) ); +} + + +TEST_CASE("[Synth] Off by same note") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 256 }; + + synth.loadSfzString(fs::current_path(), R"( + group=1 off_by=1 sample=*saw transpose=12 key=60 + group=1 off_by=1 sample=*triangle key=60 + )"); + synth.noteOn(0, 60, 85); + REQUIRE( numPlayingVoices(synth) == 1 ); + auto playingVoices = getPlayingVoices(synth); + REQUIRE( playingVoices.front()->getRegion()->sampleId.filename() == "*triangle" ); +} + From 17568510ac91e1e15c965bdf681b3b802ca71d51 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Wed, 19 Aug 2020 11:06:16 +0200 Subject: [PATCH 4/7] Disable with sampleEnd == 0 --- src/sfizz/Region.cpp | 16 ++++++++++------ src/sfizz/Region.h | 6 +++++- src/sfizz/Voice.cpp | 2 +- tests/RegionT.cpp | 7 ++++++- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index cad035b8..8843ce5c 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -92,11 +92,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) offsetCC[opcode.parameters.back()] = *value; break; case hash("end"): - if (opcode.value == "-1") { - disabled = true; - } else { - setValueFromOpcode(opcode, sampleEnd, Default::sampleEndRange); - } + setValueFromOpcode(opcode, sampleEnd, Default::sampleEndRange); break; case hash("count"): setValueFromOpcode(opcode, sampleCount, Default::sampleCountRange); @@ -1462,7 +1458,10 @@ float sfz::Region::getDelay() const noexcept uint32_t sfz::Region::trueSampleEnd(Oversampling factor) const noexcept { - return min(sampleEnd, loopRange.getEnd()) * static_cast(factor); + if (sampleEnd <= 0) + return 0; + + return min(static_cast(sampleEnd), loopRange.getEnd()) * static_cast(factor); } uint32_t sfz::Region::loopStart(Oversampling factor) const noexcept @@ -1619,3 +1618,8 @@ sfz::Region::Connection& sfz::Region::getOrCreateConnection(const ModKey& source connections.push_back(c); return connections.back(); } + +bool sfz::Region::disabled() const noexcept +{ + return (sampleEnd == 0); +} diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index 4dc5cdef..e5fbb6c0 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -261,6 +261,11 @@ struct Region { */ float getGainToEffectBus(unsigned number) const noexcept; + /** + * @brief Check if a region is disabled, if its sample end is weakly negative for example. + */ + bool disabled() const noexcept; + const NumericId id; // Sound source: sample playback @@ -272,7 +277,6 @@ struct Region { int64_t offsetRandom { Default::offsetRandom }; // offset_random CCMap offsetCC { Default::offset }; uint32_t sampleEnd { Default::sampleEndRange.getEnd() }; // end - bool disabled { false }; // end=-1 and other disabling events absl::optional sampleCount {}; // count absl::optional loopMode {}; // loopmode Range loopRange { Default::loopRange }; //loopstart and loopend diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index 86dd1cd4..3321d761 100644 --- a/src/sfizz/Voice.cpp +++ b/src/sfizz/Voice.cpp @@ -52,7 +52,7 @@ void sfz::Voice::startVoice(Region* region, int delay, int number, float value, this->region = region; - if (region->disabled) + if (region->disabled()) return; switchState(State::playing); diff --git a/tests/RegionT.cpp b/tests/RegionT.cpp index d4e68982..fa3f8f4c 100644 --- a/tests/RegionT.cpp +++ b/tests/RegionT.cpp @@ -95,7 +95,12 @@ TEST_CASE("[Region] Parsing opcodes") region.parseOpcode({ "end", "184" }); REQUIRE(region.sampleEnd == 184); region.parseOpcode({ "end", "-1" }); - REQUIRE(region.disabled); + REQUIRE(region.disabled()); + region.parseOpcode({ "end", "2" }); + REQUIRE(!region.disabled()); + REQUIRE(region.sampleEnd == 2); + region.parseOpcode({ "end", "0" }); + REQUIRE(region.disabled()); } SECTION("count") From fae0cda97139a162194d32923d68ea01006240f7 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Wed, 19 Aug 2020 11:06:26 +0200 Subject: [PATCH 5/7] Correct a compilation woe --- tests/DataHelpers.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/DataHelpers.cpp b/tests/DataHelpers.cpp index 1f0bd949..a5cb7df0 100644 --- a/tests/DataHelpers.cpp +++ b/tests/DataHelpers.cpp @@ -10,6 +10,7 @@ #include #include #include +#include void load_txt(DataPoints& dp, std::istream& in) { From f6e0bb8b3f942d59d0373db3f53e464efca72ee3 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Wed, 19 Aug 2020 19:12:33 +0200 Subject: [PATCH 6/7] Add special cases for the rate transformation in envelopes For increasing linear ramps, a 0s ramp is an increment of 1.0 For decaying exponential ramps, a 0s ramp is a multiplicator of 0.0f --- src/sfizz/ADSREnvelope.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/sfizz/ADSREnvelope.cpp b/src/sfizz/ADSREnvelope.cpp index b17bce09..78415f8a 100644 --- a/src/sfizz/ADSREnvelope.cpp +++ b/src/sfizz/ADSREnvelope.cpp @@ -20,13 +20,18 @@ Type ADSREnvelope::secondsToSamples (Type timeInSeconds) const noexcept template Type ADSREnvelope::secondsToLinRate (Type timeInSeconds) const noexcept { - timeInSeconds = std::max(timeInSeconds, config::virtuallyZero); + if (timeInSeconds == 0) + return 1.0f; + return 1 / (sampleRate * timeInSeconds); }; template Type ADSREnvelope::secondsToExpRate (Type timeInSeconds) const noexcept { + if (timeInSeconds == 0) + return 0.0f; + timeInSeconds = std::max(25e-3, timeInSeconds); return std::exp(-9.0 / (timeInSeconds * sampleRate)); }; From f6f1e68f18ab68112eebc004595f7f76c52d0e45 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Wed, 19 Aug 2020 19:13:50 +0200 Subject: [PATCH 7/7] Sustain tweaks: - Separate the sustain value from the sustain threshold after which the envelope goes from decay to sustain - If the sustain value is set to 0, the envelope is freerunning --- src/sfizz/ADSREnvelope.cpp | 10 +++++--- src/sfizz/ADSREnvelope.h | 1 + tests/SynthT.cpp | 50 +++++++++++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/sfizz/ADSREnvelope.cpp b/src/sfizz/ADSREnvelope.cpp index 78415f8a..8d92ea10 100644 --- a/src/sfizz/ADSREnvelope.cpp +++ b/src/sfizz/ADSREnvelope.cpp @@ -48,12 +48,14 @@ void ADSREnvelope::reset(const EGDescription& desc, const Region& region, this->hold = secondsToSamples(desc.getHold(state, velocity)); this->peak = 1.0; this->sustain = normalizePercents(desc.getSustain(state, velocity)); - this->sustain = max(this->sustain, config::virtuallyZero); this->start = this->peak * normalizePercents(desc.getStart(state, velocity)); releaseDelay = 0; + sustainThreshold = this->sustain + config::virtuallyZero; shouldRelease = false; - freeRunning = ((region.trigger == SfzTrigger::release) + freeRunning = ( + (region.trigger == SfzTrigger::release) + || (this->sustain == 0.0f) || (region.trigger == SfzTrigger::release_key) || (region.loopMode == SfzLoopMode::one_shot && (region.isGenerator() || region.oscillator))); currentValue = this->start; @@ -89,7 +91,7 @@ Type ADSREnvelope::getNextValue() noexcept // fallthrough case State::Decay: currentValue *= decayRate; - if (currentValue > sustain) + if (currentValue > sustainThreshold ) return currentValue; currentState = State::Sustain; @@ -159,7 +161,7 @@ void ADSREnvelope::getBlock(absl::Span output) noexcept case State::Decay: while (count < size && (currentValue *= decayRate) > sustain) output[count++] = currentValue; - if (currentValue <= sustain) { + if (currentValue <= sustainThreshold) { currentValue = sustain; currentState = State::Sustain; } diff --git a/src/sfizz/ADSREnvelope.h b/src/sfizz/ADSREnvelope.h index 892388f7..eafd2f90 100644 --- a/src/sfizz/ADSREnvelope.h +++ b/src/sfizz/ADSREnvelope.h @@ -102,6 +102,7 @@ private: Type start { 0 }; Type peak { 0 }; Type sustain { 0 }; + Type sustainThreshold { config::virtuallyZero }; int releaseDelay { 0 }; bool shouldRelease { false }; bool freeRunning { false }; diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index 5d167385..78f017b6 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -1167,6 +1167,55 @@ TEST_CASE("[Synth] end=-1 voices are immediately killed after triggering but the REQUIRE( numPlayingVoices(synth) == 0 ); } +TEST_CASE("[Synth] end=0 voices are immediately killed after triggering but they kill other voices") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 256 }; + + synth.loadSfzString(fs::current_path(), R"( + key=60 end=0 sample=*sine + key=61 end=0 sample=*silence + key=62 sample=*sine off_by=2 + key=63 end=0 sample=*saw group=2 + )"); + synth.noteOn(0, 60, 85); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOn(0, 61, 85); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOn(0, 62, 85); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + REQUIRE( numPlayingVoices(synth) == 1 ); + synth.noteOn(1, 63, 85); + synth.renderBlock(buffer); + REQUIRE( numPlayingVoices(synth) == 0 ); +} + +TEST_CASE("[Synth] ampeg_sustain = 0 puts the ampeg envelope in free-running mode, which kills the voice almost instantly in most cases") +{ + sfz::Synth synth; + sfz::AudioBuffer buffer { 2, 256 }; + + synth.loadSfzString(fs::current_path(), R"( + key=60 sample=*sine ampeg_sustain=0 + key=61 sample=*sine ampeg_sustain=0 ampeg_attack=0.1 ampeg_decay=0.1 + )"); + + synth.noteOn(0, 60, 85); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + synth.renderBlock(buffer); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); + synth.noteOn(0, 61, 85); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + // Render a bit; this does not kill the voice + for (unsigned i = 0; i < 5; ++i) + synth.renderBlock(buffer); + REQUIRE( synth.getNumActiveVoices(true) == 1 ); + // Render about half a second + for (unsigned i = 0; i < 100; ++i) + synth.renderBlock(buffer); + REQUIRE( synth.getNumActiveVoices(true) == 0 ); +} + TEST_CASE("[Synth] Off by standard") { sfz::Synth synth; @@ -1222,4 +1271,3 @@ TEST_CASE("[Synth] Off by same note") auto playingVoices = getPlayingVoices(synth); REQUIRE( playingVoices.front()->getRegion()->sampleId.filename() == "*triangle" ); } -