SCI/new sound code:

- Made the SciMusic class private, and added wrapper functions for invoking specific methods of SciMusic from outside the SoundCommandParser class
- Many SCI games keep creating and destroying sound effects constantly (i.e. many times per second). Therefore, another scheme has been devised, which replaces the mutex that was in place. Whenever a sound command is run which operates on a specific object in the play list, we disallow onTimer() from kicking in. This isn't ideal, but it does stop random deadlocks because of locked mutexes without any noticeable side effects

svn-id: r46681

7 files changed, 68 insertions, 34 deletions

diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index cdb4a1f476..8b51ce2186 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -1626,7 +1626,7 @@ bool Console::cmdSongLib(int argc, const char **argv) {
 	} while (seeker);
 	DebugPrintf("\n");
 #else
-	_vm->getEngineState()->_soundCmd->_music->printSongLib(this);
+	_vm->getEngineState()->_soundCmd->printPlayList(this);
 #endif
 
 	return true;
diff --git a/engines/sci/engine/game.cpp b/engines/sci/engine/game.cpp
index 633d01f7f4..0434008a59 100644
--- a/engines/sci/engine/game.cpp
+++ b/engines/sci/engine/game.cpp
@@ -441,7 +441,7 @@ int game_exit(EngineState *s) {
 		game_init_sound(s, SFX_STATE_FLAG_NOSOUND, s->detectDoSoundType());
 #else
 		s->_audio->stopAllAudio();
-		s->_soundCmd->_music->clearPlayList();
+		s->_soundCmd->clearPlayList();
 #endif
 	}
 
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 7677157f48..56e81c6e4b 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -428,7 +428,7 @@ void EngineState::saveLoadWithSerializer(Common::Serializer &s) {
 #ifdef USE_OLD_MUSIC_FUNCTIONS
 	sync_songlib(s, _sound._songlib);
 #else
-	_soundCmd->_music->saveLoadWithSerializer(s);
+	_soundCmd->syncPlayList(s);
 #endif
 }
 
@@ -969,7 +969,7 @@ EngineState *gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 	retval->_sound._suspended = s->_sound._suspended;
 	reconstruct_sounds(retval);
 #else
-	retval->_soundCmd->_music->reconstructSounds(meta.savegame_version);
+	retval->_soundCmd->reconstructPlayList(meta.savegame_version);
 #endif
 
 	// Message state:
diff --git a/engines/sci/sfx/music.cpp b/engines/sci/sfx/music.cpp
index fe22cf356a..e0f9111088 100644
--- a/engines/sci/sfx/music.cpp
+++ b/engines/sci/sfx/music.cpp
@@ -42,7 +42,7 @@ static int f_compare(const void *arg1, const void *arg2) {
 }
 
 SciMusic::SciMusic(SciVersion soundVersion)
-	: _soundVersion(soundVersion), _soundOn(true) {
+	: _soundVersion(soundVersion), _soundOn(true), _inCriticalSection(false) {
 
 	// Reserve some space in the playlist, to avoid expensive insertion
 	// operations
@@ -125,8 +125,6 @@ bool SciMusic::saveState(Common::OutSaveFile *pFile) {
 
 //----------------------------------------
 void SciMusic::clearPlayList() {
-	Common::StackLock lock(_mutex);
-
 	_pMixer->stopAll();
 
 	while (!_playList.empty()) {
@@ -136,8 +134,6 @@ void SciMusic::clearPlayList() {
 }
 //----------------------------------------
 void SciMusic::stopAll() {
-	Common::StackLock lock(_mutex);
-
 	SegManager *segMan = ((SciEngine *)g_engine)->getEngineState()->_segMan;	// HACK
 	
 	for (uint32 i = 0; i < _playList.size(); i++) {
@@ -352,6 +348,9 @@ void SciMusic::soundInitSnd(MusicEntry *pSnd) {
 void SciMusic::onTimer() {
 	Common::StackLock lock(_mutex);
 
+	if (_inCriticalSection)
+		return;
+
 	uint sz = _playList.size();
 	for (uint i = 0; i < sz; i++) {
 		if (_playList[i]->status != kSndStatusPlaying)
@@ -399,8 +398,6 @@ void SciMusic::doFade(MusicEntry *pSnd) {
 
 //---------------------------------------------
 void SciMusic::soundPlay(MusicEntry *pSnd) {
-	Common::StackLock lock(_mutex);
-
 	uint sz = _playList.size(), i;
 	// searching if sound is already in _playList
 	for (i = 0; i < sz && _playList[i] != pSnd; i++)
@@ -438,8 +435,6 @@ void SciMusic::soundSetVolume(MusicEntry *pSnd, byte volume) {
 }
 //---------------------------------------------
 void SciMusic::soundSetPriority(MusicEntry *pSnd, byte prio) {
-	Common::StackLock lock(_mutex);
-
 	pSnd->prio = prio;
 	sortPlayList();
 }
@@ -457,8 +452,6 @@ void SciMusic::soundKill(MusicEntry *pSnd) {
 		pSnd->pStreamAud = NULL;
 	}
 
-	Common::StackLock lock(_mutex);
-
 	uint sz = _playList.size(), i;
 	// Remove sound from playlist
 	for (i = 0; i < sz; i++) {
@@ -493,7 +486,7 @@ void SciMusic::soundSetMasterVolume(uint16 vol) {
 	_pMixer->setVolumeForSoundType(Audio::Mixer::kPlainSoundType, vol);
 }
 
-void SciMusic::printSongLib(Console *con) {
+void SciMusic::printPlayList(Console *con) {
 	Common::StackLock lock(_mutex);
 	const char *musicStatus[] = { "Stopped", "Initialized", "Paused", "Playing" };
 
@@ -504,9 +497,7 @@ void SciMusic::printSongLib(Console *con) {
 	}
 }
 
-void SciMusic::reconstructSounds(int savegame_version) {
-	Common::StackLock lock(_mutex);
-
+void SciMusic::reconstructPlayList(int savegame_version) {
 	SegManager *segMan = ((SciEngine *)g_engine)->getEngineState()->_segMan;	// HACK
 	ResourceManager *resMan = ((SciEngine *)g_engine)->getEngineState()->resMan;	// HACK
 
diff --git a/engines/sci/sfx/music.h b/engines/sci/sfx/music.h
index 16400efcd0..605eaa061c 100644
--- a/engines/sci/sfx/music.h
+++ b/engines/sci/sfx/music.h
@@ -124,8 +124,6 @@ public:
 	uint32 soundGetTempo() { return _dwTempo; }
 
 	MusicEntry *getSlot(reg_t obj) { 
-		Common::StackLock lock(_mutex);
-
 		for (uint32 i = 0; i < _playList.size(); i++) {
 			if (_playList[i]->soundObj == obj) {
 				return _playList[i];
@@ -136,13 +134,15 @@ public:
 	}
 
 	void pushBackSlot(MusicEntry *slotEntry) {
-		Common::StackLock lock(_mutex);
 		_playList.push_back(slotEntry);
 	}
 
-	void printSongLib(Console *con);
+	void printPlayList(Console *con);
+
+	void reconstructPlayList(int savegame_version);
 
-	void reconstructSounds(int savegame_version);
+	void enterCriticalSection() { _inCriticalSection = true; }
+	void leaveCriticalSection() { _inCriticalSection = false; }
 
 #ifndef USE_OLD_MUSIC_FUNCTIONS
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
@@ -173,6 +173,7 @@ private:
 
 	MusicList _playList;
 	bool _soundOn;
+	bool _inCriticalSection;
 };
 
 } // end of namespace
diff --git a/engines/sci/sfx/soundcmd.cpp b/engines/sci/sfx/soundcmd.cpp
index dba77ccf3c..d876437672 100644
--- a/engines/sci/sfx/soundcmd.cpp
+++ b/engines/sci/sfx/soundcmd.cpp
@@ -221,10 +221,22 @@ reg_t SoundCommandParser::parseCommand(int argc, reg_t *argv, reg_t acc) {
 	}
 
 	if (command < _soundCommands.size()) {
-		//if (strcmp(_soundCommands[command]->desc, "cmdUpdateCues"))
-		//printf("%s, object %04x:%04x\n", _soundCommands[command]->desc, PRINT_REG(obj));	// debug
-		debugC(2, kDebugLevelSound, "%s, object %04x:%04x", _soundCommands[command]->desc, PRINT_REG(obj));
+		if (strcmp(_soundCommands[command]->desc, "cmdUpdateCues")) {
+			//printf("%s, object %04x:%04x\n", _soundCommands[command]->desc, PRINT_REG(obj));	// debug
+			//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);
 	}
@@ -270,11 +282,6 @@ void SoundCommandParser::cmdInitHandle(reg_t obj, int16 value) {
 	PUT_SEL32(_segMan, obj, handle, obj);
 
 #ifndef USE_OLD_MUSIC_FUNCTIONS
-	// Check if a track with the same sound object is already playing
-	MusicEntry *oldSound = _music->getSlot(obj);
-	if (oldSound)
-		_music->soundKill(oldSound);
-
 	MusicEntry *newSound = new MusicEntry();
 	newSound->soundRes = 0;
 	newSound->resnum = number;
@@ -293,6 +300,12 @@ void SoundCommandParser::cmdInitHandle(reg_t obj, int16 value) {
 	newSound->fadeTicker = 0;
 	newSound->fadeTickerStep = 0;
 	newSound->status = kSndStatusStopped;
+
+	// Check if a track with the same sound object is already playing
+	MusicEntry *oldSound = _music->getSlot(obj);
+	if (oldSound)
+		_music->soundKill(oldSound);
+
 	_music->pushBackSlot(newSound);
 
 	// In SCI1.1 games, sound effects are started from here. If we can find
@@ -873,4 +886,28 @@ void SoundCommandParser::cmdSuspendSound(reg_t obj, int16 value) {
 	warning("STUB: cmdSuspendSound");
 }
 
+void SoundCommandParser::clearPlayList() {
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+	_music->clearPlayList();
+#endif
+}
+
+void SoundCommandParser::syncPlayList(Common::Serializer &s) {
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+	_music->saveLoadWithSerializer(s);
+#endif
+}
+
+void SoundCommandParser::reconstructPlayList(int savegame_version) {
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+	_music->reconstructPlayList(savegame_version);
+#endif
+}
+
+void SoundCommandParser::printPlayList(Console *con) {
+#ifndef USE_OLD_MUSIC_FUNCTIONS
+	_music->printPlayList(con);
+#endif
+}
+
 } // End of namespace Sci
diff --git a/engines/sci/sfx/soundcmd.h b/engines/sci/sfx/soundcmd.h
index 35b0f18504..b83af491b5 100644
--- a/engines/sci/sfx/soundcmd.h
+++ b/engines/sci/sfx/soundcmd.h
@@ -33,6 +33,7 @@
 
 namespace Sci {
 
+class Console;
 class SciMusic;
 class SoundCommandParser;
 typedef void (SoundCommandParser::*SoundCommand)(reg_t obj, int16 value);
@@ -53,8 +54,10 @@ public:
 #endif
 
 	reg_t parseCommand(int argc, reg_t *argv, reg_t acc);
-
-	SciMusic *_music;
+	void clearPlayList();
+	void syncPlayList(Common::Serializer &s);
+	void reconstructPlayList(int savegame_version);
+	void printPlayList(Console *con);
 
 private:
 	Common::Array<MusicEntryCommand*> _soundCommands;
@@ -62,6 +65,8 @@ private:
 	SegManager *_segMan;
 #ifdef USE_OLD_MUSIC_FUNCTIONS
 	SfxState *_state;
+#else
+	SciMusic *_music;
 #endif
 	AudioPlayer *_audio;
 	SciVersion _soundVersion;