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

TRand, TExpRand, TIRand are broken for audio-rate inputs #1278

Closed
Sciss opened this issue Dec 14, 2014 · 15 comments · Fixed by #5344
Closed

TRand, TExpRand, TIRand are broken for audio-rate inputs #1278

Sciss opened this issue Dec 14, 2014 · 15 comments · Fixed by #5344
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: server plugins

Comments

@Sciss
Copy link
Contributor

Sciss commented Dec 14, 2014

This is SC 3.6.5, not tested with SC 3.7 yet:

// kr -- ok
x = play {
  var p = Impulse.kr(1);
  var lo = \lo.kr(0);
  var hi = \hi.kr(1);
  var t = TRand.kr(lo, hi, p);
  t.poll(p, "rand");
  0
}

// ar -- moves towards hi and sticks to hi value
x = play {
  var p = Impulse.ar(1);
  var lo = \lo.ar(0);
  var hi = \hi.ar(1);
  var t = TRand.ar(lo, hi, p);
  t.poll(p, "rand");
  0
}
@telephon
Copy link
Member

Same on master.

The reason seems not to be TRand, however. The audio rate control, generated by \lo.ar(0) goes up to 1.0 by itself, although not when read as an audio reate control, but when taking it as a control rate input (ZIN0(0) in the ugen source).

Compare:

x = play {
  var p = Impulse.ar(1);
  var lo = \lo.ar(0);
  var hi = \hi.ar(1);
  var t = TRand.ar(lo, hi, p);
  t.poll(p, "rand");
  0
}
x = play {
  var p = Impulse.ar(1);
  var lo = \lo.ar(0);
  var hi = \hi.ar(1);
  var t = TRand.ar(0, 1, p);
  t.poll(p, "rand");
  0
}

The second one works fine.

If I poll the inputs inside TRand:

printf("lo: %f hi: %f\n", ZIN0(0), ZIN0(1));

I get values like this after a while:

lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000
lo: 0.999694 hi: 1.000000

@Sciss
Copy link
Contributor Author

Sciss commented Dec 14, 2014

I'm not sure I understand what you are trying to say. This has nothing to do with AudioControl:

x = play {
  var p = Impulse.ar(1);
  var lo = DC.ar(0);
  var hi = DC.ar(1);
  var t = TRand.ar(lo, hi, p);
  t.poll(p, "rand");
  0
}

Definitely TRand is broken for audio-rate lo/hi inputs. So a better title then is does not accept audio rate inputs. E.g. this works

x = play {
  var p = Impulse.ar(1);
  var lo = DC.kr(0);
  var hi = DC.kr(1);
  var t = TRand.ar(lo, hi, p);
  t.poll(p, "rand");
  0
}

@Sciss Sciss changed the title TRand.ar is broken TRand is broken for audio-rate inputs Dec 14, 2014
@telephon
Copy link
Member

Ok, yes, you are right (and it is good that it doesn't). The problem goes away if you don't call ZIN0(0)inside the LOOP1 macro:

void TRand_next_a(TRand* unit, int inNumSamples)
{
    float *trig = ZIN(2);
    float prev = unit->m_trig;
    float *out = ZOUT(0);
    float outval = unit->m_value;
    float next;

    LOOP1(inNumSamples,
        next = ZXP(trig);
        if (next > 0.f &&  prev <= 0.f) {
            float lo = ZIN0(0);
            float hi = ZIN0(1);
            float range = hi - lo;
            RGen& rgen = *unit->mParent->mRGen;
            ZXP(out) = outval = rgen.frand() * range + lo;
        } else {
            ZXP(out) = outval;
        };
        prev = next;
    )
    printf("lo: %f hi: %f\n", ZIN0(0), ZIN0(1));

    unit->m_trig = next;
    unit->m_value = outval;
}

change to:

void TRand_next_a(TRand* unit, int inNumSamples)
{
    float *trig = ZIN(2);
    float prev = unit->m_trig;
    float *out = ZOUT(0);
    float outval = unit->m_value;
    float next;
float lo = ZIN0(0);
    float hi = ZIN0(1);

    LOOP1(inNumSamples,
        next = ZXP(trig);
        if (next > 0.f &&  prev <= 0.f) {

            float range = hi - lo;
            RGen& rgen = *unit->mParent->mRGen;
            ZXP(out) = outval = rgen.frand() * range + lo;
        } else {
            ZXP(out) = outval;
        };
        prev = next;
    )
    printf("lo: %f hi: %f\n", ZIN0(0), ZIN0(1));

    unit->m_trig = next;
    unit->m_value = outval;
}

I don't know why this is though.

@Sciss Sciss changed the title TRand is broken for audio-rate inputs TRand, TExpRand, TIRand are broken for audio-rate inputs Dec 14, 2014
@Sciss
Copy link
Contributor Author

Sciss commented Dec 14, 2014

This affects at least these UGens: TRand, TIRand, TExpRand

@telephon
Copy link
Member

It affects all UGens that call ZIN0() inside LOOP1 and which have no explicit audio rate argument function.

@telephon
Copy link
Member

So in general, we can say that the following construction is broken for audio rate inputs:

LOOP1(inNumSamples,
    ...
    ZIN0(0);
    ...
)

(maybe it wasn't ever meant to work!)

@Sciss
Copy link
Contributor Author

Sciss commented Dec 14, 2014

Is there a simple way to grep all the affected cases?

@timblechmann
Copy link
Contributor

So in general, we can say that the following construction is broken for
audio rate inputs:

|LOOP1(inNumSamples,
...
ZIN0(0);
...
)
|

this code is wrong: it breaks when input and output buffers are the
same. simply store the value before entering the loop

@telephon
Copy link
Member

I'll fix these first.

@telephon
Copy link
Member

I've added proper audio rate methods, too (d0d5907)

@crucialfelix crucialfelix added bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: server plugins labels Jan 17, 2016
@Sciss
Copy link
Contributor Author

Sciss commented Feb 4, 2021

Issue doesn't seem to be fixed. Original mcve still shows TRand sticking to higher values - scsynth 3.10.2 (Built from branch 'build-3.10.2' [834c036]).

According to this forum thread, TIRand still affected as well.

x = play {
  var p = Impulse.ar(1);
  var lo = \lo.ar(0);
  var hi = \hi.ar(10);
  var t = TIRand.ar(lo, hi, p);
  t.poll(p, "rand");
  0
}

-> Synth('temp__4' : 1004)
rand: 10
rand: 10
rand: 10
rand: 10
rand: 10
rand: 10

@Sciss Sciss reopened this Feb 4, 2021
@jamshark70
Copy link
Contributor

jamshark70 commented Feb 4, 2021

For an audio rate UGen and audio rate trigger, do we expect lo and hi to be able to change at audio rate as well, or only at kr? The above proposed solution would support only the latter, but the forum user expected the former. oh, now I see that audio rate functions were added.

Are we 100% sure the forum issue matches this one? The forum issue is about getting random numbers outside the requested range, but the latest reproducer is quite different from that.

@telephon
Copy link
Member

telephon commented Feb 5, 2021

The plot looks ok:

(
{
  var p = Impulse.ar(4000);
  var lo = \lo.ar(0);
  var hi = \hi.ar(10);
  var t = TIRand.ar(lo, hi, p);
 t
}.plot(0.01)
)

Screenshot 2021-02-05 at 14 54 47

@Sciss
Copy link
Contributor Author

Sciss commented Feb 5, 2021

that plot is way too short and uses a way too high frequency to show the problem. if you record longer (plot(1)) you'll see the bias. if you decrease the frequency below ControlRate, you see the bug

(
{
  var p = Impulse.ar(400);
  var lo = \lo.ar(0);
  var hi = \hi.ar(10);
  var t = TIRand.ar(lo, hi, p);
 t
}.plot(0.5)
)

Screenshot from 2021-02-05 15-07-45

telephon added a commit to telephon/supercollider that referenced this issue Feb 5, 2021
@telephon
Copy link
Member

telephon commented Feb 5, 2021

I took a look, I am unsure, perhaps it is this?

https://github.com/telephon/supercollider/tree/topic/fix-aa-methods-of-triggered-rand

I currently can't build, because of these annoying code signing issues, so I can't test.

telephon added a commit to telephon/supercollider that referenced this issue Feb 8, 2021
Some formatting is also improved. This fixes supercollider#1278.
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: server plugins
Projects
None yet
5 participants