Fix Time::add which was broken when adding times with different rates.

6 files changed, 26 insertions, 15 deletions

diff --git a/src/rational.cc b/src/rational.cc
index 09dadab..47d57ad 100644
--- a/src/rational.cc
+++ b/src/rational.cc
@@ -31,3 +31,13 @@ Rational::integer () const
 {
 	return (numerator % denominator) == 0;
 }
+
+Rational
+sub::max (Rational const & a, Rational const & b)
+{
+	if (a.fraction() > b.fraction()) {
+		return a;
+	}
+
+	return b;
+}
diff --git a/src/rational.h b/src/rational.h
index 7ff71c0..adc40f9 100644
--- a/src/rational.h
+++ b/src/rational.h
@@ -41,5 +41,6 @@ public:
 };
 
 bool operator== (Rational const & a, Rational const & b);
+Rational max (Rational const & a, Rational const & b);
 
 }
diff --git a/src/stl_binary_writer.cc b/src/stl_binary_writer.cc
index 7ae7509..60adda4 100644
--- a/src/stl_binary_writer.cc
+++ b/src/stl_binary_writer.cc
@@ -179,7 +179,7 @@ sub::write_stl_binary (
 			for (list<Block>::const_iterator k = j->blocks.begin(); k != j->blocks.end(); ++k) {
 				t += k->text.size ();
 			}
-			longest = max (longest, t);
+			longest = std::max (longest, t);
 		}
 	}
 
diff --git a/src/sub_assert.h b/src/sub_assert.h
index 67c8abe..4c75d70 100644
--- a/src/sub_assert.h
+++ b/src/sub_assert.h
@@ -19,4 +19,4 @@
 
 #include "exceptions.h"
 
-#define SUB_ASSERT(x) if (!(x)) throw ProgrammingError (__FILE__, __LINE__);
+#define SUB_ASSERT(x) if (!(x)) throw sub::ProgrammingError (__FILE__, __LINE__);
diff --git a/src/sub_time.cc b/src/sub_time.cc
index c4e00ba..adaab31 100644
--- a/src/sub_time.cc
+++ b/src/sub_time.cc
@@ -172,24 +172,15 @@ Time::all_as_seconds () const
 	return _seconds + double(milliseconds ()) / 1000;
 }
 
-/** Add a time to this one.  This time must have an integer _rate
- *  and t must have the same rate.
- */
+/** Add a time to this one.  Both *this and t must have a specified _rate */
 void
 Time::add (Time t)
 {
 	SUB_ASSERT (_rate);
+	SUB_ASSERT (t._rate);
 
-	_seconds += t._seconds;
-	_frames += t._frames;
-
-	SUB_ASSERT (_rate.get().denominator != 0);
-	SUB_ASSERT (_rate.get().integer ());
-
-	if (_frames >= _rate.get().integer_fraction()) {
-		_frames -= _rate.get().integer_fraction();
-		++_seconds;
-	}
+	Rational result_rate = max (*_rate, *t._rate);
+	*this = Time::from_frames((all_as_seconds() + t.all_as_seconds()) * result_rate.fraction(), result_rate);
 }
 
 void
diff --git a/test/time_test.cc b/test/time_test.cc
index a4dae50..3fa9cb5 100644
--- a/test/time_test.cc
+++ b/test/time_test.cc
@@ -20,6 +20,7 @@
 #include "sub_time.h"
 #include "exceptions.h"
 #include <boost/test/unit_test.hpp>
+#include <iostream>
 
 /* Check time construction */
 BOOST_AUTO_TEST_CASE (time_construction_test)
@@ -72,3 +73,11 @@ BOOST_AUTO_TEST_CASE (time_other_test)
 	BOOST_CHECK_THROW (sub::Time::from_hmsf (2, 1, 58, 4).all_as_seconds(), sub::UnknownFrameRateError);
 	BOOST_CHECK_CLOSE (sub::Time::from_hmsf (2, 1, 58, 4, sub::Rational (24, 1)).all_as_seconds(), 7318.1667, 0.001);
 }
+
+/* Check an addition case that gave an odd result */
+BOOST_AUTO_TEST_CASE (time_add_test)
+{
+	sub::Time t = sub::Time::from_hmsf (0, 4, 8, 208, sub::Rational(1000, 1));
+	t.add (sub::Time::from_frames(54641, sub::Rational(24, 1)));
+	BOOST_CHECK_EQUAL (t, sub::Time::from_hmsf (0, 42, 4, 916, sub::Rational(1000, 1)));
+}