grame-cncm / faust

Functional programming language for signal processing and sound synthesis
http://faust.grame.fr
Other
2.51k stars 318 forks source link

en.adsr not working #115

Closed agraef closed 6 years ago

agraef commented 6 years ago

Hi @rmichon, I'm currently experimenting with (finally) getting some of my Faust examples ported to the new library. Hey, better late than never! :)

However, one issue I've been running into is that your implementations of en.adsr and en.adsre don't work for me. They just seem to trigger release immediately after the decay phase, not after the gate signal drops to zero. If I drop in Yann's original adsr implementation from music.lib, the synth works just fine.

I've attached one of my programs, a little subtractive synth, exhibiting this misbehavior. The Pd patch is also included so that you can run this using pd-faust if you've got that installed (run make beforehand), but of course you can also run the mysub.dsp program in FaustLive as a polyphonic synth instead.

mysub.zip

image

To reproduce the bug, just hook up a MIDI keyboard, play a note and hold it. It dies out immediately, but I'd expect it to be sustained until the key is released. Modify the mysub.dsp program by changing en.adsr to just adsr, recompile and run the same test again, now the note is sustained until you release the key.

agraef commented 6 years ago

Here's the code of the mysub.dsp program for easier reading:

declare name "mysub";
declare description "saw wave filtered with resonant lowpass";
declare author "Albert Graef";
declare version "1.0";
declare nvoices "16";

import("stdfaust.lib");

// control variables

// master volume and pan
vol = hslider("vol", 0.3, 0, 10, 0.01); // %
pan = hslider("pan", 0.5, 0, 1, 0.01);  // %

// ADSR envelop
attack  = hslider("attack", 0.01, 0, 1, 0.001); // sec
decay   = hslider("decay", 0.3, 0, 1, 0.001);   // sec
sustain = hslider("sustain", 0.5, 0, 1, 0.01);  // %
release = hslider("release", 0.2, 0, 1, 0.001); // sec

// filter parameters
res = hslider("resonance (dB)", 3, 0, 20, 0.1);
cutoff  = hslider("cutoff (harmonic)", 6, 1, 20, 0.1);

// voice parameters
freq    = nentry("freq", 440, 20, 20000, 1);    // Hz
gain    = nentry("gain", 1, 0, 10, 0.01);   // %
gate    = button("gate");           // 0/1

// resonant lowpass

// This is a tweaked Butterworth filter by David Werner and Patrice Tarrabia,
// see http://www.musicdsp.org and http://www.experimentalscene.com for
// details.

// res = resonance in dB above DC gain
// freq = cutoff frequency

lowpass(res,freq)   = f : (+ ~ g) : *(a)
with {
    f(x)    = a0*x+a1*x'+a2*x'';
    g(y)    = 0-b1*y-b2*y';
    a   = 1/ba.db2linear(0.5*res);

    c   = 1.0/tan(ma.PI*(freq/ma.SR));
    c2  = c*c;
    r   = 1/ba.db2linear(2.0*res);
    q   = sqrt(2.0)*r;
    a0  = 1.0/(1.0+(q*c)+(c2));
    a1  = 2.0*a0;
    a2  = a0;
    b1  = 2.0*a0*(1.0-c2);
    b2  = a0*(1.0-q*c+c2);
};

// AG: Try this as a drop-in replacement for en.adsr, it sounds different!!
adsr(a,d,s,r,t) = env ~ (_,_) : (!,_) // the 2 'state' signals are fed back
with {
    env (p2,y) =
        (t>0) & (p2|(y>=1)),          // p2 = decay-sustain phase
        (y + p1*u - (p2&(y>s))*v*y - p3*w*y)    // y  = envelop signal
    *((p3==0)|(y>=eps)) // cut off tails to prevent denormals
    with {
    p1 = (p2==0) & (t>0) & (y<1);         // p1 = attack phase
    p3 = (t<=0) & (y>0);                  // p3 = release phase
    // #samples in attack, decay, release, must be >0
    na = ma.SR*a+(a==0.0); nd = ma.SR*d+(d==0.0); nr = ma.SR*r+(r==0.0);
    // correct zero sustain level
    z = s+(s==0.0)*ba.db2linear(-60);
    // attack, decay and (-60dB) release rates
    u = 1/na; v = 1-pow(z, 1/nd); w = 1-1/pow(z*ba.db2linear(60), 1/nr);
    // values below this threshold are considered zero in the release phase
    eps = ba.db2linear(-120);
    };
};

// subtractive synth (saw wave passed through resonant lowpass)

process = os.sawtooth(freq) : ((env,freq,_) : filter) : *(env * gain)
        : (*(vol:dezipper(0.99)) : sp.panner(pan:dezipper(0.99)))
with {
  env = gate : en.adsr(attack, decay, sustain, release);
  filter(env,freq) = lowpass(env*res, ma.fmax(1/cutoff, env)*freq*cutoff);
  dezipper(c) = *(1-c) : +~*(c);
};
agraef commented 6 years ago

Massaged the program a bit so that the control variables are in a sane order:

mysub.zip

image

josmithiii commented 6 years ago

Hmmm, I get no sound at all with faust2caqt in even the attached super-simplified version which uses only "process = os.oscrs(freq) * gain <: ,;".

I do get sound in other simple f2ca tests, so this is very mysterious. This is probably a High Sierra QT4 issue. I will drill down when I can.

I added adsre, by the way, and it passed my tests at the time. Therefore I most suspect a change in one of the functions/primitives used.

On Mon, Jan 15, 2018 at 9:08 AM, Albert Graef notifications@github.com wrote:

Massaged the program a bit so that the control variables are in a sane order:

mysub.zip https://github.com/grame-cncm/faust/files/1632432/mysub.zip

[image: image] https://user-images.githubusercontent.com/2853977/34953967-e9ef09d4-fa1e-11e7-87fa-af35d839cb0f.png

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faust/issues/115#issuecomment-357741162, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFda7plAbq_9EaHQqqV_9Ghxab2Kcks5tK4YFgaJpZM4ResiG .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

rmichon commented 6 years ago

Hi @agraef and @josmithiii :),

That's because the sustain should be in percent and not a gain between 0 and 1. Since the default value of your sustain is 0.5, which is 0.5% of the gain after the attack, you don't hear any sound. I thought that's how the old ADSR used to work but I might be wrong... We can change it so that sustain is configured as a number between 0 and 1 instead of a percentage. What do you think? @josmithiii I think you get no sound because the nvoices metadata is declared which forces the compilation of a polyphonic synth that will only respond to MIDI messages. Getting rid of that line should solve your problem (at least, I think so haha).

josmithiii commented 6 years ago

Great, thanks!

I vote for sustain level between 0 and 1, but that's going to break a lot of code, so I apologize for my vote. :-)

On Mon, Jan 15, 2018 at 5:07 PM, Romain notifications@github.com wrote:

Hi @agraef https://github.com/agraef and @josmithiii https://github.com/josmithiii :),

That's because the sustain should be in percent and not a gain between 0 and 1. Since the default value of your sustain is 0.5, which is 0.5% of the gain after the attack, you don't hear any sound. I thought that's how the old ADSR used to work but I might be wrong... We can change it so that sustain is configured as a number between 0 and 1 instead of a percentage. What do you think? @josmithiii https://github.com/josmithiii I think you get no sound because the nvoices metadata is declared which forces the compilation of a polyphonic synth that will only respond to MIDI messages. Getting rid of that line should solve your problem (at least, I think so haha).

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faust/issues/115#issuecomment-357824411, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFdTAZDaOZhheagSnxJ-RzU7QSIUBks5tK_ZhgaJpZM4ResiG .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

agraef commented 6 years ago

@rmichon, thanks, I missed that. But that introduces an incompatibility with Yann's adsr which has been around forever, for (IMHO) no good reason. We use normalized (0..1 or -1..+1) values elsewhere, where it makes any sense, and mathematically a percent value is just a 0..1 value in disguise, so why not just stick to the 0..1 range there?

I can live with this, although as a mathematician, I find this inelegant (pardon my French). It's at least something we should think about. A sustain level is nothing but a relative gain value, so using percents there rather arbitrarily forces you to divide by 100 wherever this value is used. Percents are for toddlers who only know base 10. The 0..1 range is universal and for grown-up women and men who understand the significance of $e^{i\pi}+1=0$. Ok, ranted enough for now. πŸ˜ƒ

agraef commented 6 years ago

If after reading my rant everybody still thinks that sustain should be a % value then feel free to close this bug. But since almost any of my suggestions finally made it into Faust after first being furiously rejected, I'll trust you to do the right thing there. ;-)

josmithiii commented 6 years ago

If Yann's original adsr had sustain level as a 0-1 fraction, then I vote _way_morestrenuously in favor of that! The newer envelopes have not been around very long --- maybe not even in an official release yet? (MacPorts is still at version 0.9.85.)

On Mon, Jan 15, 2018 at 9:22 PM, Albert Graef notifications@github.com wrote:

If after reading my rant everybody still thinks that sustain should be a % value then feel free to close this bug. But since almost any of my suggestions finally made it into Faust after first being furiously rejected, I'll trust you to do the right thing there. ;-)

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faust/issues/115#issuecomment-357856959, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFY5z-6vSQpY_DaKmzMFp4nVodIPIks5tLDItgaJpZM4ResiG .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

agraef commented 6 years ago

Yann's version most definitely uses the 0-1 range, you can still find that in old/music.lib.

@josmithiii, sorry for the lack of updates in MacPorts, I haven't used my Mac much lately. Hope I'll get around to updating those packages soon.

rmichon commented 6 years ago

I'm totally with you on this guys! I guess the only reason why I made this change when I re-implemented adsr is because the old documentation says "percentage of max," which is kind of confusing in this case. Anyway, I just corrected this and I updated all the corresponding examples. Unfortunately, it is likely that this will break lots of code (including all the tutorials on my website), we'll have to fix things on the spot as people start complaining about this haha.

sletz commented 6 years ago

And possibly annonce this change on the mailing list?

(BTW/remainder : we'll have to write systematic/automatic libraries test code at some point...)

agraef commented 6 years ago

Cool, thanks!

@rmichon brace yourself for students running last semester's code -- using a sustain value of 90 will teach them to slowly increase volume when first testing their Faust programs, haha. ;-)

agraef commented 6 years ago

@sletz, can we also have a new release tagged from Romain's latest commit please? Would be nice to have a clean slate to which I can update all those prehistoric faust 0.x packages in Arch and Ubuntu. :) Or are you currently in progress with something? Then the big winter faust package cleanup will have to wait some more.

agraef commented 6 years ago

@rmichon, sorry, but I have to reopen this, as you forgot to update the libraries submodule on faust:master-dev. It's still at rev. b5a04a60 which dates from Dec 2!

agraef commented 6 years ago

Note that it's not enough to just update the libraries repo itself, after committing the changes in the submodule you also need to commit the changed hash in the supermodule (faust:master-dev, as it were).

Yeah, git submodules are a bit hard to use. :-/ Recommended read: https://git-scm.com/book/en/v2/Git-Tools-Submodules

rmichon commented 6 years ago

@agraef Yes, I know and I thought I took care of that. The submodule does seem to have been updated in this commit: https://github.com/grame-cncm/faust/commit/05d495b0c5a0d395ae6906eccc016b738c12a4bd (scroll down to the bottom) There might be a problem with your local repo?

rmichon commented 6 years ago

@agraef @sletz +1 for the release. Should we wait for that to announce this change to the mailing list or should we do this right away?

agraef commented 6 years ago

@rmichon Oops, sorry, brain fart on my side, I looked in the wrong place (source @ v2-5-10).

@sletz Having a release at this point would be really nice since I could then hand my faust package over to David Runge in a clean state. David is about to get this into Arch proper (official package repositories) and maintain it from there on, which will reduce my burden a little (I have to maintain way too many packages myself right now, last time I checked it was well over a hundred on Arch alone, which is insane).

agraef commented 6 years ago

@rmichon, the library submodule in faustlive (at Resources/Libs) still needs to be updated, though. Who is going to do that?

rmichon commented 6 years ago

Ah, I was going to do it but I see that @sletz already took care of it. @sletz, the man who commits faster than his shadow ;)...