Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multipoint Envelope #53

Open
pingdynasty opened this issue Aug 26, 2016 · 4 comments
Open

Multipoint Envelope #53

pingdynasty opened this issue Aug 26, 2016 · 4 comments
Assignees

Comments

@pingdynasty
Copy link
Collaborator

Multipoint envelope with rate and level per point, same as the DX7 5-point envelopes.
The constructor should ideally take an array to hold the state and the size e.g.
MultipointEnvelope(float* points, int size) where points is 2 x size.
Define a static MultipointEnvelope::create(int points) which allocates the array.
Define functions that set the points with a point index, e.g. setRate(int point, float value).

@pingdynasty
Copy link
Collaborator Author

@giuliomoro
Copy link
Collaborator

do we need membrane buttons as well?

Is it (static) `FloatArray MultipointEnvelope::create(int points)' ?

what is value in setRate ? time in seconds?
Should there not be a setLevel as well?

An ADSR Envelope is a particular case of a MultipointEnvelope, should it be refactored accordingly (inheriting from it)?

Should the transition here be linear or exponential (see #48) ?

@pingdynasty
Copy link
Collaborator Author

no static MultipointEnvelope* MultipointEnvelope::create(int points).

So across the library, the constructors should never allocate dynamic memory. The only methods that allocate and deallocate memory are the static 'create' and 'destroy' methods. And there should be at least one create (and destroy) for each non-trivial object.

Yes I think rate should be seconds, I guess, same as ADSR. And yes we need setLevel. And/or void setPoint(int index, float rate, float level). And membrane buttons.

Linear transitions, and no we don't need to refactor ADSR.

@giuliomoro
Copy link
Collaborator

giuliomoro commented Aug 29, 2016

Done cf78d8f.

Done mostly with code borrowed from ADSR ( which means a refactoring of ADSR as child class of MultipointEnvelope is desirable).

Current caveats:

  • for any given point it expects setLevel() to be called before setRate(). It expects setPoint to be called in order (i.e.: setPoint(n, seconds, value) should come before setPoint(n+1, seconds, value) ). This is because the sign of the difference between the current and the previous value is used to compute the increment. It is cheaper to only do it once when the rate/level is set than for each output sample. Also, when changing a point, the next point must be re-set as well. It is not easy to find a compromise between usability and performance here.
  • the time in seconds passed to setRate() should correspond to the actual time regardless of the level of the current and previous point (it is constant time implementation, as opposed to constant rate). The line
    194 float increment = samplePeriod / seconds * diff;
    should be doing just that, but this needs to be checked on a scope.
  • there are no hard limits imposed, so there are points where division by zero may occur (e.g.: setRate(n, 0)), or a stage may take forever (e.g.: setRate(n, 0.5), setRate(n+1, 0.5 + eps). If eps is small enough, the increment is going to be 0 or very close to zero because of limited precision. Even if the increment is non-zero, it may be small enough that when added to currentLevel it does not change its value (because of limited precision again), so the envelope will get stuck at that point because the condition will never be met. We can fix this using fixed math for the accumulator(safer but more expensive), or enforcing minimum increment values. More in general, when the difference between levels of consecutive points is very small the timing is likely to screw up.

Additional notes:

  • I believe an exponential version of this should only re-implement updateCurrentValue(), checkPointReached(), setRate(), leaving the rest of the logic in place. Also, re-implementing getSustainValue() would allow for smooth transitions of currentLevel when you are at the sustain stage and you tweak the sustain knobs: in some analog ADSRs envelopes the change in output level in this case is smoothed by the Decay parameter.
  • sustainPoint, which is the point at which the envelope stops until the gate is on, defaults to numStages - 1, so you have a multi-stage attack, then sustain, then single-stage release. This can be adjusted with setSustainPoint().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants