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

Fix nodeproxy release shape #3776

Merged

Conversation

telephon
Copy link
Member

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:

  • test how node object replacement and freeing sounds.
  • this may change the behaviour of SynthDefs that are separately made
    and which do not use the fadeTime parameter to specify the release
    time
    .

telephon added 2 commits June 10, 2018 13:17
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 });
```
@telephon telephon requested a review from LFSaw June 10, 2018 11:36
@telephon telephon added this to the 3.10 milestone Jun 10, 2018
@telephon
Copy link
Member Author

telephon commented Jun 10, 2018

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 });
// but not when you "misuse" fadeTime
SynthDef(\testFadeOut, { Out.ar(\out.kr, EnvGen.kr(Env.asr,
\gate.kr(1), doneAction:2) * PinkNoise.ar * 0.3 * SinOsc.ar(\fadeTime * 200 + 100) ) }).add;

@telephon telephon added comp: class library SC class library bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. labels Jun 10, 2018
@telephon telephon requested review from muellmusik and removed request for LFSaw June 19, 2018 19:59
@nhthn nhthn removed this from the 3.10 milestone Jun 24, 2018
@telephon telephon added this to the 3.10 milestone Jul 6, 2018
@telephon telephon requested review from adcxyz and removed request for muellmusik July 6, 2018 08:30
@telephon
Copy link
Member Author

telephon commented Jul 6, 2018

looking for a reviewer …

Copy link
Contributor

@adcxyz adcxyz left a 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!

@telephon
Copy link
Member Author

telephon commented Jul 6, 2018

OK, I'll do this. Where would you add it?

@adcxyz
Copy link
Contributor

adcxyz commented Jul 6, 2018

Ndef help file, maybe a subsection:: reserved parameter names
and same in NodeProxy help file.

@@ -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]::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which three parameters?

Copy link
Contributor

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks totally ok, I just missed that github code is not wrapping:

screen shot 2018-07-07 at 17 02 59

@adcxyz adcxyz merged commit d438757 into supercollider:develop Jul 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: class library SC class library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NodeProxy crossfade is linear
5 participants