Some cleanup / code formatting changes, and added some FIXME comments

svn-id: r25199

6 files changed, 38 insertions, 27 deletions

diff --git a/sound/mods/infogrames.cpp b/sound/mods/infogrames.cpp
index e5eda6e838..a1eeddf22d 100644
--- a/sound/mods/infogrames.cpp
+++ b/sound/mods/infogrames.cpp
@@ -236,8 +236,7 @@ bool Infogrames::load(Common::SeekableReadStream &dum) {
 void Infogrames::unload(void) {
 	stopPlay();
 
-	if (_data)
-		delete[] _data;
+	delete[] _data;
 	_data = 0;
 
 	clearVoices();
@@ -449,9 +448,9 @@ void Infogrames::interrupt() {
 		} else if (_repCount != -1) {
 			_end = true;
 			_playing = false;
-		}
-		else
+		} else {
 			init();
+		}
 	}
 }
 
diff --git a/sound/mods/module.cpp b/sound/mods/module.cpp
index 291f95fe3f..d6f1d3eb4d 100644
--- a/sound/mods/module.cpp
+++ b/sound/mods/module.cpp
@@ -33,6 +33,11 @@ bool Module::load(Common::ReadStream &st) {
 	st.read(songname, 20);
 	songname[20] = '\0';
 
+	// FIXME: We define sample to have 32 entries,
+	// yet we only setup 31 of these -- is this on
+	// purpose, or an off-by-one error? This should
+	// be clarified by either adding a comment explaining
+	// this odditiy, or by fixing the off-by-one-bug.
 	for (int i = 0; i < 31; ++i) {
 		st.read(sample[i].name, 22);
 		sample[i].name[22] = '\0';
diff --git a/sound/mods/module.h b/sound/mods/module.h
index 1e87b7e1ea..f56e42767b 100644
--- a/sound/mods/module.h
+++ b/sound/mods/module.h
@@ -29,6 +29,11 @@
 namespace Modules {
 
 /*
+ * FIXME: Is the following comment still valid? If so, 
+ * it should be marked accordingly, and maybe added to our 
+ * TODO list on the Wiki (conserving memory is always a big
+ * boon for our smaller targets).
+ *
  * Storing notes and patterns like this
  * wastes insane amounts of memory.
  *
diff --git a/sound/mods/paula.cpp b/sound/mods/paula.cpp
index 90d247096c..33a8dd7c2a 100644
--- a/sound/mods/paula.cpp
+++ b/sound/mods/paula.cpp
@@ -45,8 +45,7 @@ Paula::~Paula() {
 }
 
 void Paula::clearVoice(byte voice) {
-	if (voice >= 4)
-		return;
+	assert(voice < NUM_VOICES);
 
 	_voice[voice].data = 0;
 	_voice[voice].dataRepeat = 0;
@@ -68,12 +67,14 @@ int Paula::readBuffer(int16 *buffer, const int numSamples) {
 	int16 *p;
 	int8 *data;
 
-	_mutex.lock();
+// FIXME: Could this code be unified/merged with ProtrackerStream::generateSound?
+// They look very similar. Maybe one could be rewritten to 
+// use the other?
+	
+	Common::StackLock lock(_mutex);
 
 	memset(buffer, 0, numSamples * 2);
-	if (!_playing)
-	{
-		_mutex.unlock();
+	if (!_playing) {
 		return numSamples;
 	}
 
@@ -84,7 +85,7 @@ int Paula::readBuffer(int16 *buffer, const int numSamples) {
 			_curInt = 0;
 		}
 		nSamples = MIN(samples, _intFreq - _curInt);
-		for (voice = 0; voice < 4; voice++) {
+		for (voice = 0; voice < NUM_VOICES; voice++) {
 			if (!_voice[voice].data || (_voice[voice].period <= 0))
 				continue;
 
@@ -160,7 +161,6 @@ int Paula::readBuffer(int16 *buffer, const int numSamples) {
 		_curInt += nSamples;
 		samples -= nSamples;
 	}
-	_mutex.unlock();
 	return numSamples;
 }
 
diff --git a/sound/mods/paula.h b/sound/mods/paula.h
index be85122635..aebc5a9bbb 100644
--- a/sound/mods/paula.h
+++ b/sound/mods/paula.h
@@ -35,18 +35,19 @@ namespace Audio {
  */
 class Paula : public AudioStream {
 public:
+	static const int NUM_VOICES = 4;
+
 	Paula(bool stereo = false, int rate = 44100, int interruptFreq = 0);
 	~Paula();
 	
 	bool playing() const { return _playing; }
 	void setInterruptFreq(int freq) { _intFreq = freq; }
-	void setPanning(byte voice, byte panning)
-	{
-		if (voice < 4)
-			_voice[voice].panning = panning;
+	void setPanning(byte voice, byte panning) {
+		assert(voice < NUM_VOICES);
+		_voice[voice].panning = panning;
 	}
 	void clearVoice(byte voice);
-	void clearVoices() { int i; for (i = 0; i < 4; i++) clearVoice(i); }
+	void clearVoices() { for (int i = 0; i < NUM_VOICES; ++i) clearVoice(i); }
 	virtual void startPlay(void) {}
 	virtual void stopPlay(void) {}
 	virtual void pausePlay(bool pause) {}
@@ -67,7 +68,7 @@ protected:
 		byte volume;
 		double offset;
 		byte panning; // For stereo mixing: 0 = far left, 255 = far right
-	} _voice[4];
+	} _voice[NUM_VOICES];
 
 	int _rate;
 	int _intFreq;
@@ -78,13 +79,12 @@ protected:
 	Common::Mutex _mutex;
 
 	void mix(int16 *&buf, int8 data, int voice) {
+		const int32 tmp = ((int32) data) * _voice[voice].volume;
 		if (_stereo) {
-			*buf++ += (((int32) data) * _voice[voice].volume *
-				 (255 - _voice[voice].panning)) >> 7;
-			*buf++ += (((int32) data) * _voice[voice].volume *
-				 (_voice[voice].panning)) >> 7;
+			*buf++ += (tmp * (255 - _voice[voice].panning)) >> 7;
+			*buf++ += (tmp * (_voice[voice].panning)) >> 7;
 		} else
-			*buf++ += _voice[voice].volume * data;
+			*buf++ += tmp;
 	}
 	virtual void interrupt(void) {};
 };
diff --git a/sound/mods/protracker.cpp b/sound/mods/protracker.cpp
index ab3776624b..83fb1790cf 100644
--- a/sound/mods/protracker.cpp
+++ b/sound/mods/protracker.cpp
@@ -179,9 +179,12 @@ void ProtrackerStream::generateSound() {
 
 	_buf->ensureCapacity(samples);
 
+// FIXME: Could this code be unified/ with Paula::readBuffer?
+// They look very similar. Maybe one could be rewritten to 
+// use the other?
+
 	int16 *p = _buf->getEnd();
-	for (int i = 0; i < samples; i++)
-		p[i] = 0;
+	memset(p, 0, samples * sizeof(int16));
 
 	for (int track = 0; track < 4; track++) {
 		if (_track[track].sample > 0) {
@@ -234,8 +237,7 @@ void ProtrackerStream::generateSound() {
 				}
 			} else {
 				if (offset < slen) {
-					if ((int)(offset + samples * rate) >=
-					    slen) {
+					if ((int)(offset + samples * rate) >= slen) {
 						/* The end of the sample is the limiting factor */
 
 						int end = (int)((slen - offset) / rate);