Add comments and a helper function to test ring validity

src/sfizz/SisterVoiceRing.h

@@ -12,6 +12,14 @@ namespace sfz
 {
 
 struct SisterVoiceRing {
+    /**
+     * @brief Apply a lambda function to all sisters in a ring.
+     * This function should be robust enough to be able to kill the voice
+     * in the lambda.
+     *
+     * @param voice
+     * @param lambda
+     */
     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) noexcept
@@ -25,6 +33,12 @@ struct SisterVoiceRing {
         lambda(voice);
     }
 
+    /**
+     * @brief Count the number of sister voices in a ring
+     *
+     * @param start
+     * @return unsigned
+     */
     static unsigned countSisterVoices(const Voice* start) noexcept
     {
         if (!start)
@@ -41,6 +55,54 @@ struct SisterVoiceRing {
         ASSERT(count < config::maxVoices);
         return count;
     }
+
+    /**
+     * @brief Check if a sister voice ring is well formed
+     *
+     * @param start
+     * @return true
+     * @return false
+     */
+    static bool checkRingValidity(const Voice* start) noexcept
+    {
+        if (start == nullptr)
+            return true;
+
+        unsigned idx { 0 };
+        const Voice* ring[config::maxVoices];
+        ring[idx] = start;
+        while (idx < config::maxVoices) {
+            const auto* newVoice = ring[idx]->getNextSisterVoice();
+
+            if (newVoice == nullptr) {
+                DBG("Error in ring: " << static_cast<const void*>(ring[idx])
+                        << " next sister is null");
+                return false;
+            }
+
+            if (newVoice->getPreviousSisterVoice() != ring[idx]) {
+                DBG("Error in ring: " << static_cast<const void*>(newVoice)
+                        << " refers " << static_cast<const void*>(newVoice->getPreviousSisterVoice())
+                        << " as previous sister voice instead of "
+                        << static_cast<const void*>(ring[idx]));
+                return false;
+            }
+
+            if (newVoice == start)
+                break;
+
+            for (unsigned i = 1; i < idx; ++i) {
+                if (ring[i] == newVoice) {
+                    DBG("Error in ring: " << static_cast<const void*>(newVoice)
+                        << " already present in ring at index " << i);
+                    return false;
+                }
+            }
+            ring[++idx] = newVoice;
+        }
+
+        return true;
+    }
 };
 
 /**

src/sfizz/Synth.cpp

@@ -730,7 +730,7 @@ void sfz::Synth::renderBlock(AudioSpan<float> buffer) noexcept
             callbackBreakdown.panning += voice->getLastPanningDuration();
 
             if (voice->toBeCleanedUp())
-                    voice->reset();
+                voice->reset();
         }
     }
 

src/sfizz/VoiceStealing.cpp

@@ -13,16 +13,24 @@ sfz::Voice* sfz::VoiceStealing::steal(absl::Span<sfz::Voice*> voices) noexcept
     const auto sumEnvelope = absl::c_accumulate(voices, 0.0f, [](float sum, const Voice* v) {
         return sum + v->getAverageEnvelope();
     });
+    // We are checking the envelope to try and kill voices with relative low contribution
+    // to the output compared to the rest.
     const auto envThreshold = sumEnvelope
         / static_cast<float>(voices.size()) * config::stealingEnvelopeCoeff;
+    // We are checking the age so that voices have the time to build up attack
+    // This is not perfect because pad-type voices will take a long time to output
+    // their sound, but it's reasonable for sounds with a quick attack and longer
+    // release.
     const auto ageThreshold = voices.front()->getAge() * config::stealingAgeCoeff;
+    // This needs to be positive
+    ASSERT(ageThreshold >= 0);
 
     Voice* returnedVoice = voices.front();
     unsigned idx = 0;
     while (idx < voices.size()) {
         const auto ref = voices[idx];
 
-        if (ref->getAge() < ageThreshold) {
+        if (ref->getAge() <= ageThreshold) {
             // Went too far, we'll kill the oldest note.
             break;
         }