NamedControl doesn't see Controls created by other APIs

avdrd commented 2 years ago

I'm not entirely sure if this qualifies as a bug report rather than a feature request, but since the help page for Controls is rather ambiguous on this, because it freely mixes old and new interfaces creating an expectation of interoperability... I'm filing it as bug.

As was discussed on the forum, NamedControl doesn't see controls created with function args nor is it interoperable with the (older?) Control.names(..).kr(...) way of dynamically instantiating controls.

Environment

SuperCollider version: 3.12.1
Operating system: all

Steps to reproduce

As examples, first some artificial ones:

NamedControl doesn't see arg controls

d = SynthDef(\one, { arg xx; Out.kr(0, [xx, \xx.kr]) }).dumpUGens
/*
[ 0_Control, control, nil ]
[ 1_Control, control, nil ]
[ 2_Out, control, [ 0, 0_Control[0], 1_Control[0] ] ]
*/

Two controls with the same name are created instead of NamedControl looking up xx that was created by args. Due to the way the server resolves such conflicts, meaning stopping the search at the 1st matching name, the 2nd control is not addressable by name. SynthDesc will actually produce a warning in such cases when it builds the msgFunc, but other workflows like Ndefs don't produce any such warning but result in silent failures.

d.add
// -> a SynthDef
// WARNING: Could not build msgFunc for this SynthDesc: duplicate control name xx etc.

Ndef(\one, { arg xx; [xx, \xx.kr] }).prime;
// no warning whatsoever

NamedControl doesn't see controls issued by Control.names.kr and friends

Mixing Control.names wtih NamedControl produces similarly problematic graphs e.g.

SynthDef(\one, { Out.kr(0, [Control.names([\xx]).kr([12]), \xx.kr]) }).dumpUGens
/*
[ 0_Control, control, nil ]
[ 1_Control, control, nil ]
[ 2_Out, control, [ 0, 0_Control[0], 1_Control[0] ] ]
*/

The same is true if the order is reversed, i.e. replacing

[\xx.kr, Control.names([\xx]).kr([12])]

in the above example.

JITLib uses a Control.name([\out]) internally, duplicating controls in some cases

And more realistically, there's trouble when NamedControl is used inside a Ndef function to try to access out, because JITLib uses Contol.names internally to instantiate another out.

// reserve these for something else; JITLib should not touch 
c = Bus.control(s, 4);

y = NodeProxy.control(s, 1);
// make some self-triggering decay as simple illustration
y[0] = { var oi = In.kr(\out.ir(42)); if(oi > 0.01, oi * 0.999, 1.0) };

y.get(\out) // 4 -- seems to respect others, but...
y.trace

the output from NodeProxy goes to the wrong bus. Apparently that only happens when In.kr(\out.ir) is used that way inside the proxy function. The trace prints something like

  unit 1 In
    in  4
    out 0
// more units omitted
 unit 10 Out
    in  0 1
    out

while the input is wired correctly, the output is not! Meaning NodeProxy corrupted (or overtook) a bus (number 0) reserved for something else.

Expected vs. actual behavior

Perhaps the desirable behavior here is for a unified namespace, as much as possible.

Fix(es) proposed

I've done some prototyping work on this.

(Pre-)loading arg-generated controls in to NamedControl doesn't take all that much in the way of code changes.
Updating NamedControl from dynamically generated Contrlol.names is more complicated, but for the simple and common case where a single name is issued at a time with names, I also have some a working prototype.

On the forum, opinions have been expressed that more cases should be handled in this unification effort, like:

NamedControl being capable of properly handling multiple Control.names issued in one batch. This alas is somewhat hard to handle because Control.names issues bad ControlNames (and only correct Controls) in such cases. Thankfully the bad ControlNames issued by Control.names(list) for list.size>1 are discarded, or rather reconstructed from Controls by both the sever and by`SynthDesc as explained in this rather long expose.
Control.names.kr despite being a multi-step, state-machine process should automatically see NamedControls ... at the .kr call (and similarly AudioControl at .ar etc.) I had some vague idea how this could be handled, using some caching (explained on the forum), but I did not feel like prototyping this would be useful for me personally, so I let those who desire this "show me the code".

Also worth noting here that JITlib issues its Control.names([\out]) in ProxySynthDef after the user's function has been executed, so merely making NamedControl see all, doesn't address the latter problme. JITLib would also have to be changed to use NamedControl for \out. Ironically, GraphBuilder does do that, meaning it uses NamedControl but for things that have less impact like \fadeTime.

avdrd commented 2 years ago

For what's worth it, the bits that fix most of the uses cases I care about, meaning the 1st and 3d in the above list, are fairly easy to solve with just patches

I can upload here for convenience the more complex code that updates NamedControl from Control.names, with the caveats mentioned in the previous post, but for me personally this turned out to do nothing more, since I eliminated use of Control.names in the sources I care about.

telephon commented 2 years ago

From reading the code, the solutions above look good.

jamshark70 commented 2 years ago

From reading the code, the solutions above look good.

I agree that it would be better to have a central repository, during SynthDef building, of all control inputs.

I think there's still a question of whether this repository should be located in NamedControl, or in SynthDef.

I tend to think SynthDef would be a better place for this.

Option A: arg controls and Control.names controls create entries in the SynthDef, and all of them (and NamedControl) could check the entries in the SynthDef to reuse names that are declared or referenced multiple times.
Option B: arg controls and Control.names controls create entries in the SynthDef, which are then copied over to NamedControl -- then further args (e.g. SynthDef:wrap) would need to look at NamedControl for dups.

I realize that design decisions are, to some extent, a matter of opinion, but to me, A seems cleaner.

dyfer commented 2 years ago

Hello, does #5675 fix this?

telephon commented 2 years ago

No, that fix is just a fix foe some especially painful cases in NodeProxy. @avdrd has made a suggestion of how to fix this here: https://github.com/avdrd/supercollider/commit/82862d2c9634921d1b79d3ef3190abf3334ab5f4

(I just gave one suggestion about how to change the dependencies).

supercollider / supercollider