jamoma / jamoma2

A header-only C++ library for building dynamic and reflexive systems with an emphasis on audio and media.
MIT License
30 stars 6 forks source link

Phasor UnitTest needs to test the object output #35

Closed tap closed 9 years ago

tap commented 9 years ago

Should be easy to do for anyone that wants to do it...

nwolek commented 9 years ago

Looks like the method used for computing step size in this code is slightly different than the one I created in my Octave comparison code. So this is going to take some research to figure out which is right. The difference is about 0.00000515, but it adds up with each step.

nwolek commented 9 years ago

@tap - would also appreciate your opinion: should Phasor require an input? I kind of think it should be like UnitImpulse or WhiteNoise, but I could be convinced otherwise.

Right now it is like this: auto out = phasor( impulse() );

I think it should instead be: auto out = phasor();

nwolek commented 9 years ago

Back to difference in computing the step size, which seems to hinge on computing rampSamples first like here in JamomaCore. Not sure storing this value is necessary. YAGNI?

tap commented 9 years ago

I agree that phasor should be a generator, though maybe an optional input to toggle it on/off would be useful?

As for the ramp time in samples in JamomaCore, I think that is flawed. What if your cycle time in milliseconds is not an integer number of samples? Then you will have drift and error -- and phasor is the place you can't afford that because it is the type of object you use to drive all of your timing!

nwolek commented 9 years ago

Good tests results so far with the new algorithm. Timing looks improved. Phasor now behaves like a generator and requires no input.

Regarding starting/stopping: I think that is a better description for a Ramp. Let's not put the overhead to check starting/stopping into Phasor. Would you agree @tap?

Just making a note for later: I need to add a test that runs across multiple SampleBundle outputs then I think this issue will be closed. Unless somebody thinks of another test that is needed.

tap commented 9 years ago

Thanks @nwolek ! I appreciate your work -- and your success!

I disagree that sample accurate / synchronized start is not useful for a phasor. If you are concerned about the computation of that checking then make me a ticket and I'd be happy to investigate some ways of avoiding the overhead when it is not needed -- I believe there are things we can do there.

Takk!

nwolek commented 9 years ago

Since @tap most of this, I feel confident enough to proceed with merge into master and closing the issue.