MIDI File Saving: Multiple End of Track Events Issue

General JUCE discussion
1

Hey Jules,

When saving a MIDI file, it becomes corrupted due to duplicated events. From what I can tell, it seems the issue stems from the code in this location: MidiFile::writeTrack().

The algorithm in this method is not checking to see if there’s an end of track i[/i] event existing prior to writing this event.


The image represents a basic MIDI file I created in a DAW, loaded it with my JUCE project’s code and saved it soon thereafter. The circled regions in this image demonstrate the issue I am having, and stem from the most current tip.


Here is the hex representation of the original file.

Cheers.

2

I’m struggling slightly to see what it should be doing… Can you explain what you think the algorithm should do?

3

The algorithm should simply be omitting all the end-of-track meta-event patterns (of this liking 0x00 0xFF 0x2F 0x00).

Here is a rundown version of what I think it should do:

[code]void MidiFile::writeTrack (OutputStream& mainOut, const int trackNum)
{
MemoryOutputStream out;

const MidiMessageSequence& ms = *tracks[trackNum];

int lastTick = 0;
char lastStatusByte = 0;

for (int i = 0; i < ms.getNumEvents(); ++i)
{
    MidiMessage& mm = ms.getEventPointer(i)->message;

    /////Added:
    if (mm.isEndOfTrackMetaEvent() == true)
    { //Omit this event
        continue;
    }
    /////

    const int tick = roundToInt (mm.getTimeStamp());
    const int delta = jmax (0, tick - lastTick);
    MidiFileHelpers::writeVariableLengthInt (out, delta);
    lastTick = tick;

    const char statusByte = *(mm.getRawData());

    if ((statusByte == lastStatusByte)
         && ((statusByte & 0xf0) != 0xf0)
         && i > 0
         && mm.getRawDataSize() > 1)
    {
        out.write (mm.getRawData() + 1, mm.getRawDataSize() - 1);
    }
    else
    {
        out.write (mm.getRawData(), mm.getRawDataSize());
    }

    lastStatusByte = statusByte;
}

out.writeByte (0);
const MidiMessage m (MidiMessage::endOfTrack());
out.write (m.getRawData(), m.getRawDataSize());

mainOut.writeIntBigEndian ((int) ByteOrder::bigEndianInt ("MTrk"));
mainOut.writeIntBigEndian ((int) out.getDataSize());

mainOut << out;

}[/code]

As it is, what I did was skip over the 0x2F event by checking if an event “isEndOfTrackMetaEvent”, and continuing on if it is.

To avoid corrupting a MIDI file further, I think what needs to happen now is:

  • During the for-loop:
    • check if the previous events are 0x00 and 0xFF, and next is 0x00.
    • remove these events/skip them (whichever is better).
    • continue adding the next events.

This would thus ensure correct saving of a MIDI file (excluding the fact of going back from seconds to ticks, of course).

4

Ok, thanks, I see now. But a couple of things are bothering me…

Firstly, looking at my code, I can’t understand why it’s adding the zero byte before the ff 2f 00 marker! I’ve obviously deliberately added code to do that, and it’s been producing files that work, but there’s nothing in the midi file standard that says you should do it, and it doesn’t seem to make any sense! I wrote the class a long time ago, and wish I’d added a comment explaining that decision…

Secondly, I can’t decide whether your suggestion is correct, and that end-of-track events should be removed when writing the track… or whether it’d be better to make the readTrack() remove those events when loading it. Any thoughts?

5

Doh… sorry, ignore my comment about the mystery zero - it’s the time delta, of course…

Been a while since I looked at any midi code!

6

Haha indeed that is the time delta.

Well, if you load a MIDI file that works via JUCE code, save it right away, it will create a bogus MIDI file. Unless I’m missing something completely, it’s reading the same event at the end of writeTrack([…]).

My thoughts are that, perhaps in this case, filtering this event series in readNextTrack([…]) and writeTrack ([…]) and adding it at the end may be a safer and robust solution for a JUCE user/programmer. This may seem like overkill, may slow down the process a little bit, but who knows what a user might do in between loading tracks, and later saving tracks.

A separate method that completely cleans out a track’s End of Track event series, and adds it at the end, could be called after reading a track, and before writing it to a file, probably eliminating the need for this code in writeTrack([…]):

out.writeByte (0);
const MidiMessage m (MidiMessage::endOfTrack());
out.write (m.getRawData(), m.getRawDataSize());

ie:

void MidiFile::fixEndOfTrackEvent (MidiMessageSequence& out)
{
    //Don't bother with a blank sequence
    if(out.getNumEvents() <= 0) return;

    MidiMessageSequence seq;
    double lastTime = 0.0;

    for(int i = 0; i < out.getNumEvents(); ++i)
    {
        if(out.getEventPointer(i)->message.isEndOfTrackMetaEvent() == false)
        {
            seq.addEvent(out.getEventPointer(i)->message);
            lastTime = out.getEventPointer(i)->message.getTimeStamp();
        }
    }

    seq.addEvent(MidiMessage::endOfTrack(), lastTime);

    out = seq;
}
7

Thanks. In the end I went for your first suggestion, and just filtered them out when writing the track. I figured that even if it correctly removed the events when reading the file, people could mistakenly (or deliberately) add an end-of-track when building their data, so it’d still need to be checked for when writing.

8

Thanks Jules!