MIDI Program Change received with incorrect program number

[artur-twardowski]

Setup:

• LMMS from current master under Linux (Ubuntu 18.04.3), compiled with VST support
• MIDI keyboard (used Alesis QX49)
• a2jmidid installed

Preparation for Jack-MIDI:

• In the LMMS settings window, select MIDI Interface "Jack-MIDI", then restart LMMS
• Execute the command: a2jmidid -e (in a separate terminal or in the background)
• Open the Connections window in Qjackctl and connect the output port of your MIDI keyboard (a2j/QX49) with input port of the LMMS

Preparation for ALSA-Sequencer:

• In the LMMS settings window, select MIDI Interface "ALSA-Sequencer), then restart LMMS

Steps to reproduce:

• Add an instance of VeSTige to the song editor
• Load into VeSTige any VST instrument that supports Program Change (e. g. DSK Strings, Superwave P8)
• Trigger the Program Change message from the MIDI keyboard

Expected behavior:

• The appropriate patch is selected - if we do Program Change with the value of 30, the patch 31 will be selected (MIDI counts programs from 0, LMMS starts numbering them from 1), for Program Change "0" the first patch will be chosen.

Actual behavior:

• For Jack-MIDI, the patch number is decreased by 12 - Program Change "12" corresponds to the first patch, Program Change "30" chooses 19th patch. Programs below 12 actually select patches 245 to 256.
• For ALSA-Sequencer, always the first patch is selected

Further remarks:

• I have added a few printf()'s inside InstrumentTrack::processInEvent function; the wrong values come directly from the MIDI clients' implementations. I have also checked the actual MIDI messages sent by the keyboard, using amidi in dump mode.
• Jack-MIDI client implementation utilizes MidiClientRaw::parseData function to convert the raw MIDI data into the higher-level data structures. In the place where MIDI Note On, Note Off, Key Pressure, Channel Pressure and Program Change events are handled the number of the key is decreased by KeysPerOctave constant (=12). I assume that the subtraction is needed for the former four events (even though I don't know the purpose), but for Program Change it is unneccessary. Since the function is a member of MidiClientRaw class, I expect that ALSA-Raw MIDI and OSS-Raw MIDI interfaces will also be affected by this bug, but I did not test them.

[michaelgregorius]

Hi @artur-twardowski,

if I understand correctly you are compiling LMMS yourself? Have you tried if it works if you handle the case for MidiProgramChange separately and without subtracting KeysPerOctave in MidiClientRaw::parseData?

So the relevant code section would look as follows:

case MidiNoteOff:
case MidiNoteOn:
case MidiKeyPressure:
case MidiChannelPressure:
    m_midiParseData.m_midiEvent.setKey( m_midiParseData.m_buffer[0] - KeysPerOctave );
    m_midiParseData.m_midiEvent.setVelocity( m_midiParseData.m_buffer[1] );
    break;
case MidiProgramChange:
    m_midiParseData.m_midiEvent.setKey( m_midiParseData.m_buffer[0] );
    m_midiParseData.m_midiEvent.setVelocity( m_midiParseData.m_buffer[1] );
    break;

I don't really know LMMS' MIDI code but it looks quite strange that a value of 12 is subtracted for all these events. Does the code for the "Note On" and "Note Off" events mean that LMMS plays every note an octave lower than other DAWs?

[artur-twardowski]

Hi,

Yes, I compile it myself from the yesterday's most recent commit on master (0db83c5).
The change you've proposed is exactly the same I did locally on my computer. Moreover, I modified the MidiAlsaSeq::run() function:

switch( ev->type )
...
	case SND_SEQ_EVENT_PGMCHANGE:
		dest->processInEvent( MidiEvent(
					MidiProgramChange,
				ev->data.control.channel,
				ev->data.control.value,	0, // changed here
				source ),
				MidiTime() );
					break;

I tested these changes with both Jack-MIDI and ALSA Sequencer, they work correctly now.
I will create a pull request with that change shortly.

Regarding subtraction of an octave, it seems that #1857 might be the reason.

[michaelgregorius]

More mappings

I have investigated a bit further and found that several MIDI values, e.g. for "Note On" and "Note Off" are manipulated in MidiAlsaSeq::run where a value of KeysPerOctave is subtracted as well. If I play an A4 on my MIDI keyboard I can see the debugger that the struct ev has a correct note value of 69 (see here). This value is then changed to 69 - 12 = 57 before it is passed into the MidiEvent that is processed by MidiPort::processInEvent.

What's surprising is that for example Triple Oscillator still plays an A4 = 440 Hz (69) instead of an A3 = 220 Hz (57). This means that somewhere the whole thing has to be corrected again.

I found out that this more or less happens in the constructor of InstrumentTrack where the following line is executed:

m_baseNoteModel.setInitValue( DefaultKey );

The value of DefaultKey is 57 which corresponds to the value that an A4 is mapped to. So the base note of the Instrument Track is set to 57. The Triple Oscillator takes the frequency that it should play from the NotePlayHandle that's passed into TripleOscillator::playNote. This frequency is calculated in NotePlayHandle::updateFrequency and one of the factors that determines the frequency is the value of the base note. If we don't take master pitch and detune into account the frequency is calculated as something that looks similar to the following:

m_frequency = BaseFreq * powf( 2.0f, key - baseNoteModel)

In the case of the pressed A4 key and baseNoteModel both have a value of 57 so we multiply BaseFreq by 2^0 = 1. BaseFreq has a value of 440 and thus we play A4.

LMMS can't handle the full note range of MIDI devices

In MidiPort::processInEvent the following checking code can be found:

if( inEvent.key() < 0 || inEvent.key() >= NumKeys )
{
    return;
}

If I trigger the lowest note on my MIDI keyboard this corresponds to a "Note On" with a value of 0. Because LMMS subtracts a value of 12 in MidiAlsaSeq::run before passing the value to MidiPort::processInEvent the aforementioned check returns because it sees a value of -12.

Put differently: if you had a keyboard with 128 keys for each MIDI value then LMMS would ignore some of them and would likely also not forward them to VSTs which might be interested in the full range.

I think LMMS should not manipulate the raw MIDI data and instead should just forward it. It should be left to consumers to decide how to deal with them, i.e. if they want to ignore some of the values.

I think I will create a separate issue for this.

[PhysSong]

Fixed in #5154. @artur-twardowski Thank you!