Skip to content

Commit

Permalink
Improve handling of rates when converting to timecode strings (#1477)
Browse files Browse the repository at this point in the history
* Deal with "close" timecode rates via a heuristic, instead of misleading entries in valid_timecode_rates table.
* Renamed "valid_timecode" functions to "smpte_timecode" for clarity (old names are deprecated, but still work).
* Both `is_smpte_timecode_rate` and `nearest_smpte_timecode_rate` now adhere to ST 12-1:2014 - SMPTE Standard - Time and Control Code.

Signed-off-by: Joshua Minor <joshm@pixar.com>
  • Loading branch information
jminor authored Nov 14, 2024
1 parent f40c509 commit 3a8bad4
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 88 deletions.
4 changes: 2 additions & 2 deletions src/opentime/errorStatus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
83 changes: 42 additions & 41 deletions src/opentime/rationalTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,75 +13,68 @@ namespace opentime { namespace OPENTIME_VERSION {

RationalTime RationalTime::_invalid_time{ 0, RationalTime::_invalid_rate };

static constexpr std::array<double, 4> dropframe_timecode_rates{ {
// 23.976,
// 23.98,
// 23.97,
// 24000.0/1001.0,
29.97,
static constexpr std::array<double, 2> 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<double, 11> 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<double, 16> 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<double>::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;
}
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
{
Expand All @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down
13 changes: 10 additions & 3 deletions src/opentime/rationalTime.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 13 additions & 5 deletions src/py-opentimelineio/opentime-bindings/opentime_rationalTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 (*)(double, double)> (&RationalTime::from_seconds), "seconds"_a, "rate"_a)
.def_static("from_seconds", static_cast<RationalTime (*)(double)> (&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<bool> drop_frame) {
return rt.to_timecode(
Expand Down
Loading

0 comments on commit 3a8bad4

Please sign in to comment.