Bad Midi file when velocity==10

[blackrachmaninov]

Hello, Thank you for this very useful library. There is a weird issue when trying to export a Midi file if one value==10, for example the velocity. Only with a value of 10 .... The rest works as expected from what I tested. I attach you the Midi files. They contain a single note. The one with the velocity==12 is fine, the one with the velocity==10 has an extra Hex value and then gets corrupted.

I had a lot of variables in my code but it looks as simple as that :

   MidiMessage midiMsg;
    midiMsg.makeNoteOn(60, 10, 0);
    outputFile.addEvent(0, 0, midiMsg);
    midiMsg.makeNoteOff(60, 10, 0);
    outputFile.addEvent(0, 2, midiMsg);

Cheers,

files.zip

[craigsapp]

The problem sounds like a binary/text mode problem. character 10 in ASCII is a newline character. You are likely using Windows, and somehow the file is being written in text mode, where any character 10 (0x0a) written to the file also has character 13 (0x0d) added to the file. And this would cause a MIDI file to become invalid. I'll look to see where text mode is being used instead of binary mode for writing files.

http://stackoverflow.com/questions/5537066/strange-0x0d-being-added-to-my-binary-file

[blackrachmaninov]

Yes I am using Windows. You found the issue for sure. Thank you for answering that fast and for the link.

[craigsapp]

Hi blackrachmaninov,

The problem is definitely related to binary bytes being treated as a newline. Here are the hex bytes from your two test files:

The first one with velocity = 12 which is writing correctly:

4d 54 68 64 
00 00 00 06 
00 00 
00 01 
03 c0 
4d 54 72 6b 
00 00 00 0d 
00    90 3c 0c 
8f 00 80 3c 0c 
00 ff 2f 00 

The second one with velocity = 10 which is writing incorrectly:

4d 54 68 64 
00 00 00 06 
00 00 
00 01 
03 c0 
4d 54 72 6b 
00    00 00 0d 
00    90 3c 0d 0a  (should only be 0a with no 0d in front)
8f 00 80 3c 0d 0a  (should only be 0a with no 0d in front)
00    ff 2f 00 

Whenever a "0a" hex byte (decimal 10) is written, a "0d" (decimal 13) is being written in front of it. This is because MS-DOS/Windows uses "0d 0a" as a newline, while unix and modern OS X use "0a" only. So when writing "\n" in Windows, "0d 0a" will be written, while in unix and probably OS X, "0a" only will be written (older Apple OSes would use "0d" by itself, and so would never add an extra character).

I am suspecting that you are using an older version of the midifile library. There was a fix in January for writing binary streams to fix the problem (which only occurs in Windows), and which should have also fixed the problem that you are having: https://github.com/craigsapp/midifile/commit/24bc795092ae085018fe63c2b7a126dde037506d

Here is the line which was fixed to add ios::binary to the writing flags: https://github.com/craigsapp/midifile/blob/ee3a5355d54749af846ba607ae78bff99d95b697/src-library/MidiFile.cpp#L536

Another thing that makes me think you are using an older version of the midifile library is that the order of the parameters in MidiMessage::makeNoteOn and MidiMessage::makeNoteOff have changed: The channel use to come last as the third, optional parameter, but now it is moved to the first parameter:

     MidiMessage::makeNoteOn(channel, key, velocity);

Originally the channel was placed at then end so that it could be treated as an optional parameter with a default value of 0. But I decided that it is more sane to put the parameters in the same order as in a MIDI message, which is also channel, key, velocity. Also, I added convenience functions in the MidiFile class which can be used to avoid using an intermediate MidiMessage data structure:

    MidiFile mfile;
    mfile.addNoteOn(track, tick, channel, key, velocity);
    mfile.addNoteOff(track, tick, channel, key, velocity);

which is equivalent to the more wordy:

   MidiMessage message;
   MidiFile mfile;
   message.makeNoteOn(channel, key, velocity);
   mfile.addEvent(track, tick, message);
   message.makeNoteOff(channel, key, velocity);
   mfile.addEvent(track, tick, message);

So, if you want to keep using the old version, it will be easy to fix by adding "ios::binary" to the ostream flags in the MidiFile::write() function. If you do use the new version, then you will have to switch the order of the parameters in the adding functions so that the channel is first (otherwise, I think most everything else will be unchanged).

[blackrachmaninov]

Hi craigsapp, You are 100% right, I was using old files. I just tested with the new files and everything works fine. My bad. Thank you very much for your time and your explanation. Cheers,