get_piano_roll sustains some notes too long

[drscotthawley]

Hi Colin! I see a number of issues and PRs related to get_piano_roll(), yet I'm not sure which of them my issue fits best with so I'm opening a new one:

I notice many notes in the piano roll are sustained for much longer than they should be, resulting in long horizontal lines, independent of the choice for fs.

For example, here is a plt.imshow() for the get_piano_roll output for piano instrument in file number 028/028.mid in the POP909 Dataset:

def plot_piano_roll(pr_array): 
    plt.figure(figsize=(8, 8))
    plt.imshow(np.flipud(pr_array), aspect='auto')
    plt.show()

for instrument in pm.instruments: 
  name = instrument.name.upper()
        if  name in ['MELODY', 'PIANO']:
            print(f"get_piano_rolls: instrument.name = {name}")
            piano_rolls[name] = instrument.get_piano_roll(fs=fs)
            if name == 'PIANO':
                plot_piano_roll(piano_rolls[name])

The same part displayed in Logic Pro looks different; it does not have the long horizontal lines:

Also, when I plot it using the method from the Magenta music_generation.ipynb Colab Notebook, there are no long horizontal lines:

I've tried different fs values (from 4 to 1000) and different image sizes to see if the long horizontal lines in the first image are just some sort of plotting artifact, but... I can't get rid of them.

This issue applies to both the instrument.get_piano_roll() and the full pm.get_piano_roll(). The issue applies to several other files in that POP909 dataset, e.g. 028, 030, 038, 039, 040, 046, 047, 050, 052, 057, 061, 064, 065, 066, 068,... I stopped counting. Logic and other viewers show them ok, but get_piano_roll seems to incorrectly sustain notes for many, many measures.

Is there something I'm doing wrong that 's leading to the "long horizontal lines"? I'd like an array that reflects something more like from the other two plotting methods.

Thanks!

[drscotthawley]

So,... no doubt my own code below is not sophisticated enough for general purposes, but for now, I am able to recover something that "looks right" using a simple bit of code, which is suitable for my current purposes of 'MELODY', 'PIANO', and 'TOTAL' instruments. The plot now shows distinct notes.

This is the code I used:

def get_piano_rolls2(midi, fs):
    duration = midi.get_end_time()
    n_frames = int(np.ceil(duration * fs))
    # create a piano roll for each instrument
    piano_rolls = {'PIANO':  np.zeros((128, n_frames)),
                   'MELODY': np.zeros((128, n_frames)),
                   'TOTAL':  np.zeros((128, n_frames))}
    for instrument in midi.instruments:
        name = instrument.name.upper()
        if name in ['MELODY', 'PIANO']:
            print(f"get_piano_rolls: instrument.name = {name}")
            for note in instrument.notes:
                start = int(np.floor(note.start * fs))
                end = int(np.ceil(note.end * fs))
                piano_rolls[name][note.pitch, start:end] = note.velocity
                piano_rolls['TOTAL'][note.pitch, start:end] = note.velocity
            if name == 'PIANO': plot_piano_roll(piano_rolls[name])
    return piano_rolls

[craffel]

Hi, unfortunately I don't have time to debug this directly but I highly suspect it has to do with different conventions as to how to handle a note off event after repeated note on events (e.g. you get a note on at note N on channel C, another note on for note N on channel C, and then a note off for note N at channel c). There are (at least) thre ways to handle it:

• The note off turns off the first note on but not the second (FIFO)
• The note off turns off the second note on but not the first (LIFO)
• The note off turns off both notes

IIRC pretty_midi does FIFO. I think many DAWs/libraries have the note off turn off all notes. I don't know that there is any "correct" behavior, only conventions. I do think it would be nice if you could specify the behavior of get_piano_roll via an argument, but I will not be doing that myself (PRs welcome). If you have your own code that suits your needs, please use it (but note that the code you posted is missing a lot of features, e.g. handling pitch bends etc.).

[TimFelixBeyer]

While I still think that the current get_piano_roll implementation suffers from rounding errors in particular cases (addressed by #195), in this case the issue seems to be that the MIDI files contain several sustain events with an active value (over 64). As a result, the notes are correctly displayed as held for a long time.

In the Magenta notebook and in LogicPro (and your example), pedaling is not taken into account at all for the piano display. If you'd like the same effect, you can call get_piano_roll(pedal_threshold=None), which ignores pedaling. You can also set a custom threshold if the default 64 leads to issues for you.