Note off velocity

Hi,

I use a MidiKeyboardComponent to display notes played from an external MIDI keyboard and to allow triggering MIDI notes from the application GUI. So, incoming MIDI Note events are sent to a MidiKeyboardState object that merges them with MIDI Note events coming from the GUI component, and sends the whole thing to MIDI out.

My problem is that by doing so, I lose the Note Off velocity information coming from the external MIDI device when forwarding Note Off messages to MIDI out through the Keyboard component stuff.

So I have a minor request: could you please add support for NoteOff velocity for this stuff?

Unless I've missed something, here are the needed changes: (additions in bold)

in juce_MidiKeyboardState.h:

    virtual void handleNoteOff (MidiKeyboardState* source,
        int midiChannel, int midiNoteNumber, float velocity=0) = 0;

    void noteOff(int midiChannel, int midiNoteNumber, float velocity = 0);

    void noteOffInternal(int midiChannel, int midiNoteNumber, float velocity=0);

in juce_MidiKeyboardState.cpp:

void MidiKeyboardState::noteOff(const int midiChannel, const int midiNoteNumber, float velocity)
{
    const ScopedLock sl (lock);

    if (isNoteOn (midiChannel, midiNoteNumber))
    {
        const int timeNow = (int) Time::getMillisecondCounter();
        eventsToAdd.addEvent(MidiMessage::noteOff(midiChannel, midiNoteNumber, velocity), timeNow);
        eventsToAdd.clear (0, timeNow - 500);

        noteOffInternal (midiChannel, midiNoteNumber, velocity);
    }
}

void MidiKeyboardState::noteOffInternal(const int midiChannel, const int midiNoteNumber, float velocity)
{
    if (isNoteOn (midiChannel, midiNoteNumber))
    {
        noteStates [midiNoteNumber] &= ~(1 << (midiChannel - 1));

        for (int i = listeners.size(); --i >= 0;)
            listeners.getUnchecked(i)->handleNoteOff (this, midiChannel, midiNoteNumber, velocity);
    }
}

void MidiKeyboardState::processNextMidiEvent (const MidiMessage& message)
{
    if (message.isNoteOn())
    {
        noteOnInternal (message.getChannel(), message.getNoteNumber(), message.getFloatVelocity());
    }
    else if (message.isNoteOff())
    {
        noteOffInternal (message.getChannel(), message.getNoteNumber(), message.getFloatVelocity());
    }
    else if (message.isAllNotesOff())
    {
        for (int i = 0; i < 128; ++i)
            noteOffInternal (message.getChannel(), i);
    }

In juce_MidiMessageCollector.h:

    void handleNoteOff (MidiKeyboardState*, int midiChannel, int midiNoteNumber, float velocity=0) override;

In juce_MidiMessageCollector.cpp:

void MidiMessageCollector::handleNoteOff (MidiKeyboardState*, int midiChannel, int midiNoteNumber, float velocity)
{
    MidiMessage m (MidiMessage::noteOff (midiChannel, midiNoteNumber, velocity));
    m.setTimeStamp (Time::getMillisecondCounterHiRes() * 0.001);

    addMessageToQueue (m);
}

In juce_MidiMidiKeyboardComponent.h:

    void handleNoteOff (MidiKeyboardState*, int midiChannel, int midiNoteNumber, float velocity=0) override;

In juce_MidiMidiKeyboardComponent.cpp:

void MidiKeyboardComponent::handleNoteOff(MidiKeyboardState*, int /*midiChannel*/, int /*midiNoteNumber*/, float /*velocity*/)

void MidiKeyboardComponent::updateNoteUnderMouse (Point<int> pos, bool isDown, int fingerNum)
{
[...]

   else if (oldNoteDown >= 0)
    {
        mouseDownNotes.set (fingerNum, -1);

        if (!mouseDownNotes.contains(oldNoteDown))
        {
            if (!useMousePositionForVelocity)
                mousePositionVelocity = 1.0f;

            state.noteOff(midiChannel, oldNoteDown, mousePositionVelocity * velocity);
        }
    }
}

 

As these arguments are made optional, there will be no compatibility issue with prior Juce versions

This addition would be greatly appreciated (so I won't have to re-edit Juce code after each update, and hopefully it can be helpful to other users)

Thanks in advance!

 

Note: original post just updated with actual Juce code modifications (at least the ones I figured out and that work for me...)

Thanks - it's a good FR but it not correct to say there'd be no compatibility issues - virtual methods should never have default arguments, so it would have to be non-defaulted, and would break both user overrides of the virtual methods, and code that calls it..

Fair enough, it won't be compatible then... or isn't it possible to overload the existing virtual function with a 3 argument variant instead, so you can use one or the other? Worst case the code incompatibilities will be easy to fix and it doesn't seem to be Juce policy to preserve 100% compatibility between version (I often have to tweak a few lines of my code when I update Juce)

Anyway, you're the C++ guru, I let it up to you to figure out the best way. As long as I can pass Note Off Velocity along through the MidiKeyboard stuff, I'm happy ;-)