LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
8.16k stars 1.01k forks source link

Piano Roll, when recording, it (sometimes) plays the note twice #6277

Open oscaracena opened 2 years ago

oscaracena commented 2 years ago

Bug Summary

When recording from MIDI devices on Piano Roll, sometimes I can hear the same note twice. It appears to be a random event, but I can reproduce it using a simple MIDI script.

Steps to reproduce

  1. Create an empty project, and add an instrument (for example, an AudioFileProcessor with a kick sound as a sample).
  2. Set the instrument MIDI input to your testing device (or launch the following script and use it as tester).
  3. Open a Clip on Piano Roll, and start recording.

Script to test behavior:

#!/usr/bin/env python3

import mido
import time

note = 69
port = mido.open_output(name="TEST", virtual=True)
on = mido.Message("note_on", note=note, velocity=127)
off = mido.Message("note_off", note=note, velocity=0)

while True:
    print(".", end="", flush=True)

    port.send(on)
    time.sleep(5 / 1000)
    port.send(off)

    time.sleep(1)

Expected behavior

When a note is sent to the instrument, you always must hear it only once (not repeated). No arpeggiator, sequencer or any other effect used.

Actual behavior

Sometimes, you can hear the same note twice (with a very short delay between both sounds). In the Piano Roll, there is only one note recorded.

Screencast

https://user-images.githubusercontent.com/4654094/149574646-91a36c8b-0e0f-425a-b8b5-13ec5dad4e7c.mp4

Affected LMMS versions

oscaracena commented 2 years ago

As with #6264, the issue seems to be present only when brief notes are handled (like taps in the pads of a MIDI keyboard). I've tested it again with a script, when using 5 ms notes, the double sound is present, but with 50 ms, there is no issue.

I'll try to haunt this one too :wink:

oscaracena commented 2 years ago

Ok, some insight. PianoRoll has a method called finishRecordNote, which is a slot connected to midiNoteOff. It is on charge of adding the new note to the clip, using midiClip->addNote(). If I comment that line, the note is not added to the piano-roll (obviously), but I can hear it (without repeats). Moreover, if I add the note, but one octave up, when I hear the repeated note, it plays the two octaves (the original + the upper). So, the repetition comes from midiClip->addNote().

What I don't understand is why the note is not always repeated. Still investigating it.

oscaracena commented 2 years ago

If I add, manually, a note ahead of the recording position (the line that goes forward while recording), it will be added but not played until the recording line reaches it. I think that a new race condition could be happening here too. If the note is added just a little bit ahead of that line, then it will be played always twice (once by the instrument, and once by the piano-roll). If the note is added before the proper position (i.e. pos - 5), then there is no repetition. I checked it.

Also, if the note is long enough, the repetition will not happen. [spoiler: because the note is added on note-off. If the note is longer than Q/2, the note position is always behind the playing position].

oscaracena commented 2 years ago

I've tracked the position of the newly created notes, and also the play() method inside Song. I've got a bunch of records like this:

note1 pos: 1017, len: 12, song pos: 1019
note2 pos: 1020, len: 12, song pos: 1019
play note from song, note pos: 1020, current start: 1020

When a note is created (note1 in the previous record), its position (1017) is set before the current playing position of the song (1019). That's right. BUT the method Clip::addNote() must adjust the note position according to the current track quantization. So, the actual note used (note2 in this case) may have a position (1020) in the future of the song, so it will be played when that tick is reached.

The quantize method makes the note snap to the piano-roll grid, which is a feature we don't want to lose. To see clearly the problem, I've set the quantization to the maximum (1/1), and test the recording. The note is snapped to the next bar, as seen in this screenshot (which is, by the way, perfectly normal and expected).

PianoRoll add note in the future

This situation is a bit hard to deal with, because the song does not know if we want to hear that note or not. It may have been there for ages, or just added. We can not mark the note in any way. On the other hand, the snapping is correct, the right position should be the nearer in time. And, we can not disable the instrument sound, because the notes added before the recording line won't be played. Interesting indeed. :grimacing:

oscaracena commented 2 years ago

The only solution that I came up with is to add a flag on Note to say to the InstrumentTrack that he should skip it in the next play() iteration. Thus, inside PianoRoll::finishRecordNote(), just after the note is added, we set the flag. And inside InstrumentTrack::play(), we check if the flag is set, and if so, skip the note. We need also to clear the flag on all past notes, so it will only be used in the current recording session, and not in the next play/record. This is done inside the same play(), and also in the PianoRoll::stop(), just in case the are future notes still flagged.

There is no reason to add the flag inside the Note's copy constructor, because I don't want copies of a skipped note outside control. Only explicit setting of the flag allowed.

I've tested the implementation, and it works as expected. I'll PR it.