#1427 Double note-offs in playback

[Ted Felix]

Using kmidimon, this is easy to see. The problem is caused by both AlsaDriver::processNotesOff() and AlsaDriver::processMidiOut() sending a note-off.

Problem was originally identified by Craig McQueen in [bugs:#1426].

[Tito Latini]

Attached a patch to fix the bug.

A noteoff is scheduled also with a MappedEvent::MidiNoteOneShot, but
the noteoff for oneshot is created in `src/sound/AlsaDriver.cpp:3921'.

[Ted Felix]

I tested the patch against the following use cases:

1. MIDI thru. Just playing a keyboard with a synth hooked up. Works fine.
2. Recording. Works fine.
3. Playback. Works correctly now. Only single NoteOff's go out.
4. MIDI Export. Does not work. The patch removes all NoteOffs from the exported standard MIDI file.

It appears that the same code is used for playback and standard MIDI file export, but it's used in different ways.

[Tito Latini]

The attached patch replaces the old and should fix the bug.

The forced type in MappedEvent() is a bug generator. For example,
the type is ignored in ImmediateNote::fillWithNote()
(src/sound/ImmediateNote.cpp:80).

Note: it is unnecessary for the bug fix, but I have changed
enqueueNoteoff() with a simplest m_noteOffs.insert plus a warning msg
(feel free to change the text, perhaps it's useful the info about trackid).
I think the deletion of the old noteoff messages with the same pitch/ch is
useless because the possible corresponding noteon messages are not removed.
For example, two superimposed notes with the same pitch:

C4/ch0 ######################
            C4/ch0 ########################

When enqueueNoteoff() is called with the second noteoff, it removes
the noteoff of the first C4 but not the noteon. The result is

C4/ch0 ############# stop at the end of the composition #############
            C4/ch0 ########################

I think there are two possibilities:

a) merge all the superimposed notes with the same pitch/channel

b) ignore

The point (a) requires extra work to

- merge the imported notes

- merge the recorded notes

- merge the notes during the editing (matrix or notation)
  with command history to provide undo/redo

The point (b) is done: if a synth stops a note too soon, we hear the
break and read the warning message when the noteoff is inserted.

I'm for (b) also because some synths can play more notes with the same
pitch/channel and I have noticed superimposed notes (unison out of time)
in orchestral midi files to do a kind of chorus effect. That flexibility
is impossible after the point (a).

[Tito Latini]

Patch applied.

[Ted Felix]

[r14092], which addresses this, introduces a new bug. Now, when playing in loop mode, no note-offs are sent at the end of the loop before jumping back to the start of the loop.

[Tito Latini]

Thanks to test that case.

It should be fixed in r14095.

Now the only difference between MidiNote and MidiNoteOneShot is the
noteoff-container used for MidiNote in the internal segment mapper.

Let me know if there are other side effects.

[Ted Felix]

[r14095] appears to fix the loop issue. This one feels complete to me, but since it was pretty tough to fix, let's leave it open for a while in case other side-effects are discovered. Thanks for the hard work, Tito.

[D. Michael McIntyre]

I'm guessing this is responsible for the new bug I've been chasing for hours. It finally dawned on me to try an older Rosegarden, stupid. I compiled a new Rosegarden at some point in the middle of playing with setting up and learning how to control my new rig, and I've been trying to figure out what the blue blazing hell weird setting I must have turned on.

No, it's Rosegarden!

The transport is stopped. I have a MIDI track active, and I'm playing the keyboard, using Rosegarden to route the MIDI out to my synth. If I hold down a key, the note nevertheless cuts off almost instantly.

I hooked up KMidiMon to monitor Rosegarden's output.

With 15.08, I get a note on, then a note off after however long I held the key. With current SVN I get a note on, then a note off 9 ms later, no matter how long I hold the key. It's virtually instantaneous.

I'll look into the recent code changes and figure out what to revert or possibly fix a bit later on.

[D. Michael McIntyre]

The problem I just reported goes away after backing out revision 14095. I suggest a more careful fix for the bug in 14092 is required here. I'm going to look through the changes and see if I can get anywhere on an improved fix.

[Tito Latini]

Sorry, it is my stupid error because recently I have tested only
manual arrangements and midi files.

We have MidiNote and MidiNoteOneShot. I don't know the original
intention, but now a possible solution is to use MidiNote only for
midi input (no noteoffs on the stack but noteoff when velocity is
zero) and MidiNoteOneShot for the rest (noteoffs on the stack).
To avoid double noteoffs, an inserted MidiNote (from midi input)
is recorded as a MidiNoteOneShot. Idem for midi-import.

I'll try to write a patch with that logic
(and I'll remember to test midi input!)

[D. Michael McIntyre]

I haven't had a working MIDI keyboard in so many years it's ridiculous, so I well understand how you might have skipped that step.

I glanced through the code, and have no clue what any of it means. I am just a user with commit access too. That hasn't changed as much as you'd imagine after 13 years. It's good to see someone with some fresh energy.

[Tito Latini]

The attached patch seems to fix all the problems.

noteoff with duration zero is scheduled from the internal segment
mapper and we don't use that message in realtime otherwise it is a
duplicate. When we receive a noteoff message from midi input, the
duration is never zero but at least 1 msec (see the case for
SND_SEQ_EVENT_NOTEOFF in getMappedEventList). 1 msec is used
for midi input to "Fix zero duration record bug" (comment in the code)
and we can use that information to avoid duplicated notes.

Besides, noteon from midi input is scheduled with duration -1 and we
don't use the noteoff stack for midi input.

Tested with:

• midi input
• midi import
• midi export
• manual insertion of the events
• jumps during playback
• loop mode

[Tito Latini]

bug ko -- patch applied (r14097).

[D. Michael McIntyre]

Fix confirmed for noodling at the keyboard. If I discover any new problems, I'll report back.