ReaScript: MIDI_Sort causes extended note if first note ends where next note (FIXED)

[juliansader]

As a simple example, the following MIDI data is for two 1/8 notes:

+480 480: 90 30 41
+480 960: 80 30 00
+-960 0: 90 30 41
+480 480: 80 30 00
+32160 32640: B0 7B 00

The notes are in the wrong order (the second note has been moved in front of the first note), but each note-off is followed nicely by its note-off and the notes do not overlap. The MIDI editor does not have any problems with these notes:
* The notes are displayed correctly in the MIDI editor,
* The action "Correct overlapping notes" does not make any changes to the MIDI data,
* Deselecting the notes (or any other edits in the MIDI editor) sort the MIDI data correctly without any extended notes.

However, running MIDI_Sort deletes one of the note-offs, resulting in an extended note:

+0 0: 90 30 41
+480 480: 80 30 40
+0 480: 90 30 41
+32160 32640: B0 7B 00

If there is a slight gap between the notes (i.e. the note-off and note-on don't fall of the same PPQ position), the notes are sorted correctly.

[juliansader]

As mentioned above, almost any editing in the MIDI editor, such as merely selecting or deselecting a note, re-sorts the MIDI data without any problems.

I think that the MIDI_Sort function should simply use the same code that the MIDI editor uses to sort its data.

In fact, given that MIDI_Sort is notorious for causing extended notes, I found that a more reliable way to sort MIDI before a script exits, is to use MIDIEditor_OnCommand to run a MIDI editor action. For example,

reaper.MIDIEditor_OnCommand(editor, 40501) -- Invert selection in active take
reaper.MIDIEditor_OnCommand(editor, 40501) -- Re-invert back to original selection

This is usually slower than MIDI_Sort, but only if the MIDI is already well sorted. If the MIDI is very messy -- for example after bulk editing in a script -- MIDI_Sort is just as slow as the OnCommand method.

[schwa]

Originally Posted by juliansader

+480 480: 90 30 41 // A
+480 960: 80 30 00 // B
+-960 0: 90 30 41 // C
+480 480: 80 30 00 // D

All MIDI sorting done by reaper is stable, so

Originally Posted by juliansader

each note-off is followed nicely by its note-off

is actually not the case. Any stable sort will make sure A remains before D, resulting in CADB which is two note-ons followed by two note-offs.

The MIDI_Sort API function calls the internal sort function, but then also does another pass over the material to see if there are double note-ons or note-offs that need to be removed, or missing note-offs that need to be inserted. The last step isn't necessary for the internal sorting (unless a bug occurs) because REAPER tracks notes in separate data structures that ensure the note-on, note-off, and notation data are all kept together.

In your example case, the script has the contextual information to know which messages form individual notes. If you are setting this data all at once, I think the only robust solution is for the script to apply the logic needed to sort events that it alters. When adding or moving a note-on and associated note-off, the script needs to check to see if there is an existing note with the same pitch playing, move the existing note-off if necessary, insert the note-on, check to see if there is an existing note-on and possibly note-off that falls between the new note-on and note-off, and move or remove those messages.

If you are using the API functions to set or move notes, it's now clear to me that the API function needs to be changed apply the same logic, which will add computation time if adding many new notes at once.

[juliansader]

Originally Posted by schwa

All MIDI sorting done by reaper is stable, so

The MIDI editor uses a different (non-stable?) sorting algorithm that is much more reliable when sorting notes, and which does not cause extended notes in this example. Calling Invert selection with OnCommand (or simply doing any edit in the MIDI editor, such as selecting one of the notes with the mouse) results in correct sorting:

+0   0  : 90 30 41
+480 480: 80 30 00
+0   480: 90 30 41
+480 960: 80 30 00
+31680 32640: B0 7B 00

Would it be possible for MIDI_Sort to simply use the MIDI editor's code and algorithm? (And the MIDI editor, which can't sort notation events, can use MIDI_Sort's code for notation events.)

Originally Posted by schwa

The MIDI_Sort API function calls the internal sort function, but then also does another pass over the material to see if there are double note-ons or note-offs that need to be removed, or missing note-offs that need to be inserted.

It should also check for note-ons and note-offs at the same PPQ and pitch, to ensure that they are in sequence: on-off-on-off-on-....

Originally Posted by schwa

In your example case, the script has the contextual information to know which messages form individual notes. If you are setting this data all at once, I think the only robust solution is for the script to apply the logic needed to sort events that it alters.

Surely this is the intended job of MIDI_Sort, not of the script!?

Scripters cannot be expected to code their own sorting algorithms to replace a buggy MIDI_Sort. MIDI_Sort is one of the most fundamental API functions, and according to both you and Justin, it should be called at the end of all scripts that edit MIDI.

[schwa]

I think I understand the confusion here. The crux is that when the piano roll is open, REAPER already has events collected into different data structures that ensure note-ons, note-offs, and notation data stay together. Those data structures do not exist if the MIDI editor is closed.

The sorting functions are exactly the same and are both stable. The difference is that calling a MIDI editor action, which is only possible if the MIDI editor is open, sorts events that are already collected into note data structures. Calling the API sort function sorts the raw MIDI events. Since those events are not already grouped into note data structures, there is simply not enough context to know how to organize newly added or moved events.

Originally Posted by juliansader

Scripters cannot be expected to code their own sorting algorithms to replace a buggy MIDI_Sort.

It's not buggy! It is the same function used internally. It just doesn't have the information it needs.

Here is an example.

   0: 90 30 40  // A
1440: 80 30 00  // B
 480: 90 30 40  // C
 960: 80 30 00  // D

What should happen? The only possible sort is ACDB, which is two note-ons followed by two note-offs. If you create the events in REAPER by using the MIDI editor, REAPER already knows that A and B represent a single existing note, so it has a bunch of logic to create new events as necessary to result in three notes:

   0: 90 30 40  // A
 480: 80 30 00
 480: 90 30 40  // C 
 960: 80 30 00  // D
 960: 90 30 40
1440: 80 30 00  // B

If you are just handing that list of four events to REAPER via MIDI_SetAllEvts, there is not enough information to know that those four events are really meant to be three notes. There just isn't, there's no sort algorithm in the world that can interpret those four events as three notes.

If you are using the API MIDI_InsertNote to add events C and D to the already-existing A and B, REAPER does have enough information, because it already knows that A and B belong together. As I wrote above: it's now clear to me that the API function needs to be changed apply the same logic, which will add computation time. It may add significant computation time in the case where the MIDI editor is not already open, because REAPER has to do the work of collecting all of the existing events into note structures before interpreting the new events.

[juliansader]

I am not familiar with the inner workings of REAPER, but it does not seem to me that the problem can the result of missing information in the case of MIDI_Sort, since the MIDI editor can properly sort notes even if it only has raw MIDI data to work with - i.e. raw MIDI data uploaded by SetAllEvts, instead of proper notes created by MIDI_SetNote or MIDI_InsertNote.

Originally Posted by schwa

I think I understand the confusion here. The crux is that when the piano roll is open, REAPER already has events collected into different data structures that ensure note-ons, note-offs, and notation data stay together. Those data structures do not exist if the MIDI editor is closed.

Could MIDI_Sort not collect everything into the same structures, using the same algorithms as the MIDI editor? (Except of course that MIDI_Sort shouldn't delete notation events like the MIDI editor does).

Originally Posted by schwa

It just doesn't have the information it needs.
...
If you create the events in REAPER by using the MIDI editor, REAPER already knows that A and B represent a single existing notes, so it has a bunch of logic to create new events as necessary to result in three notes:
...
If you are using the API MIDI_InsertNote to add events C and D to the already-existing A and B, REAPER does have enough information, because it already knows that A and B belong together.

Your example is of two overlapping notes, so I am not sure why it would be sorted into *three* notes?

My original example concerned a specific case of *non-overlapping* notes in which the note-on and note-off fell on the same PPQ position. However, your new example also demonstrates the difference between MIDI_Sort and the MIDI editor - even when there is no context:

If I upload your example using SetAllEvts - i.e., the MIDI editor does not have any prior context - the MIDI editor still manages to sort the notes into something usable, whereas MIDI_Sort results in extended notes:

editor = reaper.MIDIEditor_GetActive()
take = reaper.MIDIEditor_GetTake(editor)
reaper.MIDI_SetAllEvts(take, string.pack("i4Bi4BBB", 0, 0, 3, 0x90, 60, 100)
                          .. string.pack("i4Bi4BBB", 1440, 0, 3, 0x80, 60, 0)
                          .. string.pack("i4Bi4BBB", -960, 0, 3, 0x90, 60, 100)
                          .. string.pack("i4Bi4BBB", 480, 0, 3, 0x80, 60, 0)
                          ... string.pack("i4Bi4BBB", 960*4*4 - 960, 0, 3, 0xB0, 0x7B, 0))

-- Either of these two lines:
--reaper.MIDIEditor_OnCommand(editor, 40501) -- Invert selection
reaper.MIDI_Sort(take)

Running MIDIEditor_OnCommand(editor, 40501) results in:

    +0       0:  90 3C 64
  +480     480:  90 3C 64
  +480     960:  80 3C 00
  +480    1440:  80 3C 00

which is perfectly usable, whereas MIDI_Sort results in:

    +0       0:  90 3C 64
  +480     480:  80 3C 40
    +0     480:  90 3C 64

which is obviously wrong.


If I upload my original example using SetAllEvts (as before, the MIDI editor does not have any prior context)

editor = reaper.MIDIEditor_GetActive()
take = reaper.MIDIEditor_GetTake(editor)
reaper.MIDI_SetAllEvts(take, string.pack("i4Bi4BBB", 480, 0, 3, 0x90, 60, 100)
                          .. string.pack("i4Bi4BBB", 480, 0, 3, 0x80, 60, 0)
                          .. string.pack("i4Bi4BBB", -960, 0, 3, 0x90, 60, 100)
                          .. string.pack("i4Bi4BBB", 480, 0, 3, 0x80, 60, 0)
                          .. string.pack("i4Bi4BBB", 960*4*4 - 480, 0, 3, 0xB0, 0x7B, 0))

-- Either of these two lines:
--reaper.MIDIEditor_OnCommand(editor, 40501) -- Invert selection
reaper.MIDI_Sort(take)

reaper.MIDIEditor_OnCommand gives:

    +0       0:  90 3C 64
  +480     480:  80 3C 00
    +0     480:  90 3C 64
  +480     960:  80 3C 00

which is correct, whereas MIDI_Sort gives:

    +0       0:  90 3C 64
  +480     480:  80 3C 40
    +0     480:  90 3C 64

which is incorrect.

[schwa]

[deleted really long technical post]

instead just re-quoting myself:

Originally Posted by schwa

the API function needs to be changed apply the same logic, which will add computation time. It may add significant computation time in the case where the MIDI editor is not already open, because REAPER has to do the work of collecting all of the existing events into note structures before interpreting the new events.

[juliansader]

I would much prefer a MIDI_Sort that is slower and computationally intensive -- as long as it is also reliable.