search

LancePutnam / Gamma

Generic (Sound) Synthesis Library
Other
458 stars 54 forks source link

Inconsistent LFO heights #34

Closed mhetrick closed 8 years ago

mhetrick commented 8 years ago

The LFO class has inconsistent amplitudes for the various shape outputs.

As an example, LFO.pulse() runs from -.5 to +.5, while LFO.cos() runs from -1.0 to +1.0.

It would be great if these were standardized. Currently, writing an LFO selector means testing the height of each shape and adding a scalar for the inconsistent shapes.

My vote would be for -.5 to +.5 for all bipolar LFOs, as these would maintain the same amplitude as the unipolar LFOs. I can submit a pull request with code, but I'd like to know the intended design before I do so.

LancePutnam commented 8 years ago

The design is that bipolar signals fit the range [-1,1] and unipolar signals [0,1]. I think these normalized ranges are pretty standard/expected, so would be hesitant to change them.

AFAIK, only pulse could be called nonstandard. It is +/-0.5 with half pulse width, but when the pulse width is near zero or one, then the signal reaches +/-1.

Are there other shapes that don't fit the intended design?

mhetrick commented 8 years ago

Would you be more receptive to a flag to toggle that Pulse behavior, or a different Pulse output with static amplitude? I've seen both behaviors on hardware synths, but I think static amplitude is much more common.

LancePutnam commented 8 years ago

I think a new pulse function would be best. scl::pulseU just branches on 0 and 1, so it could be generalized to take high and low values as arguments. Might not make sense to simply overload pulseU since I wouldn't consider it unipolar. A new function might be best, but a good name eludes me right now...

mhetrick commented 8 years ago

pulseMax, pulseQuant, biPulse, boundPulse, pulse2?

Let me know when you have an approved name, and I'll submit the pull request.

LancePutnam commented 8 years ago

I think pulseRange is best. Short, familiar, and mathematically accurate. I prefer naming general to specific to give better results with alphanumeric sorting. The order of interval arguments in Gamma go max, min since you often want default min=0. I suggest following the same convention to maintain consistency.

mhetrick commented 8 years ago

I've pushed two separate commits and made a pull request as #36. I've separated the commits, just in case you think either portion of the implementation is sloppy or outside of Gamma standards. I've tested it in our plug-ins, and it behaves as anticipated for us.

LancePutnam commented 8 years ago

Looks okay to me.