Opened 16 years ago

Closed 16 years ago

Last modified 14 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 16 years ago.
Fixes GM and MT-32 master volume
mt32sysexfix.diff (486 bytes ) - added by SF/logicdeluxe 16 years ago.
fixes overflows while sysEx is sent to MT-32

Download all attachments as: .zip

Change History (14)

comment:1 by fingolfin, 16 years ago

Owner: set to SF/jamieson630

comment:2 by fingolfin, 16 years ago

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

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

Owner: changed from SF/jamieson630 to lavosspawn

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

Attachment: volume.diff added

Fixes GM and MT-32 master volume

comment:6 by SF/logicdeluxe, 16 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, 16 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, 16 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, 16 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, 16 years ago

Attachment: mt32sysexfix.diff added

fixes overflows while sysEx is sent to MT-32

comment:10 by SF/jamieson630, 16 years ago

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

comment:11 by SF/jamieson630, 16 years ago

Patch looks fine to me. In CVS. Thanks!

comment:12 by fingolfin, 14 years ago

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