Opened 16 years ago

Closed 16 years ago

Last modified 16 months ago

#1615 closed defect (fixed)

Possible bug in midiparser_smf.cpp

Reported by: eriktorbjorn Owned by: SF/jamieson630
Priority: normal Component: Engine: SAGA
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 eriktorbjorn, 16 years ago

Owner: set to SF/jamieson630

comment:2 by SF/jamieson630, 16 years ago

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).

comment:3 by eriktorbjorn, 16 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 SF/jamieson630, 16 years ago

Resolution: fixed
Status: newclosed

comment:5 by SF/jamieson630, 16 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 digitall, 16 months ago

Component: Engine: SAGA
Game: Inherit the Earth
Note: See TracTickets for help on using tickets.