It would be nice to have methods for Hasher that impement these different behaviours.
Indeed, it’s necessary to move the uint32 iid part outside of the if (id != unit->m_id) – although you’d have to reverse the order of if (iid < unit->mWorld->mNumRGens) and uint32 iid = (uint32)id; because the posted version performs a < check on an uninitialized variable.
void RandID_next(RandID* unit, int inNumSamples) {
float id = ZIN0(0);
if (id != unit->m_id) {
unit->m_id = id;
};
uint32 iid = (uint32)id;
if (iid < unit->mWorld->mNumRGens) {
unit->mParent->mRGen = unit->mWorld->mRGen + iid;
}
ZOUT0(0) = 0.f;
}
This would raise CPU usage for every synth using RandID.kr – currently, unit->mParent->mRGen is set only when the ID changes, but after a change such as this, it will be set in every control block, whether changing or not. So it might be nice to add a second input, a flag, determining whether to fire every time or not, such as bool fire = ZIN0(1) > 0 and, in the id != block, set fire = true. Then if(fire && (iid < unit->mWorld->mNumRGens)). A positive input would be true in every block; a nonpositive input would initially be false, but a change in ID would override that. Default 0 for compatibility with current behavior.
It’s also necessary to send a nonzero trigger to RandSeed. The examples are using trig = 0, so RandSeed never fires, and the seeds are ignored.
With some debugging output, then I get:
init rand id 0 with seed 1789
got 0.758146 from rand id 0
init rand id 1 with seed 1789
got 0.758146 from rand id 1 -- same
got 0.524698 from rand id 0
got 0.524698 from rand id 1 -- same
Diffs:
diff --git a/server/plugins/NoiseUGens.cpp b/server/plugins/NoiseUGens.cpp
index 7bbe84643..50c581d44 100644
--- a/server/plugins/NoiseUGens.cpp
+++ b/server/plugins/NoiseUGens.cpp
@@ -790,13 +790,15 @@ void RandID_Ctor(RandID* unit) {
void RandID_next(RandID* unit, int inNumSamples) {
float id = ZIN0(0);
+ bool fire = ZIN0(1) > 0;
if (id != unit->m_id) {
unit->m_id = id;
- uint32 iid = (uint32)id;
- if (iid < unit->mWorld->mNumRGens) {
- unit->mParent->mRGen = unit->mWorld->mRGen + iid;
- }
+ fire = true;
+ };
+ uint32 iid = (uint32)id;
+ if (fire && (iid < unit->mWorld->mNumRGens)) {
+ unit->mParent->mRGen = unit->mWorld->mRGen + iid;
}
ZOUT0(0) = 0.f;
}
diff --git a/SCClassLibrary/Common/Audio/Noise.sc b/SCClassLibrary/Common/Audio/Noise.sc
index ca001d258..ab37e62bf 100644
--- a/SCClassLibrary/Common/Audio/Noise.sc
+++ b/SCClassLibrary/Common/Audio/Noise.sc
@@ -33,12 +33,12 @@ RandSeed : WidthFirstUGen {
RandID : WidthFirstUGen {
// choose which random number generator to use for this synth .
- *kr { arg id=0;
- this.multiNew('control', id)
+ *kr { arg id = 0, alwaysUpdate = 0;
+ this.multiNew('control', id, alwaysUpdate)
^0.0 // RandID has no output
}
- *ir { arg id=0;
- this.multiNew('scalar', id)
+ *ir { arg id = 0, alwaysUpdate = 0;
+ this.multiNew('scalar', id, alwaysUpdate)
^0.0 // RandID has no output
}
}
hjh
Very good! Can you make a pull request for it?
1 Like
Publish the tests and benchmarks results (just for curiosity)
Probably could, over the weekend.
Highly unlikely I would have time for that. You’re welcome to run some tests.
hjh
Sure, when I have time. If this part of the project is already part of some king of unit/property test, I can try to write along those lines.
Informal test based on this thread:
(
{
var a, b;
RandID.kr(0, 1);
RandSeed.ir(1, 88372);
a = LFDNoise0.ar(440);
RandID.kr(1, 1);
RandSeed.ir(1, 88372);
b = LFDNoise0.ar(440);
[a, b, a - b]
}.plot;
)
hjh
2 Likes
I am happy that a question turned into a bug fix! Thank you all!!!
Remind me what happens when you extend the binary interface of UGens - does this work with SynthDefs produced for the previous version (i.e. where RandID has one instead of two arguments), or does this potentially crash scsynth? Is there a default value of 0 when force is not given in the SynthDef?
Edit: I added my comments to the PR, to avoid that the discussion spreads to different places.
I could not reproduce a crash here.
hjh