#1615 closed defect (fixed)
Possible bug in midiparser_smf.cpp
| Reported by: | eriktorbjorn | Owned by: | SF/jamieson630 |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SAGA |
| Version: | Keywords: | ||
| Cc: | Game: | Inherit the Earth |
Description
I don't know if this is a bug or not, but I noticed that the special_lengths array in midiparser_smf.cpp is declared to have 16 elements, but only 15 or them are specified.
Right now, an error in that array is my best guess to why it refuses to play about half of the MIDI files included with the Wyrmkeep release of Inherit the Earth. But if so, I don't know enough about MIDI to fix it. In fact, I can't reconcile it at all with what I read at http://www.borg.com/~jglatt/tech/midispec.htm
Ticket imported from: #945497. Ticket imported from: bugs/1615.
Change History (6)
comment:1 by , 20 years ago
| Owner: | set to |
|---|
comment:2 by , 20 years ago
comment:3 by , 20 years ago
Hmm... Looks like it never gets to any case where special_lengths[X] is non-zero. At least not when I run it on this computer. It doesn't even crash, unless I run it in GDB.
On the other hand, it doesn't seem to ever get through compressToType0() either, and if that means it's corrupting memory then all bets are off, of course.
I don't know if it's relevant, but the thing that finally triggered the crash in the debugger was that it was trying to print "Bad MIDI command FE". I remember seeing a couple of messages like that yesterday.
Come to think of it, it was this that made me suspect special_lengths[], because I figured maybe FE was a valid MIDI command after all and the zero was meant for FF.
comment:4 by , 20 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:5 by , 20 years ago
Fix in CVS. The Type 0 compression wasn't handling Running Status (a form of command compression) correctly, causing parser sync problems. While I think a broader structural change would address issues like this better, this will work until I have oodles of free time on my hands to implement said change.
comment:6 by , 5 years ago
| Component: | → Engine: SAGA |
|---|---|
| Game: | → Inherit the Earth |
The link you provided is to a transmission spec, not an SMF spec. There is a difference. When dealing with SMF, always use an SMF format spec.
http://www.borg.com/~jglatt/tech/midifile.htm
That's the one I use. There are probably better ones out there, but it's become familiar to me.
Add a 0 to the end of the array initializer list. If it still fails, put a trace around Line 294 of midiparser_smf.cpp. (My version is hacked for Mac Loom right now, so the line number may be slightly off.) Something like this:
} else if (special_lengths [(event & 0x0F)] > 0) { warning ("System Common message %0X", event); // ADD copy_bytes = special_lengths [(event & 0x0F)]; } else if (event == 0xF0) {
Then report back what the trace tells you.
I'll go back and review the System Common command lengths ASAP (meaning within 24 hours hopefully).