Remove unused phase in ADSR

[apiel]

Here is a small improvement on ADSR envelope to remove noize from unused phase. Following is an example to understand the improvement:

#include <MozziGuts.h>
#include <ADSR.h>
#include <Oscil.h>
#include <tables/triangle_dist_squared_2048_int8.h>

// this should be after MozziGuts
#include <EventDelay.h>

byte gSeqBPM = 80;
unsigned int gTempo = 1000 / ((gSeqBPM * 4) / 60);
byte delayBetweenNote = 3 * gTempo;

byte envelopeValue2;

// oscillators
Oscil<TRIANGLE_DIST_SQUARED_2048_NUM_CELLS, AUDIO_RATE> aOscil(
    TRIANGLE_DIST_SQUARED_2048_DATA);

// envelopes
ADSR<CONTROL_RATE, AUDIO_RATE> envelope1;
ADSR<CONTROL_RATE, AUDIO_RATE> envelope2;

int frequency = 45;

EventDelay noteDelay;
void updateControl() {
    if (noteDelay.ready()) {
        aOscil.setFreq(frequency);
        envelope1.noteOn();
        envelope2.noteOn();
        noteDelay.start(gTempo + delayBetweenNote);
    }
    envelope1.update();
    envelope2.update();
}

int updateAudio() {
    int freq = frequency + (envelope2.next() >> 1);
    aOscil.setFreq(freq);
    return (int)((envelope1.next() * aOscil.next()) >> 1) >> 8;
}

void loop() { audioHook(); }

byte PeakLevel = 200;
byte SubstainLevel = 150;
void setup() {
    envelope1.setLevels(PeakLevel, SubstainLevel, SubstainLevel, 0);
    envelope1.setTimes(gTempo * 0 / 100, gTempo * 0 / 100, gTempo * 0 / 100,
                       gTempo * 100 / 100);

    envelope2.setLevels(200, 200, 200, 0);
    envelope2.setTimes(gTempo * 30 / 100, gTempo * 0 / 100, gTempo * 40 / 100,
                       gTempo * 30 / 100);

    startMozzi();
}

On each beat, without this fix, we hear a little noise due to some delay on the phases. If we apply the fix, the noize is removed.

[tfry-git]

Nice find.

I'm a bit put off by how many checks (meaning additional bytes) are needed to fix this, though. Looking at ADSR.h, I'm thinking, the whole thing could be restructured as an array of phases. Transitioning to the "next" phase does not really need a separate case for each phase. Would you feel like giving that a try?

At any rate, a minimal nitpick: Use update_steps, not lerp_steps for comparison. If one is zero, the other is, too, but lerp_steps is a long, requiring more cycles to check.

[apiel]

Actually, why not to base the ADSR envelope on the MultiLine?

[tfry-git]

Indeed. MutliLine2.h looks much like what I had in mind (I believe a few details could be simplified, further). Not sure, if it would be easily possible to actually base ADSR on that (i.e. using inheritance, rather than copying the code), due to that third template parameter. The solution should definitely remain source compatible. Would you like to take a closer look into that?

[sensorium]

Just a warning here that MultiLine and MultiLine2 are just drafts for my own projects which probably shouldn't be in the public release of Mozzi! ADSR came first and they would have been based on that. They probably need work, and there's probably no need for both. MultiLine2 is probably the later and possibly improved version.... I haven't tried to use them for a long while so beware... Anyway, pleased if something useful comes of my untidy housekeeping...

…

On 24 Jan 2021, at 7:44 am, Thomas Friedrichsmeier ***@***.***> wrote: Indeed. MutliLine2.h looks much like what I had in mind (I believe a few details could be simplified, further). Not sure, if it would be easily possible to actually base ADSR on that (i.e. using inheritance, rather than copying the code), due to that third template parameter. The solution should definitely remain source compatible. Would you like to take a closer look into that? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#108 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANW2NYL5UTGHNIIA2RCP43S3MYKNANCNFSM4WKYYJOQ>.

[sensorium]

Actually, it might be worth looking into basing ADSR on MultiLine2 (which should probably replace MultiLine, if it works OK)... but life is short...

…

On 24 Jan 2021, at 12:30 pm, Tim Barrass ***@***.***> wrote: Just a warning here that MultiLine and MultiLine2 are just drafts for my own projects which probably shouldn't be in the public release of Mozzi! ADSR came first and they would have been based on that. They probably need work, and there's probably no need for both. MultiLine2 is probably the later and possibly improved version.... I haven't tried to use them for a long while so beware... Anyway, pleased if something useful comes of my untidy housekeeping... > On 24 Jan 2021, at 7:44 am, Thomas Friedrichsmeier ***@***.*** ***@***.***>> wrote: > > > Indeed. MutliLine2.h looks much like what I had in mind (I believe a few details could be simplified, further). Not sure, if it would be easily possible to actually base ADSR on that (i.e. using inheritance, rather than copying the code), due to that third template parameter. The solution should definitely remain source compatible. Would you like to take a closer look into that? > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub <#108 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANW2NYL5UTGHNIIA2RCP43S3MYKNANCNFSM4WKYYJOQ>. >

[apiel]

I don't have so much time right now, but one easy solution would be to put phase* next_phase inside the phase and set it, when we set the time:

    struct phase {
        byte phase_type;
        T update_steps;
        long lerp_steps;  // signed, to match params to transition (line) type
                          // Q15n16, below
        Q8n0 level;
        phase* next_phase;
    } attack, decay, sustain, release, idle;

Let see, maybe on day I have some free time to do this approach.

[apiel]

I actually wonder if ADSR class is necessary. There could a generic Envelope class supporting multiple phases, see here https://github.com/apiel/sequencer/blob/main/sequencer_13_mini/Envelope.h

This envelop is very similar to multiline but it remove all the phases with 0 steps. Of course there could be a wrapper around to make an ADSR...