add reset and phase versions of lf_sawpos

[magnetophon]

No description provided.

[sletz]

dontReset is not used in the function?

[magnetophon]

Oops!
Fixed now.

[sletz]

Why is this dontReset so complicated? Can you explain what signal it produces?

[magnetophon]

It's a stream of 1's,with a single sample zero when reset goes from 0 to bigger then 0.
Can you think of a way to simplify it?

[josmithiii]

Hi All, We have ba.impulsify defined as impulsify = _ <: _,mem : - <: >(0)*_; which emits an impulse whenever the input signal increases. Creating a one-sample "strobe" from this as Bart is doing by subtracting it from 1 makes sense to me. It would also make sense to me to use the reset signal directly, without "debouncing" it. I can imagine possibly wanting to hold the oscillator in the reset state until some event comes along, for example. Cheers, Julius

…

On Sat, Jun 27, 2020 at 2:14 PM Bart Brouns ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In oscillators.lib <#44 (comment)> : > +// +// ``` +// lf_sawpos_reset(freq,reset) +// ``` +// +// Where: +// +// * `freq`: frequency +// * `reset`: reset the oscillator to 0 +// +//--------------------------------------------------------- +// Author: Bart Brouns +// License: STK-4.3 +lf_sawpos_reset(freq,reset) = ma.frac * dontReset ~ +(freq/ma.SR) +with { + dontReset = reset:ba.impulsify*-1+1; It's a stream on 1's,with a single sample zero when reset goes from 0 to bigger then 0. Can you think of a way to simplify it? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFLLQKFB7DA6IKY5O53RYZOMRANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[magnetophon]

Hi Julius,

@sletz Suggested the much more elegant (reset <= reset') on the slack chat.
I changed the PR to use that.

Coincidentally, while making that change, I also thought of not debouncing, but initially decided to leave it in.
After reading your comment I reconsidered and removed it.

[josmithiii]

Great, I agree with that decision - I have learned the hard way to keep low level primitives as simple as possible and build up the layers of added functionality using higher level functions

…

On Sat, Jun 27, 2020 at 5:19 PM Bart Brouns ***@***.***> wrote: Hi Julius, @sletz <https://github.com/sletz> Suggested the much more elegant (reset <= reset') on the slack chat. I changed the PR to use that. Coincidentally, while making that change, I also thought of not debouncing, but initially decided to leave it in. After reading your comment I reconsidered and removed it. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFJRTZXIYOKWGTMPBBLRY2EABANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[sletz]

I think detecting fronts should be standardised because they happens everywhere and we really want the most efficient way to write those low-level stuff, so I suggest to add in the basics.lib:

upfront(x) = x > x';        // equal 1 for upfront, 0 otherwise
downfront(x) =  x < x';  // equal 1 for downfront, 0 otherwise
front(x) = x != x';	          // equal 1 for upfront or downfront, 0 otherwise
inv_upfront(x) = x <= x';      // equal 0 for upfront, 1 otherwise
inv_downfront(x) = x >= x'; // equal 0 for downfront, 1 otherwise
inv_front(x) = x == x';          // equal 0 for upfront or downfront, 1 otherwise 

What do you guys think? Then my suggestion would be to check if some lower-level control signal can be improved using this.

[josmithiii]

Nice idea! Based on my experience, the most standard names I can think of would be rising falling changing nonincreasing nondecreasing constant Cheers, - Julius

…

On Sat, Jun 27, 2020 at 11:45 PM Stéphane Letz ***@***.***> wrote: I think detecting fronts *should be standardised* because they happens everywhere and we really want the *most efficient* way to write those low-level stuff, so I suggest to add in the basics.lib: upfront(x) = x > x'; // equal 1 for upfront, 0 otherwise downfront(x) = x < x'; // equal 1 for downfront, 0 otherwise front(x) = x != x'; // equal 1 for upfront or downfront, 0 otherwise inv_upfront(x) = x <= x'; // equal 0 for upfront, 1 otherwise inv_downfront(x) = x >= x'; // equal 0 for downfront, 1 otherwise inv_front(x) = x == x'; // equal 0 for upfront or downfront, 1 otherwise What do you guys think? Then my suggestion would be to check if some lower-level control signal can be improved using this. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFOCYYAMFLDSWLXOPPTRY3RHVANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[sletz]

I still like the idea of having the front or edge idea in the name, and also suggesting the "inverse" idea (0 becomes 1, and 1 becomes 0) so possibly:

rising_edge
falling_edge
edge ? front ?
inv_rising_edge
inv_falling_edge
inv_edge ?  inv_front

[sletz]

And more generally; trying to rewrite everything starting from the more general version with phase and reset, and specialising it using the fact the compiler will only keep the really needed code, so:

lf_sawpos_phase_reset(freq,phase,reset) = (+(phase-phase') : ma.frac * (reset <= reset')) ~ +(freq/ma.SR);
lf_sawpos_phase(freq,phase) = lf_sawpos_phase_reset(freq, phase, 0);
lf_sawpos_reset(freq,reset) = lf_sawpos_phase_reset(freq, 0, reset);
lf_sawpos(freq) = lf_sawpos_phase_reset(freq, 0, 0);

Then we can possibly define :

lf_sin_phase_reset(freq,phase,reset) = sin(2* ma.PI * lf_sawpos_phase_reset(freq,phase,reset));
lf_sin_phase(freq,phase) = sin(2* ma.PI * lf_sawpos_phase(freq, phase));
lf_sin_reset(freq,reset) = sin(2* ma.PI * lf_sawpos_reset(freq, reset));
lf_sin(freq) = sin(2* ma.PI * lf_sawpos(freq));

So you see the point here, rebuilding from scratch, starting from the most efficient general form, using the compiler optimising features to define more specialised versions, building more complex signals (the sin version here) by wrapping the basic sawpos and so on...

[magnetophon]

I like the suggestion to clean up the edge detection in the libraries.
I think that the last set of names is the best so far.

Small nitpick: As I understand it, the lf_ prefix as used in faust, marks oscillators that might alias, so lf_sin doesn't make sense.

In any case, none of the above applies to this PR, as Julius and I decided to take out the debunce altogether.

[LFSaw]

In any case, none of the above applies to this PR, as Julius and I decided to take out the debunce altogether.

I went ahead and created a new issue for this: #45

[magnetophon]

I found some bugs.
When I compile this:

import("stdfaust.lib");
freq = hslider("freq",440,0,24000,0.0001);
gain = hslider("gain",0.5,0,1,0.001);
gate = button("gate");
phase = hslider("phase", 0.5, 0, 1, 0.001)<:select2(checkbox("smooth phase"),_, si.smoo);

process =
  lf_sawpos_phase_reset(freq,phase,gate:ba.impulsify)*gate
, stephan_lf_sawpos_phase_reset(freq,phase,gate:ba.impulsify)*gate;

lf_sawpos_reset(freq,reset) = ma.frac * (reset == 0) ~ +(freq/ma.SR);
lf_sawpos_phase_reset(freq,phase,reset) = lf_sawpos_reset(freq,reset) +phase :ma.frac;

stephan_lf_sawpos_phase_reset(freq,phase,reset) = (+(phase-phase') : ma.frac * (reset == 0)) ~ +(freq/ma.SR);

I get different results depending on how I compile:

1. with faust2jack bug.dsp && ./bug:

• lf_sawpos_phase_reset works as expected
• stephan_lf_sawpos_phase_reset seems to ignore the phase slider

2. with faust2jack -midi -nvoices 1 bug.dsp && ./bug

• lf_sawpos_phase_reset
  works as expected without "smooth phase",
  has a pitch bend (described below) with "smooth phase"
• stephan_lf_sawpos_phase_reset
  seems to ignore the phase slider without "smooth phase".
  has a pitch bend (described below) with "smooth phase"

When the pitch bend is present, the oscillators start at phase 0, but with a higher pitch, and then pitch down during the course of the note.

I guess these are side-effects of the fact that with -nvoices, the dsp is turned on and off, but I'm not sure what to do about it.

[sletz]

My proposal was lf_sawpos_phase_reset(freq,phase,reset) = (+(phase-phase') : ma.frac * (reset <= reset')) ~ +(freq/ma.SR);

The reset handling is different.

[magnetophon]

The only difference is that reset is impulsified.
The above bugs remain.

[sletz]

It seems that your lf_sawpos_phase(phase,freq) = (+(phase-phase') : ma.frac) ~ +(freq/ma.SR); I started from is not working as expected also ((-;

[magnetophon]

It works when compiled without -nvoices, so iow when the dsp is "on" all the time.

Any idea how to get it working with -nvoices?
Is this a bug in the above code or in the faust compiler?

[josmithiii]

The "lf_" prefix stands for "low frequency", as in "LFO". Algorithms are generally more efficient when they can assume LF (e.g., not worry about aliasing). Speaking of prefixes, maybe "b_" could be a good prefix for all functions return 0 or 1, i.e., a one-bit signal? The "inv_" prefix works for me. The "_edge" suffix would work for me if we really detected a single edge in one rise or fall time. However, these are continuous sample-by-sample comparisons. For example, a triangle wave or sine wave input comes out as a square wave. The "edges" are then defined as the positive and negative peaks of the input wave. The name "edge" works well when the input is a "b_" signal, like a button, but not for general inputs. - Julius

…

On Sun, Jun 28, 2020 at 12:12 PM Bart Brouns ***@***.***> wrote: It works when compiled without -nvoices, so iow when the dsp is "on" all the time. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFJWPIGC7J65A4Y7ZPLRY6IZHANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[LFSaw]

Speaking of prefixes, maybe "b_" could be a good prefix for all functions return 0 or 1, i.e., a one-bit signal?

could also be g_ as in gate, or t_ as in trigger, or is_ as in a question (is_falling, is_rising), assuming 0 to be equivalent to false and 1 equivalent to true.

[josmithiii]

I like gate better than bit - more "virtual analog". To me a trigger is an impulse. "is_" works for me better than "b_" also, and is the most readable. I vote for readability above all, since I see that as the main point of wrapping elementary Faust expressions in function names.

…

On Sun, Jun 28, 2020 at 11:09 PM LFSaw ***@***.***> wrote: Speaking of prefixes, maybe "b_" could be a good prefix for all functions return 0 or 1, i.e., a one-bit signal? could also be g_ as in gate, or t_ as in trigger, or is_ as in a question (is_falling, is_rising), assuming 0 to be equivalent to false and 1 equivalent to true. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFNFJILDZVGXTQJRPHLRZAVZZANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[magnetophon]

I figured out my smooth issue is, of course, an issue that every smoothed synth parameter has.
I came up with my own solution before I found polySmooth(g,s,d).

Why does it have the d parameter?
I can only see the use case for d=1, and that is how it's used in the examples I found.

This smooth function doesn't need g either, and does the same job, as far as I can tell.

new_smooth(s) = si.smooth(s * ((1-1') < 1) );

One could even make the case that this should be the default smooth function.
The current smooth(s) would be renamed, for use in exceptional cases.
That might break some dsp "in the wild" though.

What do you think?

[josmithiii]

Good points. I did not write polySmooth, so I will let the author respond about that. I too encountered this issue and handled it with a one-sample delay similar to your solution. I would keep si.smooth() defined as is, for simplicity and backward compatibility, and define a new function si.smoothDelayed() or the like using your implementation or whatever implementation is best at any given time. Instead of polySmooth, I would prefer a name like smoothDelayedBy(delay) so that we could define smoothDelayed = smoothDelayedBy(1); smooth = smoothDelayedBy(0); assuming the compiler optimizes all these to be equivalent to the direct definitions.

…

On Mon, Jun 29, 2020 at 10:03 AM Bart Brouns ***@***.***> wrote: I figured out my smooth issue is, of course, an issue that every smoothed synth parameter has. I came up with my own solution before I found polySmooth(g,s,d). Why does it have the d parameter? I can only see the use case for d=1, and that is how it's used in the examples I found <https://github.com/search?p=2&q=polySmooth+hslider&type=Code>. This smooth function doesn't need g either, and does the same job, as far as I can tell. new_smooth(s) = si.smooth(s * ((1-1') < 1) ); One could even make the case that this should be the default smooth function. The current smooth(s) would be renamed, for use in exceptional cases. That might break some dsp "in the wild" though. What do you think? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFLIUB7O3EMSLA3PULDRZDCNNANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[magnetophon]

@josmithiii Thanks.
How does your solution work?
I don't understand how a delay comes into it, other then in creating the initialization pulse. That is not what you mean, is it?

[josmithiii]

What I had in mind was something like smoothDelayedBy(d, s) = si.smooth(s * ((1-1@d) < 1) );

…

On Mon, Jun 29, 2020 at 1:34 PM Bart Brouns ***@***.***> wrote: @josmithiii <https://github.com/josmithiii> Thanks. How does your solution work? I don't understand how a delay comes into it, other then in creating the initialization pulse. That is not what you mean, is it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFKMPRAMUBCHDKIUHWLRZD3FVANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[magnetophon]

Thanks.
Why is d variable?

[josmithiii]

We know we want cases d=0 and d=1, and larger values are to implement polySmooth. On second thought I would say @int(d) in place of @d.

…

On Mon, Jun 29, 2020 at 2:52 PM Bart Brouns ***@***.***> wrote: Thanks. Why is d variable? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFJ4SIGGZK6FFHCLSDLRZEELBANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[magnetophon]

Sorry to be dense, but if you implement smoothDelayedBy using smooth, then the case d=0 is not needed, right?
That brings me back to my original question: what is the d parameter in polySmooth used for?

[josmithiii]

Yes, sorry, I was just trying to make clear the various relationships. I should have said something like smoothDelayedBy(d, s) = *(1.0 - sd) : + ~ *(sd) with { sd = s * ((1-1@d) < 1) ); }; and THEN defined the others, so as to not imply an infinite recursion at compile time. The idea, which you'll see here and there in the Faust libraries, is to write one lowest-level function that gives all desired cases without compromise (after compilation). This helps to keep entropy low, in my opinion, and makes more clear the function relationships. However, I don't think it's worth it if the lowest-level function then becomes unreadable, unless the easier higher-level variations make everything clear, like I think they do in this case. The d parameter in polySmooth generalizes the one-sample delay of your solution (and mine) to any number of samples. Like you, I don't know when that is needed, but it's free, and it's been released (I presume) so why not keep it? It means there is no smoothing for the first d samples at the beginning of time. Maybe state initializations sometimes require some compute time in samples?

…

On Mon, Jun 29, 2020 at 3:24 PM Bart Brouns ***@***.***> wrote: Sorry to be dense, but if you implement smoothDelayedBy using smooth, then the case d=0 is not needed, right? That brings me back to my original question: what is the d parameter in polySmooth used for? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQZKFPS6NU3B45BL5XLPR3RZEIAHANCNFSM4OKDRX3Q> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

[magnetophon]

How can we move forward with this?

[josmithiii]

Since the commit just adds new functions, I see no problems with it.

[magnetophon]

@rmichon Could you have a look at this one as well?

Most of the discussion above is about providing general functions for edge/front detection and is not really relevant for this PR.
When I say not really relevant, I mean:

• Yes, general functions for edge/front detection would be nice.
• Yes, once we have those, these functions should be rewritten.
• But: this PR is useful as-is, imho.

[sletz]

Well, I think we should restart this edge/front detection discussion... Otherwise what will happen? Same kind of edge/front detection code rewritten again and again in similar use-cases, with "slight" differences that the average user will not grasp...
We (GRAME) have to provide clean and reference implementations, and so share code when it make sense.

[rmichon]

I accepted this commit because it only adds new functions as @josmithiii pointed out. However, I agree with @sletz that this conversation on edge detection is important and should be continued.

[sletz]

OK ((-;

[magnetophon]

@rmichon Thanks.
@sletz As mentioned above: Yes, general functions for edge/front detection would be nice.