EnvGen releases incorrectly for very very short envelope segments

[jamshark70]

I seem to have found a disturbing bug in EnvGen. It's worse when the envelope segments are very short, but it also occurs (mildly) for typical env-segment durations. The release segment of an Env.adsr actually goes UP before going down!!! This seems to happen always, which is really awful.

In a simple Env.adsr, we would expect the signal to rise during the attack phase, but thereafter, the slope should be <= 0 (always descending or flat). Instead, I get a very obvious spike when the gate closes and the envelope goes into the release phase.

b.free;
b = Buffer.alloc(s, 4410, 1);

a = {
    var trig = Impulse.ar(30);
    trig = Trig1.ar(trig, TRand.ar(1/120, 1/40, trig));
    RecordBuf.ar(EnvGen.ar(Env.adsr(0.01, 0.1, 0.5, 0.2), trig, timeScale: 1/30), b, loop: 0, doneAction: 2);
    Silent.ar(1)
}.play;

b.getToFloatArray(wait: -1, action: { |data| d = data });

x = List.new; d.doAdjacentPairs { |a, b, i| if(a < b) { x.add(i) } }; x

--> List[ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 1023, 1470, 1471, 1472, 1473, 1474, 1475, 1476, 1477, 1478, 1479, 1480, 1481, 1482, 1483, 2495, 2940, 2941, 2942, 2943, 2944, 2945, 2946, 2947, 2948, 2949, 2950, 2951, 2952, 2953, 3967 ]

If, in fact, slope were > 0 only during the attack segments, then we would only see 0..13, 1470..1483 etc. Instead, there's also 1470***, 2495 and 3967.

• *** Because of TRand, these will be different when you run it.

Look at the floats surrounding one of those indices -- d[1015 .. 1030]:

FloatArray[ 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.88936585187912, 0.87712335586548, 0.86504632234573, 0.85313242673874, 0.84137958288193, 0.8297855257988, 0.81834816932678 ]

Just before that point, it's holding flat at the sustain level. Then, instead of decreasing, it jumps up to almost twice its current level.

An alternate version, without using timeScale, also reproduces the issue.

a = {
    var trig = Impulse.ar(30);
    trig = Trig1.ar(trig, TRand.ar(1/120, 1/40, trig));
    RecordBuf.ar(EnvGen.ar(Env.adsr(0.01/30, 0.1/30, 0.5, 0.2/30), trig, timeScale: 1/30), b, loop: 0, doneAction: 2);
    Silent.ar(1)
}.play;

A longer envelope still reproduces the issue.

{ var trig = Impulse.ar(1);
    trig = Trig1.ar(trig, TRand.ar(3/12, 3/4, trig));
    [trig, EnvGen.ar(Env.adsr(0.01, 0.1, 0.5, 0.2), trig)]
}.plot(2);

[jamshark70]

A clean rebuild did not resolve the issue.

[jamshark70]

Converting the last example to all kr units does not reproduce the problem. So it's only EnvGen.ar, it seems.

[danstowell]

James,

I'm unable to reproduce this. The list I get when running your first
example is:

 List[ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 1470, 1471, 1472,

1473, 1474, 1475, 1476, 1477, 1478, 1479, 1480, 1481, 1482, 1483, 2940,
2941, 2942, 2943, 2944, 2945, 2946, 2947, 2948, 2949, 2950, 2951, 2952,
2953 ]

which corresponds to the three attack segments, all as should be.

(BTW please note that your message has a typo in it which confused me at
first: "1470" is not a glitch but "1023" from your list is.)

I'm on SC 3.6.6 (401b992) on Ubuntu 13.04. Please, please when reporting
things, say what version of SuperCollider you're running, at the very least.

Dan

2014/1/22 jamshark70 notifications@github.com

Converting the last example to all kr units does not reproduce the
problem. So it's only EnvGen.ar, it seems.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1023#issuecomment-32991674
.

http://www.mcld.co.uk

[jamshark70]

Currently I'm at or around:

commit 1c422a3
Author: Julian Rohrhuber jrhb@users.sourceforge.net
Date: Sun Jan 5 00:24:10 2014 +0100

help: add loop node example to EnvGen

For this problem, I would guess one of these is the issue, but EnvGen got pretty heavily reorganized and it's hard to see quickly which one of the many changes is responsible.

I can try checking out these individual commits one by one (only five) and, if they build successfully, find out which is the first one where it broke.

$ git log server/plugins/LFUGens.cpp
commit 800c443
Author: Tim Blechmann tim@klingt.org
Date: Sun Oct 20 14:53:38 2013 +0200

EnvGen: fix .kr method

Signed-off-by: Tim Blechmann <tim@klingt.org>

commit 42967f6
Author: Tim Blechmann tim@klingt.org
Date: Sun Oct 20 14:17:19 2013 +0200

Env: rename step2 to hold

'hold' shares the analogy of sample&hold and is therefore more expressive
than step2

Signed-off-by: Tim Blechmann <tim@klingt.org>

commit c51d383
Author: Tim Blechmann tim@klingt.org
Date: Sat Oct 12 16:21:22 2013 +0200

Env: provide new step2 shape, which steps to a value at the end of a shape

the semantics of Env.xyc for \step is very awkward when using, because \step
jumps to the end value of a segment. this means that Env.xzc([[0, 0, \step],
will have the value 1 between time 0 and 1.
using \step2 instead of \step, the value between 0 and 1 will be 0, and it w
at time 1

Signed-off-by: Tim Blechmann <tim@klingt.org>

commit af7c725
Author: Tim Blechmann tim@klingt.org
Date: Fri Oct 11 19:51:26 2013 +0200

plugins: EnvGen - unify envgen perform implemementation

commit dc0646d
Author: Tim Blechmann tim@klingt.org
Date: Fri Oct 11 19:27:05 2013 +0200

plugins: EnvGen - remove duplicated code

[jamshark70]

I get:

• BAD commit af7c725: plugins: EnvGen - unify envgen perform implemementation
• GOOD commit dc0646d: plugins: EnvGen - remove duplicated code

I can see that a bunch of stuff has been moved into:

template <bool CheckGateOnSustain, typename GateCheck>
static inline void EnvGen_perform

... but I don't see that the logic for shape_Curve has changed very much... except for the gateCheck() which is new in the block-calculation loop.

As a test, I started with af7c725 and commented out the two lines 2964 and 2965:

    case shape_Curve : {
        double a2 = unit->m_a2;
        double b1 = unit->m_b1;
        double grow = unit->m_grow;
        for (int i=0; i<nsmps; ++i) {

// COMMENT THESE TWO

            if (!gateCheck( i ) )
                break;

            ZXP(out) = level;
            b1 *= grow;
            level = a2 - b1;
        }
        unit->m_b1 = b1;
    } break;

And the problem went away -- the last test case that I posted no longer has the spikes, but... without the gate check, (IIUC) the gate may close in the middle of a calculation block, but this won't take effect until the next block boundary. So, just removing it is not the solution.

So, for EnvGen.ar, it's possible to exit from the block calculation early, and I'm guessing that this doesn't update the unit's state variables, (likely) causing the next block to be wrong. It's beyond my expertise to fix it, though.

[danstowell]

Beyond my expertise too I'm afraid. But from reading af7c725, here's a thought: the way that EnvGen_perform() has been unified is such that for audio-rate it does the same gate-check as before, while for control-rate the gate-check has been replaced by a simple "return false" (line 3010). This seems odd since it would always break out of the curve-generating code blocks for k-rate stuff. It would seem to me that if it were instead a "return true" then that might reinstate the old behaviour.

I emphasise "might" - this is just from reading the patch. I could easily be misreading. @timblechmann may be able to clarify.

[timblechmann]

can have a look, but it might take a few days. the audio-rate ugen has to distinguish between audio-rate and control-rate gate signals ... but having a reproducer, it should be easy to find

[telephon]

Here is a related issue: #1063 (To avoid confusion, I opened a new one instead of reopening this one)