From 6d692ba5be773a3818b5b8829397f8132b6ecf8d Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 19 Mar 2021 00:37:13 +0100 Subject: [PATCH 1/2] Rewrite the ring builder simpler, and add assertion --- src/sfizz/SisterVoiceRing.h | 32 +++++++++++--------------------- src/sfizz/Voice.h | 6 ++++++ 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/sfizz/SisterVoiceRing.h b/src/sfizz/SisterVoiceRing.h index bb6137bb3..edc6e8014 100644 --- a/src/sfizz/SisterVoiceRing.h +++ b/src/sfizz/SisterVoiceRing.h @@ -6,6 +6,7 @@ #pragma once #include "Voice.h" +#include "Debug.h" #include "absl/meta/type_traits.h" namespace sfz @@ -135,31 +136,20 @@ class SisterVoiceRingBuilder { * @param voice */ void addVoiceToRing(Voice* voice) noexcept { - if (firstStartedVoice == nullptr) - firstStartedVoice = voice; + ASSERT(!voice->isInSisterRing()); - firstStartedVoice->setPreviousSisterVoice(voice); - voice->setNextSisterVoice(firstStartedVoice); + Voice* next = head_; + if (!next) + head_ = next = voice; - if (lastStartedVoice != nullptr) { - voice->setPreviousSisterVoice(lastStartedVoice); - lastStartedVoice->setNextSisterVoice(voice); - } - - lastStartedVoice = voice; + Voice* previous = next->getPreviousSisterVoice(); + voice->setNextSisterVoice(next); + voice->setPreviousSisterVoice(previous); + next->setPreviousSisterVoice(voice); + previous->setNextSisterVoice(voice); } - /** - * @brief Apply a function to the sister ring, including the current voice. - * This function should be safe enough to even reset the sister voices, but - * if you mutate the ring significantly you should probably roll your own - * iterator. - * - * @param lambda the function to apply. - * @param voice the starting voice - */ private: - Voice* firstStartedVoice { nullptr }; - Voice* lastStartedVoice { nullptr }; + Voice* head_ { nullptr }; }; } diff --git a/src/sfizz/Voice.h b/src/sfizz/Voice.h index 595bf1c1b..b0c5b8cc4 100644 --- a/src/sfizz/Voice.h +++ b/src/sfizz/Voice.h @@ -382,6 +382,12 @@ class Voice { */ const TriggerEvent& getTriggerEvent(); +public: + /** + * @brief Check if the voice already belongs to a sister ring + */ + bool isInSisterRing() const noexcept { return this != nextSisterVoice_; } + private: struct Impl; std::unique_ptr impl_; From f6e93911aec0d0f93dea8a0381c01a86708f0390 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 19 Mar 2021 01:15:18 +0100 Subject: [PATCH 2/2] Fix breakage of sister ring and related bugs --- src/sfizz/Synth.cpp | 4 ++-- src/sfizz/Voice.cpp | 31 +++++++++++++++++++------------ src/sfizz/Voice.h | 3 ++- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index c99bcaf67..d1808b3d3 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -1066,8 +1066,8 @@ void Synth::Impl::startVoice(Region* region, int delay, const TriggerEvent& trig return; ASSERT(selectedVoice->isFree()); - selectedVoice->startVoice(region, delay, triggerEvent); - ring.addVoiceToRing(selectedVoice); + if (selectedVoice->startVoice(region, delay, triggerEvent)) + ring.addVoiceToRing(selectedVoice); } void Synth::Impl::noteOffDispatch(int delay, int noteNumber, float velocity) noexcept diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index 34b98c8d2..b2beed4ab 100644 --- a/src/sfizz/Voice.cpp +++ b/src/sfizz/Voice.cpp @@ -358,19 +358,22 @@ Voice::Impl::Impl(int voiceNumber, Resources& resources) getSCurve(); } -void Voice::startVoice(Region* region, int delay, const TriggerEvent& event) noexcept +bool Voice::startVoice(Region* region, int delay, const TriggerEvent& event) noexcept { Impl& impl = *impl_; ASSERT(event.value >= 0.0f && event.value <= 1.0f); impl.region_ = region; - if (region->disabled()) - return; impl.triggerEvent_ = event; if (impl.triggerEvent_.type == TriggerEventType::CC) impl.triggerEvent_.number = region->pitchKeycenter; + if (region->disabled()) { + impl.switchState(State::cleanMeUp); + return false; + } + impl.switchState(State::playing); ASSERT(delay >= 0); @@ -414,7 +417,7 @@ void Voice::startVoice(Region* region, int delay, const TriggerEvent& event) noe impl.currentPromise_ = impl.resources_.filePool.getFilePromise(region->sampleId); if (!impl.currentPromise_) { impl.switchState(State::cleanMeUp); - return; + return false; } impl.updateLoopInformation(); impl.speedRatio_ = static_cast(impl.currentPromise_->information.sampleRate / impl.sampleRate_); @@ -455,6 +458,8 @@ void Voice::startVoice(Region* region, int delay, const TriggerEvent& event) noe impl.resources_.modMatrix.initVoice(impl.id_, region->getId(), impl.initialDelay_); impl.saveModulationTargets(region); + + return true; } int Voice::Impl::getCurrentSampleQuality() const noexcept @@ -628,7 +633,8 @@ void Voice::renderBlock(AudioSpan buffer) noexcept ASSERT(static_cast(buffer.getNumFrames()) <= impl.samplesPerBlock_); buffer.fill(0.0f); - if (impl.region_ == nullptr) + const Region* region = impl.region_; + if (region == nullptr || region->disabled()) return; const auto delay = min(static_cast(impl.initialDelay_), buffer.getNumFrames()); @@ -637,13 +643,13 @@ void Voice::renderBlock(AudioSpan buffer) noexcept { // Fill buffer with raw data ScopedTiming logger { impl.dataDuration_ }; - if (impl.region_->isOscillator()) + if (region->isOscillator()) impl.fillWithGenerator(delayed_buffer); else impl.fillWithData(delayed_buffer); } - if (impl.region_->isStereo()) { + if (region->isStereo()) { impl.ampStageStereo(buffer); impl.panStageStereo(buffer); impl.filterStageStereo(buffer); @@ -653,12 +659,12 @@ void Voice::renderBlock(AudioSpan buffer) noexcept impl.panStageMono(buffer); } - if (!impl.region_->flexAmpEG) { + if (!region->flexAmpEG) { if (!impl.egAmplitude_.isSmoothing()) impl.switchState(State::cleanMeUp); } else { - if (impl.flexEGs_[*impl.region_->flexAmpEG]->isFinished()) + if (impl.flexEGs_[*region->flexAmpEG]->isFinished()) impl.switchState(State::cleanMeUp); } @@ -1476,12 +1482,13 @@ bool Voice::Impl::released() const noexcept bool Voice::checkOffGroup(const Region* other, int delay, int noteNumber) noexcept { Impl& impl = *impl_; - if (impl.region_ == nullptr || other == nullptr) + const Region* region = impl.region_; + if (region == nullptr || other == nullptr) return false; if (impl.triggerEvent_.type == TriggerEventType::NoteOn - && impl.region_->offBy == other->group - && (impl.region_->group != other->group || noteNumber != impl.triggerEvent_.number)) { + && region->offBy == other->group + && (region->group != other->group || noteNumber != impl.triggerEvent_.number)) { off(delay); return true; } diff --git a/src/sfizz/Voice.h b/src/sfizz/Voice.h index b0c5b8cc4..eae11f4f5 100644 --- a/src/sfizz/Voice.h +++ b/src/sfizz/Voice.h @@ -102,8 +102,9 @@ class Voice { * @param region * @param delay * @param evebt + * @return bool */ - void startVoice(Region* region, int delay, const TriggerEvent& event) noexcept; + bool startVoice(Region* region, int delay, const TriggerEvent& event) noexcept; /** * @brief Get the sample quality determined by the active region.