My midi beats goes out of ...beat

It seems that MidiFile::writeTrack ignores any endOfTrack metaEvent present in the midisequence when writing it to the file. Instead it adds it's own endOfTrackMetaEvent after the last event in the sequence.

While it might seem to be a good idea to ascertain that the mandatory endOfTrack event is added to the file if it has been forgotten, the problem is that it's timestamp is that of the event before. This not only makes it hard to store an empty, silent midi file (for later editing), it also makes it hard to keep any trailing silence in the file. You can't just put a couple of equidistant notes in the file and then loop it because there will be no pause between the last note and the first note in the next loop.

So, if there's no law against having a space before the endoftrack event, why not let writeTrack keep the timestamp of said event if it's already present in the sequence when writing it to the file?

Good idea, thanks! Try what I've checked in now...

Ähmm, it's actually worse... You must actually write the endoftrack event, not just detect it. Remove the first else in the function and it will work as intended.

Doh! Yes, that's what I meant to write of course.. I've corrected it now.

Thanks. Works fine now.

 

Oh, one more thing, in a couple of MidiMessage functions you have lookup tables with const char *[] arrays.

getGMInstrumentName(),

getGMInstrumentBankName(),

getRhythmInstrumentName(),

getControllerName()

 

Because they're not static (the arrays), they have to be initalized for every call which means that getGMInstrumentName() will have to initialize 128 pointers for every call. It's possible that this might be optimized by the compiler when not debugging, but I'm not sure about that.

 

Any reasons why not making them static (static const char* names[]) like you've done in getMidiNoteName() for example?

I'd be very surprised if compilers failed to do a good job of optimising that!

I could be overestimating compilers, but I tend to prefer local variables to static ones where possible, as they're more likely to get optimised-out by the linker if the function's not used. I'd presume that the actual code produced would be the same regardless of whether it's static or not.

(Of course if you can show me that I'm wrong about performance, then I'll happily change that policy though!)

I think you'll be surprised.

I checked the optimized (Maximize speed, favor fast code etc) release build in the debugger (VS 2012) and as you can see in the attached assembler code, MidiMessage::getGMInstrumentName() will go through a tedious array initialization wheras getGMInstrumentBankName(), getRhythmInstrumentName() etc, where i've made the arrays static will happily return a swift value right away from its preloaded arrays.

And I don't think the compiler retards to some non optimized mode just because I add some debug info, I really think this _is_ the optimized version (apart from the added debug information). It certainly is that fast at least.

<sigh> Goddamn you, Microsoft..