Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EnvGen releases incorrectly for early gate #1063

Closed
telephon opened this issue Mar 21, 2014 · 14 comments
Closed

EnvGen releases incorrectly for early gate #1063

telephon opened this issue Mar 21, 2014 · 14 comments

Comments

@telephon
Copy link
Member

@telephon telephon commented Mar 21, 2014

This bug might be related to #1023, and also to the discussion here: http://new-supercollider-mailing-lists-forums-use-these.2681727.n2.nabble.com/EnvGen-bug-td7599058.html

Here is a reproducer, creating three hanging synths:

s.plotTree;

SynthDef(\x, { |gate = 1| EnvGen.kr(Env.asr(0, 1, 0), gate, doneAction: 2) }).add;

// three synths hang:
(
3.do {
    var id = s.nextNodeID;
    s.sendMsg(\s_new, \x, id, 1, 1);
    s.sendMsg(\n_set, id, \gate, 0);
};
)

// not if the time difference is long enough:
(
fork {
    3.do {
        var id = s.nextNodeID;
        s.sendMsg(\s_new, \x, id, 1, 1);
        0.01.wait;
        s.sendMsg(\n_set, id, \gate, 0);
    }
};
)

Replacing the SynthDef by the following is a remedy, but not a really beautiful one:

SynthDef(\x, { |gate = 1| EnvGen.kr(Env.asr(0, 1, 0), gate + Impulse.kr(0), doneAction: 2) }).add;

@LucaDanieli
Copy link
Contributor

@LucaDanieli LucaDanieli commented Apr 16, 2014

Actually, what happens in the first example is that Supercollider initialize the synth with gate=0 and not 1, probably because the message arrives before the synth starts.

It behaves like this:

s.plotTree;

SynthDef(\x, { |gate = 0| Out.ar(0,SinOsc.ar()*EnvGen.kr(Env.asr(0, 1, 0), gate, doneAction: 2)) }).add;

// three synths:
(
fork {
    var id = s.nextNodeID;
    s.sendMsg(\s_new, \x, id, 1, 1);
    3.wait;
    id = s.nextNodeID;
    s.sendMsg(\s_new, \x, id, 1, 1);;
    3.wait;
    id = s.nextNodeID;
    s.sendMsg(\s_new, \x, id, 1, 1);    
};
)

Supercollider creates synths that cannot be freed, because never received the message "gate:1->0", so they are waiting for any gate message. This is why Impulse.kr makes it work fine.

Probably we should say to the program to accept n_set only when the Synth has started, but I don't know if it is a good idea.

@telephon
Copy link
Member Author

@telephon telephon commented Apr 18, 2014

If I remember correctly, the problem persists when gate = 1 already in the definition:

SynthDef(\x, { |gate = 1| Out.ar(0,SinOsc.ar()*EnvGen.kr(Env.asr(0, 1, 0), gate, doneAction: 2)) }).add;

So it is not about n_setmessages (above, you also only have s_newmessages).

@jamshark70
Copy link
Contributor

@jamshark70 jamshark70 commented Apr 18, 2014

As I have certainly pointed out before: there is no solution to this problem without a way to distinguish between these two cases:

  • I am starting this synth with gate = 0, and I intend to open the gate later.
  • I am starting this synth with gate = 1, and I want to close it (gate = 0) in the same control cycle.

The issue keeps recurring, on the mailing list and in the issue tracker, in slightly different guises but I don't remember any of these cases where somebody proposed a concrete solution to this problem.

So my opinion is, provisionally, that people like to complain about this behavior but not actually resolve the ambiguity... and that leaves the complaints in the territory of idle griping but not much more.

IMO (and this is only my opinion), if you want the gate to be open at the start of the synth, it's your responsibility as a user to make sure this is the case. If there is a solution that allows the latter of the above two options to be the default -- without precluding the former -- I'm listening, but so far, I see only grousing about the current behavior but no viable proposal that doesn't kill the option of creating a synth whose gate you can open later.

@telephon
Copy link
Member Author

@telephon telephon commented Apr 18, 2014

Wouldn't the general solution look like the following:

EnvGen(env, gate + Impulse.kr(0, 0, 1-gate))

?

@LucaDanieli
Copy link
Contributor

@LucaDanieli LucaDanieli commented Apr 18, 2014

Yes, I avoided on purpose n_set message, just to point that

...
SynthDef(\x, {|gate = 1| ...})
s.sendMsg(\s_new, \x, id, 1, 1);
s.sendMsg(\n_set, id, \gate, 0);
...

is interpreted like

...
SynthDef(\x, {|gate = 0| ...})
s.sendMsg(\s_new, \x, id, 1, 1);
...

This just because the n_set message arrives to the Synth before the Synth has started, so strangely it seems to me that (but this is just a theory that I cannot prove):

  1. SynthDef(\x, {|gate = 1| ...}) //no problems.
  2. s_new: creates a node "1000" which needs a little time to be started.
  3. n_set: sends a message to the same node (I assume that the creation of the tree is faster than the start of a synth) during that short time setting the Gate to 0.
  4. the Synth is finally started with |gate=0|
  5. Resulting Synth started: SynthDef(\x, {|gate = 0| ...})

I don't know if this is realistic, but it would explain the behaviour. And so the reason because the synths do not free in a so little time is that their initial gate was already 0 when created. For this reason the impulse works.

% If I remember correctly, the problem persists when gate = 1 already in the definition:

Yes, because in my opinion, a so rapid n_set doesn't care about the initial values of args.

@telephon
Copy link
Member Author

@telephon telephon commented Apr 18, 2014

Even if you send the messages in a bundle, the notes hang, so I don't think this is the reason:

SynthDef(\x, { |gate = 0| EnvGen.kr(Env.asr(0, 1, 0), gate, doneAction: 2) }).add;

// three synths hang:
(
3.do {
    s.bind {
        var id = s.nextNodeID;
        s.sendMsg(\s_new, \x, id, 1, 1, \gate, 1);
        s.sendMsg(\n_set, id, \gate, 0);
    };
};
)

Btw. also my above solution doesn't work, because the gate mul factor of the Impulse apparently is also set only later.
So

SynthDef(\x, { |gate = 0| EnvGen.kr(Env.asr(0, 1, 0), gate + Impulse.kr(0, 0, gate), doneAction: 2) }).add;

will have hanging notes, too.

@telephon
Copy link
Member Author

@telephon telephon commented Apr 18, 2014

As James pointed out, the difficulty is that the control is set first to 1 and then to zero before any calculation has even happened. So the UGen, when it is initialized, only sees the last value that has been set. It is like coming into a dark room and trying to know whether the light had been on a second ago and switched off, or whether it has always been off.

What we need in sclang is a way to clearly distinguish the two use cases: start at s_new – or wait until gate is set to 1.

@telephon
Copy link
Member Author

@telephon telephon commented Apr 18, 2014

One could say that: If you intend to make the envelope wait until the gate is explicitly set to 1, you should initialize the gate with 0. Otherwise, sclang will add an Impulse.kr(0) to it.

In code:
{ |gate = 0| EnvGen.kr(env, gate) } will remain unchanged.

{ |gate = 1| EnvGen.kr(env, gate) }
will be internally rewritten to:
{ |gate = 1| EnvGen.kr(env, gate + Impulse.kr(0)) }

We could leave it to the user, but the trick is very indirect and hard to understand (as this whole discussion shows).

@telephon
Copy link
Member Author

@telephon telephon commented Apr 19, 2014

OK, here the suggestion:

*new1 { arg rate, gate, levelScale, levelBias, timeScale, doneAction, envArray;
        // protect envelope from being locked when setting gate to 0 in the first cycle
        if(gate.isKindOf(OutputProxy)) {
            if(gate.source.isKindOf(Control)) {
                if(gate.source.values.at(gate.outputIndex) == 1.0) {
                    gate = gate + Impulse.perform(UGen.methodSelectorForRate(rate), 0)
                }
            }
        };
        ^super.new.rate_(rate).addToSynth.init([gate, levelScale, levelBias, timeScale, doneAction]
            ++ envArray.dereference);
    }
telephon pushed a commit that referenced this issue Apr 19, 2014
… is set to 0 too early. This fixes problems with hanging notes (fixes #1063). But this fix changes behaviour: a gate with default = 1 won't be able to be set to 0 at init time (which one would want to do in order to open the gate later on). A gate with default = 0 will work as expected, however, so this patch may be a good solution even if it is a little risky.
@telephon
Copy link
Member Author

@telephon telephon commented Apr 20, 2014

I've added this to a branch risky-fixes in the repo, so if necessary, this can be merged later.

@telephon
Copy link
Member Author

@telephon telephon commented Nov 15, 2014

I measured the delta time between init and release, and in the range [0..1] * block duration it can happen that the synth remains hanging (silently). This is because the gate control is set to 0 or < 0 before the synth is started.

I see the argument that you want to be able to start a synth with gate = 0 - but: you don't want to do this with < 0 (which indicates a release). So this points to a solution.

@telephon
Copy link
Member Author

@telephon telephon commented Nov 15, 2014



(
var numBlocksWait = 1; // releases
fork {
    s.sendBundle(0.2, ["/s_new", "default", 1000, 1, 1]);
    (s.options.blockSize / s.sampleRate * numBlocksWait).wait;
    s.sendBundle(0.2, ["/n_set", 1000, "gate", -1]);
}
);

(
var numBlocksWait = 0.5; // 0 ... 0.5: doesn't always release (silent, but hanging)
fork {
    s.sendBundle(0.2, ["/s_new", "default", 1000, 1, 1]);
    (s.options.blockSize / s.sampleRate * numBlocksWait).wait;
    s.sendBundle(0.2, ["/n_set", 1000, "gate", -1]);
}
);
@telephon
Copy link
Member Author

@telephon telephon commented Nov 15, 2014

Or better:


SynthDef(\envTest, { |gate = 1| Out.ar(0, SinOsc.ar(440) * EnvGen.kr(Env.asr(), gate, doneAction:2)) }).add

(
var numBlocksWait = 0.5; // 0 ... 0.5: doesn't release (silent, but hanging)
var dt = s.options.blockSize / s.sampleRate * numBlocksWait;
s.sendBundle(0.2, ["/s_new", "envTest", 1000, 1, 1]);
s.sendBundle(0.2 + dt, ["/n_set", 1000, "gate", -1]);
);
@telephon
Copy link
Member Author

@telephon telephon commented Nov 15, 2014

This problem is fixed by 1e34f62.

@telephon telephon closed this Feb 21, 2015
sofakid pushed a commit to sofakid/supercollider that referenced this issue Apr 6, 2015
… is set to 0 too early. This fixes problems with hanging notes (fixes supercollider#1063). But this fix changes behaviour: a gate with default = 1 won't be able to be set to 0 at init time (which one would want to do in order to open the gate later on). A gate with default = 0 will work as expected, however, so this patch may be a good solution even if it is a little risky.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants