1 files changed, 168 insertions, 118 deletions

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 9546b1503f..186fc18a5c 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -20,6 +20,9 @@
  *
  */
 
+#include "sci/sci.h"
+#include "sci/engine/state.h"
+
 #include "sci/engine/kernel.h"
 #include "sci/engine/state.h"
 #include "sci/sound/midiparser_sci.h"
@@ -53,10 +56,6 @@ MidiParser_SCI::MidiParser_SCI(SciVersion soundVersion, SciMusic *music) :
 	_masterVolume = 15;
 	_volume = 127;
 
-	_signalSet = false;
-	_signalToSet = 0;
-	_dataincAdd = false;
-	_dataincToAdd = 0;
 	_resetOnPause = false;
 	_pSnd = 0;
 }
@@ -439,20 +438,6 @@ void MidiParser_SCI::sendToDriver(uint32 midi) {
 }
 
 void MidiParser_SCI::parseNextEvent(EventInfo &info) {
-	// Set signal AFTER waiting for delta, otherwise we would set signal too soon resulting in all sorts of bugs
-	if (_dataincAdd) {
-		_dataincAdd = false;
-		_pSnd->dataInc += _dataincToAdd;
-		_pSnd->signal = 0x7f + _pSnd->dataInc;
-		debugC(4, kDebugLevelSound, "datainc %04x", _dataincToAdd);
-	}
-	if (_signalSet) {
-		_signalSet = false;
-		_pSnd->setSignal(_signalToSet);
-
-		debugC(4, kDebugLevelSound, "signal %04x", _signalToSet);
-	}
-
 	info.start = _position._playPos;
 	info.delta = 0;
 	while (*_position._playPos == 0xF8) {
@@ -474,6 +459,79 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 	case 0xC:
 		info.basic.param1 = *(_position._playPos++);
 		info.basic.param2 = 0;
+		break;
+	case 0xD:
+		info.basic.param1 = *(_position._playPos++);
+		info.basic.param2 = 0;
+		break;
+
+	case 0xB:
+		info.basic.param1 = *(_position._playPos++);
+		info.basic.param2 = *(_position._playPos++);
+		info.length = 0;
+		break;
+
+	case 0x8:
+	case 0x9:
+	case 0xA:
+	case 0xE:
+		info.basic.param1 = *(_position._playPos++);
+		info.basic.param2 = *(_position._playPos++);
+		if (info.command() == 0x9 && info.basic.param2 == 0)
+			info.event = info.channel() | 0x80;
+		info.length = 0;
+		break;
+
+	case 0xF: // System Common, Meta or SysEx event
+		switch (info.event & 0x0F) {
+		case 0x2: // Song Position Pointer
+			info.basic.param1 = *(_position._playPos++);
+			info.basic.param2 = *(_position._playPos++);
+			break;
+
+		case 0x3: // Song Select
+			info.basic.param1 = *(_position._playPos++);
+			info.basic.param2 = 0;
+			break;
+
+		case 0x6:
+		case 0x8:
+		case 0xA:
+		case 0xB:
+		case 0xC:
+		case 0xE:
+			info.basic.param1 = info.basic.param2 = 0;
+			break;
+
+		case 0x0: // SysEx
+			info.length = readVLQ(_position._playPos);
+			info.ext.data = _position._playPos;
+			_position._playPos += info.length;
+			break;
+
+		case 0xF: // META event
+			info.ext.type = *(_position._playPos++);
+			info.length = readVLQ(_position._playPos);
+			info.ext.data = _position._playPos;
+			_position._playPos += info.length;
+			break;
+		default:
+			warning(
+					"MidiParser_SCI::parseNextEvent: Unsupported event code %x",
+					info.event);
+		} // // System Common, Meta or SysEx event
+	}// switch (info.command())
+}
+
+void MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
+	if (!fireEvents) {
+		// We don't do any processing that should be done while skipping events
+		MidiParser::processEvent(info, fireEvents);
+		return;
+	}
+
+	switch (info.command()) {
+	case 0xC:
 		if (info.channel() == 0xF) {// SCI special case
 			if (info.basic.param1 != kSetSignalLoop) {
 				// At least in kq5/french&mac the first scene in the intro has
@@ -486,25 +544,47 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 				// though, so ignoring these signals in SCI0 games will result
 				// in glitches (e.g. the intro of LB1 Amiga gets stuck - bug
 				// #3297883). Refer to MusicEntry::setSignal() in sound/music.cpp.
-				if (_soundVersion <= SCI_VERSION_0_LATE ||
-					_position._playTick || info.delta) {
-					_signalSet = true;
-					_signalToSet = info.basic.param1;
+				// FIXME: SSCI doesn't start playing at the very beginning
+				// of the stream, but at a fixed location a few commands later.
+				// That is probably why this signal isn't triggered
+				// immediately there.
+				bool skipSignal = false;
+				if (_soundVersion >= SCI_VERSION_1_EARLY) {
+					if (!_position._playTick) {
+						skipSignal = true;
+						switch (g_sci->getGameId()) {
+						case GID_ECOQUEST2:
+							// In Eco Quest 2 room 530 - gonzales is supposed to dance
+							// WORKAROUND: we need to signal in this case on tick 0
+							// this whole issue is complicated and can only be properly fixed by
+							// changing the whole parser to a per-channel parser. SSCI seems to
+							// start each channel at offset 13 (may be 10 for us) and only
+							// starting at offset 0 when the music loops to the initial position.
+							if (g_sci->getEngineState()->currentRoomNumber() == 530)
+								skipSignal = false;
+							break;
+						default:
+							break;
+						}
+					}
+				}
+				if (!skipSignal) {
+					if (!_jumpingToTick) {
+						_pSnd->setSignal(info.basic.param1);
+						debugC(4, kDebugLevelSound, "signal %04x", info.basic.param1);
+					}
 				}
 			} else {
-				_loopTick = _position._playTick + info.delta;
+				_loopTick = _position._playTick;
 			}
+
+			// Done with this event.
+			return;
 		}
-		break;
-	case 0xD:
-		info.basic.param1 = *(_position._playPos++);
-		info.basic.param2 = 0;
-		break;
 
+		// Break to let parent handle the rest.
+		break;
 	case 0xB:
-		info.basic.param1 = *(_position._playPos++);
-		info.basic.param2 = *(_position._playPos++);
-
 		// Reference for some events:
 		// http://wiki.scummvm.org/index.php/SCI/Specifications/Sound/SCI0_Resource_Format#Status_Reference
 		// Handle common special events
@@ -526,43 +606,48 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 			switch (info.basic.param1) {
 			case kSetReverb:
 				// Already handled above
-				break;
+				return;
 			case kMidiHold:
 				// Check if the hold ID marker is the same as the hold ID
 				// marker set for that song by cmdSetSoundHold.
 				// If it is, loop back, but don't stop notes when jumping.
 				if (info.basic.param2 == _pSnd->hold) {
-					uint32 extraDelta = info.delta;
 					jumpToTick(_loopTick, false, false);
-					_nextEvent.delta += extraDelta;
+					// Done with this event.
+					return;
 				}
-				break;
+				return;
 			case kUpdateCue:
-				_dataincAdd = true;
-				switch (_soundVersion) {
-				case SCI_VERSION_0_EARLY:
-				case SCI_VERSION_0_LATE:
-					_dataincToAdd = info.basic.param2;
-					break;
-				case SCI_VERSION_1_EARLY:
-				case SCI_VERSION_1_LATE:
-				case SCI_VERSION_2_1:
-					_dataincToAdd = 1;
-					break;
-				default:
-					error("unsupported _soundVersion");
+				if (!_jumpingToTick) {
+					int inc;
+					switch (_soundVersion) {
+					case SCI_VERSION_0_EARLY:
+					case SCI_VERSION_0_LATE:
+						inc = info.basic.param2;
+						break;
+					case SCI_VERSION_1_EARLY:
+					case SCI_VERSION_1_LATE:
+					case SCI_VERSION_2_1:
+						inc = 1;
+						break;
+					default:
+						error("unsupported _soundVersion");
+					}
+					_pSnd->dataInc += inc;
+					debugC(4, kDebugLevelSound, "datainc %04x", inc);
+
 				}
-				break;
+				return;
 			case kResetOnPause:
 				_resetOnPause = info.basic.param2;
-				break;
+				return;
 			// Unhandled SCI commands
 			case 0x46: // LSL3 - binoculars
 			case 0x61: // Iceman (AdLib?)
 			case 0x73: // Hoyle
 			case 0xD1: // KQ4, when riding the unicorn
 				// Obscure SCI commands - ignored
-				break;
+				return;
 			// Standard MIDI commands
 			case 0x01:	// mod wheel
 			case 0x04:	// foot controller
@@ -577,84 +662,49 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 			case 0x4B:	// voice mapping
 				// TODO: is any support for this needed at the MIDI parser level?
 				warning("Unhanded SCI MIDI command 0x%x - voice mapping (parameter %d)", info.basic.param1, info.basic.param2);
-				break;
+				return;
 			default:
 				warning("Unhandled SCI MIDI command 0x%x (parameter %d)", info.basic.param1, info.basic.param2);
-				break;
+				return;
 			}
+
 		}
-		info.length = 0;
-		break;
 
-	case 0x8:
-	case 0x9:
-	case 0xA:
-	case 0xE:
-		info.basic.param1 = *(_position._playPos++);
-		info.basic.param2 = *(_position._playPos++);
-		if (info.command() == 0x9 && info.basic.param2 == 0)
-			info.event = info.channel() | 0x80;
-		info.length = 0;
+		// Break to let parent handle the rest.
 		break;
+	case 0xF: // META event
+		if (info.ext.type == 0x2F) {// end of track reached
+			if (_pSnd->loop)
+				_pSnd->loop--;
+			// QFG3 abuses the hold flag. Its scripts call kDoSoundSetHold,
+			// but sometimes there's no hold marker in the associated songs
+			// (e.g. song 110, during the intro). The original interpreter
+			// treats this case as an infinite loop (bug #3311911).
+			if (_pSnd->loop || _pSnd->hold > 0) {
+				jumpToTick(_loopTick);
+
+				// Done with this event.
+				return;
 
-	case 0xF: // System Common, Meta or SysEx event
-		switch (info.event & 0x0F) {
-		case 0x2: // Song Position Pointer
-			info.basic.param1 = *(_position._playPos++);
-			info.basic.param2 = *(_position._playPos++);
-			break;
+			} else {
+				_pSnd->status = kSoundStopped;
+				_pSnd->setSignal(SIGNAL_OFFSET);
 
-		case 0x3: // Song Select
-			info.basic.param1 = *(_position._playPos++);
-			info.basic.param2 = 0;
-			break;
+				debugC(4, kDebugLevelSound, "signal EOT");
+			}
+		}
 
-		case 0x6:
-		case 0x8:
-		case 0xA:
-		case 0xB:
-		case 0xC:
-		case 0xE:
-			info.basic.param1 = info.basic.param2 = 0;
-			break;
+		// Break to let parent handle the rest.
+		break;
 
-		case 0x0: // SysEx
-			info.length = readVLQ(_position._playPos);
-			info.ext.data = _position._playPos;
-			_position._playPos += info.length;
-			break;
+	default:
+		// Break to let parent handle the rest.
+		break;
+	}
 
-		case 0xF: // META event
-			info.ext.type = *(_position._playPos++);
-			info.length = readVLQ(_position._playPos);
-			info.ext.data = _position._playPos;
-			_position._playPos += info.length;
-			if (info.ext.type == 0x2F) {// end of track reached
-				if (_pSnd->loop)
-					_pSnd->loop--;
-				// QFG3 abuses the hold flag. Its scripts call kDoSoundSetHold,
-				// but sometimes there's no hold marker in the associated songs
-				// (e.g. song 110, during the intro). The original interpreter
-				// treats this case as an infinite loop (bug #3311911).
-				if (_pSnd->loop || _pSnd->hold > 0) {
-					// We need to play it again...
-					uint32 extraDelta = info.delta;
-					jumpToTick(_loopTick);
-					_nextEvent.delta += extraDelta;
-				} else {
-					_pSnd->status = kSoundStopped;
-					_pSnd->setSignal(SIGNAL_OFFSET);
-
-					debugC(4, kDebugLevelSound, "signal EOT");
-				}
-			}
-			break;
-		default:
-			warning(
-					"MidiParser_SCI::parseNextEvent: Unsupported event code %x",
-					info.event);
-		} // // System Common, Meta or SysEx event
-	}// switch (info.command())
+
+	// Let parent handle the rest
+	MidiParser::processEvent(info, fireEvents);
 }
 
 byte MidiParser_SCI::getSongReverb() {