5 files changed, 160 insertions, 146 deletions

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 7640c5f314..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,11 +56,6 @@ MidiParser_SCI::MidiParser_SCI(SciVersion soundVersion, SciMusic *music) :
 	_masterVolume = 15;
 	_volume = 127;
 
-	_signalSet = false;
-	_signalToSet = 0;
-	_dataincAdd = false;
-	_dataincToAdd = 0;
-	_jumpToHoldTick = false;
 	_resetOnPause = false;
 	_pSnd = 0;
 }
@@ -440,26 +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);
-	}
-	if (_jumpToHoldTick) {
-		_jumpToHoldTick = false;
-		_pSnd->inFastForward = true;
-		jumpToTick(_loopTick, false, false);
-		_pSnd->inFastForward = false;
-	}
-
 	info.start = _position._playPos;
 	info.delta = 0;
 	while (*_position._playPos == 0xF8) {
@@ -481,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
@@ -497,27 +548,43 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 				// of the stream, but at a fixed location a few commands later.
 				// That is probably why this signal isn't triggered
 				// immediately there.
-				if (_soundVersion <= SCI_VERSION_0_LATE ||
-					_position._playTick || info.delta) {
-					if (!_pSnd->inFastForward) {
-						_signalSet = true;
-						_signalToSet = info.basic.param1;
+				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
@@ -539,50 +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.
-				// We need to wait for the delta of the current event before
-				// jumping, thus the jump will be performed on the next
-				// parseNextEvent() call, like with the signal set events.
-				// In LSL6, room 360, song 381, this ends up jumping forward
-				// one tick (the hold marker occurs at playtick 27, with
-				// _loopTick being 15 and the event itself having a delta of
-				// 13, total = 28) - bug #3614566.
 				if (info.basic.param2 == _pSnd->hold) {
-					_jumpToHoldTick = true;
+					jumpToTick(_loopTick, false, false);
+					// Done with this event.
+					return;
 				}
-				break;
+				return;
 			case kUpdateCue:
-				if (!_pSnd->inFastForward) {
-					_dataincAdd = true;
+				if (!_jumpingToTick) {
+					int inc;
 					switch (_soundVersion) {
 					case SCI_VERSION_0_EARLY:
 					case SCI_VERSION_0_LATE:
-						_dataincToAdd = info.basic.param2;
+						inc = info.basic.param2;
 						break;
 					case SCI_VERSION_1_EARLY:
 					case SCI_VERSION_1_LATE:
 					case SCI_VERSION_2_1:
-						_dataincToAdd = 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
@@ -597,93 +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) {
-					// TODO: this jump is also vulnerable to the same lockup as
-					// the MIDI hold one above. However, we can't perform the
-					// jump on the next tick like with the MIDI hold jump above,
-					// as there aren't any subsequent MIDI events after this one.
-					// This assert is here to detect cases where the song ends
-					// up jumping forward, like with bug #3614566 (see above).
-					assert(_loopTick + info.delta < _position._playTick);
-
-					uint32 extraDelta = info.delta;
-					_pSnd->inFastForward = true;
-					jumpToTick(_loopTick);
-					_pSnd->inFastForward = false;
-					_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() {
diff --git a/engines/sci/sound/midiparser_sci.h b/engines/sci/sound/midiparser_sci.h
index 7bd68994c8..5784dca1ab 100644
--- a/engines/sci/sound/midiparser_sci.h
+++ b/engines/sci/sound/midiparser_sci.h
@@ -89,6 +89,7 @@ public:
 
 protected:
 	void parseNextEvent(EventInfo &info);
+	void processEvent(const EventInfo &info, bool fireEvents = true);
 	byte *midiMixChannels();
 	byte *midiFilterChannels(int channelMask);
 	byte midiGetNextChannel(long ticker);
@@ -106,11 +107,6 @@ protected:
 	byte _masterVolume; // the overall master volume (same for all tracks)
 	byte _volume; // the global volume of the current track
 
-	bool _signalSet;
-	int16 _signalToSet;
-	bool _dataincAdd;
-	int16 _dataincToAdd;
-	bool _jumpToHoldTick;
 	bool _resetOnPause;
 
 	bool _channelUsed[16];
diff --git a/engines/sci/sound/music.cpp b/engines/sci/sound/music.cpp
index 1628a22386..8c6d0d6431 100644
--- a/engines/sci/sound/music.cpp
+++ b/engines/sci/sound/music.cpp
@@ -521,11 +521,7 @@ void SciMusic::soundPlay(MusicEntry *pSnd) {
 				pSnd->pMidiParser->jumpToTick(0);
 			else {
 				// Fast forward to the last position and perform associated events when loading
-				pSnd->inFastForward = true;
-				// we set this flag, so that the midiparser doesn't set any signals for scripts
-				// if we don't do this, at least accessing the debugger will reset previously set signals
 				pSnd->pMidiParser->jumpToTick(pSnd->ticker, true, true, true);
-				pSnd->inFastForward = false;
 			}
 
 			// Restore looping and hold
@@ -765,7 +761,6 @@ MusicEntry::MusicEntry() {
 	resourceId = 0;
 
 	isQueued = false;
-	inFastForward = false;
 
 	dataInc = 0;
 	ticker = 0;
diff --git a/engines/sci/sound/music.h b/engines/sci/sound/music.h
index 5924a0fd12..40236c8445 100644
--- a/engines/sci/sound/music.h
+++ b/engines/sci/sound/music.h
@@ -65,12 +65,11 @@ public:
 	uint16 resourceId;
 
 	bool isQueued; // for SCI0 only!
-	bool inFastForward; // if we are currently fast-forwarding (disables any signals to scripts)
 
 	uint16 dataInc;
 	uint16 ticker;
 	uint16 signal;
-	byte priority;
+	int16 priority; // must be int16, at least in Laura Bow 1, main music (object conMusic) uses priority -1
 	uint16 loop;
 	int16 volume;
 	int16 hold;
diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp
index b0102a002b..e36c5705ab 100644
--- a/engines/sci/sound/soundcmd.cpp
+++ b/engines/sci/sound/soundcmd.cpp
@@ -116,7 +116,10 @@ void SoundCommandParser::processInitSound(reg_t obj) {
 	newSound->resourceId = resourceId;
 	newSound->soundObj = obj;
 	newSound->loop = readSelectorValue(_segMan, obj, SELECTOR(loop));
-	newSound->priority = readSelectorValue(_segMan, obj, SELECTOR(priority)) & 0xFF;
+	if (_soundVersion <= SCI_VERSION_0_LATE)
+		newSound->priority = readSelectorValue(_segMan, obj, SELECTOR(priority));
+	else
+		newSound->priority = readSelectorValue(_segMan, obj, SELECTOR(priority)) & 0xFF;
 	if (_soundVersion >= SCI_VERSION_1_EARLY)
 		newSound->volume = CLIP<int>(readSelectorValue(_segMan, obj, SELECTOR(vol)), 0, MUSIC_VOLUME_MAX);
 	newSound->reverb = -1;	// initialize to SCI invalid, it'll be set correctly in soundInitSnd() below
@@ -428,7 +431,7 @@ reg_t SoundCommandParser::kDoSoundUpdate(int argc, reg_t *argv, reg_t acc) {
 	int16 objVol = CLIP<int>(readSelectorValue(_segMan, obj, SELECTOR(vol)), 0, 255);
 	if (objVol != musicSlot->volume)
 		_music->soundSetVolume(musicSlot, objVol);
-	uint32 objPrio = readSelectorValue(_segMan, obj, SELECTOR(priority));
+	int16 objPrio = readSelectorValue(_segMan, obj, SELECTOR(priority));
 	if (objPrio != musicSlot->priority)
 		_music->soundSetPriority(musicSlot, objPrio);
 	return acc;