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.

Sign up for GitHub

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

Jump to bottom

Topic/use named controls in node proxy #5675

Merged
merged 4 commits into from
Jan 8, 2022
Merged

Topic/use named controls in node proxy #5675

merged 4 commits into from
Jan 8, 2022

Conversation

telephon
Copy link
Member

@telephon telephon commented Dec 31, 2021 •
edited
Loading

Purpose and Motivation

As @avdrd has noticed (thanks!), node proxies may collide with uses of NamedControl: #5648
This is a lightweight, partial fix. A complete fix will require touching SynthDef more deeply.

To-do list

  • Code is tested
  • All tests are passing
  • This PR is ready for review

@telephon
class library: use NamedControl in ProxySynthDef
3c75a39 
This makes the control visible for other named controls in the UGen function.
@telephon
class library: use named control in proxy wrappers
bb446cd 
This makes them consistent with named controls in the UGen functions.
@avdrd
Copy link
Contributor

avdrd commented Jan 1, 2022 •
edited
Loading

I guess you didn't see #5655 or I wrote a way TLDR summary for it... I guess I can close that now.

Please let me know if you plan to make your own PR for other stuff like avdrd@82862d2, so I don't waste my own time with that.

@telephon
Copy link
Member Author

telephon commented Jan 2, 2022

I guess you didn't see #5655 or I wrote a way TLDR summary for it... I guess I can close that now.

Please let me know if you plan to make your own PR for other stuff like avdrd@82862d2, so I don't waste my own time with that.

Ah sorry, I didn't see that you had made a separate one. I only saw your blobs you posted. So not a waste of time at all, because I more or less was able to copy your suggestions.

Please let me know if you plan to make your own PR for other stuff like avdrd/supercollider@82862d2, so I don't waste my own time with that.

No, I had no plans to do this because I have very little time right now. I'd be glad if you can make a PR for that separately, if you have time for it.

Thanks for your contributions, this is very valuable.

@telephon telephon requested a review from adcxyz January 2, 2022 09:39
@adcxyz
Copy link
Contributor

adcxyz commented Jan 2, 2022

Hi @telephon and @avdrd,
just tested your two pull requests as follows,
and found that the \filter role works in @avdrd PR 5655,
but not in 5675 - don't see yet why that is.

Otherwise, are there reasons speaking for/against implicit
vs. explicit creation, \ctl.kr versus NamedControl(\ctl) ?

s.boot;
// assume server has 2 ins, 2 outs, 
// so first ndef gets first free audio bus at index 4, 
s.scope(6); 
// this does not test yet, just the base comparison: 
// should show on the ndef's bus only 
n = Ndef(\noise, {  WhiteNoise.ar(0.2) });
n.index; // should be on bus 4

//// this tests double-using an existing controlname, \out : 
n = Ndef(\noise, { \out.ir /* has side effect */; WhiteNoise.ar(0.2) })

// with current develop branch, this wrongly plays on audio bus 0!
// with pull/5679, it also plays wrongly on bus 0! 
// with pull/5655, it plays on bus 4 as expected, 

// does it create a doubled out control? 
n.objects[0].synthDef.allControlNames collect: _.name
// on develop yes: 
// -> [ out, gate, fadeTime, out ] 
// on 5675 and 5655, no -> OK


// test with second control \mix5 
n = Ndef(\noise);
n.play;
n[5] = \mix -> { \mix5.kr.poll; Dust2.ar(10) };

// just one \mix5 
n.objects[5].synthDef.allControlNames collect: _.name;
n.set(\mix5, 0.25); // gets softer
// works with 5675 and 5655

// test filter role: 
n.filter(10, { |in| Ringz.ar(in, 567, 0.2) });
// baseline case works with 5655, fails with 5675!!
n.set(\wet10, 0.1); // gets softer

// test with doubled wet control 
// works with 5655, fails with 5675!!
n[10] = \filter -> { |in| \wet10.kr.poll; Ringz.ar(in, 567, 0.05) };

@telephon
Copy link
Member Author

telephon commented Jan 3, 2022

Interesting. This is exactly the same:

{ NamedControl.kr("t", 1).dump.source.dump }.asSynthDef
{ \t.kr(1).dump.source.dump }.asSynthDef
Instance of OutputProxy {    (0x7f89990b1158, gc=74, fmt=00, flg=00, set=04)
  instance variables [11]
    synthDef : instance of SynthDef (0x7f89604e3db8, size=16, set=4)
    inputs : nil
    rate : Symbol 'control'
    synthIndex : Integer 1
    specialIndex : Integer 0
    antecedents : nil
    descendants : nil
    widthFirstAntecedents : nil
    source : instance of Control (0x7f8960606fd8, size=10, set=4)
    outputIndex : Integer 0
    name : nil
}
Instance of Control {    (0x7f8960606fd8, gc=74, fmt=00, flg=00, set=04)
  instance variables [10]
    synthDef : instance of SynthDef (0x7f89604e3db8, size=16, set=4)
    inputs : nil
    rate : Symbol 'control'
    synthIndex : Integer 1
    specialIndex : Integer 1
    antecedents : nil
    descendants : nil
    widthFirstAntecedents : nil
    channels : instance of Array (0x7f89603fce98, size=1, set=2)
    values : instance of Array (0x7f89910b29c8, size=1, set=2)
}
-> a SynthDef
Instance of OutputProxy {    (0x7f8990fcea08, gc=74, fmt=00, flg=00, set=04)
  instance variables [11]
    synthDef : instance of SynthDef (0x7f896063d5f8, size=16, set=4)
    inputs : nil
    rate : Symbol 'control'
    synthIndex : Integer 1
    specialIndex : Integer 0
    antecedents : nil
    descendants : nil
    widthFirstAntecedents : nil
    source : instance of Control (0x7f899906ca58, size=10, set=4)
    outputIndex : Integer 0
    name : nil
}
Instance of Control {    (0x7f899906ca58, gc=74, fmt=00, flg=00, set=04)
  instance variables [10]
    synthDef : instance of SynthDef (0x7f896063d5f8, size=16, set=4)
    inputs : nil
    rate : Symbol 'control'
    synthIndex : Integer 1
    specialIndex : Integer 1
    antecedents : nil
    descendants : nil
    widthFirstAntecedents : nil
    channels : instance of Array (0x7f89b0a126e8, size=1, set=2)
    values : instance of Array (0x7f8998d4eda8, size=1, set=2)
}
-> a SynthDef

@telephon
Copy link
Member Author

telephon commented Jan 3, 2022 •
edited
Loading

Otherwise, are there reasons speaking for/against implicit
vs. explicit creation, \ctl.kr versus NamedControl(\ctl) ?

well, I tend to go to the more verbose and less roundabout version in class libraries, that's all.
Also there is already a conversion to symbol in NamedControl, no need for a second call.

@telephon
Copy link
Member Author

telephon commented Jan 3, 2022

should work now …

adcxyz
adcxyz approved these changes Jan 8, 2022
View reviewed changes
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.

works as intended, good to go!

@adcxyz adcxyz merged commit 7e3d574 into supercollider:develop Jan 8, 2022
@dyfer dyfer mentioned this pull request Jan 12, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adcxyz adcxyz adcxyz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@telephon @avdrd @adcxyz