Fix Timestamp::addMsecs; some cleanup

svn-id: r47212

3 files changed, 56 insertions, 13 deletions

diff --git a/sound/timestamp.cpp b/sound/timestamp.cpp
index 6cf0c8fa9f..6157d87d32 100644
--- a/sound/timestamp.cpp
+++ b/sound/timestamp.cpp
@@ -43,7 +43,8 @@ Timestamp::Timestamp(uint32 ms, int fr) {
 	_framerateFactor = 1000 / gcd(1000, fr);
 	_framerate = fr * _framerateFactor;
 
-	_numberOfFrames = (ms % 1000) * _framerate / 1000;
+	// Note that _framerate is always divisible by 1000.
+	_numberOfFrames = (ms % 1000) * (_framerate / 1000);
 }
 
 Timestamp::Timestamp(uint s, int frames, int fr) {
@@ -53,7 +54,7 @@ Timestamp::Timestamp(uint s, int frames, int fr) {
 	_framerateFactor = 1000 / gcd(1000, fr);
 	_framerate = fr * _framerateFactor;
 	_numberOfFrames = 0;
-	*this = addFrames(frames);
+	addFramesIntern(frames * _framerateFactor);
 }
 
 Timestamp Timestamp::convertToFramerate(int newFramerate) const {
@@ -123,26 +124,33 @@ Timestamp Timestamp::addFrames(int frames) const {
 
 	// The frames are given in the original framerate, so we have to
 	// adjust by _framerateFactor accordingly.
-	ts._numberOfFrames += frames * _framerateFactor;
+	ts.addFramesIntern(frames * _framerateFactor);
 
-	if (ts._numberOfFrames < 0) {
-		int secsub = 1 + (-ts._numberOfFrames / ts._framerate);
+	return ts;
+}
 
-		ts._numberOfFrames += ts._framerate * secsub;
-		ts._secs -= secsub;
-	}
+void Timestamp::addFramesIntern(int frames) {
+	_numberOfFrames += frames;
 
-	ts._secs += (ts._numberOfFrames / ts._framerate);
-	ts._numberOfFrames %= ts._framerate;
+	if (_numberOfFrames < 0) {
+		int secsub = 1 + (-_numberOfFrames / _framerate);
 
-	return ts;
+		_numberOfFrames += _framerate * secsub;
+		_secs -= secsub;
+	}
+
+	// Wrap around if necessary
+	_secs += (_numberOfFrames / _framerate);
+	_numberOfFrames %= _framerate;
 }
 
 Timestamp Timestamp::addMsecs(int ms) const {
+	assert(ms >= 0);
 	Timestamp ts(*this);
 	ts._secs += ms / 1000;
 	// Add the remaining frames. Note that _framerate is always divisible by 1000.
-	return ts.addFrames((ms % 1000) * (_framerate / 1000));
+	ts.addFramesIntern((ms % 1000) * (ts._framerate / 1000));
+	return ts;
 }
 
 int Timestamp::frameDiff(const Timestamp &ts) const {
@@ -174,6 +182,7 @@ int Timestamp::msecsDiff(const Timestamp &ts) const {
 }
 
 uint32 Timestamp::msecs() const {
+	// Note that _framerate is always divisible by 1000.
 	return _secs * 1000 + _numberOfFrames / (_framerate / 1000);
 }
 
diff --git a/sound/timestamp.h b/sound/timestamp.h
index 699723e79b..7c3b81ca32 100644
--- a/sound/timestamp.h
+++ b/sound/timestamp.h
@@ -164,6 +164,8 @@ public:
 protected:
 
 	int cmp(const Timestamp &ts) const;
+
+	void addFramesIntern(int frames);
 };
 
 
diff --git a/test/sound/timestamp.h b/test/sound/timestamp.h
index 213c49ad7f..c5bd24c951 100644
--- a/test/sound/timestamp.h
+++ b/test/sound/timestamp.h
@@ -5,7 +5,7 @@
 class TimestampTestSuite : public CxxTest::TestSuite
 {
 	public:
-	void test_diff_add() {
+	void test_diff_add_frames() {
 		const Audio::Timestamp a(10000, 1000);
 		const Audio::Timestamp b(10001, 1000);
 		const Audio::Timestamp c(10002, 1000);
@@ -23,6 +23,38 @@ class TimestampTestSuite : public CxxTest::TestSuite
 		TS_ASSERT_EQUALS(c.frameDiff(a), 2);
 	}
 
+	void test_diff_add_msecs() {
+		Audio::Timestamp ts0(3, 22050);
+		Audio::Timestamp ts1(0, 22050);
+		Audio::Timestamp ts2(0, 22050);
+
+		TS_ASSERT_EQUALS(ts0.msecs(), 3);
+		TS_ASSERT_EQUALS(ts0.totalNumberOfFrames(), 3 * 22050 / 1000);
+		TS_ASSERT_EQUALS(ts0.numberOfFrames(), 3 * 22050 / 1000);
+
+		ts1 = ts1.addFrames(53248);
+		TS_ASSERT_EQUALS(ts1.secs(), 2);
+		TS_ASSERT_EQUALS(ts1.msecs(), 53248 * 1000 / 22050);
+		TS_ASSERT_EQUALS(ts1.totalNumberOfFrames(), 53248);
+		TS_ASSERT_EQUALS(ts1.numberOfFrames(), 53248 - 2 * 22050);
+		ts1 = ts1.addMsecs(47);
+		TS_ASSERT_EQUALS(ts1.secs(), 2);
+		TS_ASSERT_EQUALS(ts1.msecs(), 2414+47);
+		TS_ASSERT_EQUALS(ts1.totalNumberOfFrames(), 47*22050 / 1000 + 53248);
+		TS_ASSERT_EQUALS(ts1.numberOfFrames(), 47*22050 / 1000 + 53248 - 2 * 22050);
+
+		ts2 = ts2.addMsecs(47);
+		TS_ASSERT_EQUALS(ts2.secs(), 0);
+		TS_ASSERT_EQUALS(ts2.msecs(), 47);
+		TS_ASSERT_EQUALS(ts2.totalNumberOfFrames(), 47*22050 / 1000);
+		TS_ASSERT_EQUALS(ts2.numberOfFrames(), 47*22050 / 1000);
+		ts2 = ts2.addFrames(53248);
+		TS_ASSERT_EQUALS(ts2.secs(), 2);
+		TS_ASSERT_EQUALS(ts2.msecs(), 2414+47);
+		TS_ASSERT_EQUALS(ts2.totalNumberOfFrames(), 47*22050 / 1000 + 53248);
+		TS_ASSERT_EQUALS(ts2.numberOfFrames(), 47*22050 / 1000 + 53248 - 2 * 22050);
+	}
+
 	void test_ticks() {
 		const Audio::Timestamp a(1234, 60);
 		const Audio::Timestamp b(5678, 60);