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.

Sign up for GitHub

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

Jump to bottom

Fixing the arpeggiator #3858

Merged
merged 1 commit into from
Oct 9, 2017
Merged

Fixing the arpeggiator #3858

merged 1 commit into from
Oct 9, 2017

Conversation

zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Oct 5, 2017 •
edited
Loading

Fixing some of the issues with the arpeggiator.

  • The decay of the last arpeggio note is added to the length of the master note so after the first run there will be extra notes played along with the arpeggio, forming chords or doubling the notes for increased volume.
    * * *Fix reverted* * *

  • Sort mode. If the envelopes are off, the notes that should be silent are playing as ordinary notes while waiting for their turn. Solving this by using the same hack as the 'skip' algorithm as it had the same issue.
    Set as master note.

Adresses #3342

Issues remaining

  • Sort mode. Sometimes notes are skipped.
  • Sort mode. There is no way to know in which order the notes are being handled without displacing them. This is a pretty good way to deal with it actually but it would be better if the default would be to start with the lowest note and then work our way up. To fix this we probably need to sort the cnphv after key value.
  • Skip function. This may only apply to lb302... If you drag an lb302 over a track with envelopes those affect the lb302 in such a way that notes keep sounding, in a legato kind of way, for as long as notes after it are skipped.

Edit: New arpeggiator meta issue opened here: #3880

@zonkmachine zonkmachine added this to the 1.2.0 milestone Oct 5, 2017
@zonkmachine
Copy link
Member Author

zonkmachine commented Oct 5, 2017 •
edited
Loading

Note:
Arpeggiator sometimes drops notes when handling more than one base note at a time. This is true for all sync modes and is not related to this PR.

@zonkmachine
Copy link
Member Author

zonkmachine commented Oct 6, 2017 •
edited
Loading

This fixes #340
Testing appreciated. I'll merge this tomorrow if no one objects.

@zonkmachine zonkmachine changed the title Decay length shouldn't affect the length of the arpeggiated master note Fixing the arpeggiator Oct 7, 2017
zonkmachine
zonkmachine commented Oct 7, 2017
View reviewed changes
src/core/InstrumentFunctions.cpp Outdated
@@ -378,9 +378,7 @@ void InstrumentFunctionArpeggio::processNote( NotePlayHandle * _n )
// used for calculating remaining frames for arp-note, we have to add
// arp_frames-1, otherwise the first arp-note will not be setup
// correctly... -> arp_frames frames silence at the start of every note!
int cur_frame = ( ( m_arpModeModel.value() != FreeMode ) ?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm dropping this test altogether but perhaps it should be kept and changed to m_arpModeModel.value() = SyncMode ?

@zonkmachine
Copy link
Member Author

zonkmachine commented Oct 7, 2017 •
edited
Loading

I went ahead and fixed up some other issues. Merge will wait.

@zonkmachine
Copy link
Member Author

I think I'm done with the arpeggiator now for 1.2 . I reverted one of the changes but the rest is fine to go and also took the liberty of bumping the default arp time from 100ms to 200ms. half the speed that is. Else it will just fire away and scare our beloved beginners. The fixes here basically takes care of regressions from 1.1.3.

I'll merge this in a day if there is no objection.

@zonkmachine zonkmachine force-pushed the arpnotedecay branch 2 times, most recently from 839fdb3 to b2ea4b6 Compare October 9, 2017 03:55
@zonkmachine
Copy link
Member Author

zonkmachine commented Oct 9, 2017 •
edited
Loading

Two of the fixes didn't pass testing and had to be reverted. I'll open a new ticket (meta) for the remaining issues once this PR is merged.
Edit: #3880

@zonkmachine
Arpeggiator fixes
3a03181 
Set 'master note' for silent notes on sort mode.
Prevent silent notes to play as ordinary notes in the background
when they are waiting for their turn to arpeggiate.

Remove unnecessary test (it will always return TRUE).

Slower default speed (200ms).
@zonkmachine zonkmachine merged commit 6da8737 into LMMS:stable-1.2 Oct 9, 2017
@zonkmachine zonkmachine mentioned this pull request Oct 10, 2017
Closed
@zonkmachine zonkmachine deleted the arpnotedecay branch October 10, 2017 04:48
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
@zonkmachine
Arpeggiator fixes (LMMS#3858)
c6475e8 
Set 'master note' for silent notes on sort mode.
Prevent silent notes to play as ordinary notes in the background
when they are waiting for their turn to arpeggiate.

Remove unnecessary test (it will always return TRUE).

Slower default speed (200ms).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.
1 participant
@zonkmachine