-
Notifications
You must be signed in to change notification settings - Fork 761
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
Fix nodeproxy release shape #3776
Fix nodeproxy release shape #3776
Conversation
Setting the gate of an envelope to a negative value makes a forced release. What remained unnoticed here was that the forced release shape is always linear, regardless of the envelope’s shape. This commit replaces the forced release by a normal release. The fadeTime is set, so forcing wasn’t necessary. This fixes supercollider#3775. testing: - test how node object replacement and freeing sounds. - this may change the behaviour of SynthDefs that are separately made and which do not define a `fadeTime` parameter to specify the release time. A subsequent commit will try to minimize such impact.
As mentioned in the previous commit, some externally created SynthDefs may not have a `fadeTime` argument. Their behaviour would change with the current PR. To minimize impact, we detect if there is a fadeTime argument in the SynthDef, and fall back to the old behaviour if we find none. Here are examples for the two cases: ``` SynthDef(\testFadeOut, { Out.ar(\out.kr, EnvGen.kr(Env.asr, \gate.kr(1), doneAction:2) * PinkNoise.ar * 0.3) }).add; Ndef(\x, \testFadeOut).play; Ndef(\x).objects.first.hasFadeTimeControl == false // true Ndef(\x).fadeTime = 4; Ndef(\x, { Silent.ar }); SynthDef(\testFadeOut, { Out.ar(\out.kr, EnvGate.new * PinkNoise.ar * 0.3) }).add; Ndef(\x, \testFadeOut).play; Ndef(\x).objects.first.hasFadeTimeControl == true // true Ndef(\x).fadeTime = 4; Ndef(\x, { Silent.ar }); ```
For reference, I'm adding two examples for special cases that are affected by this fix: // this will still release as expected
SynthDef(\testFadeOut, { Out.ar(\out.kr, EnvGen.kr(Env.asr,
\gate.kr(1), doneAction:2) * PinkNoise.ar * 0.3) }).add;
Ndef(\x, \testFadeOut).play;
Ndef(\x).objects.first.hasFadeTimeControl.not // true
Ndef(\x).fadeTime = 4;
Ndef(\x, { Silent.ar });
// also this will still release as expected
SynthDef(\testFadeOut, { Out.ar(\out.kr, EnvGate.new * PinkNoise.ar *
0.3) }).add;
Ndef(\x, \testFadeOut).play;
Ndef(\x).objects.first.hasFadeTimeControl // true
Ndef(\x).fadeTime = 4;
Ndef(\x, { Silent.ar });
|
looking for a reviewer … |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely improves behavior.
that it fails if people add a fadeTime control is to be expected - you could construct the same glitch with everything with any synthdef-wrapping function.
please add/update info about reserved control names (including \fadeTime) to documentation!
OK, I'll do this. Where would you add it? |
Ndef help file, maybe a subsection:: reserved parameter names |
@@ -189,6 +189,23 @@ Ndef(\maus, { LFNoise0.kr(1 ! 8) + 1 }); // now 8 parallel channels of audio are | |||
|
|||
|
|||
|
|||
subsection::Reserved parameters | |||
Three parameters are automatically specified if they don't exist in a given UGen function. You can override their use: code::[\out, \gate, \fadeTime]:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which three parameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\out, \gate, \fadeTime. this reads OK to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting the gate of an envelope to a negative value makes a forced release. What remained unnoticed here was that the forced release shape is always linear, regardless of the envelope’s shape. This commit replaces the forced release by a normal release. The fadeTime is set,
so forcing wasn’t necessary.
This fixes #3775.
It changes how crossfading sounds.
testing:
and which do not use the
fadeTime
parameter to specify the releasetime.