Corrected #63

- The envelope does not sustain if the region has `trigger=release`
- Change the envelope setup to pass the region rather than
individual envelope parameters

src/sfizz/ADSREnvelope.cpp

@@ -12,25 +12,25 @@
 namespace sfz {
 
 template <class Type>
-void ADSREnvelope<Type>::reset(int attack, int release, Type sustain, int delay, int decay, int hold, Type start, Type depth) noexcept
+void ADSREnvelope<Type>::reset(const Region& region, const MidiState& state, int delay, uint8_t velocity, float sampleRate) noexcept
 {
-    ASSERT(start <= 1.0f);
+    auto secondsToSamples = [sampleRate](auto timeInSeconds) {
-    ASSERT(sustain <= 1.0f);
+        return static_cast<int>(timeInSeconds * sampleRate);
+    };
 
-    sustain = clamp<Type>(sustain, 0.0, 1.0);
+    const auto ccArray = state.getCCArray();
-    start = clamp<Type>(start, 0.0, 1.0);
+    this->delay = delay + secondsToSamples(region.amplitudeEG.getDelay(ccArray, velocity));
+    this->attack = secondsToSamples(region.amplitudeEG.getAttack(ccArray, velocity));
+    this->decay = secondsToSamples(region.amplitudeEG.getDecay(ccArray, velocity));
+    this->release = secondsToSamples(region.amplitudeEG.getRelease(ccArray, velocity));
+    this->hold = secondsToSamples(region.amplitudeEG.getHold(ccArray, velocity));
+    this->peak = 1.0;
+    this->sustain =  normalizePercents(region.amplitudeEG.getSustain(ccArray, velocity));
+    this->start = this->peak * normalizePercents(region.amplitudeEG.getStart(ccArray, velocity));
 
-    currentState = State::Done;
-    this->delay = delay;
-    this->attack = attack;
-    this->decay = decay;
-    this->release = release;
-    this->hold = hold;
-    this->start = depth * start;
-    this->sustain = depth * sustain;
-    this->peak = depth;
     releaseDelay = 0;
     shouldRelease = false;
+    freeRunning = ((region.trigger == SfzTrigger::release) || (region.trigger == SfzTrigger::release_key));
     step = 0.0;
     currentValue = this->start;
     currentState = State::Delay;
@@ -53,7 +53,7 @@ Type ADSREnvelope<Type>::getNextValue() noexcept
             return start;
 
         currentState = State::Attack;
-        step = (static_cast<Type>(1.0) - currentValue) / (attack > 0 ? attack : 1);
+        step = (peak - currentValue) / (attack > 0 ? attack : 1);
         [[fallthrough]];
     case State::Attack:
         if (attack-- > 0) {
@@ -62,7 +62,7 @@ Type ADSREnvelope<Type>::getNextValue() noexcept
         }
 
         currentState = State::Hold;
-        currentValue = 1.0;
+        currentValue = peak;
         [[fallthrough]];
     case State::Hold:
         if (hold-- > 0)
@@ -81,6 +81,8 @@ Type ADSREnvelope<Type>::getNextValue() noexcept
         currentValue = sustain;
         [[fallthrough]];
     case State::Sustain:
+        if (freeRunning)
+            shouldRelease = true;
         return currentValue;
     case State::Release:
         if (release-- > 0) {
@@ -152,6 +154,8 @@ void ADSREnvelope<Type>::getBlock(absl::Span<Type> output) noexcept
         currentState = State::Sustain;
         [[fallthrough]];
     case State::Sustain:
+        if (freeRunning)
+            shouldRelease = true;
         break;
     case State::Release:
         length = min(remainingSamples, release);

src/sfizz/ADSREnvelope.h

@@ -6,6 +6,8 @@
 
 #pragma once
 #include "LeakDetector.h"
+#include "Region.h"
+#include "MidiState.h"
 #include <absl/types/span.h>
 namespace sfz {
 /**
@@ -19,19 +21,15 @@ class ADSREnvelope {
 public:
     ADSREnvelope() = default;
     /**
-     * @brief Resets the ADSR envelope. There's alot of parameter but what can you do.
+     * @brief Resets the ADSR envelope given a Region, the current midi state, and a delay and
-     * They all match the SFZ specification.
+     * trigger velocity
      *
-     * @param attack
+     * @param region
-     * @param release
+     * @param state
-     * @param sustain
      * @param delay
-     * @param decay
+     * @param velocity
-     * @param hold
-     * @param start
-     * @param depth
      */
-    void reset(int attack, int release, Type sustain = 1.0, int delay = 0, int decay = 0, int hold = 0, Type start = 0.0, Type depth = 1) noexcept;
+    void reset(const Region& region, const MidiState& state, int delay, uint8_t velocity, float sampleRate) noexcept;
     /**
      * @brief Get the next value for the envelope
      *
@@ -91,6 +89,7 @@ private:
     Type sustain { 0 };
     int releaseDelay { 0 };
     bool shouldRelease { false };
+    bool freeRunning { false };
     LEAK_DETECTOR(ADSREnvelope);
 };
 

src/sfizz/Voice.cpp

@@ -109,23 +109,7 @@ void sfz::Voice::startVoice(Region* region, int delay, int number, uint8_t value
     initialDelay = delay + static_cast<uint32_t>(region->getDelay() * sampleRate);
     baseFrequency = midiNoteFrequency(number);
     bendStepFactor = centsFactor(region->bendStep);
-    prepareEGEnvelope(initialDelay, value);
+    egEnvelope.reset(*region, resources.midiState, delay, value, sampleRate);
-}
-
-void sfz::Voice::prepareEGEnvelope(int delay, uint8_t velocity) noexcept
-{
-    auto secondsToSamples = [this](auto timeInSeconds) {
-        return static_cast<int>(timeInSeconds * sampleRate);
-    };
-    const auto& ccArray = resources.midiState.getCCArray();
-    egEnvelope.reset(
-        secondsToSamples(region->amplitudeEG.getAttack(ccArray, velocity)),
-        secondsToSamples(region->amplitudeEG.getRelease(ccArray, velocity)),
-        normalizePercents(region->amplitudeEG.getSustain(ccArray, velocity)),
-        delay + secondsToSamples(region->amplitudeEG.getDelay(ccArray, velocity)),
-        secondsToSamples(region->amplitudeEG.getDecay(ccArray, velocity)),
-        secondsToSamples(region->amplitudeEG.getHold(ccArray, velocity)),
-        normalizePercents(region->amplitudeEG.getStart(ccArray, velocity)));
 }
 
 bool sfz::Voice::isFree() const noexcept

src/sfizz/Voice.h

@@ -224,13 +224,6 @@ private:
      * @param buffer
      */
     void fillWithGenerator(AudioSpan<float> buffer) noexcept;
-    /**
-     * @brief Computes the values for the envelope depending on the note or CC number and the velocity/cc value
-     *
-     * @param delay
-     * @param velocity
-     */
-    void prepareEGEnvelope(int delay, uint8_t velocity) noexcept;
     /**
      * @brief The function processing a mono sample source
      *

tests/ADSREnvelopeT.cpp

@@ -45,14 +45,18 @@ TEST_CASE("[ADSREnvelope] Basic state")
 TEST_CASE("[ADSREnvelope] Attack")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 0);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+
+    envelope.reset(region, state, 0, 0, 100.0f);
     std::array<float, 5> output;
     std::array<float, 5> expected { 0.5f, 1.0f, 1.0f, 1.0f, 1.0f };
     for (auto& out : output)
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
 
-    envelope.reset(2, 0);
+    envelope.reset(region, state, 0, 0, 100.0f);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
     REQUIRE(approxEqual<float>(output, expected));
@@ -61,14 +65,18 @@ TEST_CASE("[ADSREnvelope] Attack")
 TEST_CASE("[ADSREnvelope] Attack again")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(3, 0);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.03f;
+
+    envelope.reset(region, state, 0, 0, 100.0f);
     std::array<float, 5> output;
     std::array<float, 5> expected { 0.33333f, 0.66667f, 1.0f, 1.0f, 1.0f };
     for (auto& out : output)
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
 
-    envelope.reset(3, 0);
+    envelope.reset(region, state, 0, 0, 100.0f);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
     REQUIRE(approxEqual<float>(output, expected));
@@ -77,7 +85,12 @@ TEST_CASE("[ADSREnvelope] Attack again")
 TEST_CASE("[ADSREnvelope] Release")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 4);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+    region.amplitudeEG.release = 0.04f;
+
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(2);
     std::array<float, 8> output;
     std::array<float, 8> expected { 0.5f, 1.0f, 0.08409f, 0.00707f, 0.000594604f, 0.00005f, 0.0f, 0.0f };
@@ -85,7 +98,7 @@ TEST_CASE("[ADSREnvelope] Release")
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
 
-    envelope.reset(2, 4);
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(2);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
@@ -95,15 +108,20 @@ TEST_CASE("[ADSREnvelope] Release")
 TEST_CASE("[ADSREnvelope] Delay")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 4, 1.0f, 2);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+    region.amplitudeEG.release = 0.04f;
+    region.amplitudeEG.delay = 0.02f;
     std::array<float, 10> output;
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(4);
     std::array<float, 10> expected { 0.0f, 0.0f, 0.5f, 1.0f, 0.08409f, 0.00707f, 0.000594604f, 0.00005f, 0.0f, 0.0f };
     for (auto& out : output)
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
 
-    envelope.reset(2, 4, 1.0f, 2);
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(4);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
@@ -113,14 +131,20 @@ TEST_CASE("[ADSREnvelope] Delay")
 TEST_CASE("[ADSREnvelope] Lower sustain")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 4, 0.5f, 2);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+    region.amplitudeEG.release = 0.04f;
+    region.amplitudeEG.delay = 0.02f;
+    region.amplitudeEG.sustain = 50.0f;
     std::array<float, 10> output;
+    envelope.reset(region, state, 0, 0, 100.0f);
     std::array<float, 10> expected { 0.0f, 0.0f, 0.5f, 1.0f, 0.5f, 0.5f, 0.5f, 0.5f, 0.5f, 0.5f };
     for (auto& out : output)
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
 
-    envelope.reset(2, 4, 0.5, 2);
+    envelope.reset(region, state, 0, 0, 100.0f);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
     REQUIRE(approxEqual<float>(output, expected));
@@ -129,14 +153,21 @@ TEST_CASE("[ADSREnvelope] Lower sustain")
 TEST_CASE("[ADSREnvelope] Decay")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 4, 0.5f, 2, 2);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+    region.amplitudeEG.release = 0.04f;
+    region.amplitudeEG.delay = 0.02f;
+    region.amplitudeEG.sustain = 50.0f;
+    region.amplitudeEG.decay = 0.02f;
     std::array<float, 10> output;
+    envelope.reset(region, state, 0, 0, 100.0f);
     std::array<float, 10> expected { 0.0f, 0.0f, 0.5f, 1.0f, 0.707107f, 0.5f, 0.5f, 0.5f, 0.5f, 0.5 };
     for (auto& out : output)
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
 
-    envelope.reset(2, 4, 0.5f, 2, 2);
+    envelope.reset(region, state, 0, 0, 100.0f);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
     REQUIRE(approxEqual<float>(output, expected));
@@ -145,14 +176,22 @@ TEST_CASE("[ADSREnvelope] Decay")
 TEST_CASE("[ADSREnvelope] Hold")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 4, 0.5f, 2, 2, 2);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+    region.amplitudeEG.release = 0.04f;
+    region.amplitudeEG.delay = 0.02f;
+    region.amplitudeEG.sustain = 50.0f;
+    region.amplitudeEG.decay = 0.02f;
+    region.amplitudeEG.hold = 0.02f;
     std::array<float, 12> output;
+    envelope.reset(region, state, 0, 0, 100.0f);
     std::array<float, 12> expected { 0.0f, 0.0f, 0.5f, 1.0f, 1.0f, 1.0f, 0.707107f, 0.5f, 0.5f, 0.5f, 0.5f, 0.5f };
     for (auto& out : output)
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
 
-    envelope.reset(2, 4, 0.5f, 2, 2, 2);
+    envelope.reset(region, state, 0, 0, 100.0f);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
     REQUIRE(approxEqual<float>(output, expected));
@@ -161,7 +200,15 @@ TEST_CASE("[ADSREnvelope] Hold")
 TEST_CASE("[ADSREnvelope] Hold with release")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 4, 0.5f, 2, 2, 2);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+    region.amplitudeEG.release = 0.04f;
+    region.amplitudeEG.delay = 0.02f;
+    region.amplitudeEG.sustain = 50.0f;
+    region.amplitudeEG.decay = 0.02f;
+    region.amplitudeEG.hold = 0.02f;
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(8);
     std::array<float, 14> output;
     std::array<float, 14> expected { 0.0f, 0.0f, 0.5f, 1.0f, 1.0f, 1.0f, 0.707107f, 0.5f, 0.05f, 0.005f, 0.0005f, 0.00005f, 0.0f, 0.0f };
@@ -169,7 +216,7 @@ TEST_CASE("[ADSREnvelope] Hold with release")
         out = envelope.getNextValue();
 
     REQUIRE(approxEqual<float>(output, expected));
-    envelope.reset(2, 4, 0.5f, 2, 2, 2);
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(8);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));
@@ -179,14 +226,22 @@ TEST_CASE("[ADSREnvelope] Hold with release")
 TEST_CASE("[ADSREnvelope] Hold with release 2")
 {
     sfz::ADSREnvelope<float> envelope;
-    envelope.reset(2, 4, 0.5f, 2, 2, 2);
+    sfz::MidiState state;
+    sfz::Region region { state };
+    region.amplitudeEG.attack = 0.02f;
+    region.amplitudeEG.release = 0.04f;
+    region.amplitudeEG.delay = 0.02f;
+    region.amplitudeEG.sustain = 50.0f;
+    region.amplitudeEG.decay = 0.02f;
+    region.amplitudeEG.hold = 0.02f;
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(4);
     std::array<float, 14> output;
     std::array<float, 14> expected { 0.0f, 0.0f, 0.5f, 1.0f, 0.08409f, 0.00707f, 0.000594604f, 0.00005f, 0.0f, 0.0f, 0.0f, 0.0 };
     for (auto& out : output)
         out = envelope.getNextValue();
     REQUIRE(approxEqual<float>(output, expected));
-    envelope.reset(2, 4, 0.5f, 2, 2, 2);
+    envelope.reset(region, state, 0, 0, 100.0f);
     envelope.startRelease(4);
     absl::c_fill(output, -1.0f);
     envelope.getBlock(absl::MakeSpan(output));

tests/SynthT.cpp

@@ -162,3 +162,18 @@ TEST_CASE("[Synth] Releasing after the initial and normal mode does not trigger
     synth.renderBlock(buffer);
     REQUIRE( !synth.getVoiceView(0)->isFree() );
 }
+
+TEST_CASE("[Synth] Trigger=release and an envelope properly kills the voice at the end of the envelope")
+{
+    sfz::Synth synth;
+    synth.setSamplesPerBlock(1024);
+    sfz::AudioBuffer<float> buffer(2, 1024);
+    synth.setNumVoices(1);
+    synth.loadSfzFile(fs::current_path() / "tests/TestFiles/envelope_trigger_release.sfz");
+    synth.noteOn(10, 60, 63);
+    synth.noteOff(10, 60, 63);
+    REQUIRE( !synth.getVoiceView(0)->isFree() );
+    for (int i = 0; i < 10; ++i)
+        synth.renderBlock(buffer);
+    REQUIRE( synth.getVoiceView(0)->isFree() );
+}

tests/TestFiles/envelope_trigger_release.sfz

@@ -0,0 +1,13 @@
+
+<group>
+lovel=0
+hivel=127
+
+<region>
+trigger=release
+sample=*noise
+loop_mode=one_shot
+ampeg_attack=0.02
+ampeg_decay=0.02
+ampeg_release=0
+ampeg_sustain=0