Ndef simple channel expansion question

Questions
21

Cool :grin: (Of course in #5 I hadn’t anticipated future fixes.)

It might be helpful to format it as a git patch, to help JITLib experts review it. This thread has a lot of short code snippets, some of which are quotes for purposes of analysis, while others are test cases, and a few would be the code changes. (The ideal way forward would be a PR – forum threads drift further and further back in the rearview, while a PR with an open status attaches accountability for someone to review it.)

hjh

1 Like
22

Thanks to suggestions here, I’ve written a cleaned-up version of my BusPlug.value bugfix that doesn’t stuff more methods in the base classes.

+ BusPlug {
	value { | something |
		var n;
		if(UGen.buildSynthDef.isNil) { ^this }; // only return when in ugen graph.
		if(something.notNil) {
			if(something.isKindOf(UGen.class) or: { something.isKindOf(UGen) }) {
				// UGen.numChannels always 1, so let InBus.new1 ultimately called by this.ar/.kr
				// auto-detect numChannels by keeping n=nil for the call to this.ar/.kr below.
				// Certainly need to do this in the case of UGens classes that can't be
				// flagged by something.isUGen, which is an instance method,
				// e.g. CombN.isUGen == false while CombN.ar.isUGen == true
				//("is UG or UGC" + something + n).postln
			} {
				n = something.numChannels
			}
		};
		//"bpv2: % is_UGen:% is_UGen.class:% n:% ".format(something, something.isKindOf(UGen), something.isKindOf(UGen.class), n).postln;
		^if(something.respondsTo(\rate) and: { something.rate == 'audio'} or: { this.rate == \audio }) {
			this.ar(n)
		} {
			this.kr(n)
		}
	}
}
23

+1 to doing a PR when you’re ready so that this work can make it into the codebase

24

I didn’t see this discussion, I’m not always logged in here. Thank you for your analysis and sorry for the opacity. So just one comment on the intent. The idea is the following:

When reading from a node proxy:

  1. If you do not specify what number of channels your signal should have, it should just return whatever it is, but always in an array (this guarantees that you can rely on calling array functions). So Ndef.ar(\x) will always be an array, but may vary depending on the number of channels Ndef(\x) has.
  2. If you do not even want to specify the rate, then you can leave out the ar and it should just return whatever it is. This happens via asUGenInput, which is called in UGen:multiNewList 
Ndef(\z, { MouseX.kr(1, 9) * [2, 3, 4] });

Ndef(\x, { Ndef(\z).asUGenInput.postln; 0 }) // [ an OutputProxy, an OutputProxy, an OutputProxy ]

Ndef(\z, { MouseX.kr(1, 9) });

Ndef(\x, { Ndef(\z).asUGenInput.postln; 0 }) // [ an OutputProxy ]

The problem that I see is that some filter UGens like CombN don’t call asUGenInput, but:

asAudioRateInput { |for|
		var result = this.value(for);
		^if(result.rate != \audio) { K2A.ar(result) } { result }
	}

Now BusPlug overrides asUGenInput, in order to avoid the described behaviour:

asUGenInput {
		^this.value(nil)
	}

but not asAudioRateInput.

Just all in all, it is an example of someone (me) failing to simplify enough.

So adding this in BusPlug should fix it:

asAudioRateInput {
		^this.ar(nil)
	}
25

True, it would fix it in this case. But there can be other call paths that call BusPlug.value with a UGen. Trying to find and fix all the call sites in all UGens seems much more brittle than making BusPlug.value expand on UGen something. Can you see a case where the latter would be semantically wrong?

26

Sorry, this has remained unanswered: I can’t see a use case where you want to pass a UGen to BusPlug.value. It is extra information for functions if they want to decide what to return, but a BusPlug can’t do that.