#5528 closed defect (fixed)
KQ5: MT-32 music tones are off
| Reported by: | SF/envisaged0ne | Owned by: | bluegr |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCI |
| Version: | Keywords: | ||
| Cc: | Game: | King's Quest 5 |
Description
When playing KQ5 through the MT-32, the tones are all off. Thus the music doesn't sound accurate.
Ticket imported from: #3117252. Ticket imported from: bugs/5528.
Change History (7)
comment:1 by , 14 years ago
comment:2 by , 14 years ago
| Summary: | MT-32 music tones are off → KQ5: MT-32 music tones are off |
|---|
comment:3 by , 14 years ago
This should be fixed now after all the reverb related changes. Please, test the latest daily build (r54485 or newer)
comment:5 by , 14 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
comment:6 by , 14 years ago
| Resolution: | wontfix → fixed |
|---|
comment:7 by , 14 years ago
| Owner: | set to |
|---|
Note:
See TracTickets
for help on using tickets.
The problem here appears to be related to how reverb is handled, as the sound in KQ5 gets too much reverb.
This appears to ahve 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 http://wiki.scummvm.org/index.php/SCI/Specifications/Sound/SCI0_Resource_Format 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).