Opened 17 years ago

Closed 17 years ago

Last modified 15 years ago

#1261 closed defect (fixed)

MT-32 execution checksum errors

Reported by: SF/logicdeluxe Owned by: SF/jamieson630
Priority: normal Component: Engine: Sky
Keywords: Cc:
Game: Beneath a Steel Sky

Description

With this game in native MT-32 mode, several "execution checksum error" and "execution buffer overflow" appear in the display. Seems there are sent some bad SysEx in this game. (No problems with SCUMM or Simon games, though) At least the "execution buffer overflow" is a bug in the original interpreter and happens whenever I load a savegame for instance, but with the checksums, there must be something wrong with ScummVM. The overflows should be fixable with some delay after SysEx is sent. The other error must be a rather old bug lying in the original sources. I have no clue, what the MT-32 is supposed to do when this would work. At least I hear no difference in the music between the original and ScummVM so far.

Ticket imported from: #817286. Ticket imported from: bugs/1261.

Attachments (2)

volume.diff (1.2 KB ) - added by SF/logicdeluxe 17 years ago.
Fixes GM and MT-32 master volume
mt32sysexfix.diff (486 bytes ) - added by SF/logicdeluxe 17 years ago.
fixes overflows while sysEx is sent to MT-32

Download all attachments as: .zip

Change History (14)

comment:1 by fingolfin, 17 years ago

Owner: set to SF/jamieson630

comment:2 by fingolfin, 17 years ago

Assigning to Jamieson, our MIDI-god. But feel free to unassign if you doN't feel responsible.

comment:3 by SF/jamieson630, 17 years ago

BASS music is LavosSpawn's scope of responsibility. Reassigning. I'll look at it if it's sent back to me, but I'm trying to stay out of the BASS music code for the time being.

comment:4 by SF/jamieson630, 17 years ago

Owner: changed from SF/jamieson630 to lavosspawn

comment:5 by SF/jamieson630, 17 years ago

Oh alright, I poked around a bit anyway. There was one invalid checksum computation that I found (wasn't doing & 0x7F to mask high-order bit). So I fixed that in CVS. Don't know if that takes care of all the checksum errors.

I've asked this before, and I'll ask it again -- *why* are we trying to set a master volume using SysEx, for both GM and MT-32? While it might work for GM (I've never seen that notation, though), it almost definitely won't work for MT-32. I think the volume solution should be based solely on channel volumes (control change 7 messages) and avoid any SysEx goofiness.

Finally, someone (LavosSpawn) should see about implementing SysEx delays when sending large amounts of SysEx data. Even if the original interpreter had this problem, it's one that we can easily circumvent.

by SF/logicdeluxe, 17 years ago

Attachment: volume.diff added

Fixes GM and MT-32 master volume

comment:6 by SF/logicdeluxe, 17 years ago

In fact master volume in GM is defined with SysEx 7f 7f 04 01 volume. The valid volume is 2 byte in the order msb lsb and the range is from 00 to 7f. (I know, using a 14 bit value here is sick.)

The manufacturer id's 7e and 7f are so called "universal sysex". Actually they aren't real SysEx as they are understood from many different hardware.

With MT-32 (which in fact doesn't understand that GM SysEx) the master volume is set with SysEx 41 10 16 12 10 00 16 volume checksum. The valid volume range is from 00 to 64.

As I measured several wavetables, so I am pretty sure, volume scales (channels independent as well as master volume) are almost identical between MT-32 and GM, so just the range has to be scaled. There is no need for dynamic compression here like it is required with the velocity.

I corrected both gmmusic.cpp and mt32music.cpp.

However, the overflow, which this bug report actually is about, still has to be fixed.

comment:7 by SF/logicdeluxe, 17 years ago

Oops! In mt32music.cpp just delete either the memcpy line or leave sysEx[7] = volume; alone after that! What you prefer. I accidentally assigned them twice.

comment:8 by SF/jamieson630, 17 years ago

I put your patch in for mt32music.cpp. I didn't mod the log transform in gmmusic.cpp because I'm not sure where that came from. (It may be from the original BASS source.)

Of great use, now, would be if you could add those g_system- >delay_msec() calls after all MT-32 SysEx transmissions and determine which transmissions are problematic. Once we've figured out where exactly the "execution buffer overflow" errors are occurring, we can add delays to the appropriate transmissions. (I'm guessing that indiscriminantly putting delays after every single SysEx transmission would cause small freezes during gameplay.)

comment:9 by SF/logicdeluxe, 17 years ago

Well, it slows the game, but this only happens when a music is loaded, hence gameplay shouldn't be affected much. And the delay you need isn't that long anyway. I tried it and at least 3 ms are required to make it work. With 5 ms we really should be on the save side. I guess, when the game originally was written, computers were that slow the code itself did enough delay, so the error probably didn't occour on 386 cpus back then. Btw. some other games back then spend up to a minute just sending SysEx. This one and also the SCUMM games are really fast with that! I did a patch.

by SF/logicdeluxe, 17 years ago

Attachment: mt32sysexfix.diff added

fixes overflows while sysEx is sent to MT-32

comment:10 by SF/jamieson630, 17 years ago

Owner: changed from lavosspawn to SF/jamieson630
Resolution: fixed
Status: newclosed

comment:11 by SF/jamieson630, 17 years ago

Patch looks fine to me. In CVS. Thanks!

comment:12 by fingolfin, 15 years ago

Component: Engine: Sky
Game: Beneath a Steel Sky
Note: See TracTickets for help on using tickets.