juce-framework / JUCE

JUCE is an open-source cross-platform C++ application framework for desktop and mobile applications, including VST, VST3, AU, AUv3, LV2 and AAX audio plug-ins.
https://juce.com
Other
6.31k stars 1.67k forks source link

MPEZoneLayout: reset MidiRPNDetector once RPN message processed #1331

Closed LaurentClerc closed 5 months ago

LaurentClerc commented 5 months ago

This is to address this issue raised on the JUCE forum: https://forum.juce.com/t/juce-mpe-mode-configuration-implementation-issue/59537

As detailed in the MIDI spec - the MPE Configuration Message is an RPN message:

Message Format: [Bn 64 06] [Bn 65 00] [Bn 06 <mm>]
 n = MIDI Channel Number
 n=0: Lower Zone Master Channel
 n=F: Upper Zone Master Channel
 All other values are invalid and should be ignored
 mm = Number of Member MIDI Channels in the Zone
 mm=0: MPE is Off (No Channels)
 mm=1 to F: Assigns that number of MIDI Channels to the Zone

It only expects the MSB portion of the Parameter Value - and ignores the LSB. With that in mind, it would be best to reset the MidiRPNDetector object after the MSB value is received and avoid any spurious updates happening after (since otherwise the MidiRPNDetector object would remain active to receive any further MSB and LSB messages - as per the Running Status implementation)

reuk commented 5 months ago

it would be best to reset the MidiRPNDetector object after the MSB value is received and avoid any spurious updates happening after

I'm not convinced. The MIDI 1.0 spec says the following:

After the reception of Non-Registered (or Registered) Parameter Numbers has been enabled, the receiver should wait until it receives both the LSB and MSB for a parameter number... The receiver should be able to respond accordingly if the transmitter sends only an LSB or MSB to change the parameter number.

I interpret this as meaning that, if we send Bn 64 06 Bn 65 00 to select the MPE Configuration parameter, then we should remember the MSB and LSB of this parameter number. If we then send a parameter number LSB message, Bn 64 00, then this should select the master pitch bend range parameter, with no need to send the parameter number MSB again.

Resetting the MidiRPNDetector detector would cause it to forget the LSB and MSB of the selected parameter number, so this use-case would fail.

The spec also says:

It is recommended that the LSB and MSB be sent each time a new parameter number is selected

From the forum post, it sounds like the problem is caused by combining two MIDI streams, where each stream attempts to modify a different registered parameter without selecting the intended RPN each time.

How are the MIDI input streams combined? Are you relying on the OS to combine the streams, or do you combine them yourself? If you're doing this yourself, then I think the correct way to implement this is for the combiner to maintain a separate RPN state for each of the input streams, and also for the output stream. Whenever a data-entry, data-increment, or data-decrement message is encountered on either input stream, if the selected RPN on that input differs from the selected RPN on the output, then insert additional messages in the output stream to select the correct RPN.

LaurentClerc commented 5 months ago

I agree that the Midi 1.0 spec with regards to RPN and NRPN messages should allow for receiving individual MSB and LSB messages after the Parameter value portion of the message has been received. This is the general use case of RPN/NRPN messages. However in the specific case of MPE MCM config message, the specification states that for MCM config message the LSB is ignored:

The MPE Configuration message (AMEI/MMA CA-034) is defined as Registered Parameter Number "00 06". The MSB of Data Entry represents the number of MIDI Channels assigned, as explained below. The LSB of Data Entry is not used.

So once the MSB data entry (CC6) portion of the RPN message has been received, there is no need to wait for an LSB message - as it is not used as part of the MPE configuration.

...each stream attempts to modify a different registered parameter without selecting the intended RPN each time

In our specific case only one of the senders is failing to emit a full NRPN message (from a third party piece of hardware) and causing these issues.

I think the correct way to implement this is for the combiner to maintain a separate RPN state for each of the input streams

I had initially hoped to handle it this way, but the Midi streams are being combined at the DAW level (in this case Ableton) before being received by our plugin - so there's no way to differentiate between streams and handle RPN states for different sources from within our plugin.

LaurentClerc commented 5 months ago

As an alternative, adding a member function to the MPEZoneLayout class to reset the MidiRPNDetector object would be another (and safer) approach perhaps.

reuk commented 5 months ago

I had initially hoped to handle it this way, but the Midi streams are being combined at the DAW level (in this case Ableton) before being received by our plugin

Have you submitted this as a bug report? Ableton might not be aware of this behaviour. If the DAW is producing a MIDI stream that sets the RPN such that following data-entry messages apply to the wrong RPN, that sounds like it could be a bug. If you've not tested the latest version of Live, that might be worth trying too.

As an alternative, adding a member function to the MPEZoneLayout class to reset the MidiRPNDetector object would be another (and safer) approach perhaps.

I'm not sure about this. Having some kind of reset seems reasonable, but not for resolving the issue you're facing. I think there are valid use-cases where the sender of the MIDI messages might assume that the receiver has remembered the currently-selected RPN, so forgetting/resetting the selected RPN after selecting the MPE configuration RPN might also cause problems. If you wanted to keep the functionality you suggested, you'd also need to call processNextMidiEvent on each event, instead of processNextMidiBuffer, and you'd need to add your own check after processing each message to decide whether to call reset().

If you really want the behaviour you're suggesting, I think you could implement it by using your own MidiRPNDetector (i.e. avoid MPEZoneLayout::processNextMidiBuffer), and calling setLowerZone or setUpperZone on the MPEZoneLayout as appropriate. However, as mentioned previously, I think this could break things if the DAW decides to just send multiple data-entry messages without preceding RPN-selection messages when configuring MPE zone size and/or pitchbend range.

LaurentClerc commented 5 months ago

Thanks for the pointers. Much appreciated.