RobinSchmidt / RS-MET

Codebase for RS-MET products (Robin Schmidt's Music Engineering Tools)
Other
56 stars 6 forks source link

EQ and Delay effects are "exploding", no idea how to debug #176

Closed elanhickler closed 6 years ago

elanhickler commented 6 years ago

Hey Robin, the EQ and Delay effects I added explode when in use. Using an EQ curve other than flat or a delay feedback other than 0 quickly has the effect going to infinite amplitude. Is there something I could have done wrong? Unfortunately I've made a handful of changes to my fork of RS-MET so you can't debug with your base RS-MET. If you learned to use branches you could temporarily use my branch.................................................... but maybe you can tell me how to debug or what I did wrong.

RobinSchmidt commented 6 years ago

did you make changes to the dsp code? especially in the coefficient computations? for debugging i would set a breakpoint in rosic::Equalizer::getSample (and watch it explode sample-by-sample). the class has a std::vector "bands" of TwoPoleFilter units. i would open the bands vector and investigate the variables - filter coefficients and such and look for anything strange...maybe also something like a cutoff frequency above half the sample-rate. do the plots still plot correctly or are they messed up, too?

elanhickler commented 6 years ago

nooo no no no, this is your DSP code, I don't want to debug that!

All I'm doing is adding your effects to my plugin like a normal person would do.

Any code I'm changing is cosmetic, mostly widget colors and sometimes I need access to a setting on some protected member, but none of this has anything to do with EQ or Delay.

elanhickler commented 6 years ago

All I do is override paint and resized for my own Equalizer Editor. image

classes in the Module image

instantion in Module image

simple process block function for Module image

elanhickler commented 6 years ago

M-Ref dev which has an EQ still works, so I must have some kind of feedback loop.............. HOWW

elanhickler commented 6 years ago

specifically the EQ is self-resonating, but it's kinda ugly sounding, it's distorted, buzzy, not typical normal self-oscillation.

edit: and it crashes when you remove a point. why is it so easy to fuck everything up. doesn't happen with M-Rev dev.

RobinSchmidt commented 6 years ago

ok, so what would i have to do to reproduce this over here?

RobinSchmidt commented 6 years ago

oh - wait - i'm getting a crash when removing a band, too...

elanhickler commented 6 years ago

I would have to make my library compatible with the base RS-MET again or you'd have to get on my fork temporarily.

Edit: ok

RobinSchmidt commented 6 years ago

wait - i'll look into the crash i'm getting in toolchain first.. who knows, maybe it automagically fixes the other stuff too

RobinSchmidt commented 6 years ago

oh fuck! this was a big bug! i recently renamed ParameterObserver::parameterIsGoingToBeDeleted to the shorter parameterWillBeDeleted - but apparently i did not replace all overrides. i actually did a text replacement in the whole solution but apparently something went wrong. so we had a lot of dangling pointers when parameters were dynamically deleted at runtime (which is what happens when you remove an eq band, for example). because the deletion notification was broken. i fixed that now. it fixes the crash when removing the selected band in the eq. ...maybe it fixes the blow-up, too?

elanhickler commented 6 years ago

no more crash deleting points, but plugin still explodes

also, could you remove "reset" for delay or make it optional when changing time?

If you have smoothing on delay time, the plugin freezes as the smoother set the delay time over and over, causing a reset on the buffer over and over, freezing the software.

elanhickler commented 6 years ago

oops forgot to say which delay: Quadrifex Ping Pong delay

image

RobinSchmidt commented 6 years ago

ok, yes - i removed the reset. does not really seem to make sense anyway. as for the blow-up - yes, i guess i need to try that myself. so how shall we do this? do you want to make yourself compatible with my version or should temporarily i switch to your fork (if so, how would i do this?)

elanhickler commented 6 years ago

I'm ALWAYS always asLFIsndfkljsdf always available for you online if you want to hop on and I can direct you in doing this. It would be easy for me to kinda give you a tutorial on branches and the possible pitfalls.

👍

RobinSchmidt commented 6 years ago

is it that complicated? i actually prefer to read stuff, tbh. maybe i should work my way through one of the many git tutorials online. can you recommend one?

edit: i actually thought that forks and branches are quite different concepts (fork: someone else makes a copy of the repo and edits it, branch: several lines of development within one repo ...or something)

elanhickler commented 6 years ago

i prefer not to read so I can't recommend one.

But don't you use a git GUI like SourceTree? If so, maybe there's a tutorial on sourcetree. Or maybe you'd rather learn how git actually work, but I like to learn the opposite way. Using SourceTree GUI gives me a good idea of how it works.

elanhickler commented 6 years ago

main difference between our branches is some hack code I added to change button colors: image (more specifically, I changed the "enabled" color)

RobinSchmidt commented 6 years ago

how about i just add that button-painter delegation as i did for sliders (did you actually try to use that already)? then you don't need a "hack" anymore but can "officially" change the painting

elanhickler commented 6 years ago

that should work.

So I can add it only to specific buttons?

didn't use the slider painter yet.

RobinSchmidt commented 6 years ago

yes - you would just call myButton->setPainter(myButtonPainter) - assuming you have a myButtonPainter object lying around somewhere. it will only affect those buttons to which you pass the painter, different buttons can use totally different painters, too, etc.

elanhickler commented 6 years ago

sounds good!

RobinSchmidt commented 6 years ago

ha! it was actually already there. but the naming conventions were slightly different from the corresponding slider stuff, so i just made that consistent. you need to subclass RButtonPainter, implement the paint method there, somewhere have an object of that subclass and pass it to the buttons that should use it via setPainter()

RobinSchmidt commented 6 years ago

let's hope, it works as expected...

elanhickler commented 6 years ago

RButtonPainter is quite a wimpy class

image

Isn't this kind of thing the perfect use case for passing in a function like std::function? I dunno the details of how it'd work, just an idea.

edit: oh wait, the point of a class is you could make the painter more complex with constructor options. I guess that's good. I could also put an std::function as a member in my RButtonPainter superclass. Nevermind.

elanhickler commented 6 years ago

aaaaghh!!!!!!!!!!! image

elanhickler commented 6 years ago

alright, you're going to have to make some things unprotected....... ill do it and make a pull request.

elanhickler commented 6 years ago

actually no, you'll probably want to make a function for something like this.

RobinSchmidt commented 6 years ago

ah, ok - yeah - what do you need? getFont()? something else? edit: getFont has been added

RobinSchmidt commented 6 years ago

oh wait, the point of a class is you could make the painter more complex with constructor options.

yeah - or something like: setButtonUpImage, setButtonDownImage, etc.

RobinSchmidt commented 6 years ago

and drawBitmapFontText is a global function, not a member function of RButton, so the b-> must be deleted for this call

elanhickler commented 6 years ago

i need function:

AudioModule::getModuleNameAppendix() { return moduleNameAppendix; }

and then we are compatible.

RobinSchmidt commented 6 years ago

done

RobinSchmidt commented 6 years ago

Isn't this kind of thing the perfect use case for passing in a function like std::function

the perfect use case for this is actually my new rsRootFinder class, for example:

template<class T>
T rsRootFinder<T>::bisection(std::function<T(T)>& f, T xL, T xR, T y)
{
  static const int maxNumIterations = 60; // should be enough for double-precision
  T tol = std::numeric_limits<T>::epsilon();
  T fL  = f(xL) - y;
  T xM, fM;
  for(int i = 1; i <= maxNumIterations; i++) {
    xM = T(0.5)*(xL+xR);
    fM = f(xM) - y;
    if(fM == 0 || xR-xL <= fabs(xM*tol)) 
      return xM; // done
    if(fL*fM > 0) { xL = xM; fL = fM; }
    else          { xR = xM;          }
  }
  //rsError("rsRootFinder::bisection failed to converge");
  return xM;
}

i'm using it with a lambda-function in my unit-test suite. nice. i guess a function object (functor) will work just as well. ...and regular function pointers will work anyway, i think. i can assign all these things (lambda-functiosn, functors, function-pointers) to a std::function object with a matching signature, right?

RobinSchmidt commented 6 years ago

...yup - just tested it. works like clockwork. really nice. already have some idea how to use that for ultra-precise zero-crossing location in audio-signals...

RobinSchmidt commented 6 years ago

namely - define a functor class that sinc-interpolates the audio signal and use the root-finder to find the zero-crossing of that sinc-interpolant functor. elegant!

elanhickler commented 6 years ago

does this mean you will make a better frequency locking tuner? we still need that for OTS (well, it just has a weird unexpected drift problem at the moment)

RobinSchmidt commented 6 years ago

iff the problem is the precision of the zero-crossing locations, then probably yes

RobinSchmidt commented 6 years ago

currently, i fit a polynomial interpolant and determine the zero crossing of that interpolant - which should not really be soo bad, i think. however, the sinc-interpolant is actually the proper interpolant, so it may improve things (of course, it will probably be costly in terms of cpu - but as it's a non-realtime algo anyway, that may be acceptable)

RobinSchmidt commented 6 years ago

btw. - there are some audio-dsp textbooks and papers that state things like that you can locate zero-crossings only to sample-accuracy. ha ha!

elanhickler commented 6 years ago

good. now you are more superior than them!

RobinSchmidt commented 6 years ago

do recent samplers actually allow to set loop points with subsample accuracy? it's a long time ago i used a sampler but back then you could set loop points only to integer sample values

elanhickler commented 6 years ago

I don't think there's a point in subsample accurate loop points unless an algorithm is detecting a good loop point.

RobinSchmidt commented 6 years ago

think of looping a single cycle. or maybe two or 3 cycles. if the loop points are accurate only up to sample instants, the frequency may audibly diverge from where it should be

elanhickler commented 6 years ago

ok

RobinSchmidt commented 6 years ago

this comes to my mind because there was a time when i created sampler patches with a short sample from the beginning of the recording. think of an acoustic guitar - let it go through its transient and after a few tenths of a second, enter a loop of a single cycle. you would then need only a very short sample (say, half a second) to capture the transient and the steady state part would be created by looping a cycle. but here, you need to be subsample accurate. ...and a subsample-accurate zero-crossing finder could give you your loop points - but the sampler must then support subsample loop points

RobinSchmidt commented 6 years ago

a bit similar to this "LA" (linear arithmetic - whyever, the term makes no sense to me) synthesis by roland - just that we take the steady part also from the sample instead of synthesizing it by other means

elanhickler commented 6 years ago

don't be anti-crossfade

RobinSchmidt commented 6 years ago

i think, crossfade is nice for longer loops - but for very short loops containing just one or a few cycles - you should not need a crossfade to "cover up" a signal discontinuity (especially, if the discontinuity is only an artifact of temporal loop-point inaccuracy). also, crossfades don't solve the frequency deviation problem

elanhickler commented 6 years ago

uhh, but there is a discontinuity no matter what, unless the waveform is perfectly synthesized. Might as well average the two cycles with a 100% crossfade to make it perfect.

RobinSchmidt commented 6 years ago

there is a discontinuity no matter what

why - because the next cycle is slightly different from the current? ok, but i think this effect is very much (orders of magnitude maybe?) below the (worst case) artifacts that can be expected from having slightly wrong loop points

Might as well average the two cycles with a 100% crossfade to make it perfect.

hmmm...ok...yes - that should work. but it doesn't solve the frequency deviation problen. ...maybe you could solve it by using a different speed in the loop than in the transient - but i really think, subsample loop points are the more elegant and more proper solution to the problem