Opened 9 years ago

Closed 9 years ago

Last modified 9 months ago

#9261 closed patch

MT-32 reverb config parsed incorrectly

Reported by: SF/sven3000 Owned by: bluegr
Priority: normal Component: Engine: SCI
Keywords: MT-32 Cc:
Game:

Description

ScummVM doesn't correctly parse the reverb data in the MT-32 patch resource. It also doesn't send the default reverb settings to the MT-32. This problem seems to apply to every SCI0 and SCI1 game.

In the MidiPlayer_Midi::readMt32Patch (engines/sci/sound/driver/midi.cpp) function, the _reverbConfig array is initialized in the wrong order. It is read row-wise while it is stored column-wise. The attached patch includes a fix for this function.

In addition, the reverb settings stored in _reverbConfig[0] need to be sent to the MT-32 somewhere. The real SCI engine (observed by running the games in DOSBox) sends this command before it first plays music.

I think the best place to put it would probably be MidiParser_SCI::sendInitCommands (engines/sci/sound/midiparser_sci.cpp), but that seems to send it more often than the real SCI does. If you know a better place to put it, be my guest.

Because I wasn't sure where to put it, the attached patch does NOT include this! It only fixes the reverb config parsing!

This behaviour was observed with ScummVM 1.2.0 and svn revision 54451, on Windows 7 x64, and with multiple games including KQ4 and KQ5.

Ticket imported from: #3117434. Ticket imported from: patches/1366.

Attachments (1)

mt32_reverb.patch (635 bytes) - added by SF/sven3000 9 years ago.
Patch to fix MT-32 reverb config data parsing (does not fix sending initial reverb issue)

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by SF/sven3000

Attachment: mt32_reverb.patch added

Patch to fix MT-32 reverb config data parsing (does not fix sending initial reverb issue)

comment:1 Changed 9 years ago by jvprat

Summary: MT-32 reverb config parsed incorrectlySCI: MT-32 reverb config parsed incorrectly

comment:2 Changed 9 years ago by bluegr

Moving this to patches

comment:3 Changed 9 years ago by bluegr

Component: Engine: SCI
Game: King's Quest 4
Summary: SCI: MT-32 reverb config parsed incorrectlyMT-32 reverb config parsed incorrectly

comment:4 Changed 9 years ago by bluegr

Patch added in r54453, after discussing with waltervn.

As for setting the default reverb value, this is a separate issue. It seems that not all SCI versions did this, so I've added it as a TODO for now, till we sort it out

Closing this, thanks!

comment:5 Changed 9 years ago by bluegr

Owner: set to bluegr
Status: newclosed

comment:6 Changed 9 months ago by digitall

Component: Engine: SCI
Keywords: MT-32 added
Note: See TracTickets for help on using tickets.