From e1da55ccc75e308bbed5ea89b9e7227eed64422d Mon Sep 17 00:00:00 2001 From: reuk Date: Wed, 19 Apr 2023 13:27:30 +0100 Subject: [PATCH] MidiRPN: Adjust MSB and LSB order for improved conformance to the MIDI 1.0 spec --- .../juce_audio_basics/midi/juce_MidiRPN.cpp | 364 ++++++++++-------- modules/juce_audio_basics/midi/juce_MidiRPN.h | 37 +- .../mpe/juce_MPEZoneLayout.cpp | 9 +- 3 files changed, 240 insertions(+), 170 deletions(-) diff --git a/modules/juce_audio_basics/midi/juce_MidiRPN.cpp b/modules/juce_audio_basics/midi/juce_MidiRPN.cpp index ae5fc500f2..f16e73a76e 100644 --- a/modules/juce_audio_basics/midi/juce_MidiRPN.cpp +++ b/modules/juce_audio_basics/midi/juce_MidiRPN.cpp @@ -23,42 +23,46 @@ namespace juce { -MidiRPNDetector::MidiRPNDetector() noexcept -{ -} - -MidiRPNDetector::~MidiRPNDetector() noexcept -{ -} - bool MidiRPNDetector::parseControllerMessage (int midiChannel, int controllerNumber, int controllerValue, MidiRPNMessage& result) noexcept +{ + auto parsed = tryParse (midiChannel, controllerNumber, controllerValue); + + if (! parsed.has_value()) + return false; + + result = *parsed; + return true; +} + +std::optional MidiRPNDetector::tryParse (int midiChannel, + int controllerNumber, + int controllerValue) { jassert (midiChannel > 0 && midiChannel <= 16); jassert (controllerNumber >= 0 && controllerNumber < 128); jassert (controllerValue >= 0 && controllerValue < 128); - return states[midiChannel - 1].handleController (midiChannel, controllerNumber, controllerValue, result); + return states[midiChannel - 1].handleController (midiChannel, controllerNumber, controllerValue); } void MidiRPNDetector::reset() noexcept { - for (int i = 0; i < 16; ++i) + for (auto& state : states) { - states[i].parameterMSB = 0xff; - states[i].parameterLSB = 0xff; - states[i].resetValue(); - states[i].isNRPN = false; + state.parameterMSB = 0xff; + state.parameterLSB = 0xff; + state.resetValue(); + state.isNRPN = false; } } //============================================================================== -bool MidiRPNDetector::ChannelState::handleController (int channel, - int controllerNumber, - int value, - MidiRPNMessage& result) noexcept +std::optional MidiRPNDetector::ChannelState::handleController (int channel, + int controllerNumber, + int value) noexcept { switch (controllerNumber) { @@ -68,13 +72,11 @@ bool MidiRPNDetector::ChannelState::handleController (int channel, case 0x64: parameterLSB = uint8 (value); resetValue(); isNRPN = false; break; case 0x65: parameterMSB = uint8 (value); resetValue(); isNRPN = false; break; - case 0x06: valueMSB = uint8 (value); return sendIfReady (channel, result); - case 0x26: valueLSB = uint8 (value); break; - - default: break; + case 0x06: valueMSB = uint8 (value); valueLSB = 0xff; return sendIfReady (channel); + case 0x26: valueLSB = uint8 (value); return sendIfReady (channel); } - return false; + return {}; } void MidiRPNDetector::ChannelState::resetValue() noexcept @@ -84,32 +86,28 @@ void MidiRPNDetector::ChannelState::resetValue() noexcept } //============================================================================== -bool MidiRPNDetector::ChannelState::sendIfReady (int channel, MidiRPNMessage& result) noexcept +std::optional MidiRPNDetector::ChannelState::sendIfReady (int channel) noexcept { - if (parameterMSB < 0x80 && parameterLSB < 0x80) - { - if (valueMSB < 0x80) - { - result.channel = channel; - result.parameterNumber = (parameterMSB << 7) + parameterLSB; - result.isNRPN = isNRPN; + if (parameterMSB >= 0x80 || parameterLSB >= 0x80 || valueMSB >= 0x80) + return {}; - if (valueLSB < 0x80) - { - result.value = (valueMSB << 7) + valueLSB; - result.is14BitValue = true; - } - else - { - result.value = valueMSB; - result.is14BitValue = false; - } + MidiRPNMessage result{}; + result.channel = channel; + result.parameterNumber = (parameterMSB << 7) + parameterLSB; + result.isNRPN = isNRPN; - return true; - } + if (valueLSB < 0x80) + { + result.value = (valueMSB << 7) + valueLSB; + result.is14BitValue = true; + } + else + { + result.value = valueMSB; + result.is14BitValue = false; } - return false; + return result; } //============================================================================== @@ -132,25 +130,26 @@ MidiBuffer MidiRPNGenerator::generate (int midiChannel, jassert (parameterNumber >= 0 && parameterNumber < 16384); jassert (value >= 0 && value < (use14BitValue ? 16384 : 128)); - uint8 parameterLSB = uint8 (parameterNumber & 0x0000007f); - uint8 parameterMSB = uint8 (parameterNumber >> 7); + auto parameterLSB = uint8 (parameterNumber & 0x0000007f); + auto parameterMSB = uint8 (parameterNumber >> 7); uint8 valueLSB = use14BitValue ? uint8 (value & 0x0000007f) : 0x00; uint8 valueMSB = use14BitValue ? uint8 (value >> 7) : uint8 (value); - uint8 channelByte = uint8 (0xb0 + midiChannel - 1); + auto channelByte = uint8 (0xb0 + midiChannel - 1); MidiBuffer buffer; buffer.addEvent (MidiMessage (channelByte, isNRPN ? 0x62 : 0x64, parameterLSB), 0); buffer.addEvent (MidiMessage (channelByte, isNRPN ? 0x63 : 0x65, parameterMSB), 0); - // sending the value LSB is optional, but must come before sending the value MSB: + buffer.addEvent (MidiMessage (channelByte, 0x06, valueMSB), 0); + + // According to the MIDI spec, whenever a MSB is received, the corresponding LSB will + // be reset. Therefore, the LSB should be sent after the MSB. if (use14BitValue) buffer.addEvent (MidiMessage (channelByte, 0x26, valueLSB), 0); - buffer.addEvent (MidiMessage (channelByte, 0x06, valueMSB), 0); - return buffer; } @@ -168,134 +167,196 @@ public: void runTest() override { - beginTest ("7-bit RPN"); + // From the MIDI 1.0 spec: + // If 128 steps of resolution is sufficient the second byte (LSB) of the data value can be + // omitted. If both the MSB and LSB are sent initially, a subsequent fine adjustment only + // requires the sending of the LSB. The MSB does not have to be retransmitted. If a + // subsequent major adjustment is necessary the MSB must be transmitted again. When an MSB + // is received, the receiver should set its concept of the LSB to zero. + + beginTest ("Individual MSB is parsed as 7-bit"); + { + MidiRPNDetector detector; + expect (! detector.tryParse (2, 101, 0)); + expect (! detector.tryParse (2, 100, 7)); + + auto parsed = detector.tryParse (2, 6, 42); + expect (parsed.has_value()); + + expectEquals (parsed->channel, 2); + expectEquals (parsed->parameterNumber, 7); + expectEquals (parsed->value, 42); + expect (! parsed->isNRPN); + expect (! parsed->is14BitValue); + } + + beginTest ("LSB without preceding MSB is ignored"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (2, 101, 0, rpn)); - expect (! detector.parseControllerMessage (2, 100, 7, rpn)); - expect (detector.parseControllerMessage (2, 6, 42, rpn)); - - expectEquals (rpn.channel, 2); - expectEquals (rpn.parameterNumber, 7); - expectEquals (rpn.value, 42); - expect (! rpn.isNRPN); - expect (! rpn.is14BitValue); + expect (! detector.tryParse (2, 101, 0)); + expect (! detector.tryParse (2, 100, 7)); + expect (! detector.tryParse (2, 38, 42)); } - beginTest ("14-bit RPN"); + beginTest ("LSB following MSB is parsed as 14-bit"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (1, 100, 44, rpn)); - expect (! detector.parseControllerMessage (1, 101, 2, rpn)); - expect (! detector.parseControllerMessage (1, 38, 94, rpn)); - expect (detector.parseControllerMessage (1, 6, 1, rpn)); - - expectEquals (rpn.channel, 1); - expectEquals (rpn.parameterNumber, 300); - expectEquals (rpn.value, 222); - expect (! rpn.isNRPN); - expect (rpn.is14BitValue); + expect (! detector.tryParse (1, 101, 2)); + expect (! detector.tryParse (1, 100, 44)); + + expect (detector.tryParse (1, 6, 1).has_value()); + + auto lsbParsed = detector.tryParse (1, 38, 94); + expect (lsbParsed.has_value()); + + expectEquals (lsbParsed->channel, 1); + expectEquals (lsbParsed->parameterNumber, 300); + expectEquals (lsbParsed->value, 222); + expect (! lsbParsed->isNRPN); + expect (lsbParsed->is14BitValue); + } + + beginTest ("Multiple LSB following MSB re-use the MSB"); + { + MidiRPNDetector detector; + expect (! detector.tryParse (1, 101, 2)); + expect (! detector.tryParse (1, 100, 43)); + + expect (detector.tryParse (1, 6, 1).has_value()); + + expect (detector.tryParse (1, 38, 94).has_value()); + expect (detector.tryParse (1, 38, 95).has_value()); + expect (detector.tryParse (1, 38, 96).has_value()); + + auto lsbParsed = detector.tryParse (1, 38, 97); + expect (lsbParsed.has_value()); + + expectEquals (lsbParsed->channel, 1); + expectEquals (lsbParsed->parameterNumber, 299); + expectEquals (lsbParsed->value, 225); + expect (! lsbParsed->isNRPN); + expect (lsbParsed->is14BitValue); + } + + beginTest ("Sending a new MSB resets the LSB"); + { + MidiRPNDetector detector; + expect (! detector.tryParse (1, 101, 3)); + expect (! detector.tryParse (1, 100, 43)); + + expect (detector.tryParse (1, 6, 1).has_value()); + expect (detector.tryParse (1, 38, 94).has_value()); + + auto newMsb = detector.tryParse (1, 6, 2); + expect (newMsb.has_value()); + + expectEquals (newMsb->channel, 1); + expectEquals (newMsb->parameterNumber, 427); + expectEquals (newMsb->value, 2); + expect (! newMsb->isNRPN); + expect (! newMsb->is14BitValue); } beginTest ("RPNs on multiple channels simultaneously"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (1, 100, 44, rpn)); - expect (! detector.parseControllerMessage (2, 101, 0, rpn)); - expect (! detector.parseControllerMessage (1, 101, 2, rpn)); - expect (! detector.parseControllerMessage (2, 100, 7, rpn)); - expect (! detector.parseControllerMessage (1, 38, 94, rpn)); - expect (detector.parseControllerMessage (2, 6, 42, rpn)); - - expectEquals (rpn.channel, 2); - expectEquals (rpn.parameterNumber, 7); - expectEquals (rpn.value, 42); - expect (! rpn.isNRPN); - expect (! rpn.is14BitValue); - - expect (detector.parseControllerMessage (1, 6, 1, rpn)); - - expectEquals (rpn.channel, 1); - expectEquals (rpn.parameterNumber, 300); - expectEquals (rpn.value, 222); - expect (! rpn.isNRPN); - expect (rpn.is14BitValue); + expect (! detector.tryParse (1, 100, 44)); + expect (! detector.tryParse (2, 101, 0)); + expect (! detector.tryParse (1, 101, 2)); + expect (! detector.tryParse (2, 100, 7)); + expect (detector.tryParse (1, 6, 1).has_value()); + + auto channelTwo = detector.tryParse (2, 6, 42); + expect (channelTwo.has_value()); + + expectEquals (channelTwo->channel, 2); + expectEquals (channelTwo->parameterNumber, 7); + expectEquals (channelTwo->value, 42); + expect (! channelTwo->isNRPN); + expect (! channelTwo->is14BitValue); + + auto channelOne = detector.tryParse (1, 38, 94); + expect (channelOne.has_value()); + + expectEquals (channelOne->channel, 1); + expectEquals (channelOne->parameterNumber, 300); + expectEquals (channelOne->value, 222); + expect (! channelOne->isNRPN); + expect (channelOne->is14BitValue); } beginTest ("14-bit RPN with value within 7-bit range"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (16, 100, 0 , rpn)); - expect (! detector.parseControllerMessage (16, 101, 0, rpn)); - expect (! detector.parseControllerMessage (16, 38, 3, rpn)); - expect (detector.parseControllerMessage (16, 6, 0, rpn)); - - expectEquals (rpn.channel, 16); - expectEquals (rpn.parameterNumber, 0); - expectEquals (rpn.value, 3); - expect (! rpn.isNRPN); - expect (rpn.is14BitValue); + expect (! detector.tryParse (16, 100, 0)); + expect (! detector.tryParse (16, 101, 0)); + expect (detector.tryParse (16, 6, 0).has_value()); + + auto parsed = detector.tryParse (16, 38, 3); + expect (parsed.has_value()); + + expectEquals (parsed->channel, 16); + expectEquals (parsed->parameterNumber, 0); + expectEquals (parsed->value, 3); + expect (! parsed->isNRPN); + expect (parsed->is14BitValue); } beginTest ("invalid RPN (wrong order)"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (2, 6, 42, rpn)); - expect (! detector.parseControllerMessage (2, 101, 0, rpn)); - expect (! detector.parseControllerMessage (2, 100, 7, rpn)); + expect (! detector.tryParse (2, 6, 42)); + expect (! detector.tryParse (2, 101, 0)); + expect (! detector.tryParse (2, 100, 7)); } beginTest ("14-bit RPN interspersed with unrelated CC messages"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (16, 3, 80, rpn)); - expect (! detector.parseControllerMessage (16, 100, 0 , rpn)); - expect (! detector.parseControllerMessage (16, 4, 81, rpn)); - expect (! detector.parseControllerMessage (16, 101, 0, rpn)); - expect (! detector.parseControllerMessage (16, 5, 82, rpn)); - expect (! detector.parseControllerMessage (16, 5, 83, rpn)); - expect (! detector.parseControllerMessage (16, 38, 3, rpn)); - expect (! detector.parseControllerMessage (16, 4, 84, rpn)); - expect (! detector.parseControllerMessage (16, 3, 85, rpn)); - expect (detector.parseControllerMessage (16, 6, 0, rpn)); - - expectEquals (rpn.channel, 16); - expectEquals (rpn.parameterNumber, 0); - expectEquals (rpn.value, 3); - expect (! rpn.isNRPN); - expect (rpn.is14BitValue); + expect (! detector.tryParse (16, 3, 80)); + expect (! detector.tryParse (16, 100, 0)); + expect (! detector.tryParse (16, 4, 81)); + expect (! detector.tryParse (16, 101, 0)); + expect (! detector.tryParse (16, 5, 82)); + expect (! detector.tryParse (16, 5, 83)); + expect (detector.tryParse (16, 6, 0).has_value()); + expect (! detector.tryParse (16, 4, 84).has_value()); + expect (! detector.tryParse (16, 3, 85).has_value()); + + auto parsed = detector.tryParse (16, 38, 3); + expect (parsed.has_value()); + + expectEquals (parsed->channel, 16); + expectEquals (parsed->parameterNumber, 0); + expectEquals (parsed->value, 3); + expect (! parsed->isNRPN); + expect (parsed->is14BitValue); } beginTest ("14-bit NRPN"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (1, 98, 44, rpn)); - expect (! detector.parseControllerMessage (1, 99 , 2, rpn)); - expect (! detector.parseControllerMessage (1, 38, 94, rpn)); - expect (detector.parseControllerMessage (1, 6, 1, rpn)); - - expectEquals (rpn.channel, 1); - expectEquals (rpn.parameterNumber, 300); - expectEquals (rpn.value, 222); - expect (rpn.isNRPN); - expect (rpn.is14BitValue); + expect (! detector.tryParse (1, 98, 44)); + expect (! detector.tryParse (1, 99 , 2)); + expect (detector.tryParse (1, 6, 1).has_value()); + + auto parsed = detector.tryParse (1, 38, 94); + expect (parsed.has_value()); + + expectEquals (parsed->channel, 1); + expectEquals (parsed->parameterNumber, 300); + expectEquals (parsed->value, 222); + expect (parsed->isNRPN); + expect (parsed->is14BitValue); } beginTest ("reset"); { MidiRPNDetector detector; - MidiRPNMessage rpn; - expect (! detector.parseControllerMessage (2, 101, 0, rpn)); + expect (! detector.tryParse (2, 101, 0)); detector.reset(); - expect (! detector.parseControllerMessage (2, 100, 7, rpn)); - expect (! detector.parseControllerMessage (2, 6, 42, rpn)); + expect (! detector.tryParse (2, 100, 7)); + expect (! detector.tryParse (2, 6, 42)); } } }; @@ -346,25 +407,24 @@ private: //============================================================================== void expectContainsRPN (const MidiBuffer& midiBuffer, MidiRPNMessage expected) { - MidiRPNMessage result = MidiRPNMessage(); + std::optional result; MidiRPNDetector detector; for (const auto metadata : midiBuffer) { const auto midiMessage = metadata.getMessage(); - if (detector.parseControllerMessage (midiMessage.getChannel(), - midiMessage.getControllerNumber(), - midiMessage.getControllerValue(), - result)) - break; + result = detector.tryParse (midiMessage.getChannel(), + midiMessage.getControllerNumber(), + midiMessage.getControllerValue()); } - expectEquals (result.channel, expected.channel); - expectEquals (result.parameterNumber, expected.parameterNumber); - expectEquals (result.value, expected.value); - expect (result.isNRPN == expected.isNRPN); - expect (result.is14BitValue == expected.is14BitValue); + expect (result.has_value()); + expectEquals (result->channel, expected.channel); + expectEquals (result->parameterNumber, expected.parameterNumber); + expectEquals (result->value, expected.value); + expect (result->isNRPN == expected.isNRPN); + expect (result->is14BitValue == expected.is14BitValue); } }; diff --git a/modules/juce_audio_basics/midi/juce_MidiRPN.h b/modules/juce_audio_basics/midi/juce_MidiRPN.h index f1fb9ff01e..1f4fcf3c9c 100644 --- a/modules/juce_audio_basics/midi/juce_MidiRPN.h +++ b/modules/juce_audio_basics/midi/juce_MidiRPN.h @@ -68,10 +68,10 @@ class JUCE_API MidiRPNDetector { public: /** Constructor. */ - MidiRPNDetector() noexcept; + MidiRPNDetector() noexcept = default; /** Destructor. */ - ~MidiRPNDetector() noexcept; + ~MidiRPNDetector() noexcept = default; /** Resets the RPN detector's internal state, so that it forgets about previously received MIDI CC messages. @@ -79,26 +79,39 @@ public: void reset() noexcept; //============================================================================== - /** Takes the next in a stream of incoming MIDI CC messages and returns true - if it forms the last of a sequence that makes an RPN or NPRN. - - If this returns true, then the RPNMessage object supplied will be - filled-out with the message's details. - (If it returns false then the RPNMessage object will be unchanged). - */ + /** @see tryParse() */ + [[deprecated ("Use tryParse() instead")]] bool parseControllerMessage (int midiChannel, int controllerNumber, int controllerValue, MidiRPNMessage& result) noexcept; + /** Takes the next in a stream of incoming MIDI CC messages and returns + a MidiRPNMessage if the current message produces a well-formed RPN or NRPN. + + Note that senders are expected to send the MSB before the LSB, but senders are + not required to send a LSB at all. Therefore, tryParse() will return a non-null + optional on all MSB messages (provided a parameter number has been set), and will + also return a non-null optional for each LSB that follows the initial MSB. + + This behaviour allows senders to transmit a single MSB followed by multiple LSB + messages to facilitate fine-tuning of parameters. + + The result of parsing a MSB will always be a 7-bit value. + The result of parsing a LSB that follows an MSB will always be a 14-bit value. + */ + std::optional tryParse (int midiChannel, + int controllerNumber, + int controllerValue); + private: //============================================================================== struct ChannelState { - bool handleController (int channel, int controllerNumber, - int value, MidiRPNMessage&) noexcept; + std::optional handleController (int channel, int controllerNumber, + int value) noexcept; void resetValue() noexcept; - bool sendIfReady (int channel, MidiRPNMessage&) noexcept; + std::optional sendIfReady (int channel) noexcept; uint8 parameterMSB = 0xff, parameterLSB = 0xff, valueMSB = 0xff, valueLSB = 0xff; bool isNRPN = false; diff --git a/modules/juce_audio_basics/mpe/juce_MPEZoneLayout.cpp b/modules/juce_audio_basics/mpe/juce_MPEZoneLayout.cpp index e51f314dae..b86e179b19 100644 --- a/modules/juce_audio_basics/mpe/juce_MPEZoneLayout.cpp +++ b/modules/juce_audio_basics/mpe/juce_MPEZoneLayout.cpp @@ -108,14 +108,11 @@ void MPEZoneLayout::processNextMidiEvent (const MidiMessage& message) if (! message.isController()) return; - MidiRPNMessage rpn; - - if (rpnDetector.parseControllerMessage (message.getChannel(), + if (auto parsed = rpnDetector.tryParse (message.getChannel(), message.getControllerNumber(), - message.getControllerValue(), - rpn)) + message.getControllerValue())) { - processRpnMessage (rpn); + processRpnMessage (*parsed); } }