I enabled the one bar count in mode in my edit (currently using the midi recording demo):
edit->setCountInMode(te::Edit::CountIn::oneBar);
Which seems to work great. If I start recording, the count in starts and then it starts recording after one bar has passed. (Side question: Is there a way to tell if the count in is currently happening?)
However, if I play midi notes during the count in, I hit the following assertion on line 114 of juce_MidiMessageSequence.cpp after I stop the recording:
int MidiMessageSequence::getIndexOfMatchingKeyUp (int index) const noexcept
{
if (auto* meh = list[index])
{
if (auto* noteOff = meh->noteOffObject)
{
for (int i = index; i < list.size(); ++i)
if (list.getUnchecked(i) == noteOff)
return i;
jassertfalse; // we've somehow got a pointer to a note-off object that isn't in the sequence
}
}
return -1;
}
I get the same number of assertions as the number of notes that were played during the count in before the recording starts. Is there something I am doing wrong? Nothing bad seems to happen, I just hit the assertion.
Hmm, the assertion is indicating that the note-off is present in the sequence but the note-on event isn’t there.
I’m a bit slammed to dig in to this right now especially as it’s not causing a problem.
If you wanted to take a look yourself I would start by seeing if the note-on is added to the recorded sequence at all. Firstly try MidiInputDeviceInstanceBase::handleIncomingMidiMessage and then when the final recorded sequence is built in MidiInputDeviceInstanceBase::applyLastRecordingToEdit.
I might have found the problem in MidiInputDeviceInstanceBase::applyLastRecordingToEdit. For example I begain recording, the count in began, and I played a single C2 and made sure the note on and off events occurred while the count in was still happening and before the recording actually started.
In this case both the note on and not off event get added to the events to delete since the isOutsideClipAndNotNoteOffMessage returns true for any message that occurs before the recording start position, regardless of its a note off or not (there may be a reason for this logic though and perhaps the method just doesnt have the best name):
// This is at line 854
const auto isOutsideClipAndNotNoteOff = [startPos, maxEndPos] (const juce::MidiMessage& m)
{
return m.getTimeStamp() < startPos || (m.getTimeStamp() > maxEndPos && ! m.isNoteOff());
};
...
...
...
// This is at line 894
if (isOutsideClipAndNotNoteOff (m.message))
eventsToDelete.add (i);
Then when the events are deleted
if (! eventsToDelete.isEmpty())
{
// N.B. eventsToDelete is in reverse order so iterate forwards when deleting
for (int index : eventsToDelete)
recorded.deleteEvent (index, true);
}
Since the events are ordered in reverse of when they came in, the note off gets deleted first. That works fine, but the assert occurs when the note on is to be deleted. I think there needs to be a call to
updateMatchedPairs() after a note is deleted from the sequence, since it seems like the sequence still thinks the note on has a matching note off even though it was deleted just before.
Adding the call to updateMatchedPairs seems to resolve the assertion issue.
if (! eventsToDelete.isEmpty())
{
// N.B. eventsToDelete is in reverse order so iterate forwards when deleting
for (int index : eventsToDelete)
{
recorded.deleteEvent (index, true);
recorded.updateMatchedPairs();
}
}
Thanks for digging in to it.
Calling updateMatchedPairs after every deletion seems very costly though as it could take a bit of time if the sequence is long… And I think that would mess with the eventsToDelete indices because it would modify the sequence?
I wonder if it’s possible to just not delete the events that are would cause assertions (the note-ons that have no note-offs?) and then call updateMatchedPairs afterwards?
I think it’s to avoid having dangling note-offs in the sequence.
I dont’t think updateMatchedPairs will remove dangling note-offs.
Maybe the only way to do this without hitting the assertion is to only delete the indexed events ( recorded.deleteEvent (index, false);) and then manually iterate the sequence for note-offs and if getIndexOfMatchingKeyUp returns -1 delete the event…
I think that would probably be equivalent to the same thing and only add a few extra iterations of the sequence.
alright I changed the delete event call to not delete the matching note offs, then looped over the events looking for orphaned note ons and deleted them.
The sequences produced are the same as the ones using the original method of deletion, just without throwing assertions. I just did a quick few tests though with 24 note sequences and manually verified they were the same.
if (! eventsToDelete.isEmpty())
{
// N.B. eventsToDelete is in reverse order so iterate forwards when deleting
for (int index = eventsToDelete.size(); --index >= 0;)
recorded.deleteEvent (index, false);
// Look for orphaned note ons that do not have a matching note off.
for (int index = 0; index < recorded.getNumEvents(); index++)
{
if (recorded.getEventPointer(index)->message.isNoteOn())
if (recorded.getIndexOfMatchingKeyUp(index) == -1)
recorded.deleteEvent (index, false);
}
}
