lmms icon indicating copy to clipboard operation
lmms copied to clipboard

Arpeggiator - Wrong sequence with multiple notes and sort mode

Open zonkmachine opened this issue 3 years ago • 12 comments

Bug Summary

Arpeggio sequence over more than one note and mode sort, comes out wrong. (Chord: Octave, Direction 'Up', and Range 2 octaves)

Instead of going through the notes of the chord in one octave before advancing to the next octave, it loops over one note in the chord in different octaves before advancing to the next note.

In essence, manually entering a chord in the piano roll, and arpeggiating it over sort mode, should give you the same result as if you entered the same base note and selected the same chord in the arpeggiator chord menu. All other settings being equal.

The responsible line of code is here: https://github.com/LMMS/lmms/blob/4821606465a5c46109f048466f84e55aeeb31d6e/src/core/InstrumentFunctions.cpp#L400

With this chord: arpchord

I get this arpeggio: arpget

But this is what I want and which is the behavior of other arpeggiators. Hardware synths, Logic, LV2 arpeggiators with Ardour: arpwant

Steps to reproduce

Enter a chord in the piano roll. Dial in an arpeggio based around sort mode and chord: octave. Demo: arpsequence.mmp.zip Chord that arpeggiates wrong followed by explicit notes showing again how it's acting today and last the wanted result.

Version

Any version of lmms

zonkmachine avatar Aug 27 '22 11:08 zonkmachine

'+' this could possibly be a one line fix. '-' the fix will be destructive but there probably isn't many projects out there using the arpeggiator in this way. It just sounds odd, and not in a good way.

zonkmachine avatar Aug 27 '22 16:08 zonkmachine

This is potentially an easy fix so tagging this for 1.3 for now.

zonkmachine avatar Aug 28 '22 13:08 zonkmachine

Agreeing that this should be fixed as suggested. The arp is faulty and skipping notes. If that is fixed I won't consider that compatibility breaking either. Rather I have some old projects that are broken in 1.3 due to the arp being so buggy...

allejok96 avatar Aug 28 '22 19:08 allejok96

I just found a project that seems to use it. Uh. https://lmms.io/lsp/?action=show&file=20371 (It was uploaded recently, but apparently has been made last year according to the description.) I can't quite tell what's going on though, I think it's just the arpeggio set to octave (0 range) over a two-note chord, but perhaps we should edit the piano roll TCO itself if a project's been made in a version of 1.3 exhibiting this behaviour? Would it be too complex a fix? Because I can believe that there are possibly other projects made by users using the broken feature.

Monospace-V avatar Nov 01 '22 07:11 Monospace-V

I just found a project that seems to use it.

More than one track use arpeggio in that project and with different settings. It also displays other issues like notes not being played on arpeggio over more than one note simultaneously. I provided a demo project in the first post that displays this issue specifically. First an arpeggiated chord. Then the same arpeggio recreated in notes followed by an example of how I think it should have worked.

but perhaps we should edit the piano roll TCO itself if a project's been made in a version of 1.3 exhibiting this behaviour? Would it be too complex a fix? Because I can believe that there are possibly other projects made by users using the broken feature.

That would be way to complex. How would you know what the user would prefer? It could differ from track to track. Also, note that the behaviour is in all versions of lmms.

zonkmachine avatar Nov 02 '22 18:11 zonkmachine

Ah, my mistake. Somewhere over the thread I misinterpreted. Then surely every project that's ever used the arps on chords with sort mode will sound different? If we cannot ensure backward compatibility, then what can we do? The user needs to know that their arpeggio will sound different if they use sort mode with chords, on this version of LMMS. We could just fix it and leave it to the user, but I think even I may have possibly used sort mode at some point over chords and accepted the sound. (Unless it could be the first commit on a potential 1.4... 🤔 but we haven't thought of that yet. And this does need fixing.)

Monospace-V avatar Nov 03 '22 00:11 Monospace-V

Then surely every project that's ever used the arps on chords with sort mode will sound different?

Yes. Chances are that there aren't many who use the arpeggiator in this way as it sounds a bit wonky though, but yes, that's correct. There could be ways to deal with it but I doubt it would be worth it. We try not to break backward compatibility but not when it's really a broken feature.

then what can we do?

Inform the user in the release notes?

zonkmachine avatar Nov 03 '22 18:11 zonkmachine

Inform the user in the release notes?

I am in favour of this...though, I am worried users may not read the release notes properly, especially in case of a stable release. However, I cannot see much better ways to do this.

Monospace-V avatar Nov 03 '22 23:11 Monospace-V

may not read the release notes properly

Ultimately this is on the user, not the software maintainers. It's generally good practice to start and end projects on the same version, and complete backwards compatibility is a luxury many programs don't have. I have made use of the wonky output of this broken sort mode, and yet I will be first in line to celebrate it being fixed :) an example

slapkev avatar Nov 04 '22 03:11 slapkev

I think some additional clarification on the desired behaviour would be useful. The choice of "octave" as the chord in the initial example means it doesn't demonstrate how the chord should be reflected in the arpeggio, since the "octave" chord consists of the root note only.

There are three different things to arpeggiate across - the arpeggio chord, the octave range, and the piano roll notes. The current behaviour is each note of the chord is played, within each octave, for each piano roll note. In pseudocode,

for note in piano_roll:
  for octave in octave_range:
    for degree in arp_chord:
      play(note + octave * 12 + degree)

Call this piano-octave-arp order. The requirements in the description narrow down the acceptable orders somewhat:

[The bug is that] instead of going through the notes of the chord in one octave before advancing to the next octave, it loops over one note in the chord in different octaves before advancing to the next note.

This means that "octave" needs to come before "piano" in the order.

In essence, manually entering a chord in the piano roll, and arpeggiating it over sort mode, should give you the same result as if you entered the same base note and selected the same chord in the arpeggiator chord menu. All other settings being equal.

This means that "piano" and "arp" need to be the same side of "octave".

This leaves two orders: octave-piano-arp, and octave-arp-piano. The current PR does something slightly different, effectively octave-sort(arp-piano). The following screenshot demonstrates the following configurations, with range set to 2 octaves and chord set to "5" (the root and a perfect fifth): underlying piano roll chord, current behaviour (piano-octave-arp), octave-piano-arp, octave-arp-piano, PR #7025 (octave-sort(arp-piano)): image

I own no hardware synths with sufficiently complex arpeggiators, nor a copy of Logic, so I can't say what the "correct" behaviour should be. I'm not convinced combining an arpeggiator chord with a piano roll chord to form some kind of "cartesian product chord" is musically useful, so the particular choice of behaviour may not matter. Intuitively, I would expect octave-arp-piano order to be used - conceptually, the octave range stacks copies of the arp on top of each other, then this big arp is applied to the chord in the piano roll. This also means that sort mode behaves in a similar way to free or sync, just with an arpeggiated piano roll chord rather than the plain written one. Octave-piano-arp order feels weird - some parts of the arpeggiator are applied before arpeggiating the piano roll, some after. Octave-sort(arp-piano) can split apart both the piano roll chord and the arp chord, and has the same oddity where different parts of the arpeggiator are applied in different ways.

There is also a technical motivation for avoiding octave-sort(arp-piano) order - in our current arpeggiator implementation, it requires a temporary vector in which to sort notes before processing. This incurs a heap allocation for each piano roll note in each arp step (though I think this can be optimised to just the root note in each step), which is something we should strive to avoid in real-time code.

In the future, I would hope we would have a MIDI effect chain, where any number of stackers and arpeggiators (among other effects) could be added as the user pleases. This would offer much more flexibility and precise control over the result. In this scenario, octave-sort(arp-piano) could be built first by applying a stacker with the arp chord, then feeding that into an arpeggiator with an octave chord selected. This is another argument for avoiding that order, and instead choosing one which couldn't be created in such a manner.

DomClark avatar Apr 25 '24 00:04 DomClark

There are three different things to arpeggiate across - the arpeggio chord, the octave range, and the piano roll notes.

The piano roll notes here could also possibly be the notes from the note stacking tool, even though today note stacking is processed after the arpeggio. It seem to me more logical to actually do it the other way around. Not that it seem useful to do it either way, you typically use the arpeggiator or the note stacking tool. Possibly with note stacking just set to octave if they ever are used together.

I own no hardware synths with sufficiently complex arpeggiators, nor a copy of Logic, so I can't say what the "correct" behaviour should be.

Hardware synths don't seem to have complex arpeggiators built in. Usually it's up, down, updown, downup and random and then a setting for number of octaves and that's about it. They usually are either sorting note input or remembers the order you press the keys and play the arpeggio non linearly. I don't think there is a canonical way an arpeggiator should behave for more complex input. I think we're really talking design here and it's up to us to choose, although I don't have a preference personally, I think you're making a good case for the octave-arp-piano over the solution by @szeli1 (https://github.com/LMMS/lmms/pull/7025).

There is also a technical motivation for avoiding octave-sort(arp-piano) order - in our current arpeggiator implementation, it requires a temporary vector in which to sort notes before processing. This incurs a heap allocation for each piano roll note in each arp step (though I think this can be optimised to just the root note in each step), which is something we should strive to avoid in real-time code.

Invoking the Pareto principle here, do you think this is inefficient enough to warrant a rewrite? Is the arpeggiator used that much and the sorting slowing down the execution to an extent where it needs to be looked at for https://github.com/LMMS/lmms/pull/7025? I personally don't think so. I'm happy to lay that refactoring on later generations anyway.

In the future, I would hope we would have a MIDI effect chain, where any number of stackers and arpeggiators (among other effects) could be added as the user pleases. This would offer much more flexibility and precise control over the result. In this scenario, octave-sort(arp-piano) could be built first by applying a stacker with the arp chord, then feeding that into an arpeggiator with an octave chord selected. This is another argument for avoiding that order, and instead choosing one which couldn't be created in such a manner.

I've mentioned elsewhere that the whole 'note stacking/arpeggio tab' is misplaced. It rather belongs to the track and on other sequencers you would find the arpeggiator in a context menu on the head of the track. Taking from memory I believe this is how it worked on Logic.

I still think https://github.com/LMMS/lmms/pull/7025 is fine to merge. Any difference in behavior in future refactoring is a bit too theoretical, user cases that typically wouldn't arise, and something I would be perfectly happy to accept the breaking of full compatibility with older versions for.

zonkmachine avatar May 04 '24 11:05 zonkmachine

I would expect octave-arp-piano order to be used

Let's implement this.

szeli1 avatar May 04 '24 13:05 szeli1