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

Envelope resets the amplitude of an oscillator to one on play #3

Closed
defrost256 opened this issue Sep 5, 2018 · 3 comments
Closed
Labels
enhancement New feature or request

Comments

@defrost256
Copy link
Contributor

In the Env.play() method the value for the envelope data for the SegmentedEnvelope assumes the value of the attack is 1f instead of the expected input.amp, taking control away from the developer.
Easy fix:

SegmentedEnvelope env = new SegmentedEnvelope(new double[] {
				attackTime, input.amp, // attack
				// gradual decay towards sustain level across entire sustain period
				sustainTime, sustainLevel, // sustain
				releaseTime, 0.0 });

instead of

SegmentedEnvelope env = new SegmentedEnvelope(new double[] {
				attackTime, 1.0, // attack
				// gradual decay towards sustain level across entire sustain period
				sustainTime, sustainLevel, // sustain
				releaseTime, 0.0 });

On another note I think sustain level might be more intuitive represented as a fraction of amplitude as well

SegmentedEnvelope env = new SegmentedEnvelope(new double[] {
				attackTime, input.amp, // attack
				// gradual decay towards sustain level across entire sustain period
				sustainTime, sustainLevel * input.amp, // sustain
				releaseTime, 0.0 });

I'm going to make a PR for the first one shortly, since that's an actual problem, but I'll wait with the second one in case there is discussion to be had about it.
Keep up the good work!

defrost256 added a commit to defrost256/processing-sound that referenced this issue Sep 5, 2018
Set the attack amplitude of the Envelope to the amplitude of the input SoundObject.

Fixing Issue processing#3
@kevinstadler kevinstadler reopened this Sep 6, 2018
@kevinstadler
Copy link
Collaborator

Oops sorry I didn't see the more contentious second point about the sustain level in the issue! Yes I think representing the sustain level as a fraction of the overall amplitude makes sense, so if you're happy to do a pull request for that go on!

defrost256 added a commit to defrost256/processing-sound that referenced this issue Sep 6, 2018
Changed sustain level to be relative to the amplitude of the input SoundObject as discussed in Issue processing#3
@defrost256
Copy link
Contributor Author

Hey, thanks for listening, I am afraid these changes might break the library for "legacy" users. Hope they won't come at me with torches and pitchforks 🔥

@kevinstadler
Copy link
Collaborator

I think if you let the envelope's amplitude alone it's fully compatible with the old version of the library, where the absolute amplitude couldn't be controlled in the first place! Thanks for the PR, there's another sound card-selection bug fix in the pipeline so this one should be available from the content manager soon...

@kevinstadler kevinstadler added the enhancement New feature or request label Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants