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 time modifier #3372

Closed
wants to merge 9 commits into from
Closed

Conversation

zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Feb 19, 2017

envelope

Just some things I've wanted to test since #411
Also in response to issue #3223
I think It's a good idea to make dangerously long envelopes a conscious decision.

I never liked the 0 - 2.0 value for the envelope times. This fixes it right up and instead adds a check box to turn on longer envelope times.

TODO

  • The upgrade path isn't 'tuned in' just yet.
  • The check box context menu lacks a 'help' button which this knob would benefit from.
  • One could also scrap the check box and just tweak the knob value range to 0 - 1.0 .

@jasp00 You suggest here to make the envelope knobs response linear. If this is still a plan now would probably be a good time.
https://github.com/LMMS/lmms/blob/master/src/core/EnvelopeAndLfoParameters.cpp#L407

@zonkmachine
Copy link
Member Author

I bump the buffer amount to 1024. It's the fastest way to reduce the risk of running out of buffers like in #3223 . You could go higher but there is a time penalty to it. I haven't looked at precisely how much the cost in CPU cycles is to do this. From my testing, pretty much negligible.

@jasp00
Copy link
Member

jasp00 commented Mar 17, 2017

You suggest here to make the envelope knobs response linear. If this is still a plan now would probably be a good time.

It is still a plan because it is an optimization to avoid the calculations from expKnobVal(); for a (-1.0, 2.0) range, it means to switch to (-1.0, 4.0) and upgrade old data. This can be done in a different PR.

I never liked the 0 - 2.0 value for the envelope times. This fixes it right up and instead adds a check box to turn on longer envelope times.

Then the knob is probably not the proper widget.

@ivhacks
Copy link
Contributor

ivhacks commented Mar 17, 2017

I think I have found a bug: When I open a project file that was made using normal master LMMS, the envelopes on my instruments are way too long. I tried a few different project files and I've seen this behavior on all of them. When I made a project using my build of this pull request, it saved and reopened properly. Here's links to a video of this behavior and the MMPZ.

https://drive.google.com/open?id=0B8dZ3IVOKKA6VXN2aTdzT0JkSGc

lmms.io/lsp/?action=show&file=10488

Interesting feature, could be very useful :)

@grejppi
Copy link
Contributor

grejppi commented Mar 17, 2017

The choice between 1x and 5x feels arbitrary to me. What if it was an LCD spin box with a range of 1-5 instead?

@Umcaruje Umcaruje added this to the 1.3.0 milestone May 14, 2017
@zonkmachine
Copy link
Member Author

I'm closing for the time being.

@zonkmachine
Copy link
Member Author

PS. Thanks for the reviews.

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

Successfully merging this pull request may close these issues.

5 participants