Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#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 SF/sven3000, 13 years ago

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).

comment:2 by jvprat, 13 years ago

Summary: MT-32 music tones are offKQ5: MT-32 music tones are off

comment:3 by bluegr, 13 years ago

This should be fixed now after all the reverb related changes. Please, test the latest daily build (r54485 or newer)

comment:4 by SF/envisaged0ne, 13 years ago

This has been resolved. Music sounds fine now

comment:5 by SF/envisaged0ne, 13 years ago

Resolution: wontfix
Status: newclosed

comment:6 by jvprat, 13 years ago

Resolution: wontfixfixed

comment:7 by jvprat, 13 years ago

Owner: set to bluegr
Note: See TracTickets for help on using tickets.