Summary. SingleGroupMidi1ToBytestreamTranslator::dispatch() (modules/juce_audio_basics/midi/ump/juce_UMPMidi1ToBytestreamTranslator.h) captures the packet time into pendingSysExTime only in its ongoingSysex branch. A SysEx short enough to fit one UMP packet is SysEx7::Kind::complete, which the extractor reports directly as lastSysex, so the time is never captured and the message is delivered with whatever pendingSysExTime last held: 0.0 on a fresh translator, or – since only reset() clears it – the timestamp of the previous multi-packet SysEx.
Present in 8.0.12 and unchanged on develop at dcf6bba08d (9.0.1).
How it shows up. Any SysEx of up to six data bytes. Mine was F0 00 00 66 14 00 F7 – a Mackie / Logic Control device query that a Mac control-surface scan sends to every MIDI destination, arriving at an iPad over IDAM. An iOS standalone plugin auto-opens every MIDI input (shouldAutoOpenMidiDevices defaults to true), and AudioProcessorPlayer::handleIncomingMidiMessage passes the message to MidiMessageCollector::addMessageToQueue unstamped, which trips
jassert (! approximatelyEqual (message.getTimeStamp(), 0.0));
In a Release build there is no crash, but the arithmetic is still wrong: sampleNumber = (0 - 0.001 * lastCallbackTime) * sampleRate is large and negative, so the “older than a second” guard (sampleNumber > sampleRate) can never fire, and removeNextBlockOfMessages() clamps the message to sample 0 instead of dropping it. The stale case is quieter: a short SysEx after a long one is simply timed as the earlier message.
Fix. Mirror the guard the ongoingSysex branch already has:
case SysexExtractorCallbackKind::lastSysex:
{
if (pendingSysExData.empty())
pendingSysExTime = time;
pendingSysExData.insert (pendingSysExData.end(), bytes.begin(), bytes.end());
...
Test. The attached patch also adds a case to UniversalMidiPacketTests in juce_UMP_test.cpp, “Single-packet SysEx7 messages carry the packet’s own timestamp”: it dispatches the query above as one packet at t=123.456 and expects 123.456; a start+end pair at 10.0/10.5 and expects 10.0 (first packet’s time – unchanged behaviour); the single packet again at 20.0 and expects 20.0; and a MIDI clock at 77.25 and expects 77.25 (real-time messages take the notSysex path and were never affected).
Against unpatched develop the new case fails exactly twice –
Expected value: 123.456, Actual value: 0
Expected value: 20, Actual value: 10
-- and with the one-line fix the whole “Universal MIDI Packet” test is 1706 passes, 0 failures. Same results on 8.0.12.
Patch against develop (dcf6bba08d) – git apply from the repo root. Happy to open a PR instead if that is preferred.
diff --git a/modules/juce_audio_basics/midi/ump/juce_UMPMidi1ToBytestreamTranslator.h b/modules/juce_audio_basics/midi/ump/juce_UMPMidi1ToBytestreamTranslator.h
index 7715b0919e..41e5856429 100644
--- a/modules/juce_audio_basics/midi/ump/juce_UMPMidi1ToBytestreamTranslator.h
+++ b/modules/juce_audio_basics/midi/ump/juce_UMPMidi1ToBytestreamTranslator.h
@@ -274,6 +274,17 @@ public:
case SysexExtractorCallbackKind::lastSysex:
{
+ // A SysEx that fits in a single UMP packet (SysEx7::Kind::complete) never
+ // passes through the ongoingSysex branch, so without this the message
+ // would be delivered with whatever pendingSysExTime last held: 0.0 on a
+ // fresh translator (tripping the timestamp assertion in
+ // MidiMessageCollector::addMessageToQueue), or -- since only reset()
+ // clears it -- the timestamp of the PREVIOUS multi-packet SysEx.
+ // Covered by "Single-packet SysEx7 messages carry the packet's own
+ // timestamp" in juce_UMP_test.cpp.
+ if (pendingSysExData.empty())
+ pendingSysExTime = time;
+
pendingSysExData.insert (pendingSysExData.end(), bytes.begin(), bytes.end());
if (pendingSysExData.empty())
diff --git a/modules/juce_audio_basics/midi/ump/juce_UMP_test.cpp b/modules/juce_audio_basics/midi/ump/juce_UMP_test.cpp
index 8a583189a7..afcd254fd6 100644
--- a/modules/juce_audio_basics/midi/ump/juce_UMP_test.cpp
+++ b/modules/juce_audio_basics/midi/ump/juce_UMP_test.cpp
@@ -168,6 +168,65 @@ public:
}
}
+ beginTest ("Single-packet SysEx7 messages carry the packet's own timestamp");
+ {
+ // A SysEx that fits in one UMP packet is SysEx7::Kind::complete, which the
+ // bytestream translator reports straight to its lastSysex branch. That
+ // branch used to leave pendingSysExTime alone (only ongoingSysex wrote it),
+ // so the message went out stamped with whatever the member last held: 0.0
+ // on a fresh translator, or -- since only reset() clears it -- the time of
+ // the previous multi-packet SysEx.
+ SingleGroupMidi1ToBytestreamTranslator translator (0);
+
+ struct Delivered { MidiMessage message; double time; };
+ std::vector<Delivered> delivered;
+
+ const auto collect = [&] (const BytesOnGroup& b, double t)
+ {
+ delivered.push_back ({ makeMidiMessage (b, t), t });
+ };
+
+ const auto lastTime = [&] { return delivered.empty() ? -1.0 : delivered.back().time; };
+
+ // F0 00 00 66 14 00 F7 -- a Mackie / Logic Control device query. Five data
+ // bytes fit one SysEx7 packet.
+ const std::byte query[] { std::byte { 0x00 }, std::byte { 0x00 }, std::byte { 0x66 },
+ std::byte { 0x14 }, std::byte { 0x00 } };
+ const auto single = Factory::makeSysExIn1Packet (0, Span<const std::byte> (query, std::size (query)));
+
+ translator.dispatch (View { single.data() }, 123.456, collect);
+ expectEquals ((int) delivered.size(), 1);
+ expect (! delivered.empty() && delivered.back().message.isSysEx());
+ expect (! delivered.empty() && delivered.back().message.getRawDataSize() == 7);
+ expectEquals (lastTime(), 123.456);
+
+ // A multi-packet SysEx keeps its FIRST packet's time, as it always did...
+ const std::byte longData[] { std::byte { 0x01 }, std::byte { 0x02 }, std::byte { 0x03 }, std::byte { 0x04 },
+ std::byte { 0x05 }, std::byte { 0x06 }, std::byte { 0x07 }, std::byte { 0x08 } };
+ const auto start = Factory::makeSysExStart (0, Span<const std::byte> (longData, 6));
+ const auto end = Factory::makeSysExEnd (0, Span<const std::byte> (longData + 6, 2));
+
+ translator.dispatch (View { start.data() }, 10.0, collect);
+ translator.dispatch (View { end.data() }, 10.5, collect);
+ expectEquals ((int) delivered.size(), 2);
+ expect (! delivered.empty() && delivered.back().message.getRawDataSize() == 10);
+ expectEquals (lastTime(), 10.0);
+
+ // ...and a single-packet SysEx after it is stamped with its own time, not
+ // the previous message's.
+ translator.dispatch (View { single.data() }, 20.0, collect);
+ expectEquals ((int) delivered.size(), 3);
+ expectEquals (lastTime(), 20.0);
+
+ // Real-time messages were never affected: they take the notSysex path,
+ // which passes the time straight through.
+ const auto clock = Factory::makeTimingClock (0);
+ translator.dispatch (View { clock.data() }, 77.25, collect);
+ expectEquals ((int) delivered.size(), 4);
+ expect (! delivered.empty() && delivered.back().message.isMidiClock());
+ expectEquals (lastTime(), 77.25);
+ }
+
beginTest ("UMP SysEx7 messages interspersed with utility messages convert to bytestream");
{
const auto sysEx = createRandomSysEx (random, 100);
-- Julius O. Smith III (CCRMA, Stanford / moForte)