From 5b23a251ceefaecdd4d46e1b27835adaf7dd7f99 Mon Sep 17 00:00:00 2001
From: Willem Jan Palenstijn
Date: Thu, 19 Sep 2013 23:53:12 +0200
Subject: SCI: Fix too strict assert triggering in LSL5

---
 engines/sci/sound/midiparser_sci.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 7640c5f314..4b4333a37c 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -663,7 +663,10 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 					// 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);
+					// (Exception: delta == 0, in which case the loop points
+					// at the previous event, which is fine.)
+					assert(_loopTick + info.delta < _position._playTick ||
+					       ((_loopTick == _position._playTick && info.delta == 0)));
 
 					uint32 extraDelta = info.delta;
 					_pSnd->inFastForward = true;
-- 
cgit v1.2.3


From 11d425b76c7d6cdae8b72a1c3c438cc74e9c37a0 Mon Sep 17 00:00:00 2001
From: Willem Jan Palenstijn
Date: Fri, 20 Sep 2013 23:34:46 +0200
Subject: SCI: Move MIDI event processing out of parseNextEvent

---
 engines/sci/sound/midiparser_sci.cpp | 261 +++++++++++++++++------------------
 engines/sci/sound/midiparser_sci.h   |   6 +-
 2 files changed, 130 insertions(+), 137 deletions(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 4b4333a37c..0af8e7aedd 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -53,11 +53,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 +435,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 +456,76 @@ 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) {
+
+	// TODO: Properly handle fireEvents
+
+	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 +542,23 @@ 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 (_soundVersion <= SCI_VERSION_0_LATE || _position._playTick) {
 					if (!_pSnd->inFastForward) {
-						_signalSet = true;
-						_signalToSet = info.basic.param1;
+						_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 +580,51 @@ 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;
+					_pSnd->inFastForward = true;
+					jumpToTick(_loopTick, false, false);
+					_pSnd->inFastForward = false;
+					// Done with this event.
+					return;
 				}
-				break;
+				return;
 			case kUpdateCue:
 				if (!_pSnd->inFastForward) {
-					_dataincAdd = true;
+					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;
+					_pSnd->signal = 0x7f + 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,96 +639,51 @@ 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) {
+				_pSnd->inFastForward = true;
+				jumpToTick(_loopTick);
+				_pSnd->inFastForward = false;
+
+				// 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).
-					// (Exception: delta == 0, in which case the loop points
-					// at the previous event, which is fine.)
-					assert(_loopTick + info.delta < _position._playTick ||
-					       ((_loopTick == _position._playTick && info.delta == 0)));
-
-					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];
-- 
cgit v1.2.3


From 3792af8e955bea5a07359c808361a16b15481327 Mon Sep 17 00:00:00 2001
From: Willem Jan Palenstijn
Date: Sat, 21 Sep 2013 09:30:42 +0200
Subject: AUDIO: Simplify SCI inFastForward flag by moving it to MidiParser

---
 engines/sci/sound/midiparser_sci.cpp | 8 ++------
 engines/sci/sound/music.cpp          | 5 -----
 engines/sci/sound/music.h            | 1 -
 3 files changed, 2 insertions(+), 12 deletions(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 0af8e7aedd..40ae91d83d 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -543,7 +543,7 @@ void MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
 				// That is probably why this signal isn't triggered
 				// immediately there.
 				if (_soundVersion <= SCI_VERSION_0_LATE || _position._playTick) {
-					if (!_pSnd->inFastForward) {
+					if (!_jumpingToTick) {
 						_pSnd->setSignal(info.basic.param1);
 						debugC(4, kDebugLevelSound, "signal %04x", info.basic.param1);
 					}
@@ -586,15 +586,13 @@ void MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
 				// 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) {
-					_pSnd->inFastForward = true;
 					jumpToTick(_loopTick, false, false);
-					_pSnd->inFastForward = false;
 					// Done with this event.
 					return;
 				}
 				return;
 			case kUpdateCue:
-				if (!_pSnd->inFastForward) {
+				if (!_jumpingToTick) {
 					int inc;
 					switch (_soundVersion) {
 					case SCI_VERSION_0_EARLY:
@@ -658,9 +656,7 @@ void MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
 			// (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) {
-				_pSnd->inFastForward = true;
 				jumpToTick(_loopTick);
-				_pSnd->inFastForward = false;
 
 				// Done with this event.
 				return;
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..1f798c90d7 100644
--- a/engines/sci/sound/music.h
+++ b/engines/sci/sound/music.h
@@ -65,7 +65,6 @@ 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;
-- 
cgit v1.2.3


From 4443793b97761ab1dc1fb312a4092211356b008e Mon Sep 17 00:00:00 2001
From: m-kiewitz
Date: Sat, 21 Sep 2013 14:27:16 +0200
Subject: SCI: sfx/music priority int16 fixes bug #3615038

it seems that sound system up till SCI0_LATE uses int16, afterwards it seems they changed to byte
main music object (conMusic) in Laura Bow 1 uses -1 as priority. This was truncated to 255 till now, which resulted in many sound effects not getting played, because those used priority 0
---
 engines/sci/sound/music.h      | 2 +-
 engines/sci/sound/soundcmd.cpp | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/music.h b/engines/sci/sound/music.h
index 5924a0fd12..b582e7f1a9 100644
--- a/engines/sci/sound/music.h
+++ b/engines/sci/sound/music.h
@@ -70,7 +70,7 @@ public:
 	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..90ad51b5d8 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));
+	int32 objPrio = readSelectorValue(_segMan, obj, SELECTOR(priority));
 	if (objPrio != musicSlot->priority)
 		_music->soundSetPriority(musicSlot, objPrio);
 	return acc;
-- 
cgit v1.2.3


From 158d12e555abe05e6e08a026c156097e48b2802f Mon Sep 17 00:00:00 2001
From: m-kiewitz
Date: Sat, 21 Sep 2013 14:34:42 +0200
Subject: SCI: abbrev. ffs to FE and priority check fix

---
 engines/sci/sound/soundcmd.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp
index 90ad51b5d8..e36c5705ab 100644
--- a/engines/sci/sound/soundcmd.cpp
+++ b/engines/sci/sound/soundcmd.cpp
@@ -431,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);
-	int32 objPrio = readSelectorValue(_segMan, obj, SELECTOR(priority));
+	int16 objPrio = readSelectorValue(_segMan, obj, SELECTOR(priority));
 	if (objPrio != musicSlot->priority)
 		_music->soundSetPriority(musicSlot, objPrio);
 	return acc;
-- 
cgit v1.2.3


From a6d902df2827b91dc641b6f51c0a070b70a09179 Mon Sep 17 00:00:00 2001
From: Willem Jan Palenstijn
Date: Sat, 21 Sep 2013 14:42:54 +0200
Subject: SCI: Handle !fireEvents in processEvent

---
 engines/sci/sound/midiparser_sci.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 40ae91d83d..3332edd5a6 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -521,8 +521,11 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 }
 
 void MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
-
-	// TODO: Properly handle 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:
-- 
cgit v1.2.3


From 97b255ab33fa5fcd4507573e77cd42a8406e1b55 Mon Sep 17 00:00:00 2001
From: m-kiewitz
Date: Sat, 21 Sep 2013 19:41:45 +0200
Subject: SCI: fix dataInc signalling fixes bug #3035159

we set signal in parseNextEvent on dataInc events, which then effectively triggered 2 cues through kDoSoundUpdateCues instead of one. Fixes Freddy Pharkas Ballad intro on floppy + demos
---
 engines/sci/sound/midiparser_sci.cpp | 1 -
 1 file changed, 1 deletion(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 4b4333a37c..b367eeead0 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -444,7 +444,6 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 	if (_dataincAdd) {
 		_dataincAdd = false;
 		_pSnd->dataInc += _dataincToAdd;
-		_pSnd->signal = 0x7f + _pSnd->dataInc;
 		debugC(4, kDebugLevelSound, "datainc %04x", _dataincToAdd);
 	}
 	if (_signalSet) {
-- 
cgit v1.2.3


From f1b0a7740855f9c16c537f161772483a9f850e96 Mon Sep 17 00:00:00 2001
From: Martin Kiewitz
Date: Sun, 22 Sep 2013 15:40:51 +0200
Subject: SCI: fix music start code fixes eq2 bug #3037267

we start at offset 10 for sound SCI1 games. This is hardcoded in the interpreter. Also removing not handling signals on tick 0. This fixes Eco Quest 2 / Gonzales dancing in room 530. Thanks to wjp for the help.
---
 engines/sci/sound/midiparser_sci.cpp | 34 +++++++++++++++-------------------
 engines/sci/sound/midiparser_sci.h   |  1 +
 engines/sci/sound/music.cpp          | 20 +++++++++++++++++++-
 3 files changed, 35 insertions(+), 20 deletions(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index d6ff4f6cc8..83b5bf520f 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -103,6 +103,18 @@ bool MidiParser_SCI::loadMusic(SoundResource::Track *track, MusicEntry *psnd, in
 	return true;
 }
 
+bool MidiParser_SCI::jumpToOffset(uint32 offset) {
+	if (_activeTrack >= _numTracks)
+		return false;
+
+	assert(!_jumpingToTick); // This function must not be called while within MidiParser::jumpToTick()
+
+	resetTracking();
+	_position._playPos = _tracks[_activeTrack] + offset;
+	parseNextEvent(_nextEvent);
+	return true;
+}
+
 byte MidiParser_SCI::midiGetNextChannel(long ticker) {
 	byte curr = 0xFF;
 	long closest = ticker + 1000000, next = 0;
@@ -531,25 +543,9 @@ void MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
 	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
-				// a song that sets signal to 4 immediately on tick 0. Signal
-				// isn't set at that point by sierra sci and it would cause the
-				// castle daventry text to get immediately removed, so we
-				// currently filter it. Sierra SCI ignores them as well at that
-				// time. However, this filtering should only be performed for
-				// SCI1 and newer games. Signalling is done differently in SCI0
-				// 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.
-				// 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.
-				if (_soundVersion <= SCI_VERSION_0_LATE || _position._playTick) {
-					if (!_jumpingToTick) {
-						_pSnd->setSignal(info.basic.param1);
-						debugC(4, kDebugLevelSound, "signal %04x", info.basic.param1);
-					}
+				if (!_jumpingToTick) {
+					_pSnd->setSignal(info.basic.param1);
+					debugC(4, kDebugLevelSound, "signal %04x", info.basic.param1);
 				}
 			} else {
 				_loopTick = _position._playTick;
diff --git a/engines/sci/sound/midiparser_sci.h b/engines/sci/sound/midiparser_sci.h
index 5784dca1ab..098fbc2ccb 100644
--- a/engines/sci/sound/midiparser_sci.h
+++ b/engines/sci/sound/midiparser_sci.h
@@ -60,6 +60,7 @@ public:
 	bool loadMusic(byte *, uint32) {
 		return false;
 	}
+	bool jumpToOffset(uint32 offset);
 	void sendInitCommands();
 	void unloadMusic();
 	void setMasterVolume(byte masterVolume);
diff --git a/engines/sci/sound/music.cpp b/engines/sci/sound/music.cpp
index 8c6d0d6431..fe017868b5 100644
--- a/engines/sci/sound/music.cpp
+++ b/engines/sci/sound/music.cpp
@@ -518,7 +518,25 @@ void SciMusic::soundPlay(MusicEntry *pSnd) {
 			pSnd->hold = -1;
 
 			if (pSnd->status == kSoundStopped)
-				pSnd->pMidiParser->jumpToTick(0);
+				if (_soundVersion <= SCI_VERSION_0_LATE) {
+					// SCI0 sound subsystem seems to start at offset 0
+					// Not starting at offset 0 for 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.
+					pSnd->pMidiParser->jumpToOffset(0);
+				} else {
+					// SCI1 sound subsystem starts at offset 10 (and also sets loop offset to 0)
+					// At least in kq5/french&mac the first scene in the intro has
+					//  a song that sets signal to 4 immediately on tick 0. Signal
+					//  isn't set at that point by sierra sci and it would cause the
+					//  castle daventry text to get immediately removed.
+					// Also Eco Quest 2 Gonzales Dances music (room 530) requires a signal
+					//  to get set exactly at tick 0. We previously didn't handle signals
+					//  on tick 0 for SCI1. Which then resulted in broken dance animations.
+					//  See bug #3037267
+					// FIXME: maybe also change looping logic to use offset instead of ticks
+					pSnd->pMidiParser->jumpToOffset(10);
+				}
 			else {
 				// Fast forward to the last position and perform associated events when loading
 				pSnd->pMidiParser->jumpToTick(pSnd->ticker, true, true, true);
-- 
cgit v1.2.3


From 551e263165c5906e4fb6b27de3f42d960553bd9e Mon Sep 17 00:00:00 2001
From: Martin Kiewitz
Date: Sun, 22 Sep 2013 20:13:33 +0200
Subject: SCI: revert fix music start code

add workaround for eq2
the issue is known, but can't be properly fixed without rewriting the midiparser into a channel specific parser
previous commit caused issues in kq5/french and others
---
 engines/sci/sound/midiparser_sci.cpp | 57 ++++++++++++++++++++++++++----------
 engines/sci/sound/midiparser_sci.h   |  1 -
 engines/sci/sound/music.cpp          | 20 +------------
 3 files changed, 43 insertions(+), 35 deletions(-)

(limited to 'engines/sci/sound')

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 83b5bf520f..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"
@@ -103,18 +106,6 @@ bool MidiParser_SCI::loadMusic(SoundResource::Track *track, MusicEntry *psnd, in
 	return true;
 }
 
-bool MidiParser_SCI::jumpToOffset(uint32 offset) {
-	if (_activeTrack >= _numTracks)
-		return false;
-
-	assert(!_jumpingToTick); // This function must not be called while within MidiParser::jumpToTick()
-
-	resetTracking();
-	_position._playPos = _tracks[_activeTrack] + offset;
-	parseNextEvent(_nextEvent);
-	return true;
-}
-
 byte MidiParser_SCI::midiGetNextChannel(long ticker) {
 	byte curr = 0xFF;
 	long closest = ticker + 1000000, next = 0;
@@ -543,9 +534,45 @@ void MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
 	case 0xC:
 		if (info.channel() == 0xF) {// SCI special case
 			if (info.basic.param1 != kSetSignalLoop) {
-				if (!_jumpingToTick) {
-					_pSnd->setSignal(info.basic.param1);
-					debugC(4, kDebugLevelSound, "signal %04x", info.basic.param1);
+				// At least in kq5/french&mac the first scene in the intro has
+				// a song that sets signal to 4 immediately on tick 0. Signal
+				// isn't set at that point by sierra sci and it would cause the
+				// castle daventry text to get immediately removed, so we
+				// currently filter it. Sierra SCI ignores them as well at that
+				// time. However, this filtering should only be performed for
+				// SCI1 and newer games. Signalling is done differently in SCI0
+				// 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.
+				// 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;
diff --git a/engines/sci/sound/midiparser_sci.h b/engines/sci/sound/midiparser_sci.h
index 098fbc2ccb..5784dca1ab 100644
--- a/engines/sci/sound/midiparser_sci.h
+++ b/engines/sci/sound/midiparser_sci.h
@@ -60,7 +60,6 @@ public:
 	bool loadMusic(byte *, uint32) {
 		return false;
 	}
-	bool jumpToOffset(uint32 offset);
 	void sendInitCommands();
 	void unloadMusic();
 	void setMasterVolume(byte masterVolume);
diff --git a/engines/sci/sound/music.cpp b/engines/sci/sound/music.cpp
index fe017868b5..8c6d0d6431 100644
--- a/engines/sci/sound/music.cpp
+++ b/engines/sci/sound/music.cpp
@@ -518,25 +518,7 @@ void SciMusic::soundPlay(MusicEntry *pSnd) {
 			pSnd->hold = -1;
 
 			if (pSnd->status == kSoundStopped)
-				if (_soundVersion <= SCI_VERSION_0_LATE) {
-					// SCI0 sound subsystem seems to start at offset 0
-					// Not starting at offset 0 for 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.
-					pSnd->pMidiParser->jumpToOffset(0);
-				} else {
-					// SCI1 sound subsystem starts at offset 10 (and also sets loop offset to 0)
-					// At least in kq5/french&mac the first scene in the intro has
-					//  a song that sets signal to 4 immediately on tick 0. Signal
-					//  isn't set at that point by sierra sci and it would cause the
-					//  castle daventry text to get immediately removed.
-					// Also Eco Quest 2 Gonzales Dances music (room 530) requires a signal
-					//  to get set exactly at tick 0. We previously didn't handle signals
-					//  on tick 0 for SCI1. Which then resulted in broken dance animations.
-					//  See bug #3037267
-					// FIXME: maybe also change looping logic to use offset instead of ticks
-					pSnd->pMidiParser->jumpToOffset(10);
-				}
+				pSnd->pMidiParser->jumpToTick(0);
 			else {
 				// Fast forward to the last position and perform associated events when loading
 				pSnd->pMidiParser->jumpToTick(pSnd->ticker, true, true, true);
-- 
cgit v1.2.3