MidiRPN: Adjust MSB and LSB order for improved conformance to the MIDI 1.0 spec

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<MidiRPNMessage> 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<MidiRPNMessage> 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<MidiRPNMessage> 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<MidiRPNMessage> 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);
     }
 };
 

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<MidiRPNMessage> tryParse (int midiChannel,
+                                            int controllerNumber,
+                                            int controllerValue);
+
 private:
     //==============================================================================
     struct ChannelState
     {
-        bool handleController (int channel, int controllerNumber,
-                               int value, MidiRPNMessage&) noexcept;
+        std::optional<MidiRPNMessage> handleController (int channel, int controllerNumber,
+                                                        int value) noexcept;
         void resetValue() noexcept;
-        bool sendIfReady (int channel, MidiRPNMessage&) noexcept;
+        std::optional<MidiRPNMessage> sendIfReady (int channel) noexcept;
 
         uint8 parameterMSB = 0xff, parameterLSB = 0xff, valueMSB = 0xff, valueLSB = 0xff;
         bool isNRPN = false;

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);
     }
 }