Arpeggio Sort mode counts one extra step

[DeRobyJ]

Use sort mode and notice it counts one more note, leaving silence between repetitions.

It's actually quite unconstant.

I hope this video will help, it's a bit out of sync tho
https://www.youtube.com/watch?v=RO2Yru1qrdM

[Fastigium]

Confirmed, that is some strange behavior right there. Do you know if this affects all versions? Or did it work correctly in an earlier version?

[DeRobyJ]

I used it on the same project the same way and it changed right after upgrading.
Now I'm back at stable and it works fine.

In the instrumentfunctions.CPP there is very little change in the sort arpeggio part, so I wonder if it depends on something else

[musikBear]

it actually even make one chord in the sort sequence
That is a critical, and 1.2 cant be released with this arp-bug

[Umcaruje]

That is a critical, and 1.2 cant be released with this arp-bug

But having a completely broken MIDI Export is ok!?

[Fastigium]

I used it on the same project the same way and it changed right after upgrading.

Ah, so it's a regression, that gives some hope of an easy fix. @zonkmachine Do you by any chance feel like working your bisect magic on this 😇?

[zonkmachine]

@zonkmachine Do you by any chance feel like working your bisect magic on this 😇?

On the case.

[zonkmachine]

a8211873b56ae5decd23e6894d437038120f0325 is the first bad commit
commit a8211873b56ae5decd23e6894d437038120f0325
Author: Vesa <contact.diizy@nbl.fi>
Date:   Sun Aug 24 15:44:36 2014 +0300

    Fix arpeggio to work better with the new way to handle note offsets
    Ok, not really related to memory management, but was something that needed doing and it's easier to test things when things work properly

a821187

[musikBear]

But having a completely broken MIDI Export is ok!?
imo, yes. midi-export could be postponed to 1.2.x. It is not a base-function, like arpeggio, and postponing is no big deal, so again imo - The two belongs to different levels of bugs.

[tresf]

imo, yes. midi-export could be postponed to 1.2.x. It is not a base-function, like arpeggio, and postponing is no big deal, so again imo - The two belongs to different levels of bugs.

@musikBear Please stop trying to manage our release features.

[Fastigium]

@zonkmachine Thanks a bunch, that was very helpful!

Right, I've hunted down the cause of the bug. NotePlayHandles generated by arpeggio or stacking used to be managed by their parent NotePlayHandles, but that changed with the commit zonkmachine found. Now those NotePlayHandles are added directly to the mixer. This causes some methods to return incorrect results, as they expect to find only parent NotePlayHandles in the mixer. Those methods (NotePlayHandle::index() and NotePlayHandle::nphsOfInstrumentTrack()) are almost exclusively used by sorted arpeggios, otherwise this problem might have been noticed much sooner.

I'll make a pull request, but it'll have to wait a little. My head hurts of all this bug chasing 😵. I have to start pacing myself, I think.

[softrabbit]

My head hurts of all this bug chasing

@Fastigium, quick, write a wiki page before you forget all the dirty details! Didn't you drill pretty deep into some other part of LMMS the other day as well?

[Fastigium]

Didn't you drill pretty deep into some other part of LMMS the other day as well?

@softrabbit Yeah, that resulted in #2586. That one should probably be merged soon. I was just hoping someone would look over the code and say if it looks good or not.

By the way, I'm working on a pull request for this bug as we speak. Writing a wiki page sounds like just as much work as just fixing it 😋. My previous comment contains enough information for me to pick things up where I left off.

[Fastigium]

Right, pull request made (#2603)! Another one that took hours to come up with and that only changes a few lines. I swear I've got the largest hours invested to lines of code touched ratio 😁.

At any rate, testing welcome!

[DeRobyJ]

If you happen to have a win64 build, I'm here to test it ;)

[Fastigium]

@DeRobyJ Here you go: https://drive.google.com/uc?export=download&id=0ByJw4p5_PU1cZTJZWmhhRzdrUk0
This should make arpeggio sort mode work like it did in 1.1.3 again. Please let me know if it does!

[DeRobyJ]

Yep, works!

[Fastigium]

@DeRobyJ Thanks for reporting, the next RC will include the fix!