SCI: Some SFX player cleanup

svn-id: r40956

4 files changed, 108 insertions, 146 deletions

diff --git a/engines/sci/sfx/core.cpp b/engines/sci/sfx/core.cpp
index d819ef7b9b..9eca47e0cf 100644
--- a/engines/sci/sfx/core.cpp
+++ b/engines/sci/sfx/core.cpp
@@ -58,7 +58,7 @@ void sfx_reset_player() {
 
 int sfx_get_player_polyphony() {
 	if (player)
-		return player->polyphony;
+		return player->_polyphony;
 	else
 		return 0;
 }
@@ -344,14 +344,6 @@ int sfx_play_iterator_pcm(SongIterator *it, song_handle_t handle) {
 
 #define DELAY (1000000 / SFX_TICKS_PER_SEC)
 
-static void _sfx_timer_callback(void *data) {
-	/* First run the player, to give it a chance to fill
-	** the audio buffer  */
-
-	if (player)
-		player->maintenance();
-}
-
 void sfx_init(sfx_state_t *self, ResourceManager *resmgr, int flags) {
 	song_lib_init(&self->songlib);
 	self->song = NULL;
@@ -360,18 +352,13 @@ void sfx_init(sfx_state_t *self, ResourceManager *resmgr, int flags) {
 	self->soundSync = NULL;
 	self->audioResource = NULL;
 
+	player = NULL;
+
 	if (flags & SFX_STATE_FLAG_NOSOUND) {
-		player = NULL;
-		sciprintf("[SFX] Sound disabled.\n");
+		warning("[SFX] Sound disabled");
 		return;
 	}
 
-	// TODO: Implement platform policy here?
-	player = new NewPlayer();
-	//player = new PolledPlayer();
-	//player = new RealtimePlayer();
-
-
 #ifdef DEBUG_SONG_API
 	fprintf(stderr, "[sfx-core] Initialising: flags=%x\n", flags);
 #endif
@@ -381,67 +368,36 @@ void sfx_init(sfx_state_t *self, ResourceManager *resmgr, int flags) {
 	/*-------------------*/
 
 	if (!resmgr) {
-		sciprintf("[SFX] Warning: No resource manager present, cannot initialise player\n");
-		player = NULL;
-	} else if (player->init(resmgr, DELAY / 1000)) {
-		sciprintf("[SFX] Song player '%s' reported error, disabled\n", player->name);
-		delete player;
-		player = NULL;
+		warning("[SFX] Warning: No resource manager present, cannot initialise player");
+		return;
 	}
 
-	if (!player)
+	player = new NewPlayer();
+
+	if (!player) {
 		sciprintf("[SFX] No song player found\n");
-	else
-		sciprintf("[SFX] Using song player '%s', v%s\n", player->name, player->version);
-
-	/*------------------*/
-	/* Initialise timer */
-	/*------------------*/
-
-	// We initialise the timer last, so there is no possibility of the
-	// timer callback being triggered while the mixer or player are
-	// still being initialized.
-	if (strcmp(player->name, "realtime") == 0) {
-		// FIXME: Merge this timer code into RealtimePlayer itself
-		if (!g_system->getTimerManager()->installTimerProc(&_sfx_timer_callback, DELAY, NULL)) {
-			warning("[SFX] " __FILE__": Timer failed to initialize");
-			warning("[SFX] Disabled sound support");
-			delete player;
-			player = NULL;
-		}
+		return;
+	}
+
+	if (player->init(resmgr, DELAY / 1000)) {
+		warning("[SFX] Song player reported error, disabled");
+		delete player;
+		player = NULL;
 	}
 }
 
 void sfx_exit(sfx_state_t *self) {
-	g_system->getTimerManager()->removeTimerProc(&_sfx_timer_callback);
-
-	// The timer API guarantees no more callbacks are running or will be
-	// run from this point onward, so we can now safely exit the mixer and
-	// player.
-
 #ifdef DEBUG_SONG_API
 	fprintf(stderr, "[sfx-core] Uninitialising\n");
 #endif
 
-	// WARNING: The mixer may hold feeds from the player, so we must
-	// stop the mixer BEFORE stopping the player.
-	// FIXME Player "new" frees its own feeds, so we only need to stop any
-	// remaining sfx feeds after stopping the player.
-	if (strcmp(player->name, "new") != 0)
-		g_system->getMixer()->stopAll();
-
-	// FIXME: change players to stop their own audio streams
 	if (player) {
-		// See above: This must happen AFTER stopping the mixer
 		player->exit();
 		delete player;
 		player = 0;
 	}
 
-	// FIXME: player is deleted here by the code above, so this will crash.
-	// Is that code needed?
-	//if (strcmp(player->name, "new") == 0)
-	//	g_system->getMixer()->stopAll();
+	g_system->getMixer()->stopAll();
 
 	song_lib_free(self->songlib);
 
diff --git a/engines/sci/sfx/player.h b/engines/sci/sfx/player.h
index 104a50db09..f6275bd820 100644
--- a/engines/sci/sfx/player.h
+++ b/engines/sci/sfx/player.h
@@ -37,14 +37,11 @@ namespace Sci {
 
 class SfxPlayer {
 public:
-	const char *name;
-	const char *version;
-
 	/** Number of voices that can play simultaneously */
-	int polyphony;
+	int _polyphony;
 
 public:
-	SfxPlayer() : name(0), version(0), polyphony(0) {}
+	SfxPlayer() : _polyphony(0) {}
 	virtual ~SfxPlayer() {}
 
 	virtual Common::Error init(ResourceManager *resmgr, int expected_latency) = 0;
@@ -96,12 +93,6 @@ public:
 	** Returns   : (int) Common::kNoError on success, Common::kUnknownError on failure
 	*/
 
-	virtual void maintenance() {}
-	/* Regularly called maintenance function
-	** This function is called frequently and regularly (if present), it can be
-	** used to emit sound.
-	*/
-
 	virtual void tell_synth(int buf_nr, byte *buf) = 0;
 	/* Pass a raw MIDI event to the synth
 	Parameters: (int) argc: Length of buffer holding the midi event
diff --git a/engines/sci/sfx/player/new_player.cpp b/engines/sci/sfx/player/new_player.cpp
index e725fc9da6..c5b158574f 100644
--- a/engines/sci/sfx/player/new_player.cpp
+++ b/engines/sci/sfx/player/new_player.cpp
@@ -36,37 +36,36 @@
 
 namespace Sci {
 
-// TODO: Turn the following static vars into member vars
-static MidiPlayer *mididrv;
+NewPlayer::NewPlayer() {
+	_mididrv = 0;
 
-static SongIterator *play_it = NULL;
-static Audio::Timestamp wakeup_time;
-static Audio::Timestamp current_time;
-static uint32 play_pause_diff;
+	_iterator = NULL;
+	_pauseTimeDiff = 0;
 
-static int play_paused = 0;
-static int play_it_done = 0;
-static uint32 tempo;
+	_paused = false;
+	_iteratorIsDone = false;
+	_tempo = 0;
 
-static Common::Mutex *mutex;
-static int volume = 15;
+	_mutex = 0;
+	_volume = 15;
+}
 
-static void play_song(SongIterator *it) {
-	while (play_it && wakeup_time.msecsDiff(current_time) <= 0) {
+void NewPlayer::play_song(SongIterator *it) {
+	while (_iterator && _wakeupTime.msecsDiff(_currentTime) <= 0) {
 		int delay;
 		byte buf[8];
 		int result;
 
-		switch ((delay = songit_next(&(play_it),
+		switch ((delay = songit_next(&(_iterator),
 		                             buf, &result,
 		                             IT_READER_MASK_ALL
 		                             | IT_READER_MAY_FREE
 		                             | IT_READER_MAY_CLEAN))) {
 
 		case SI_FINISHED:
-			delete play_it;
-			play_it = NULL;
-			play_it_done = 1;
+			delete _iterator;
+			_iterator = NULL;
+			_iteratorIsDone = true;
 			return;
 
 		case SI_IGNORE:
@@ -76,16 +75,16 @@ static void play_song(SongIterator *it) {
 			break;
 
 		case SI_PCM:
-			sfx_play_iterator_pcm(play_it, 0);
+			sfx_play_iterator_pcm(_iterator, 0);
 			break;
 
 		case 0:
-			static_cast<MidiDriver *>(mididrv)->send(buf[0], buf[1], buf[2]);
+			static_cast<MidiDriver *>(_mididrv)->send(buf[0], buf[1], buf[2]);
 
 			break;
 
 		default:
-			wakeup_time = wakeup_time.addFrames(delay);
+			_wakeupTime = _wakeupTime.addFrames(delay);
 		}
 	}
 }
@@ -94,17 +93,19 @@ void NewPlayer::tell_synth(int buf_nr, byte *buf) {
 	byte op1 = (buf_nr < 2 ? 0 : buf[1]);
 	byte op2 = (buf_nr < 3 ? 0 : buf[2]);
 
-	static_cast<MidiDriver *>(mididrv)->send(buf[0], op1, op2);
+	static_cast<MidiDriver *>(_mididrv)->send(buf[0], op1, op2);
 }
 
-static void player_timer_callback(void *refCon) {
-	Common::StackLock lock(*mutex);
+void NewPlayer::player_timer_callback(void *refCon) {
+	NewPlayer *player = (NewPlayer *)refCon;
+	assert(refCon);
+	Common::StackLock lock(*player->_mutex);
 
-	if (play_it && !play_it_done && !play_paused) {
-		play_song(play_it);
+	if (player->_iterator && !player->_iteratorIsDone && !player->_paused) {
+		player->play_song(player->_iterator);
 	}
 
-	current_time = current_time.addFrames(1);
+	player->_currentTime = player->_currentTime.addFrames(1);
 }
 
 /* API implementation */
@@ -114,111 +115,104 @@ Common::Error NewPlayer::init(ResourceManager *resmgr, int expected_latency) {
 
 	switch(musicDriver) {
 	case MD_ADLIB:
-		mididrv = new MidiPlayer_Adlib();
+		_mididrv = new MidiPlayer_Adlib();
 		break;
 	case MD_PCJR:
-		mididrv = new MidiPlayer_PCJr();
+		_mididrv = new MidiPlayer_PCJr();
 		break;
 	case MD_PCSPK:
-		mididrv = new MidiPlayer_PCSpeaker();
+		_mididrv = new MidiPlayer_PCSpeaker();
 		break;
 	default:
 		break;
 	}
 
-	assert(mididrv);
+	assert(_mididrv);
 
-	this->polyphony = mididrv->getPolyphony();
+	_polyphony = _mididrv->getPolyphony();
 
-	tempo = mididrv->getBaseTempo();
+	_tempo = _mididrv->getBaseTempo();
     uint32 time = g_system->getMillis();
-	current_time = Audio::Timestamp(time, 1000000 / tempo);
-	wakeup_time = Audio::Timestamp(time, SFX_TICKS_PER_SEC);
+	_currentTime = Audio::Timestamp(time, 1000000 / _tempo);
+	_wakeupTime = Audio::Timestamp(time, SFX_TICKS_PER_SEC);
 
-	mutex = new Common::Mutex();
+	_mutex = new Common::Mutex();
 
-	mididrv->setTimerCallback(NULL, player_timer_callback);
-	mididrv->open(resmgr);
-	mididrv->setVolume(volume);
+	_mididrv->setTimerCallback(this, player_timer_callback);
+	_mididrv->open(resmgr);
+	_mididrv->setVolume(_volume);
 
 	return Common::kNoError;
 }
 
 Common::Error NewPlayer::add_iterator(SongIterator *it, uint32 start_time) {
-	mutex->lock();
-	SIMSG_SEND(it, SIMSG_SET_PLAYMASK(mididrv->getPlayMask()));
-	SIMSG_SEND(it, SIMSG_SET_RHYTHM(mididrv->hasRhythmChannel()));
+	Common::StackLock lock(*_mutex);
+	SIMSG_SEND(it, SIMSG_SET_PLAYMASK(_mididrv->getPlayMask()));
+	SIMSG_SEND(it, SIMSG_SET_RHYTHM(_mididrv->hasRhythmChannel()));
 
-	if (play_it == NULL) {
+	if (_iterator == NULL) {
 		// Resync with clock
-		current_time = Audio::Timestamp(g_system->getMillis(), 1000000 / tempo);
-		wakeup_time = Audio::Timestamp(start_time, SFX_TICKS_PER_SEC);
+		_currentTime = Audio::Timestamp(g_system->getMillis(), 1000000 / _tempo);
+		_wakeupTime = Audio::Timestamp(start_time, SFX_TICKS_PER_SEC);
 	}
 
-	play_it = sfx_iterator_combine(play_it, it);
-	play_it_done = 0;
-	mutex->unlock();
+	_iterator = sfx_iterator_combine(_iterator, it);
+	_iteratorIsDone = false;
 
 	return Common::kNoError;
 }
 
 Common::Error NewPlayer::stop(void) {
-	debug(3, "Player: Stopping song iterator %p", (void *)play_it);
-	mutex->lock();
-	delete play_it;
-	play_it = NULL;
+	debug(3, "Player: Stopping song iterator %p", (void *)_iterator);
+	Common::StackLock lock(*_mutex);
+	delete _iterator;
+	_iterator = NULL;
 	for (int i = 0; i < MIDI_CHANNELS; i++)
-		static_cast<MidiDriver *>(mididrv)->send(0xb0 + i, SCI_MIDI_CHANNEL_NOTES_OFF, 0);
-	mutex->unlock();
+		static_cast<MidiDriver *>(_mididrv)->send(0xb0 + i, SCI_MIDI_CHANNEL_NOTES_OFF, 0);
 
 	return Common::kNoError;
 }
 
 Common::Error NewPlayer::iterator_message(const SongIterator::Message &msg) {
-	Common::StackLock lock(*mutex);
-	if (!play_it) {
+	Common::StackLock lock(*_mutex);
+	if (!_iterator) {
 		return Common::kUnknownError;
 	}
 
-	songit_handle_message(&play_it, msg);
+	songit_handle_message(&_iterator, msg);
 
 	return Common::kNoError;
 }
 
 Common::Error NewPlayer::pause(void) {
-	Common::StackLock lock(*mutex);
+	Common::StackLock lock(*_mutex);
 
-	play_paused = 1;
-	play_pause_diff = wakeup_time.msecsDiff(current_time);
+	_paused = true;
+	_pauseTimeDiff = _wakeupTime.msecsDiff(_currentTime);
 
-	mididrv->playSwitch(false);
+	_mididrv->playSwitch(false);
 
 	return Common::kNoError;
 }
 
 Common::Error NewPlayer::resume(void) {
-	Common::StackLock lock(*mutex);
+	Common::StackLock lock(*_mutex);
 
-	wakeup_time = Audio::Timestamp(current_time.msecs() + play_pause_diff, SFX_TICKS_PER_SEC);
-	mididrv->playSwitch(true);
-	play_paused = 0;
+	_wakeupTime = Audio::Timestamp(_currentTime.msecs() + _pauseTimeDiff, SFX_TICKS_PER_SEC);
+	_mididrv->playSwitch(true);
+	_paused = false;
 
 	return Common::kNoError;
 }
 
 Common::Error NewPlayer::exit(void) {
-	mididrv->close();
-	delete mididrv;
-	delete mutex;
-	delete play_it;
-	play_it = NULL;
+	_mididrv->close();
+	delete _mididrv;
+	delete _mutex;
+	delete _iterator;
+	_iterator = NULL;
 
 	return Common::kNoError;
 }
 
-NewPlayer::NewPlayer() {
-	name = "new";
-	version = "0.1";
-}
-
 } // End of namespace Sci
diff --git a/engines/sci/sfx/player/new_player.h b/engines/sci/sfx/player/new_player.h
index 63f9888bc2..38150af0da 100644
--- a/engines/sci/sfx/player/new_player.h
+++ b/engines/sci/sfx/player/new_player.h
@@ -30,7 +30,28 @@
 
 namespace Sci {
 
+class MidiPlayer;
+class SongIterator;
+
 class NewPlayer : public SfxPlayer {
+protected:
+	MidiPlayer *_mididrv;
+
+	SongIterator *_iterator;
+	Audio::Timestamp _wakeupTime;
+	Audio::Timestamp _currentTime;
+	uint32 _pauseTimeDiff;
+
+	bool _paused;
+	bool _iteratorIsDone;
+	uint32 _tempo;
+
+	Common::Mutex *_mutex;
+	int _volume;
+
+	void play_song(SongIterator *it);
+	static void player_timer_callback(void *refCon);
+
 public:
 	NewPlayer();