diff --git a/src/opentime/errorStatus.cpp b/src/opentime/errorStatus.cpp index 13cd16db2..de4dbe009 100644 --- a/src/opentime/errorStatus.cpp +++ b/src/opentime/errorStatus.cpp @@ -13,9 +13,9 @@ ErrorStatus::outcome_to_string(Outcome o) case OK: return std::string(); case INVALID_TIMECODE_RATE: - return "invalid timecode rate"; + return "SMPTE timecode does not support this rate"; case INVALID_TIMECODE_STRING: - return "string is not a valid timecode string"; + return "string is not a SMPTE timecode string"; case TIMECODE_RATE_MISMATCH: return "timecode specifies a frame higher than its rate"; case INVALID_TIME_STRING: diff --git a/src/opentime/rationalTime.cpp b/src/opentime/rationalTime.cpp index f4f2b9d18..0082e9139 100644 --- a/src/opentime/rationalTime.cpp +++ b/src/opentime/rationalTime.cpp @@ -13,75 +13,68 @@ namespace opentime { namespace OPENTIME_VERSION { RationalTime RationalTime::_invalid_time{ 0, RationalTime::_invalid_rate }; -static constexpr std::array dropframe_timecode_rates{ { - // 23.976, - // 23.98, - // 23.97, - // 24000.0/1001.0, - 29.97, +static constexpr std::array dropframe_timecode_rates{ { 30000.0 / 1001.0, - 59.94, 60000.0 / 1001.0, } }; +// See the official source of these numbers here: +// ST 12-1:2014 - SMPTE Standard - Time and Control Code +// https://ieeexplore.ieee.org/document/7291029 +// static constexpr std::array smpte_timecode_rates{ - { 1.0, - 12.0, - 24000.0 / 1001.0, + { 24000.0 / 1001.0, 24.0, 25.0, 30000.0 / 1001.0, 30.0, + 48000.0 / 1001.0, 48.0, 50.0, 60000.0 / 1001.0, - 60.0 } -}; - -static constexpr std::array valid_timecode_rates{ - { 1.0, - 12.0, - 23.97, - 23.976, - 23.98, - 24000.0 / 1001.0, - 24.0, - 25.0, - 29.97, - 30000.0 / 1001.0, - 30.0, - 48.0, - 50.0, - 59.94, - 60000.0 / 1001.0, - 60.0 } + 60.0 + } }; +// deprecated in favor of `is_smpte_timecode_rate` bool RationalTime::is_valid_timecode_rate(double fps) { - auto b = valid_timecode_rates.begin(), e = valid_timecode_rates.end(); + return is_smpte_timecode_rate(fps); +} + +bool +RationalTime::is_smpte_timecode_rate(double fps) +{ + auto b = smpte_timecode_rates.begin(), e = smpte_timecode_rates.end(); return std::find(b, e, fps) != e; } +// deprecated in favor of `is_smpte_timecode_rate` double RationalTime::nearest_valid_timecode_rate(double rate) +{ + return nearest_smpte_timecode_rate(rate); +} + +double +RationalTime::nearest_smpte_timecode_rate(double rate) { double nearest_rate = 0; double min_diff = std::numeric_limits::max(); - for (auto valid_rate: smpte_timecode_rates) + for (auto smpte_rate: smpte_timecode_rates) { - if (valid_rate == rate) + if (smpte_rate == rate) { return rate; } - auto diff = std::abs(rate - valid_rate); + auto diff = std::abs(rate - smpte_rate); if (diff >= min_diff) { continue; } min_diff = diff; - nearest_rate = valid_rate; + nearest_rate = smpte_rate; } return nearest_rate; } @@ -200,7 +193,7 @@ RationalTime::from_timecode( double rate, ErrorStatus* error_status) { - if (!RationalTime::is_valid_timecode_rate(rate)) + if (!RationalTime::is_smpte_timecode_rate(rate)) { if (error_status) { @@ -331,7 +324,7 @@ RationalTime::from_time_string( double rate, ErrorStatus* error_status) { - if (!RationalTime::is_valid_timecode_rate(rate)) + if (!RationalTime::is_smpte_timecode_rate(rate)) { set_error( time_string, @@ -460,7 +453,12 @@ RationalTime::to_timecode( return std::string(); } - if (!is_valid_timecode_rate(rate)) + // It is common practice to use truncated or rounded values + // like 29.97 instead of exact SMPTE rates like 30000/1001 + // so as a convenience we will snap the rate to the nearest + // SMPTE rate if it is close enough. + double nearest_smpte_rate = nearest_smpte_timecode_rate(rate); + if (abs(nearest_smpte_rate - rate) > 0.1) { if (error_status) { @@ -469,6 +467,9 @@ RationalTime::to_timecode( return std::string(); } + // Let's assume this is the rate instead of the given rate. + rate = nearest_smpte_rate; + bool rate_is_dropframe = is_dropframe_rate(rate); if (drop_frame == IsDropFrameRate::ForceYes and not rate_is_dropframe) { @@ -504,11 +505,11 @@ RationalTime::to_timecode( } else { - if ((rate == 29.97) or (rate == 30000 / 1001.0)) + if (rate == 30000 / 1001.0) { dropframes = 2; } - else if (rate == 59.94) + else if (rate == 60000 / 1001.0) { dropframes = 4; } @@ -582,7 +583,7 @@ RationalTime::to_nearest_timecode( { *error_status = ErrorStatus(); - double nearest_rate = nearest_valid_timecode_rate(rate); + double nearest_rate = nearest_smpte_timecode_rate(rate); return to_timecode(nearest_rate, drop_frame, error_status); } diff --git a/src/opentime/rationalTime.h b/src/opentime/rationalTime.h index 12dacbc3a..00cce4bfc 100644 --- a/src/opentime/rationalTime.h +++ b/src/opentime/rationalTime.h @@ -171,13 +171,20 @@ class RationalTime start_time._rate }; } - /// @brief Returns true if the rate is valid for use with timecode. + /// @brief Returns true is the rate is supported by SMPTE timecode. + [[deprecated("Use is_smpte_timecode_rate() instead")]] static bool is_valid_timecode_rate(double rate); - /// @brief Returns the first valid timecode rate that has the least - /// difference from rate. + /// @brief Returns true is the rate is supported by SMPTE timecode. + static bool is_smpte_timecode_rate(double rate); + + /// @brief Returns the SMPTE timecode rate nearest to the given rate. + [[deprecated("Use nearest_smpte_timecode_rate() instead")]] static double nearest_valid_timecode_rate(double rate); + /// @brief Returns the SMPTE timecode rate nearest to the given rate. + static double nearest_smpte_timecode_rate(double rate); + /// @brief Convert a frame number and rate into a time. static constexpr RationalTime from_frames(double frame, double rate) noexcept diff --git a/src/py-opentimelineio/opentime-bindings/opentime_rationalTime.cpp b/src/py-opentimelineio/opentime-bindings/opentime_rationalTime.cpp index 3537a8485..f0b80d810 100644 --- a/src/py-opentimelineio/opentime-bindings/opentime_rationalTime.cpp +++ b/src/py-opentimelineio/opentime-bindings/opentime_rationalTime.cpp @@ -102,14 +102,22 @@ Compute the duration of samples from first to last (including last). This is not For example, the duration of a clip from frame 10 to frame 15 is 6 frames. Result will be in the rate of start_time. )docstring") - .def_static("is_valid_timecode_rate", &RationalTime::is_valid_timecode_rate, "rate"_a, "Returns true if the rate is valid for use with timecode.") + .def_static("is_valid_timecode_rate", &RationalTime::is_valid_timecode_rate, "rate"_a, + "Deprecated. Please use `is_smpte_timecode_rate` instead. This function will be removed in a future release.") + .def_static("is_smpte_timecode_rate", &RationalTime::is_smpte_timecode_rate, "rate"_a, + "Returns true if the rate is valid for use with SMPTE timecode.") .def_static("nearest_valid_timecode_rate", &RationalTime::nearest_valid_timecode_rate, "rate"_a, - "Returns the first valid timecode rate that has the least difference from the given value.") - .def_static("from_frames", &RationalTime::from_frames, "frame"_a, "rate"_a, "Turn a frame number and rate into a :class:`~RationalTime` object.") + "Deprecated. Please use `nearest_smpte_timecode_rate` instead. This function will be removed in a future release.") + .def_static("nearest_smpte_timecode_rate", &RationalTime::nearest_smpte_timecode_rate, "rate"_a, + "Returns the first SMPTE timecode rate that has the least difference from the given value.") + .def_static("from_frames", &RationalTime::from_frames, "frame"_a, "rate"_a, + "Turn a frame number and rate into a :class:`~RationalTime` object.") .def_static("from_seconds", static_cast (&RationalTime::from_seconds), "seconds"_a, "rate"_a) .def_static("from_seconds", static_cast (&RationalTime::from_seconds), "seconds"_a) - .def("to_frames", (int (RationalTime::*)() const) &RationalTime::to_frames, "Returns the frame number based on the current rate.") - .def("to_frames", (int (RationalTime::*)(double) const) &RationalTime::to_frames, "rate"_a, "Returns the frame number based on the given rate.") + .def("to_frames", (int (RationalTime::*)() const) &RationalTime::to_frames, + "Returns the frame number based on the current rate.") + .def("to_frames", (int (RationalTime::*)(double) const) &RationalTime::to_frames, "rate"_a, + "Returns the frame number based on the given rate.") .def("to_seconds", &RationalTime::to_seconds) .def("to_timecode", [](RationalTime rt, double rate, std::optional drop_frame) { return rt.to_timecode( diff --git a/tests/test_opentime.py b/tests/test_opentime.py index 81f03bd7f..74143e29f 100755 --- a/tests/test_opentime.py +++ b/tests/test_opentime.py @@ -200,29 +200,30 @@ def test_long_running_timecode_24(self): def test_timecode_23976_fps(self): # This should behave exactly like 24 fps + ntsc_23976 = 24000 / 1001.0 timecode = "00:00:01:00" - t = otio.opentime.RationalTime(value=24, rate=23.976) - self.assertEqual(t, otio.opentime.from_timecode(timecode, 23.976)) + t = otio.opentime.RationalTime(value=24, rate=ntsc_23976) + self.assertEqual(t, otio.opentime.from_timecode(timecode, ntsc_23976)) timecode = "00:01:00:00" - t = otio.opentime.RationalTime(value=24 * 60, rate=23.976) - self.assertEqual(t, otio.opentime.from_timecode(timecode, 23.976)) + t = otio.opentime.RationalTime(value=24 * 60, rate=ntsc_23976) + self.assertEqual(t, otio.opentime.from_timecode(timecode, ntsc_23976)) timecode = "01:00:00:00" - t = otio.opentime.RationalTime(value=24 * 60 * 60, rate=23.976) - self.assertEqual(t, otio.opentime.from_timecode(timecode, 23.976)) + t = otio.opentime.RationalTime(value=24 * 60 * 60, rate=ntsc_23976) + self.assertEqual(t, otio.opentime.from_timecode(timecode, ntsc_23976)) timecode = "24:00:00:00" - t = otio.opentime.RationalTime(value=24 * 60 * 60 * 24, rate=23.976) - self.assertEqual(t, otio.opentime.from_timecode(timecode, 23.976)) + t = otio.opentime.RationalTime(value=24 * 60 * 60 * 24, rate=ntsc_23976) + self.assertEqual(t, otio.opentime.from_timecode(timecode, ntsc_23976)) timecode = "23:59:59:23" t = otio.opentime.RationalTime( value=24 * 60 * 60 * 24 - 1, - rate=(24000 / 1001.0) + rate=ntsc_23976 ) self.assertEqual( - t, otio.opentime.from_timecode(timecode, (24000 / 1001.0)) + t, otio.opentime.from_timecode(timecode, ntsc_23976) ) def test_converting_negative_values_to_timecode(self): @@ -318,15 +319,17 @@ def test_dropframe_timecode_2997fps(self): ] } + ntsc_2997 = otio.opentime.RationalTime.nearest_smpte_timecode_rate(29.97) + self.assertEqual(ntsc_2997, 30000 / 1001.0) for time_key, time_values in test_values.items(): for value, tc in time_values: - t = otio.opentime.RationalTime(value, 29.97) + t = otio.opentime.RationalTime(value, ntsc_2997) self.assertEqual( tc, otio.opentime.to_timecode( - t, rate=29.97, drop_frame=True + t, rate=ntsc_2997, drop_frame=True ) ) - t1 = otio.opentime.from_timecode(tc, rate=29.97) + t1 = otio.opentime.from_timecode(tc, rate=ntsc_2997) self.assertEqual(t, t1) def test_timecode_ntsc_2997fps(self): @@ -351,12 +354,16 @@ def test_timecode_ntsc_2997fps(self): ) def test_timecode_infer_drop_frame(self): + # These rates are all non-integer SMPTE rates. + # When `to_timecode` is called without specifying + # a value for `drop_frame`, it will infer that these + # should be displayed as drop-frame timecode. frames = 1084319 rates = [ (29.97, '10:03:00;05'), (30000.0 / 1001.0, '10:03:00;05'), (59.94, '05:01:30;03'), - (60000.0 / 1001.0, '05:01:11;59') + (60000.0 / 1001.0, '05:01:30;03') ] for rate, timecode in rates: t = otio.opentime.RationalTime(frames, rate) @@ -373,12 +380,12 @@ def test_timecode_2997(self): (17983, '00:09:59:13', '00:10:00;01'), (17984, '00:09:59:14', '00:10:00;02'), ] - + ntsc_2997 = 30000 / 1001.0 for value, tc, dftc in ref_values: - t = otio.opentime.RationalTime(value, 29.97) - to_dftc = otio.opentime.to_timecode(t, rate=29.97, drop_frame=True) - to_tc = otio.opentime.to_timecode(t, rate=29.97, drop_frame=False) - to_auto_tc = otio.opentime.to_timecode(t, rate=29.97) + t = otio.opentime.RationalTime(value, ntsc_2997) + to_dftc = otio.opentime.to_timecode(t, rate=ntsc_2997, drop_frame=True) + to_tc = otio.opentime.to_timecode(t, rate=ntsc_2997, drop_frame=False) + to_auto_tc = otio.opentime.to_timecode(t, rate=ntsc_2997) # 29.97 should auto-detect dftc for backward compatability self.assertEqual(to_dftc, to_auto_tc) @@ -388,10 +395,10 @@ def test_timecode_2997(self): self.assertEqual(tc, to_tc) # Check they convert back - t1 = otio.opentime.from_timecode(to_dftc, rate=29.97) + t1 = otio.opentime.from_timecode(to_dftc, rate=ntsc_2997) self.assertEqual(t1, t) - t2 = otio.opentime.from_timecode(to_tc, rate=29.97) + t2 = otio.opentime.from_timecode(to_tc, rate=ntsc_2997) self.assertEqual(t2, t) def test_faulty_formatted_timecode_24(self): @@ -411,11 +418,16 @@ def test_faulty_formatted_timecode_24(self): with self.assertRaises(ValueError): otio.opentime.from_timecode('01:00:13;23', 24) + def test_faulty_time_string(self): + with self.assertRaises(ValueError): + otio.opentime.from_time_string("bogus", 24) + def test_invalid_rate_to_timecode_functions(self): - t = otio.opentime.RationalTime(100, 29.98) + # Use a bogus rate, expecting `to_timecode` to complain + t = otio.opentime.RationalTime(100, 999) with self.assertRaises(ValueError): - otio.opentime.to_timecode(t, 29.98) + otio.opentime.to_timecode(t, 777) with self.assertRaises(ValueError): otio.opentime.to_timecode(t) @@ -691,46 +703,53 @@ def test_passing_ndf_tc_at_df_rate(self): DF_TC = "01:00:02;05" NDF_TC = "00:59:58:17" frames = 107957 + ntsc_2997 = otio.opentime.RationalTime.nearest_smpte_timecode_rate(29.97) + self.assertEqual(ntsc_2997, 30000 / 1001.0) tc1 = otio.opentime.to_timecode( - otio.opentime.RationalTime(frames, 29.97) + otio.opentime.RationalTime(frames, ntsc_2997) ) self.assertEqual(tc1, DF_TC) tc2 = otio.opentime.to_timecode( - otio.opentime.RationalTime(frames, 29.97), - 29.97, + otio.opentime.RationalTime(frames, ntsc_2997), + ntsc_2997, drop_frame=False ) self.assertEqual(tc2, NDF_TC) - t1 = otio.opentime.from_timecode(DF_TC, 29.97) + t1 = otio.opentime.from_timecode(DF_TC, ntsc_2997) self.assertEqual(t1.value, frames) - t2 = otio.opentime.from_timecode(NDF_TC, 29.97) + t2 = otio.opentime.from_timecode(NDF_TC, ntsc_2997) self.assertEqual(t2.value, frames) - def test_nearest_valid_timecode_rate(self): - invalid_valid_rates = ( + def test_nearest_smpte_timecode_rate(self): + rate_pairs = ( (23.97602397602397, 24000.0 / 1001.0), (23.97, 24000.0 / 1001.0), (23.976, 24000.0 / 1001.0), (23.98, 24000.0 / 1001.0), (29.97, 30000.0 / 1001.0), (59.94, 60000.0 / 1001.0), + (24.0, 24.0), + (23.999999, 24.0), + (29.999999, 30.0), + (30.01, 30.0), + (60.01, 60.0) ) - for invalid_rate, nearest_valid_rate in invalid_valid_rates: + for wonky_rate, smpte_rate in rate_pairs: self.assertTrue( - otio.opentime.RationalTime.is_valid_timecode_rate( - nearest_valid_rate + otio.opentime.RationalTime.is_smpte_timecode_rate( + smpte_rate ) ) self.assertEqual( - otio.opentime.RationalTime.nearest_valid_timecode_rate( - invalid_rate + otio.opentime.RationalTime.nearest_smpte_timecode_rate( + wonky_rate ), - nearest_valid_rate, + smpte_rate, ) @@ -1334,7 +1353,7 @@ def test_to_timecode_mixed_rates(self): t = otio.opentime.from_timecode(timecode, 24) self.assertEqual(timecode, otio.opentime.to_timecode(t)) self.assertEqual(timecode, otio.opentime.to_timecode(t, 24)) - self.assertNotEqual(timecode, otio.opentime.to_timecode(t, 12)) + self.assertNotEqual(timecode, otio.opentime.to_timecode(t, 48)) time1 = otio.opentime.RationalTime(24.0, 24.0) time2 = otio.opentime.RationalTime(1.0, 1.0)