From 897ddc1a8fc23cd1f298fea6d8767a116e817f54 Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Sun, 15 May 2022 01:47:30 +0200 Subject: [PATCH] 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 | 6 +++--- src/sfizz/ADSREnvelope.h | 7 +++++++ src/sfizz/Config.h | 13 ++++--------- src/sfizz/Synth.cpp | 7 ++++++- src/sfizz/Synth.h | 8 ++++++++ src/sfizz/Voice.cpp | 1 + src/sfizz/VoiceManager.cpp | 32 +++++++++++++++++++++++++------- src/sfizz/VoiceManager.h | 8 +++++++- tests/SynthT.cpp | 24 ++++++++++++++++++++++-- tests/TestHelpers.cpp | 38 ++++++++++++++++++++++++++++++-------- tests/TestHelpers.h | 24 ++++++++++++++++++++---- 11 files changed, 133 insertions(+), 35 deletions(-) diff --git a/src/sfizz/ADSREnvelope.cpp b/src/sfizz/ADSREnvelope.cpp index 6167b6896..819434de1 100644 --- a/src/sfizz/ADSREnvelope.cpp +++ b/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 output) noexcept { - if (desc_ && desc_->dynamic) { + if (dynamic_) { int processed = 0; int remaining = static_cast(output.size()); while(remaining > 0) { diff --git a/src/sfizz/ADSREnvelope.h b/src/sfizz/ADSREnvelope.h index a970a1d42..17007d95d 100644 --- a/src/sfizz/ADSREnvelope.h +++ b/src/sfizz/ADSREnvelope.h @@ -76,6 +76,12 @@ class ADSREnvelope { */ 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 @@ class ADSREnvelope { const EGDescription* desc_ { nullptr }; const MidiState& midiState_; float triggerVelocity_ { 0.0f }; + bool dynamic_ { false }; int delay { 0 }; Float attackStep { 0 }; Float decayRate { 0 }; diff --git a/src/sfizz/Config.h b/src/sfizz/Config.h index 9bfc24250..5aa96541f 100644 --- a/src/sfizz/Config.h +++ b/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, - * accounting for the overflow factor. + * @brief Minimum number of overflow voices to add + * */ - inline constexpr int calculateActualVoices(int polyphony) - { - return - (int(polyphony * config::overflowVoiceMultiplier) < int(config::maxVoices)) ? - int(polyphony * config::overflowVoiceMultiplier) : int(config::maxVoices); - } + static constexpr int minOverflowVoices { 4 }; + /** * @brief The smoothing time constant per "smooth" steps */ diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index e8b74b737..d589fafde 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -1042,6 +1042,12 @@ int Synth::getNumActiveVoices() const noexcept std::min(impl.numVoices_, activeVoices) : activeVoices; } +std::vector 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); } diff --git a/src/sfizz/Synth.h b/src/sfizz/Synth.h index 251ad06b2..eb98a4535 100644 --- a/src/sfizz/Synth.h +++ b/src/sfizz/Synth.h @@ -555,6 +555,14 @@ class Synth final { * @return int */ int getNumActiveVoices() const noexcept; + + /** + * @brief Get the active voices as a view + * + * @return std::vector + */ + std::vector getActiveVoices() const noexcept; + /** * @brief Get the total number of voices in the synth (the polyphony) * diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index 396ebcf57..6380e26d8 100644 --- a/src/sfizz/Voice.cpp +++ b/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 diff --git a/src/sfizz/VoiceManager.cpp b/src/sfizz/VoiceManager.cpp index 7440f5cf8..73e7476b3 100644 --- a/src/sfizz/VoiceManager.cpp +++ b/src/sfizz/VoiceManager.cpp @@ -104,6 +104,14 @@ const PolyphonyGroup* VoiceManager::getPolyphonyGroupView(int idx) noexcept return &polyphonyGroups_[idx]; } +std::vector VoiceManager::getActiveVoices() const noexcept +{ + std::vector 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) { - return voice.isFree(); - }); + Voice* freeVoice = nullptr; + for (auto& v: list_) { + if (v.isFree()) { + freeVoice = &v; + break; + } - if (freeVoice != list_.end()) - return &*freeVoice; + if (v.offedOrFree()) { + 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 diff --git a/src/sfizz/VoiceManager.h b/src/sfizz/VoiceManager.h index 785f271cd..354976b5e 100644 --- a/src/sfizz/VoiceManager.h +++ b/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 + */ + std::vector 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 list_; std::vector activeVoices_; std::vector temp_; diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index e071b2c3a..1b77c276f 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -989,11 +989,11 @@ TEST_CASE("[Synth] Sustain threshold") REQUIRE( playingSamples(synth) == std::vector { "*sine", "*sine" } ); synth.noteOn(0, 62, 85); synth.renderBlock(buffer); - REQUIRE( playingSamples(synth) == std::vector { "*silence", "*sine", "*sine" } ); + REQUIRE( playingSamples(synth) == std::vector { "*sine", "*sine", "*silence" } ); synth.cc(0, 64, 64); synth.noteOff(0, 62, 85); synth.renderBlock(buffer); - REQUIRE( playingSamples(synth) == std::vector { "*silence", "*sine", "*sine" } ); + REQUIRE( playingSamples(synth) == std::vector {"*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 buffer { 2, static_cast(synth.getSamplesPerBlock()) }; + synth.loadSfzString(fs::current_path() / "tests/TestFiles/polyphony.sfz", R"( + sample=*sine ampeg_release=10 + )"); + synth.noteOn(0, 60, 63); + synth.renderBlock(buffer); + REQUIRE( playingNotes(synth) == std::vector { 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 { i - 1, i } ); + } +} diff --git a/tests/TestHelpers.cpp b/tests/TestHelpers.cpp index 4a81175e8..08b66d0c4 100644 --- a/tests/TestHelpers.cpp +++ b/tests/TestHelpers.cpp @@ -92,8 +92,8 @@ unsigned numActiveVoices(const sfz::Synth& synth) const std::vector playingSamples(const sfz::Synth& synth) { std::vector samples; - for (int i = 0; i < synth.getNumVoices(); ++i) { - const auto* voice = synth.getVoiceView(i); + auto activeVoices = synth.getActiveVoices(); + 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 playingSamples(const sfz::Synth& synth) const std::vector playingVelocities(const sfz::Synth& synth) { std::vector velocities; - for (int i = 0; i < synth.getNumVoices(); ++i) { - const auto* voice = synth.getVoiceView(i); + auto activeVoices = synth.getActiveVoices(); + for (const auto* voice: activeVoices) { if (!voice->released()) velocities.push_back(voice->getTriggerEvent().value); } return velocities; } +const std::vector playingNotes(const sfz::Synth& synth) +{ + std::vector notes; + auto activeVoices = synth.getActiveVoices(); + for (const auto* voice: activeVoices) { + if (!voice->released()) + notes.push_back(voice->getTriggerEvent().number); + } + return notes; +} + const std::vector activeSamples(const sfz::Synth& synth) { std::vector samples; - for (int i = 0; i < synth.getNumVoices(); ++i) { - const auto* voice = synth.getVoiceView(i); + auto activeVoices = synth.getActiveVoices(); + for (const auto* voice: activeVoices) { if (!voice->isFree()) { const sfz::Region* region = voice->getRegion(); if (region) @@ -130,14 +141,25 @@ const std::vector activeSamples(const sfz::Synth& synth) const std::vector activeVelocities(const sfz::Synth& synth) { std::vector velocities; - for (int i = 0; i < synth.getNumVoices(); ++i) { - const auto* voice = synth.getVoiceView(i); + auto activeVoices = synth.getActiveVoices(); + for (const auto* voice: activeVoices) { if (!voice->isFree()) velocities.push_back(voice->getTriggerEvent().value); } return velocities; } +const std::vector activeNotes(const sfz::Synth& synth) +{ + std::vector 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 lines, int numRegions) { for (int regionIdx = 0; regionIdx < numRegions; ++regionIdx) { diff --git a/tests/TestHelpers.h b/tests/TestHelpers.h index 2145555eb..8faa28b55 100644 --- a/tests/TestHelpers.h +++ b/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 */ const std::vector playingSamples(const sfz::Synth& synth); @@ -98,15 +98,23 @@ const std::vector playingSamples(const sfz::Synth& synth); * @brief Get the playing notes velocities * * @param synth - * @return unsigned + * @return std::vector */ const std::vector playingVelocities(const sfz::Synth& synth); +/** + * @brief Get the playing notes + * + * @param synth + * @return std::vector + */ +const std::vector playingNotes(const sfz::Synth& synth); + /** * @brief Get the active samples * * @param synth - * @return unsigned + * @return std::vector */ const std::vector activeSamples(const sfz::Synth& synth); @@ -114,10 +122,18 @@ const std::vector activeSamples(const sfz::Synth& synth); * @brief Get the active notes velocities * * @param synth - * @return unsigned + * @return std::vector */ const std::vector activeVelocities(const sfz::Synth& synth); +/** + * @brief Get the active notes + * + * @param synth + * @return std::vector + */ +const std::vector activeNotes(const sfz::Synth& synth); + /** * @brief Create the default dot graph representation for standard regions *