LMMS / LMMS/lmms

Notes stuck on after changing output channel mid-note

Open
#8,030 1 comment 0 reactions 0 assignees Claimed by @regulus79 View on GitHub
bug
Dominant language
C++
Stars
10.4k
Forks
1.3k
Avg merge
2d 13h
Merged PRs (30d)
7

Description

### System Information

Arch Linux, GNOME

### LMMS Version(s)

master(?)

### Most Recent Working Version

_No response_

### Bug Summary

Midi events are routed in an odd way in LMMS. There are two functions, `InstrumentTrack::processInEvent`, and `InstrumentTrack::processOutEvent` which both do similar, but different things. The in event function handles spawning NotePlayHandles and such, and the out event function actually sends the real midi signals to the plugin.

Interestingly, the in event function does not directly send any message to the plugin. It simply creates a new `NotePlayHandle`, and when that starts playing, it in turn forcefully calls `InstrumentTrack::processOutEvent` with a MidiNoteOn event to actually tell the plugin to start processing. And once the `NotePlayHandle` is done, it likewise calls `InstrumentTrack::processOutEvent` with a MidiNoteOff event to get the plugin to stop.

And for whatever reason, `InstrumentTrack::processOutEvent` doesn't only send the event to the plugin, but also does some pre-processing steps, such as applying master key, and changing the output channel depending on the configuration under the MIDI tab of an instrument.

This system is susceptible to bugs. If the preprocessing in `InstrumentTrack::processOutEvent` changes for whatever reason during the lifetime of a note, it's possible that the NoteOff signal will be on a different key/channel than the original NoteOn signal.

Because of this, on plugins which support multi-channel midi input, when holding down a note and changing the midi output channel, when you release the note, it doesn't stop.

Also--you may be curious, why doesn't this happen when changing the little square above the piano to change the base note? Well, fortunately, someone thought of that in https://github.com/LMMS/lmms/commit/d1eb9886fd810807cf55df40813b986c92019ca9 and made sure to record the original master key when a NotePlayHandle starts, and have it compute the difference between that and the current base note when sending the NoteOff signal. This prevents notes from staying on forever whenever you move the square. However, I don't think this system is ideal.

### Expected Behaviour

Notes should stop when you release them, even if the output channel has changed.

My proposed solution is that maybe we shouldn't be doing that preprocessing in `InstrumentTrack::processOutEvent`. Maybe we should move that stuff to `InstrumentTrack::processInEvent`, that way any calls to `InstrumentTrack::processOutEvent` will not be modified before being sent to the plugin. (This would also simplify the current way `NotePlayHandle` has to store the old base note.)

### Steps To Reproduce

1. On a midi-based instrument which supports multiple midi channels, press a key.
2. While holding down that key, increment the CHAN lcd under the midi output section of the plugin.
3. Release the key.
4. Because the note on message was sent on channel 1, but the note off message was sent on channel 2, the note never stops.

Note: zynaddsubfx, opulenz, and some VSTs do not differentiate midi channels. This bug should only occur on plugins which treat events on different channels as distinct (Vital, for instance).

(PS: Zynaddsubfx does support multi-channel input, but for some reason we force all the input to go through channel 1. Why? I'm not sure.)

### Logs

_No response_

### Screenshots / Minimum Reproducible Project

_No response_

### Please search the issue tracker for existing bug reports before submitting your own.

- [x] I have searched all existing issues and confirmed that this is not a duplicate.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with InstrumentTrack::processInEvent and InstrumentTrack::processOutEvent, then inspect NotePlayHandle and the existing master-key handling described in commit d1eb9886fd810807cf55df40813b986c92019ca9. Reproduce with a multi-channel MIDI instrument by changing CHAN while holding a note; done means the note stops after release despite the channel change.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
audio-video-rtc, desktop
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.