Merge pull request #272 from paulfd/sister-voices

Sister voices

src/sfizz/SisterVoiceRing.h

@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: BSD-2-Clause
+
+// This code is part of the sfizz library and is licensed under a BSD 2-clause
+// license. You should have receive a LICENSE.md file along with the code.
+// If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz
+
+#include "Voice.h"
+#include "absl/meta/type_traits.h"
+
+namespace sfz
+{
+
+struct SisterVoiceRing {
+    template<class F, class T,
+        absl::enable_if_t<std::is_same<Voice, absl::remove_const_t<T>>::value, int> = 0>
+    static void applyToRing(T* voice, F&& lambda)
+    {
+        auto v = voice->getNextSisterVoice();
+        while (v != voice) {
+            const auto next = v->getNextSisterVoice();
+            lambda(v);
+            v = next;
+        }
+        lambda(voice);
+    }
+
+    static unsigned countSisterVoices(const Voice* start)
+    {
+        if (!start)
+            return 0;
+
+        unsigned count = 0;
+        auto next = start;
+        do
+        {
+            count++;
+            next = next->getNextSisterVoice();
+        } while (next != start && count < config::maxVoices);
+
+        ASSERT(count < config::maxVoices);
+        return count;
+    }
+};
+
+/**
+ * @brief RAII helper to build sister voice rings.
+ * Closes the doubly-linked list on destruction.
+ *
+ */
+class SisterVoiceRingBuilder {
+public:
+    ~SisterVoiceRingBuilder() {
+        if (lastStartedVoice != nullptr) {
+            ASSERT(firstStartedVoice);
+            lastStartedVoice->setNextSisterVoice(firstStartedVoice);
+            firstStartedVoice->setPreviousSisterVoice(lastStartedVoice);
+        }
+    }
+
+    /**
+     * @brief Add a voice to the sister ring
+     *
+     * @param voice
+     */
+    void addVoiceToRing(Voice* voice) {
+        if (firstStartedVoice == nullptr)
+            firstStartedVoice = voice;
+
+        if (lastStartedVoice != nullptr) {
+            voice->setPreviousSisterVoice(lastStartedVoice);
+            lastStartedVoice->setNextSisterVoice(voice);
+        }
+
+        lastStartedVoice = 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 };
+};
+
+}

src/sfizz/Synth.cpp

@@ -15,6 +15,7 @@
 #include "absl/algorithm/container.h"
 #include "absl/memory/memory.h"
 #include "absl/strings/str_replace.h"
+#include "SisterVoiceRing.h"
 #include <algorithm>
 #include <chrono>
 #include <iostream>
@@ -550,49 +551,38 @@ sfz::Voice* sfz::Synth::findFreeVoice() noexcept
         / static_cast<float>(voiceViewArray.size()) * config::stealingEnvelopeCoeff;
     const auto ageThreshold = voiceViewArray.front()->getAge() * config::stealingAgeCoeff;
 
-    auto tempSpan = resources.bufferPool.getStereoBuffer(samplesPerBlock);
-    const auto killVoice = [&] (Voice* v) {
-        renderVoiceToOutputs(*v, *tempSpan);
-        v->reset();
-    };
-
     Voice* returnedVoice = voiceViewArray.front();
     unsigned idx = 0;
     while (idx < voiceViewArray.size()) {
-        const auto refIdx = idx;
         const auto ref = voiceViewArray[idx];
-        idx++;
 
         if (ref->getAge() < ageThreshold) {
-            unsigned killIdx = 1;
-            while (killIdx < voiceViewArray.size()
-                    && sisterVoices(returnedVoice, voiceViewArray[killIdx])) {
-                killVoice(voiceViewArray[killIdx]);
-                killIdx++;
-            }
-            // std::cout << "Went too far, picking the oldest voice and killing "
-            //           << killIdx << " voices" << '\n';
-            killVoice(returnedVoice);
+            // Went too far, we'll kill the oldest note.
             break;
         }
 
-        float maxEnvelope = ref->getAverageEnvelope();
-        while (idx < voiceViewArray.size()
-                && sisterVoices(ref, voiceViewArray[idx])) {
-            maxEnvelope = max(maxEnvelope, voiceViewArray[idx]->getAverageEnvelope());
-            idx++;
-        }
+        float maxEnvelope { 0.0f };
+        SisterVoiceRing::applyToRing(ref, [&](Voice* v) {
+            maxEnvelope = max(maxEnvelope, v->getAverageEnvelope());
+        });
 
         if (maxEnvelope < envThreshold) {
             returnedVoice = ref;
-            // std::cout << "Killing " << idx - refIdx << " voices" << '\n';
-            for (unsigned j = refIdx; j < idx; j++) {
-                killVoice(voiceViewArray[j]);
-            }
             break;
         }
+
+        // Jump over the sister voices in the set
+        do { idx++; }
+        while (idx < voiceViewArray.size() && sisterVoices(ref, voiceViewArray[idx]));
     }
-    assert(returnedVoice->isFree());
+
+    auto tempSpan = resources.bufferPool.getStereoBuffer(samplesPerBlock);
+    SisterVoiceRing::applyToRing(returnedVoice, [&] (Voice* v) {
+        renderVoiceToOutputs(*v, *tempSpan);
+        v->reset();
+    });
+    ASSERT(returnedVoice->isFree());
+
     return returnedVoice;
 }
 
@@ -803,20 +793,25 @@ void sfz::Synth::noteOff(int delay, int noteNumber, uint8_t velocity) noexcept
 void sfz::Synth::noteOffDispatch(int delay, int noteNumber, float velocity) noexcept
 {
     const auto randValue = randNoteDistribution(Random::randomGenerator);
+    SisterVoiceRingBuilder ring;
+
     for (auto& region : noteActivationLists[noteNumber]) {
         if (region->registerNoteOff(noteNumber, velocity, randValue)) {
             auto voice = findFreeVoice();
             if (voice == nullptr)
                 continue;
 
             voice->startVoice(region, delay, noteNumber, velocity, Voice::TriggerType::NoteOff);
+            ring.addVoiceToRing(voice);
         }
     }
 }
 
 void sfz::Synth::noteOnDispatch(int delay, int noteNumber, float velocity) noexcept
 {
     const auto randValue = randNoteDistribution(Random::randomGenerator);
+    SisterVoiceRingBuilder ring;
+
     for (auto& region : noteActivationLists[noteNumber]) {
         if (region->registerNoteOn(noteNumber, velocity, randValue)) {
             unsigned activeNotesInGroup { 0 };
@@ -868,6 +863,7 @@ void sfz::Synth::noteOnDispatch(int delay, int noteNumber, float velocity) noexc
                 continue;
 
             voice->startVoice(region, delay, noteNumber, velocity, Voice::TriggerType::NoteOn);
+            ring.addVoiceToRing(voice);
         }
     }
 }
@@ -905,13 +901,16 @@ void sfz::Synth::hdcc(int delay, int ccNumber, float normValue) noexcept
     for (auto& voice : voices)
         voice->registerCC(delay, ccNumber, normValue);
 
+    SisterVoiceRingBuilder ring;
+
     for (auto& region : ccActivationLists[ccNumber]) {
         if (region->registerCC(ccNumber, normValue)) {
             auto voice = findFreeVoice();
             if (voice == nullptr)
                 continue;
 
             voice->startVoice(region, delay, ccNumber, normValue, Voice::TriggerType::CC);
+            ring.addVoiceToRing(voice);
         }
     }
 }

src/sfizz/Voice.cpp

@@ -671,6 +671,30 @@ void sfz::Voice::reset() noexcept
 
     filters.clear();
     equalizers.clear();
+
+    removeVoiceFromRing();
+}
+
+void sfz::Voice::setNextSisterVoice(Voice* voice) noexcept
+{
+    // Should never be null
+    ASSERT(voice);
+    nextSisterVoice = voice;
+}
+
+void sfz::Voice::setPreviousSisterVoice(Voice* voice) noexcept
+{
+    // Should never be null
+    ASSERT(voice);
+    previousSisterVoice = voice;
+}
+
+void sfz::Voice::removeVoiceFromRing() noexcept
+{
+    previousSisterVoice->setNextSisterVoice(nextSisterVoice);
+    nextSisterVoice->setPreviousSisterVoice(previousSisterVoice);
+    previousSisterVoice = this;
+    nextSisterVoice = this;
 }
 
 float sfz::Voice::getAverageEnvelope() const noexcept

src/sfizz/Voice.h

@@ -216,6 +216,40 @@ class Voice {
      */
     void reset() noexcept;
 
+    /**
+     * @brief Set the next voice in the "sister voice" ring
+     * The sister voices are voices that started on the same event.
+     * This has to be set by the synth. A voice will remove itself from
+     * the ring upon reset.
+     *
+     * @param voice
+     */
+    void setNextSisterVoice(Voice* voice) noexcept;
+
+    /**
+     * @brief Set the previous voice in the "sister voice" ring
+     * The sister voices are voices that started on the same event.
+     * This has to be set by the synth. A voice will remove itself from
+     * the ring upon reset.
+     *
+     * @param voice
+     */
+    void setPreviousSisterVoice(Voice* voice) noexcept;
+
+    /**
+     * @brief Get the next sister voice in the ring
+     *
+     * @return Voice*
+     */
+    Voice* getNextSisterVoice() const noexcept { return nextSisterVoice; };
+
+    /**
+     * @brief Get the previous sister voice in the ring
+     *
+     * @return Voice*
+     */
+    Voice* getPreviousSisterVoice() const noexcept { return previousSisterVoice; };
+
     /**
      * @brief Get the mean squared power of the last rendered block. This is used
      * to determine which voice to steal if there are too many notes flying around.
@@ -303,6 +337,12 @@ class Voice {
     void panStageStereo(AudioSpan<float> buffer) noexcept;
     void filterStageMono(AudioSpan<float> buffer) noexcept;
     void filterStageStereo(AudioSpan<float> buffer) noexcept;
+
+    /**
+     * @brief Remove the voice from the sister ring
+     *
+     */
+    void removeVoiceFromRing() noexcept;
     /**
      * @brief Initialize frequency and gain coefficients for the oscillators.
      */
@@ -365,6 +405,9 @@ class Voice {
     Duration panningDuration;
     Duration filterDuration;
 
+    Voice* nextSisterVoice { this };
+    Voice* previousSisterVoice { this };
+
     std::normal_distribution<float> noiseDist { 0, config::noiseVariance };
 
     std::array<OnePoleFilter<float>, 2> channelEnvelopeFilters;

tests/SynthT.cpp

@@ -5,6 +5,7 @@
 // If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz
 
 #include "sfizz/Synth.h"
+#include "sfizz/SisterVoiceRing.h"
 #include "sfizz/SfzHelpers.h"
 #include "sfizz/NumericId.h"
 #include "catch2/catch.hpp"
@@ -543,3 +544,79 @@ TEST_CASE("[Synth] sample quality")
     synth.allSoundOff();
     synth.disableFreeWheeling();
 }
+
+
+TEST_CASE("[Synth] Sister voices")
+{
+    sfz::Synth synth;
+    synth.loadSfzString(fs::current_path(), R"(
+        <region> key=61 sample=*sine
+        <region> key=62 sample=*sine
+        <region> key=62 sample=*sine
+        <region> key=63 sample=*saw
+        <region> key=63 sample=*saw
+        <region> key=63 sample=*saw
+    )");
+    synth.noteOn(0, 61, 85);
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(0)) == 1 );
+    REQUIRE( synth.getVoiceView(0)->getNextSisterVoice() == synth.getVoiceView(0) );
+    REQUIRE( synth.getVoiceView(0)->getPreviousSisterVoice() == synth.getVoiceView(0) );
+    synth.noteOn(0, 62, 85);
+    REQUIRE( synth.getNumActiveVoices() == 3 );
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(1)) == 2 );
+    REQUIRE( synth.getVoiceView(1)->getNextSisterVoice() == synth.getVoiceView(2) );
+    REQUIRE( synth.getVoiceView(1)->getPreviousSisterVoice() == synth.getVoiceView(2) );
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(2)) == 2 );
+    REQUIRE( synth.getVoiceView(2)->getNextSisterVoice() == synth.getVoiceView(1) );
+    REQUIRE( synth.getVoiceView(2)->getPreviousSisterVoice() == synth.getVoiceView(1) );
+    synth.noteOn(0, 63, 85);
+    REQUIRE( synth.getNumActiveVoices() == 6 );
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(3)) == 3 );
+    REQUIRE( synth.getVoiceView(3)->getNextSisterVoice() == synth.getVoiceView(4) );
+    REQUIRE( synth.getVoiceView(3)->getPreviousSisterVoice() == synth.getVoiceView(5) );
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(4)) == 3 );
+    REQUIRE( synth.getVoiceView(4)->getNextSisterVoice() == synth.getVoiceView(5) );
+    REQUIRE( synth.getVoiceView(4)->getPreviousSisterVoice() == synth.getVoiceView(3) );
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(5)) == 3 );
+    REQUIRE( synth.getVoiceView(5)->getNextSisterVoice() == synth.getVoiceView(3) );
+    REQUIRE( synth.getVoiceView(5)->getPreviousSisterVoice() == synth.getVoiceView(4) );
+}
+
+TEST_CASE("[Synth] Apply function on sisters")
+{
+    sfz::Synth synth;
+    sfz::AudioBuffer<float> buffer { 2, 256 };
+    synth.loadSfzString(fs::current_path(), R"(
+        <region> key=63 sample=*saw
+        <region> key=63 sample=*saw
+        <region> key=63 sample=*saw
+    )");
+    synth.noteOn(0, 63, 85);
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(0)) == 3 );
+    float start = 1.0f;
+    sfz::SisterVoiceRing::applyToRing(synth.getVoiceView(0), [&](const sfz::Voice* v) {
+        start += static_cast<float>(v->getTriggerNumber());
+    });
+    REQUIRE( start == 1.0f + 3.0f * 63.0f );
+}
+
+TEST_CASE("[Synth] Sisters and off-by")
+{
+    sfz::Synth synth;
+    sfz::AudioBuffer<float> buffer { 2, 256 };
+    synth.loadSfzString(fs::current_path(), R"(
+        <region> key=62 sample=*sine
+        <group> group=1 off_by=2 <region> key=62 sample=*sine
+        <group> group=2 <region> key=63 sample=*saw
+    )");
+    synth.noteOn(0, 62, 85);
+    REQUIRE( synth.getNumActiveVoices() == 2 );
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(0)) == 2 );
+    synth.renderBlock(buffer);
+    REQUIRE( synth.getNumActiveVoices() == 2 );
+    synth.noteOn(0, 63, 85);
+    REQUIRE( synth.getNumActiveVoices() == 3 );
+    synth.renderBlock(buffer);
+    REQUIRE( synth.getNumActiveVoices() == 2 );
+    REQUIRE( sfz::SisterVoiceRing::countSisterVoices(synth.getVoiceView(0)) == 1 );
+}