King's Quest 4 SCI MT-32 problem

Sven · Post by **Sven** » Tue Nov 23, 2010 11:02 am

I have a real Roland MT-32 connected to my PC via a USB MIDI cable. I have set up KQ4 in ScummVM with the music device set to the MIDI cable, and the "True Roland MT-32" box checked.

When playing the game, I get repeated messages of "WARNING: Ignoring midi message d0". Additionally, the unicorn music often gets stuck on the last note (the note keeps playing until I turn off the MT-32; even quitting the game doens't stop it).

This problem happens on ScummVM 1.2 and on the latest daily. It happens with two versions of KQ4 (1.000.111 and 1.006.004). It doesn't happen if I play the exact same game versions under DOSBox.

My operating system is Windows 7 x64.

EDIT: The same behaviour is also observed using the MT-32 emulator.

It's fairly simple to reproduce. Start a new game (you can skip the introduction). Walk around until you find the unicorn, and wait for it to walk off screen, or leave the screen yourself. The issue occurs only if the music is interrupted before it ends (either because the game speed is high enough that the unicorn leaves the screen before that time, or you left yourself).

Sven · Post by **Sven** » Tue Nov 23, 2010 1:12 pm

I've done some digging, and the problem lies in the handling of MIDI sustain controller messages.

KQ4 sends a sustain message on channel 4 during the unicorn song (b4 40 7f), which turns sustain on full. When the music ends prematurely, an "all notes off" message is sent on all channels, including 4 (b4 7b 00), but a sustain off (b4 40 00) is not sent, causing the note to hang.

The music data contains a couple of sustain off messages itself, so if you leave the screen at exactly the right time or let the music finish, it doesn't hang.

Looking at the MIDI data sent by DOSBox, it seems to pair every "all notes off" with a "sustain off".

Modifying the MidiParser_SCI::allNotesOff() function in engines/sci/sound/midiparser_sci.cpp to send a sustain off message along with the all notes off message seems to fix this problem.

This means modifying the existing loop that sends the all notes off message as follows.

Code: Select all

for &#40;i = 0; i < 16; ++i&#41; &#123;
    if &#40;_channelRemap&#91;i&#93; != -1&#41;
    &#123;
        sendToDriver&#40;0xB0 | i, 0x7b, 0&#41;; // All notes off
        sendToDriver&#40;0xB0 | i, 0x40, 0&#41;; // Sustain off &#40;*&#41;
    &#125;
&#125;

The line marked with (*) was added.

Though this appears to fix the issue, I haven't really done any testing to see if this could cause problems in other situations or with other MIDI devices.

This doesn't solve the "Ignoring MIDI event" issue, which appears to be unrelated. That can be solved with a simple fix in the void MidiPlayer_Midi::send(uint32 b) function in engines/sci/sound/drivers/midi.cpp (treating them the same as 0xe0), but again I'm not sure what the side-effects of doing this are, if any.

Raziel · Post by **Raziel** » Tue Nov 23, 2010 1:37 pm

While i love to read the in-depth tech and coding bits and pieces

this would be suited perfectly for a bug report.
Even more with your suggested solution.

As i also play all the games (where possible) through the USB connected MT-32 i'd love to see this issue fixed.

Could you please file a bug report with your suggested solution to the ScummVM bug tracker as not all the devs read here and such posted issues tends to get lost and forgotten.

Thank you

Sven · Post by **Sven** » Tue Nov 23, 2010 2:19 pm

Thanks for the suggestion.

I have filed two bug reports: one for the sustain issue and one for the ignored midi event issue.

md5 · Post by **md5** » Tue Nov 23, 2010 2:51 pm

Impressive!

Not a lot of people have deep knowledge of how MIDI works. Your patches seem to be correct at a first glance, we'll check them out with waltervn. Your fixes might address some similar problems where music hangs. If you do have any more ideas/patches, feel free to send them over

Thanks again!

Sven · Post by **Sven** » Tue Nov 23, 2010 3:24 pm

Thanks, I'm happy to help. I really like the undither feature of ScummVM so I like to play these SCI0 games in ScummVM rather than DOSBox if possible. Also, ScummVM's aspect ratio correction is way better than DOSBox.

On the second issue (the ignored midi event), I just noticed the code also ignores A0 (polyphonic aftertouch). Although I haven't seen that show up anywhere yet, you might want to add code to handle that too, same as with D0. I've added a comment to the bug report about that as well.

Although actually, checking some MT-32 documentation, I don't think it actually supports either polyphonic or channel aftertouch... which begs the question as to why KQ4 is sending those message (or one of them, anyway). Still, it should just ignore them so I don't think there's any harm in sending them.

I don't know if this code is also used for SCI games with GM support (either with Sierra's after-market GM patch or because they always supported it; e.g. KQ6 and SQ5), in which case it might legitimately send aftertouch messages and you would want to handle them properly.

md5 · Post by **md5** » Tue Nov 23, 2010 4:07 pm

Having discussed this with waltervn, he said that these weren't sent from the SCI drivers, and were ignored. MT-32 doesn't support these, so we ignore them as well.

Your fix for sustain seems correct and has been applied, thanks!

BobbinT · Post by **BobbinT** » Tue Nov 23, 2010 5:05 pm

btw, this also happens using purely MT32 Emulator on my end. Let me know if the latest svn has resolved this issue so that I can test it too.

Raziel · Post by **Raziel** » Tue Nov 23, 2010 6:38 pm

An insider when it comes to MIDI and MT-32

We must keep him close

envisaged0ne · Post by **envisaged0ne** » Wed Nov 24, 2010 4:24 am

I'll file a bug report on it, but wanted to post that KQ5's music through the MT-32 doesn't sound right. The tones are all off.

Sven · Post by **Sven** » Wed Nov 24, 2010 4:36 am

CD or floppy version of KQ5?

EDIT: I just tried the CD version (only version I have on hand at the moment). I'm getting way over the top reverb, it sounds as if it's playing in a cathedral. Is that what you're hearing?

envisaged0ne · Post by **envisaged0ne** » Wed Nov 24, 2010 4:51 am

Sven wrote:CD or floppy version of KQ5?

EDIT: I just tried the CD version (only version I have on hand at the moment). I'm getting way over the top reverb, it sounds as if it's playing in a cathedral. Is that what you're hearing?

It's the CD version. And yes, that's exactly what I'm hearing.

Edit: I just tried it with the floppy version and it does the same thing too.

Sven · Post by **Sven** » Wed Nov 24, 2010 5:18 am

I've checked the MIDI data sent by ScummVM and DOSBox (the official SCI parser). ScummVM sends a sysex that sets reverb to room mode, duration 7, level 7 (which is the maximum). The official SCI engine doesn't send any reverb-related SYSEX commands as far as I can see.

I will continue to investigate.

Sven · Post by **Sven** » Wed Nov 24, 2010 5:39 am

Okay, this has something to do with how SCI sound data is interpreted. In the function MidiParser_SCI::parseNextEvent in engines/sci/sound/midiparser_sci.cpp, it finds an event with data BF 50 7F.

F is interpreted as "SCI special" with 50 meaning a reverb change. That function contains a comment referring to this article which suggests reverb changes should have an argument between 0 and 10, and the code for MidiPlayer_Midi::setReverb (engines/sci/sound/driver/midi.cpp) also suggests that.

Obviously 0x7F (127) is not between 0 and 10, so it gets clipped to 10 which seems to match the maximum reverb config.

I'm unfortunately not familiar with how the SCI resource format actually works. It seems that one of three scenarios is likely here:
1. The MIDI data isn't read properly.
2. Byte sequence BF 50 doesn't actually indicate a reverb change.
3. A reverb change with parameter 127 shouldn't be interpreted the way it is now (treating it as 10).

envisaged0ne · Post by **envisaged0ne** » Wed Nov 24, 2010 5:50 am

Wow, that's awesome how you were able to narrow down the prob! Hopefully one of the dev's can work it out from there