SCI/new music code:

- Resolved a deadlock with the mixer, and added appropriate mutexes (a result of the fact that SCI mixes MIDI and digital audio in the same list)
- Fixed sound playing when loading games, by properly resetting the MIDI driver
- Reverted savegame version to 14 - the changes in versions 15 and 16 don't have any effect on the currently enabled old music code, and the new music code is disabled by default, and is still prone to changes
- Now saving/loading signal, loop and hold for each sound, as well as reverb
- Added stub code for setting reverb and channel hold
- The signal, loop and hold values of each song are cached, like in SSCI and like what happens in Greg's SCI implementation. This allows a clear separation of the engine code from the rest of the engine. Reverted commits 46792 and 46797
- Removed duplicate song list accessing code
- Song cues are now updated in kAnimate for SCI0, like the old music code does, to compensate for the fact that SCI0 didn't poll for music changes via cmdUpdateCues, like what SCI01 and newer do
- Cleanup

svn-id: r46812

8 files changed, 302 insertions, 231 deletions

diff --git a/engines/sci/engine/kgraphics.cpp b/engines/sci/engine/kgraphics.cpp
index 51ad54d2c1..83b9eb0f75 100644
--- a/engines/sci/engine/kgraphics.cpp
+++ b/engines/sci/engine/kgraphics.cpp
@@ -962,6 +962,9 @@ reg_t kAnimate(EngineState *s, int argc, reg_t *argv) {
 	// Take care of incoming events (kAnimate is called semi-regularly)
 #ifdef USE_OLD_MUSIC_FUNCTIONS
 	process_sound_events(s);
+#else
+	if (s->detectDoSoundType() <= SCI_VERSION_0_LATE)
+		s->_soundCmd->updateSci0Cues();
 #endif
 
 	s->_gui->animate(castListReference, cycle, argc, argv);
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 1c3ae06cec..75f191256f 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -116,8 +116,8 @@ void MusicEntry::saveLoadWithSerializer(Common::Serializer &s) {
 		uint32 restoreTime = 0;
 		s.syncAsSint32LE(restoreTime);
 		ticker = restoreTime * 60 / 1000;
-		s.skip(4);	// loop
-		s.skip(4);	// hold
+		s.syncAsSint32LE(loop);
+		s.syncAsSint32LE(hold);
 		// volume and dataInc will be synced from the sound objects
 		// when the sound list is reconstructed in gamestate_restore()
 		volume = 100;
@@ -133,9 +133,11 @@ void MusicEntry::saveLoadWithSerializer(Common::Serializer &s) {
 		s.syncAsSint16LE(resnum);
 		s.syncAsSint16LE(dataInc);
 		s.syncAsSint16LE(ticker);
+		s.syncAsSint16LE(signal);
 		s.syncAsByte(prio);
-		s.skip(1, VER(15), VER(15));
+		s.syncAsSint16LE(loop);
 		s.syncAsByte(volume);
+		s.syncAsByte(hold);
 		s.syncAsByte(fadeTo);
 		s.syncAsSint16LE(fadeStep);
 		s.syncAsSint32LE(fadeTicker);
@@ -628,16 +630,19 @@ void SciMusic::saveLoadWithSerializer(Common::Serializer &s) {
 		s.syncAsByte(_soundOn);
 		s.syncAsByte(masterVolume);
 	} else if (s.isLoading()) {
-		if (s.getVersion() >= 15) {
+		if (s.getVersion() >= 14) {
 			s.syncAsByte(_soundOn);
 			s.syncAsByte(masterVolume);
+			s.syncAsByte(_reverb);
 		} else {
 			_soundOn = true;
 			masterVolume = 15;
+			_reverb = 0;
 		}
 
 		soundSetSoundOn(_soundOn);
 		soundSetMasterVolume(masterVolume);
+		setReverb(_reverb);
 	}
 
 	if (s.isSaving())
diff --git a/engines/sci/engine/savegame.h b/engines/sci/engine/savegame.h
index ae3badc6da..8a4d8e7d3b 100644
--- a/engines/sci/engine/savegame.h
+++ b/engines/sci/engine/savegame.h
@@ -36,7 +36,7 @@ namespace Sci {
 struct EngineState;
 
 enum {
-	CURRENT_SAVEGAME_VERSION = 16,
+	CURRENT_SAVEGAME_VERSION = 14,
 	MINIMUM_SAVEGAME_VERSION = 9
 };
 
diff --git a/engines/sci/sfx/midiparser.cpp b/engines/sci/sfx/midiparser.cpp
index 6e8dd56bf5..85ffd083b9 100644
--- a/engines/sci/sfx/midiparser.cpp
+++ b/engines/sci/sfx/midiparser.cpp
@@ -83,12 +83,10 @@ void MidiParser_SCI::unloadMusic() {
 }
 
 void MidiParser_SCI::parseNextEvent(EventInfo &info) {
-	SegManager *segMan = ((SciEngine *)g_engine)->getEngineState()->_segMan;	// HACK
-
 	// Set signal AFTER waiting for delta, otherwise we would set signal too soon resulting in all sorts of bugs
 	if (_signalSet) {
 		_signalSet = false;
-		PUT_SEL32V(segMan, _pSnd->soundObj, signal, _signalToSet);
+		_pSnd->signal = _signalToSet;
 		debugC(2, kDebugLevelSound, "signal %04x", _signalToSet);
 	}
 
@@ -131,7 +129,19 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 		info.basic.param1 = *(_position._play_pos++);
 		info.basic.param2 = *(_position._play_pos++);
 		if (info.channel() == 0xF) {// SCI special
-			if (info.basic.param1 == 0x60) {
+			// Reference for some events:
+			// http://wiki.scummvm.org/index.php/SCI/Specifications/Sound/SCI0_Resource_Format#Status_Reference
+			// Also, sci/sfx/iterator/iterator.cpp, function BaseSongIterator::parseMidiCommand()
+			switch (info.basic.param1) {
+			case 0x50:	// set volume
+				// This is documented to be "reverb", but it looks like channel
+				// volume, at least in SCI11, so treat it as such
+				_pSnd->volume = info.basic.param2;
+				break;
+			case 0x52:	// set hold
+				_pSnd->hold = info.basic.param2;
+				break;
+			case 0x60:	// update dataInc
 				switch (_soundVersion) {
 				case SCI_VERSION_0_EARLY:
 				case SCI_VERSION_0_LATE:
@@ -146,10 +156,13 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 				default:
 					break;
 				}
+				break;
+			case 0x1:	// unknown (example case: LB2CD)
+			case 0xA:	// unknown (example case: LB2CD)
+			default:
+				warning("Unhandled SCI SysEx 0x%x (parameter %d)", info.basic.param1, info.basic.param2);
+				break;
 			}
-			// BF 50 x - set reverb to x
-			// BF 60 x - dataInc++
-			// BF 52 x - bHold=x
 		}
 		if (info.basic.param1 == 7) // channel volume change -scale it
 			info.basic.param2 = info.basic.param2 * _volume / 0x7F;
@@ -200,25 +213,15 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 			info.ext.data = _position._play_pos;
 			_position._play_pos += info.length;
 			if (info.ext.type == 0x2F) {// end of track reached
-				// We deviate from SSCI here: in SSCI, the loop is cached, whereas it's
-				// read directly from the sound code here (changed in rev. r46792).
-				// Theoretically, this shouldn't matter at all, as the loop should always
-				// be in sync and should never be changed. In SCI01 and later, the loop is
-				// set from the scripts via cmdSetHandleLoop, so again if there is any
-				// problem with this approach, it'll likely be apparent in SCI0 games
-				// (but no problems have been encountered because of this so far)
-				int16 loop = GET_SEL32V(segMan, _pSnd->soundObj, loop);
-				if (loop)
-					loop--;
-				PUT_SEL32V(segMan, _pSnd->soundObj, loop, loop);
-				if (loop) {
+				if (_pSnd->loop)
+					_pSnd->loop--;
+				if (_pSnd->loop) {
 					// We need to play it again...
 					jumpToTick(_loopTick);
 				} else {
 					_pSnd->status = kSoundStopped;
-					PUT_SEL32V(segMan, _pSnd->soundObj, signal, 0xFFFF);
-					if (_soundVersion <= SCI_VERSION_0_LATE)
-						PUT_SEL32V(segMan, _pSnd->soundObj, state, kSoundStopped);
+					_pSnd->signal = SIGNAL_OFFSET;
+
 					debugC(2, kDebugLevelSound, "signal EOT");
 				}
 			}
diff --git a/engines/sci/sfx/music.cpp b/engines/sci/sfx/music.cpp
index b745819648..d185334359 100644
--- a/engines/sci/sfx/music.cpp
+++ b/engines/sci/sfx/music.cpp
@@ -38,8 +38,7 @@
 namespace Sci {
 
 SciMusic::SciMusic(ResourceManager *resMan, SegManager *segMan, SciVersion soundVersion)
-	: _resMan(resMan), _segMan(segMan), _soundVersion(soundVersion),
-	  _soundOn(true), _inCriticalSection(false) {
+	: _soundVersion(soundVersion), _soundOn(true) {
 
 	// Reserve some space in the playlist, to avoid expensive insertion
 	// operations
@@ -103,30 +102,26 @@ void SciMusic::init() {
 void SciMusic::clearPlayList() {
 	_pMixer->stopAll();
 
+	_mutex.lock();
 	while (!_playList.empty()) {
 		soundStop(_playList[0]);
 		soundKill(_playList[0]);
 	}
-}
-
-void SciMusic::stopAll() {
-	const MusicList::iterator end = _playList.end();
-	for (MusicList::iterator i = _playList.begin(); i != end; ++i) {
-		if (_soundVersion <= SCI_VERSION_0_LATE)
-			PUT_SEL32V(_segMan, (*i)->soundObj, state, kSoundStopped);
-		else
-			PUT_SEL32V(_segMan, (*i)->soundObj, signal, SIGNAL_OFFSET);
-
-		(*i)->dataInc = 0;
-		soundStop(*i);
-	}
+	_mutex.unlock();
 }
 
 void SciMusic::miditimerCallback(void *p) {
 	SciMusic *aud = (SciMusic *)p;
+
+	Common::StackLock lock(aud->_mutex);
 	aud->onTimer();
 }
 
+void SciMusic::soundSetSoundOn(bool soundOnFlag) {
+	_soundOn = soundOnFlag;
+	_pMidiDrv->playSwitch(soundOnFlag);
+}
+
 uint16 SciMusic::soundGetVoices() {
 	switch (_midiType) {
 	case MD_PCSPK:
@@ -142,6 +137,35 @@ uint16 SciMusic::soundGetVoices() {
 	}
 }
 
+MusicEntry *SciMusic::getSlot(reg_t obj) {
+	Common::StackLock lock(_mutex);
+
+	const MusicList::iterator end = _playList.end();
+	for (MusicList::iterator i = _playList.begin(); i != end; ++i) {
+		if ((*i)->soundObj == obj)
+			return *i;
+	}
+
+	return NULL;
+}
+
+void SciMusic::setReverb(byte reverb) {
+	_reverb = reverb;
+
+	// TODO: actually set reverb for MT-32
+
+	// A good test case for this are the first two rooms in Longbow:
+	// reverb is set for the first room (the cave) and is subsequently
+	// cleared when Robin exits the cave
+}
+
+void SciMusic::resetDriver() {
+	_pMidiDrv->close();
+	_pMidiDrv->open();
+	_pMidiDrv->setTimerCallback(this, &miditimerCallback);
+	_dwTempo = _pMidiDrv->getBaseTempo();
+}
+
 static int f_compare(const void *arg1, const void *arg2) {
 	return ((const MusicEntry *)arg2)->prio - ((const MusicEntry *)arg1)->prio;
 }
@@ -312,6 +336,7 @@ void SciMusic::soundInitSnd(MusicEntry *pSnd) {
 			pSnd->hCurrentAud = Audio::SoundHandle();
 		} else {
 			// play MIDI track
+			_mutex.lock();
 			if (pSnd->pMidiParser == NULL) {
 				pSnd->pMidiParser = new MidiParser_SCI();
 				pSnd->pMidiParser->setMidiDriver(_pMidiDrv);
@@ -323,69 +348,23 @@ void SciMusic::soundInitSnd(MusicEntry *pSnd) {
 			// Find out what channels to filter for SCI0
 			channelFilterMask = pSnd->soundRes->getChannelFilterMask(_pMidiDrv->getPlayMask(_soundVersion));
 			pSnd->pMidiParser->loadMusic(track, pSnd, channelFilterMask, _soundVersion);
+
+			pSnd->pMidiParser->jumpToTick(pSnd->ticker);	// for resuming when loading
+			_mutex.unlock();
 		}
 	}
 }
 
 void SciMusic::onTimer() {
-	Common::StackLock lock(_mutex);
-
-	if (_inCriticalSection)
-		return;
-
 	const MusicList::iterator end = _playList.end();
 	for (MusicList::iterator i = _playList.begin(); i != end; ++i) {
 		(*i)->onTimer(_soundVersion, _pMixer);
 	}
 }
 
-void MusicEntry::onTimer(SciVersion soundVersion, Audio::Mixer *mixer) {
-	if (status != kSoundPlaying)
-		return;
-	if (pMidiParser) {
-		if (fadeStep)
-			doFade();
-		pMidiParser->onTimer();
-		ticker = (uint16)pMidiParser->getTick();
-	} else if (pStreamAud) {
-		if (!mixer->isSoundHandleActive(hCurrentAud)) {
-			ticker = 0xFFFF;
-			status = kSoundStopped;
-
-			// Signal the engine scripts that the sound is done playing
-			// FIXME: is there any other place this can be triggered properly?
-			SegManager *segMan = ((SciEngine *)g_engine)->getEngineState()->_segMan;	// HACK
-			PUT_SEL32V(segMan, soundObj, signal, SIGNAL_OFFSET);
-			if (soundVersion <= SCI_VERSION_0_LATE)
-				PUT_SEL32V(segMan, soundObj, state, kSoundStopped);
-		} else {
-			ticker = (uint16)(mixer->getSoundElapsedTime(hCurrentAud) * 0.06);
-		}
-	}
-}
-
-void MusicEntry::doFade() {
-	// This is called from inside onTimer, where the mutex is already locked
-
-	if (fadeTicker)
-		fadeTicker--;
-	else {
-		fadeTicker = fadeTickerStep;
-		volume += fadeStep;
-		SegManager *segMan = ((SciEngine *)g_engine)->getEngineState()->_segMan;	// HACK
-		if (((fadeStep > 0) && (volume >= fadeTo)) || ((fadeStep < 0) && (volume <= fadeTo))) {
-			volume = fadeTo;
-			fadeStep = 0;
-			PUT_SEL32V(segMan, soundObj, signal, 0xFFFF);
-		}
-		PUT_SEL32V(segMan, soundObj, vol, volume);
-
-		pMidiParser->setVolume(volume);
-	}
-}
-
-
 void SciMusic::soundPlay(MusicEntry *pSnd) {
+	_mutex.lock();
+
 	uint sz = _playList.size(), i;
 	// searching if sound is already in _playList
 	for (i = 0; i < sz && _playList[i] != pSnd; i++)
@@ -394,14 +373,18 @@ void SciMusic::soundPlay(MusicEntry *pSnd) {
 		_playList.push_back(pSnd);
 		sortPlayList();
 	}
+	
+	_mutex.unlock();
 
 	if (pSnd->pStreamAud && !_pMixer->isSoundHandleActive(pSnd->hCurrentAud)) {
 		_pMixer->playInputStream(Audio::Mixer::kSFXSoundType, &pSnd->hCurrentAud,
 				pSnd->pStreamAud, -1, pSnd->volume, 0, false);
 	} else if (pSnd->pMidiParser) {
+		_mutex.lock();
 		pSnd->pMidiParser->setVolume(pSnd->volume);
 		if (pSnd->status == kSoundStopped)
 			pSnd->pMidiParser->jumpToTick(0);
+		_mutex.unlock();
 	}
 
 	pSnd->status = kSoundPlaying;
@@ -411,18 +394,26 @@ void SciMusic::soundStop(MusicEntry *pSnd) {
 	pSnd->status = kSoundStopped;
 	if (pSnd->pStreamAud)
 		_pMixer->stopHandle(pSnd->hCurrentAud);
+
+	_mutex.lock();
 	if (pSnd->pMidiParser)
 		pSnd->pMidiParser->stop();
+	_mutex.unlock();
 }
 
 void SciMusic::soundSetVolume(MusicEntry *pSnd, byte volume) {
-	if (pSnd->pStreamAud)
+	if (pSnd->pStreamAud) {
 		_pMixer->setChannelVolume(pSnd->hCurrentAud, volume);
-	else if (pSnd->pMidiParser)
+	} else if (pSnd->pMidiParser) {
+		_mutex.lock();
 		pSnd->pMidiParser->setVolume(volume);
+		_mutex.unlock();
+	}
 }
 
 void SciMusic::soundSetPriority(MusicEntry *pSnd, byte prio) {
+	Common::StackLock lock(_mutex);
+
 	pSnd->prio = prio;
 	sortPlayList();
 }
@@ -431,15 +422,18 @@ void SciMusic::soundKill(MusicEntry *pSnd) {
 	pSnd->status = kSoundStopped;
 
 	if (pSnd->pMidiParser) {
+		_mutex.lock();
 		pSnd->pMidiParser->unloadMusic();
 		delete pSnd->pMidiParser;
 		pSnd->pMidiParser = NULL;
+		_mutex.unlock();
 	}
 	if (pSnd->pStreamAud) {
 		_pMixer->stopHandle(pSnd->hCurrentAud);
 		pSnd->pStreamAud = NULL;
 	}
 
+	_mutex.lock();
 	uint sz = _playList.size(), i;
 	// Remove sound from playlist
 	for (i = 0; i < sz; i++) {
@@ -450,6 +444,7 @@ void SciMusic::soundKill(MusicEntry *pSnd) {
 			break;
 		}
 	}
+	_mutex.unlock();
 }
 
 void SciMusic::soundPause(MusicEntry *pSnd) {
@@ -457,10 +452,13 @@ void SciMusic::soundPause(MusicEntry *pSnd) {
 	if (pSnd->status != kSoundPlaying)
 		return;
 	pSnd->status = kSoundPaused;
-	if (pSnd->pStreamAud)
+	if (pSnd->pStreamAud) {
 		_pMixer->pauseHandle(pSnd->hCurrentAud, true);
-	else if (pSnd->pMidiParser)
+	} else if (pSnd->pMidiParser) {
+		_mutex.lock();
 		pSnd->pMidiParser->pause();
+		_mutex.unlock();
+	}
 }
 
 void SciMusic::soundResume(MusicEntry *pSnd) {
@@ -498,55 +496,6 @@ void SciMusic::printPlayList(Console *con) {
 	}
 }
 
-void SciMusic::reconstructPlayList(int savegame_version) {
-	// Stop the music driver
-	if (_pMidiDrv) {
-		_pMidiDrv->close();
-		delete _pMidiDrv;
-	}
-
-	Common::StackLock lock(_mutex);
-
-	// Reinit the music driver
-	init();
-
-	const MusicList::iterator end = _playList.end();
-	for (MusicList::iterator i = _playList.begin(); i != end; ++i) {
-		if (savegame_version < 14) {
-			if (_soundVersion >= SCI_VERSION_1_EARLY) {
-				(*i)->dataInc = GET_SEL32V(_segMan, (*i)->soundObj, dataInc);
-				(*i)->volume = GET_SEL32V(_segMan, (*i)->soundObj, vol);
-			} else {
-				(*i)->volume = 100;
-			}
-		}
-
-		(*i)->soundRes = new SoundResource((*i)->resnum, _resMan, _soundVersion);
-		soundInitSnd(*i);
-		if ((*i)->pMidiParser)
-			(*i)->pMidiParser->jumpToTick((*i)->ticker);
-		// Replay all paused sounds - a lot of games pause sounds while saving, so
-		// we need to restore the correct sound state
-		if ((*i)->status == kSoundPaused) {
-			soundPlay(*i);
-		}
-	}
-}
-
-// There is some weird code going on in sierra sci. it checks the first 2 playlist entries if they are 0
-//  we should never need to care about this, anyway this is just meant as note
-MusicList::iterator SciMusic::enumPlayList(MusicList::iterator slotLoop) {
-	if (!slotLoop) {
-		if (_playList.begin() == _playList.end())
-			return NULL;
-		return _playList.begin();
-	}
-	slotLoop++;
-	if (slotLoop == _playList.end())
-		return NULL;
-	return slotLoop;
-}
-
 MusicEntry::MusicEntry() {
 	soundObj = NULL_REG;
 
@@ -555,8 +504,11 @@ MusicEntry::MusicEntry() {
 
 	dataInc = 0;
 	ticker = 0;
+	signal = 0;
 	prio = 0;
+	loop = 0;
 	volume = 0;
+	hold = 0;
 
 	pauseCounter = 0;
 
@@ -574,4 +526,39 @@ MusicEntry::MusicEntry() {
 MusicEntry::~MusicEntry() {
 }
 
+void MusicEntry::onTimer(SciVersion soundVersion, Audio::Mixer *mixer) {
+	if (status != kSoundPlaying)
+		return;
+	if (pMidiParser) {
+		if (fadeStep)
+			doFade();
+		pMidiParser->onTimer();
+		ticker = (uint16)pMidiParser->getTick();
+	} else if (pStreamAud) {
+		if (!mixer->isSoundHandleActive(hCurrentAud)) {
+			ticker = SIGNAL_OFFSET;
+			signal = SIGNAL_OFFSET;
+			status = kSoundStopped;
+		} else {
+			ticker = (uint16)(mixer->getSoundElapsedTime(hCurrentAud) * 0.06);
+		}
+	}
+}
+
+void MusicEntry::doFade() {
+	if (fadeTicker)
+		fadeTicker--;
+	else {
+		fadeTicker = fadeTickerStep;
+		volume += fadeStep;
+		if (((fadeStep > 0) && (volume >= fadeTo)) || ((fadeStep < 0) && (volume <= fadeTo))) {
+			volume = fadeTo;
+			fadeStep = 0;
+			signal = SIGNAL_OFFSET;
+		}
+
+		pMidiParser->setVolume(volume);
+	}
+}
+
 } // End of namespace Sci
diff --git a/engines/sci/sfx/music.h b/engines/sci/sfx/music.h
index fb047cd14f..8eca6bb483 100644
--- a/engines/sci/sfx/music.h
+++ b/engines/sci/sfx/music.h
@@ -65,6 +65,11 @@ class MusicEntry
 #endif
 {
 public:
+	// Do not get these directly for the sound objects!
+	// It's a bad idea, as the sound code (i.e. the SciMusic
+	// class) should be as separate as possible from the rest
+	// of the engine
+
 	reg_t soundObj;
 
 	SoundResource *soundRes;
@@ -72,8 +77,11 @@ public:
 
 	uint16 dataInc;
 	uint16 ticker;
+	uint16 signal;
 	byte prio;
-	int16 volume;
+	uint16 loop;
+	byte volume;
+	byte hold;
 
 	int16 pauseCounter;
 
@@ -121,7 +129,6 @@ public:
 #endif
 	void onTimer();
 	void clearPlayList();
-	void stopAll();
 
 	// sound and midi functions
 	void soundInitSnd(MusicEntry *pSnd);
@@ -135,42 +142,43 @@ public:
 	uint16 soundGetMasterVolume();
 	void soundSetMasterVolume(uint16 vol);
 	uint16 soundGetSoundOn() const { return _soundOn; }
-	void soundSetSoundOn(bool soundOnFlag) {
-		_soundOn = soundOnFlag;
-		_pMidiDrv->playSwitch(soundOnFlag);
-	}
+	void soundSetSoundOn(bool soundOnFlag);
 	uint16 soundGetVoices();
 	uint32 soundGetTempo() const { return _dwTempo; }
 
-	MusicEntry *getSlot(reg_t obj) { 
-		const MusicList::iterator end = _playList.end();
-		for (MusicList::iterator i = _playList.begin(); i != end; ++i) {
-			if ((*i)->soundObj == obj) {
-				return *i;
-			}
-		}
-
-		return NULL;
-	}
+	MusicEntry *getSlot(reg_t obj);
 
 	void pushBackSlot(MusicEntry *slotEntry) {
+		Common::StackLock lock(_mutex);
 		_playList.push_back(slotEntry);
 	}
 
 	void printPlayList(Console *con);
 
-	void reconstructPlayList(int savegame_version);
-	MusicList::iterator enumPlayList(MusicList::iterator slotLoop);
+	// The following two methods are NOT thread safe - make sure that
+	// the mutex is always locked before calling them
+	MusicList::iterator getPlayListStart() { return _playList.begin(); }
+	MusicList::iterator getPlayListEnd() { return _playList.end(); }
+
+	void sendMidiCommand (uint32 cmd) {
+		Common::StackLock lock(_mutex);
+		_pMidiDrv->send(cmd);
+	}
 
-	void enterCriticalSection() { _inCriticalSection = true; }
-	void leaveCriticalSection() { _inCriticalSection = false; }
+	void setReverb(byte reverb);
 
-	void sendMidiCommand (uint32 cmd) { _pMidiDrv->send(cmd); }
+	void resetDriver();
 
 #ifndef USE_OLD_MUSIC_FUNCTIONS
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 #endif
 
+	// Mutex for music code. Used to guard access to the song playlist, to the
+	// MIDI parser and to the MIDI driver/player. Note that guarded code must NOT
+	// include references to the mixer, otherwise there will probably be situations
+	// where a deadlock can occur
+	Common::Mutex _mutex;
+
 protected:
 	byte findAudEntry(uint16 nAud, byte&oVolume, uint32& oOffset, uint32&oSize);
 	void sortPlayList();
@@ -185,7 +193,6 @@ protected:
 	Audio::Mixer *_pMixer;
 	MidiPlayer *_pMidiDrv;
 	MidiDriverType _midiType;
-	Common::Mutex _mutex;
 
 	uint32 _dwTempo;
 	bool _bMultiMidi; // use adlib's digital track if midi track don't have one
@@ -194,10 +201,7 @@ private:
 
 	MusicList _playList;
 	bool _soundOn;
-	bool _inCriticalSection;
-
-	SegManager *_segMan;
-	ResourceManager *_resMan;
+	byte _reverb;
 };
 
 } // End of namespace Sci
diff --git a/engines/sci/sfx/soundcmd.cpp b/engines/sci/sfx/soundcmd.cpp
index e5647d77e9..02951d7e8b 100644
--- a/engines/sci/sfx/soundcmd.cpp
+++ b/engines/sci/sfx/soundcmd.cpp
@@ -170,7 +170,7 @@ SoundCommandParser::SoundCommandParser(ResourceManager *resMan, SegManager *segM
 		SOUNDCOMMAND(cmdUpdateCues);
 		SOUNDCOMMAND(cmdSendMidi);
 		SOUNDCOMMAND(cmdReverb);
-		SOUNDCOMMAND(cmdHoldHandle);
+		SOUNDCOMMAND(cmdSetHandleHold);
 		break;
 	case SCI_VERSION_1_LATE:
 		SOUNDCOMMAND(cmdVolume);
@@ -185,7 +185,7 @@ SoundCommandParser::SoundCommandParser(ResourceManager *resMan, SegManager *segM
 		SOUNDCOMMAND(cmdStopHandle);
 		SOUNDCOMMAND(cmdPauseHandle);
 		SOUNDCOMMAND(cmdFadeHandle);
-		SOUNDCOMMAND(cmdHoldHandle);
+		SOUNDCOMMAND(cmdSetHandleHold);
 		SOUNDCOMMAND(cmdDummy);
 		SOUNDCOMMAND(cmdSetHandleVolume);
 		SOUNDCOMMAND(cmdSetHandlePriority);
@@ -234,17 +234,7 @@ reg_t SoundCommandParser::parseCommand(int argc, reg_t *argv, reg_t acc) {
 			debugC(2, kDebugLevelSound, "%s, object %04x:%04x", _soundCommands[command]->desc, PRINT_REG(obj));
 		}
 
-		// If the command is operating on an object of the sound list, don't allow onTimer to kick in till the
-		// command is done
-#ifndef USE_OLD_MUSIC_FUNCTIONS
-		if (obj != NULL_REG)
-			_music->enterCriticalSection();
-#endif
 		(this->*(_soundCommands[command]->sndCmd))(obj, value);
-#ifndef USE_OLD_MUSIC_FUNCTIONS
-		if (obj != NULL_REG)
-			_music->leaveCriticalSection();
-#endif
 	} else {
 		warning("Invalid sound command requested (%d), valid range is 0-%d", command, _soundCommands.size() - 1);
 	}
@@ -288,6 +278,7 @@ void SoundCommandParser::cmdInitHandle(reg_t obj, int16 value) {
 	if (number && _resMan->testResource(ResourceId(kResourceTypeSound, number)))
 		newSound->soundRes = new SoundResource(number, _resMan, _soundVersion);
 	newSound->soundObj = obj;
+	newSound->loop = GET_SEL32V(_segMan, obj, loop);
 	newSound->prio = GET_SEL32V(_segMan, obj, pri) & 0xFF;
 	newSound->volume = CLIP<int>(GET_SEL32V(_segMan, obj, vol), 0, Audio::Mixer::kMaxChannelVolume);
 
@@ -296,8 +287,6 @@ void SoundCommandParser::cmdInitHandle(reg_t obj, int16 value) {
 	if (oldSound)
 		cmdDisposeHandle(obj, value);
 
-	_music->pushBackSlot(newSound);
-
 	// In SCI1.1 games, sound effects are started from here. If we can find
 	// a relevant audio resource, play it, otherwise switch to synthesized
 	// effects. If the resource exists, play it using map 65535 (sound
@@ -311,8 +300,12 @@ void SoundCommandParser::cmdInitHandle(reg_t obj, int16 value) {
 		if (newSound->soundRes)
 			_music->soundInitSnd(newSound);
 	}
+
+	_music->pushBackSlot(newSound);
+
 #endif
 
+	// Notify the engine
 	if (_soundVersion <= SCI_VERSION_0_LATE)
 		PUT_SEL32V(_segMan, obj, state, kSoundInitialized);
 	else
@@ -420,6 +413,7 @@ void SoundCommandParser::cmdPlayHandle(reg_t obj, int16 value) {
 		PUT_SEL32V(_segMan, obj, state, kSoundPlaying);
 	}
 
+	musicSlot->loop = GET_SEL32V(_segMan, obj, loop);
 	musicSlot->prio = GET_SEL32V(_segMan, obj, priority);
 	// vol selector doesnt get used before sci1late
 	if (_soundVersion < SCI_VERSION_1_LATE)
@@ -503,6 +497,7 @@ void SoundCommandParser::cmdStopHandle(reg_t obj, int16 value) {
 		PUT_SEL32V(_segMan, obj, signal, SIGNAL_OFFSET);
 
 	musicSlot->dataInc = 0;
+	musicSlot->signal = 0;
 	_music->soundStop(musicSlot);
 #endif
 }
@@ -518,14 +513,16 @@ void SoundCommandParser::cmdPauseHandle(reg_t obj, int16 value) {
 		changeHandleStatus(obj, value ? SOUND_STATUS_SUSPENDED : SOUND_STATUS_PLAYING);
 #else
 
+	Common::StackLock lock(_music->_mutex);
 	MusicEntry *musicSlot = NULL;
 	MusicList::iterator slotLoop = NULL;
 
 	if (!obj.segment) {
-		// Pausing/Resuming whole playlist was introduced sci1late (soundversion-wise)
+		// Pausing/Resuming the whole playlist was introduced 
+		// in the SCI1 late sound scheme
 		if (_soundVersion <= SCI_VERSION_1_EARLY)
 			return;
-		slotLoop = _music->enumPlayList(NULL);
+		slotLoop = _music->getPlayListStart();
 		musicSlot = *slotLoop;
 	} else {
 		musicSlot = _music->getSlot(obj);
@@ -545,10 +542,12 @@ void SoundCommandParser::cmdPauseHandle(reg_t obj, int16 value) {
 			else
 				_music->soundResume(musicSlot);
 		}
+
 		if (slotLoop) {
-			slotLoop = _music->enumPlayList(slotLoop);
-			if (slotLoop)
-				musicSlot = *slotLoop;
+			if (slotLoop == _music->getPlayListEnd())
+				break;
+			else
+				musicSlot = *(slotLoop++);
 		}
 	} while (slotLoop);
 #endif
@@ -563,9 +562,7 @@ void SoundCommandParser::cmdResumeHandle(reg_t obj, int16 value) {
 #ifdef USE_OLD_MUSIC_FUNCTIONS
 	changeHandleStatus(obj, SOUND_STATUS_PLAYING);
 #else
-	MusicEntry *musicSlot = NULL;
-
-	musicSlot = _music->getSlot(obj);
+	MusicEntry *musicSlot = _music->getSlot(obj);
 	if (!musicSlot) {
 		warning("cmdResumeHandle: Slot not found");
 		return;
@@ -675,6 +672,7 @@ void SoundCommandParser::cmdUpdateHandle(reg_t obj, int16 value) {
 		return;
 	}
 
+	musicSlot->loop = GET_SEL32V(_segMan, obj, loop);
 	int16 objVol = CLIP<int>(GET_SEL32V(_segMan, obj, vol), 0, 255);
 	if (objVol != musicSlot->volume)
 		_music->soundSetVolume(musicSlot, objVol);
@@ -760,36 +758,59 @@ void SoundCommandParser::cmdUpdateCues(reg_t obj, int16 value) {
 		PUT_SEL32V(_segMan, obj, frame, frame);
 	}
 #else
+	updateCues(obj);
+#endif
+}
+
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+
+void SoundCommandParser::updateSci0Cues() {
+	Common::StackLock(_music->_mutex);
+
+	const MusicList::iterator end = _music->getPlayListEnd();
+	for (MusicList::iterator i = _music->getPlayListStart(); i != end; ++i) {
+		updateCues((*i)->soundObj);
+	}
+}
 
+void SoundCommandParser::updateCues(reg_t obj) {
 	MusicEntry *musicSlot = _music->getSlot(obj);
 	if (!musicSlot) {
 		warning("cmdUpdateCues: Slot not found");
 		return;
 	}
 
-	uint16 signal = GET_SEL32V(_segMan, obj, signal);
-	uint16 dataInc = musicSlot->dataInc;
-
-	switch (signal) {
+	switch (musicSlot->signal) {
 		case 0:
-			if (dataInc != GET_SEL32V(_segMan, obj, dataInc)) {
-				PUT_SEL32V(_segMan, obj, dataInc, dataInc);
-				PUT_SEL32V(_segMan, obj, signal, dataInc + 127);
+			if (musicSlot->dataInc != GET_SEL32V(_segMan, obj, dataInc)) {
+				PUT_SEL32V(_segMan, obj, dataInc, musicSlot->dataInc);
+				PUT_SEL32V(_segMan, obj, signal, musicSlot->dataInc + 127);
 			}
 			break;
-		case 0xFFFF:
-			cmdStopHandle(obj, value);
+		case SIGNAL_OFFSET:
+			cmdStopHandle(obj, 0);
 			break;
 		default:
+			// Sync the signal of the sound object
+			PUT_SEL32V(_segMan, obj, signal, musicSlot->signal);
 			break;
 	}
 
-	uint16 ticker = musicSlot->ticker;
-	PUT_SEL32V(_segMan, obj, min, ticker / 3600);
-	PUT_SEL32V(_segMan, obj, sec, ticker % 3600 / 60);
-	PUT_SEL32V(_segMan, obj, frame, ticker);
-#endif
+	// Signal the game when a digital sound effect is done playing
+	if (musicSlot->pStreamAud && musicSlot->status == kSoundStopped && 
+		musicSlot->signal == SIGNAL_OFFSET) {
+		cmdStopHandle(obj, 0);
+	}
+
+	musicSlot->signal = 0;
+
+	if (_soundVersion >= SCI_VERSION_1_EARLY) {
+		PUT_SEL32V(_segMan, obj, min, musicSlot->ticker / 3600);
+		PUT_SEL32V(_segMan, obj, sec, musicSlot->ticker % 3600 / 60);
+		PUT_SEL32V(_segMan, obj, frame, musicSlot->ticker);
+	}
 }
+#endif
 
 void SoundCommandParser::cmdSendMidi(reg_t obj, int16 value) {
 #ifdef USE_OLD_MUSIC_FUNCTIONS
@@ -801,28 +822,25 @@ void SoundCommandParser::cmdSendMidi(reg_t obj, int16 value) {
 }
 
 void SoundCommandParser::cmdReverb(reg_t obj, int16 value) {
-	// TODO
-	// This function has one parameter, enabling the reverb effect
-	// in MT-32 if the parameter is non-zero. This is either the
-	// reverb level, or delay time. The reverb type is probably fixed
-	// to 1 ("room"). I'm not quite sure how and if this works for
-	// Adlib.
-	// Refer to http://www.midi.org/techspecs/midimessages.php
-	// and http://www.youngmonkey.ca/nose/audio_tech/synth/Roland-MT32.html
-	// Also, /sound/softsynth/mt32/synth.h is a good reference
-	// A good test case for this are the first two rooms in Longbow:
-	// reverb is set for the first room (the cave) and is subsequently
-	// cleared when Robin exits the cave
-	warning("STUB: cmdReverb (%d)", obj.toUint16());
-}
-
-void SoundCommandParser::cmdHoldHandle(reg_t obj, int16 value) {
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+	_music->setReverb(obj.toUint16() & 0xF);
+#endif
+}
+
+void SoundCommandParser::cmdSetHandleHold(reg_t obj, int16 value) {
 #ifdef USE_OLD_MUSIC_FUNCTIONS
 	SongHandle handle = FROBNICATE_HANDLE(obj);
 	_state->sfx_song_set_hold(handle, value);
 #else
-	// TODO: implement this...
-	warning("STUB: cmdHoldHandle");
+	MusicEntry *musicSlot = _music->getSlot(obj);
+	if (!musicSlot) {
+		warning("cmdSetHandleHold: Slot not found");
+		return;
+	}
+
+	musicSlot->hold = value;
+
+	// TODO: actually handle channel hold!
 #endif
 }
 
@@ -833,7 +851,18 @@ void SoundCommandParser::cmdGetAudioCapability(reg_t obj, int16 value) {
 
 void SoundCommandParser::cmdStopAllSounds(reg_t obj, int16 value) {
 #ifndef USE_OLD_MUSIC_FUNCTIONS
-	_music->stopAll();
+	Common::StackLock(_music->_mutex);
+
+	const MusicList::iterator end = _music->getPlayListEnd();
+	for (MusicList::iterator i = _music->getPlayListStart(); i != end; ++i) {
+		if (_soundVersion <= SCI_VERSION_0_LATE)
+			PUT_SEL32V(_segMan, (*i)->soundObj, state, kSoundStopped);
+		else
+			PUT_SEL32V(_segMan, (*i)->soundObj, signal, SIGNAL_OFFSET);
+
+		(*i)->dataInc = 0;
+		_music->soundStop(*i);
+	}
 #endif
 }
 
@@ -892,10 +921,12 @@ void SoundCommandParser::cmdSetHandleLoop(reg_t obj, int16 value) {
 		return;
 	}
 	if (value == -1) {
-		PUT_SEL32V(_segMan, obj, loop, 0xFFFF);
+		musicSlot->loop = 0xFFFF;
 	} else {
-		PUT_SEL32V(_segMan, obj, loop, 1); // actually plays the music once
+		musicSlot->loop = 1; // actually plays the music once
 	}
+
+	PUT_SEL32V(_segMan, obj, loop, musicSlot->loop);
 #endif
 }
 
@@ -906,19 +937,42 @@ void SoundCommandParser::cmdSuspendSound(reg_t obj, int16 value) {
 
 void SoundCommandParser::clearPlayList() {
 #ifndef USE_OLD_MUSIC_FUNCTIONS
+	Common::StackLock lock(_music->_mutex);
 	_music->clearPlayList();
 #endif
 }
 
 void SoundCommandParser::syncPlayList(Common::Serializer &s) {
 #ifndef USE_OLD_MUSIC_FUNCTIONS
+	Common::StackLock lock(_music->_mutex);
 	_music->saveLoadWithSerializer(s);
 #endif
 }
 
 void SoundCommandParser::reconstructPlayList(int savegame_version) {
 #ifndef USE_OLD_MUSIC_FUNCTIONS
-	_music->reconstructPlayList(savegame_version);
+
+	Common::StackLock lock(_music->_mutex);
+
+	const MusicList::iterator end = _music->getPlayListEnd();
+	for (MusicList::iterator i = _music->getPlayListStart(); i != end; ++i) {
+		if (savegame_version < 14) {
+			(*i)->dataInc = GET_SEL32V(_segMan, (*i)->soundObj, dataInc);
+			(*i)->signal = GET_SEL32V(_segMan, (*i)->soundObj, signal);
+
+			if (_soundVersion >= SCI_VERSION_1_EARLY)
+				(*i)->volume = GET_SEL32V(_segMan, (*i)->soundObj, vol);
+			else
+				(*i)->volume = 100;
+		}
+
+		(*i)->soundRes = new SoundResource((*i)->resnum, _resMan, _soundVersion);
+		_music->soundInitSnd(*i);
+		if ((*i)->status == kSoundPlaying)
+			cmdPlayHandle((*i)->soundObj, 0);
+	}
+
+	_music->resetDriver();
 #endif
 }
 
diff --git a/engines/sci/sfx/soundcmd.h b/engines/sci/sfx/soundcmd.h
index 1d88c393fd..bb37d1f0b3 100644
--- a/engines/sci/sfx/soundcmd.h
+++ b/engines/sci/sfx/soundcmd.h
@@ -59,6 +59,10 @@ public:
 	void reconstructPlayList(int savegame_version);
 	void printPlayList(Console *con);
 
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+	void updateSci0Cues();
+#endif
+
 private:
 	Common::Array<MusicEntryCommand*> _soundCommands;
 	ResourceManager *_resMan;
@@ -75,6 +79,17 @@ private:
 	reg_t _acc;
 	int _midiCmd, _controller, _param;
 
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+	/**
+	 * Synchronizes the current state of the music list to the rest of the engine, so that
+	 * the changes that the sound thread makes to the music are registered with the engine
+	 * scripts. In SCI0, we invoke this from kAnimate (which is called very often). SCI01
+	 * and later have a specific callback function, cmdUpdateCues, which is called regularly
+	 * by the engine scripts themselves, so the engine itself polls for changes to the music
+	 */
+	void updateCues(reg_t obj);
+#endif
+
 	void cmdInitHandle(reg_t obj, int16 value);
 	void cmdPlayHandle(reg_t obj, int16 value);
 	void cmdDummy(reg_t obj, int16 value);
@@ -94,7 +109,7 @@ private:
 	void cmdUpdateCues(reg_t obj, int16 value);
 	void cmdSendMidi(reg_t obj, int16 value);
 	void cmdReverb(reg_t obj, int16 value);
-	void cmdHoldHandle(reg_t obj, int16 value);
+	void cmdSetHandleHold(reg_t obj, int16 value);
 	void cmdGetAudioCapability(reg_t obj, int16 value);
 	void cmdSetHandleVolume(reg_t obj, int16 value);
 	void cmdSetHandlePriority(reg_t obj, int16 value);