Skip to content

Call unit constructors in Graph_Ctor #700

Closed
wants to merge 2 commits into from

4 participants

@kaoskorobase

Before, unit constructors were called the first time the graph process
function was run, leading to subtle bugs when /n_set commands could
set controls of not fully initialized synths (see [1]).

[1] https://www.listarc.bham.ac.uk/lists/sc-dev/msg46691.html

@kaoskorobase kaoskorobase Call unit constructors in Graph_Ctor
Before, unit constructors were called the first time the graph process
function was run, leading to subtle bugs when /n_set commands could
set controls of not fully initialized synths (see [1]).

[1] https://www.listarc.bham.ac.uk/lists/sc-dev/msg46691.html
a0126d6
@kaoskorobase

Here's some code that triggers the bug:

SynthDef(\testfest, { | gate = 1 |
    SinOsc.ar * EnvGen.kr(Env.asr(0.1, 1, 0.1), gate: gate, doneAction: 2)
}).send(s);

(
OSCFunc({ |msg| msg[1].postln }, '/n_end');

fork {
    50.do { |i|
        var n = 1000 + i,
            x = s.sendBundle(0.2, ["/s_new", "testfest", n, 1, 1]);
        (32/44100).wait;
        s.sendBundle(0.2, ["/n_set", n, "gate", 0]);
    };
    1.wait;
    s.sendMsg("/g_dumpTree", 1);
};
)

// Toggling gate to 1 and back to 0 will free the remaining synths
s.sendBundle(0.2, *([["/error", -1]] ++ 50.collect { |i| ["/n_set", 1000+i, "gate", 1] }));
s.sendBundle(0.2, *([["/error", -1]] ++ 50.collect { |i| ["/n_set", 1000+i, "gate", 0] }));
@timblechmann

e292a93 implements the same for supernova

@timblechmann

actually there is a subtle issue: in some use cases the synth constructors have to be called in the order of execution:

  • construct synth S1 and synth S2 at the same time
  • S1 writes to bus B1, S2 maps B1 as control value

in this case, S1 has to be constructed before S2, as the computation of the first sample of S2 depends on the sample that S1 generates at the time of construction.
can have quite nasty effects if the value of B1 is used as upper bound in a LinExp (-> nan)

not sure if this should be considered as a "user bug" ...

@kaoskorobase

Good call. AFAICS there are two possibilities:

  • Initialize synths in order of creation (which might be different from order of execution) and make it the user's responsibility to specify the correct order
  • Initialize synths lazily as before my patch and delay the effects of n_set commands until just after the synth has been fully initialized (i.e. after unit constructors have been called). Here's one way it could work:
    • Graph gets a flag mInitialized that is false initially
    • Graph_Ctor creates an additional array of controls mInitControls
    • Any commands modifying synth controls operate on mInitControls as long as mInitialized is false
    • In Graph_FirstCalc the unit constructors are called, mInitControls is copied over to mControls and mInitialized is set to true
    • The same treatment would probably be needed for mMapControls in order to be consistent.

Thoughts?

@timblechmann

lazy initialization and delaying the effect of controls is probably the most robust solution. i'll probably implement this approach for supernova

@muellmusik

Has this developed any further? Should it be closed?

@telephon telephon pushed a commit that referenced this pull request Mar 20, 2014
Julian Rohrhuber class library: protect EnvGate when released too early
This is a temporary precaustion, see discussion about #700.
4faaf12
@timblechmann timblechmann referenced this pull request Apr 20, 2014
Julian Rohrhuber class library: protect EnvGen from being unable to be freed when gate…
… 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.
ed32472
@crucialfelix
SuperCollider member

Closing due it being stale and it wouldn't merge now. Submit a new PR if you can. thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.