Skip to content

Commit

Permalink
Fix parsing of time_strings lacking leading zeroes (#1297)
Browse files Browse the repository at this point in the history 
Fixes #1293

fixes parsing of time strings without leading zeroes.
enforces that a negative sign can only appear in the left most position
implementation does not allocate memory or copy strings
implementation does not allow exponential notation and other things that std does allow but are inappropriate for time strings
compatible with strings produced by ffprobe
associated tests
adds C based tests corresponding to the existing Python based tests.
  • Loading branch information
@meshula
meshula authored Sep 15, 2022
1 parent 64b0e31 commit 78941dd
Show file tree
Hide file tree
Showing 4 changed files with 279 additions and 38 deletions.
221 changes: 186 additions & 35 deletions src/opentime/rationalTime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,103 @@ is_dropframe_rate(double rate)
return std::find(b, e, rate) != e;
}

static bool
parseFloat(char const* pCurr, char const* pEnd, bool allow_negative, double* result)
{
if (pCurr >= pEnd || !pCurr)
{
*result = 0.0;
return false;
}

double ret = 0.0;
double sign = 1.0;

if (*pCurr == '+')
{
++pCurr;
}
else if (*pCurr == '-')
{
if (!allow_negative)
{
*result = 0.0;
return false;
}
sign = -1.0;
++pCurr;
}

// get integer part
//
// Note that uint64_t is used because overflow is well defined for
// unsigned integers, but it is undefined behavior for signed integers,
// and floating point values are couched in the specification with
// the caveat that an implementation may be IEEE-754 compliant, or only
// partially compliant.
//
uint64_t uintPart = 0;
while (pCurr < pEnd)
{
char c = *pCurr;
if (c < '0' || c > '9')
{
break;
}
uint64_t accumulated = uintPart * 10 + c - '0';
if (accumulated < uintPart)
{
// if there are too many digits, resulting in an overflow, fail
*result = 0.0;
return false;
}
uintPart = accumulated;
++pCurr;
}

ret = static_cast<double>(uintPart);
if (uintPart != static_cast<uint64_t>(ret))
{
// if the double cannot be casted precisely back to uint64_t, fail
// A double has 15 digits of precision, but a uint64_t can encode more.
*result = 0.0;
return false;
}

// check for end of string or delimiter
if (pCurr == pEnd || *pCurr == '\0')
{
*result = sign * ret;
return true;
}

// if the next character is not a decimal point, the string is malformed.
if (*pCurr != '.')
{
*result = 0.0; // zero consistent with earlier error condition
return false;
}

++pCurr; // skip decimal

double position_scale = 0.1;
while (pCurr < pEnd)
{
char c = *pCurr;
if (c < '0' || c > '9')
{
break;
}
ret = ret + static_cast<double>(c - '0') * position_scale;
++pCurr;
position_scale *= 0.1;
}

*result = sign * ret;
return true;
}


RationalTime
RationalTime::from_timecode(
std::string const& timecode, double rate, ErrorStatus* error_status)
Expand Down Expand Up @@ -205,55 +302,109 @@ RationalTime::from_timecode(
return RationalTime{ double(value), rate };
}

static void
set_error(std::string const& time_string,
ErrorStatus::Outcome code,
ErrorStatus* err)
{
if (err) {
*err = ErrorStatus(
code,
string_printf(
"Error: '%s' - %s",
time_string.c_str(),
ErrorStatus::outcome_to_string(code).c_str()));
}
}

RationalTime
RationalTime::from_time_string(
std::string const& time_string, double rate, ErrorStatus* error_status)
{
if (!RationalTime::is_valid_timecode_rate(rate))
if (!RationalTime::is_valid_timecode_rate(rate))
{
if (error_status)
{
*error_status = ErrorStatus(ErrorStatus::INVALID_TIMECODE_RATE);
}
set_error(time_string, ErrorStatus::INVALID_TIMECODE_RATE, error_status);
return RationalTime::_invalid_time;
}

std::vector<std::string> fields(3, std::string());

// split the fields
int last_pos = 0;

for (int i = 0; i < 2; i++)
const char* start = time_string.data();
const char* end = start + time_string.length();
char* current = const_cast<char*>(end);
char* parse_end = current;
char* prev_parse_end = current;

double power[3] = {
1.0, // seconds
60.0, // minutes
3600.0 // hours
};

double accumulator = 0.0;
int radix = 0;
while (start <= current)
{
fields[i] = time_string.substr(last_pos, 2);
last_pos = last_pos + 3;
}

fields[2] = time_string.substr(last_pos, time_string.length());

double hours, minutes, seconds;
if (*current == ':')
{
parse_end = current + 1;
char c = *parse_end;
if (c != '\0' && c != ':')
{
if (c< '0' || c > '9')
{
set_error(time_string, ErrorStatus::INVALID_TIME_STRING, error_status);
return RationalTime::_invalid_time;
}
double val = 0.0;
if (!parseFloat(parse_end, prev_parse_end + 1, false, &val))
{
set_error(time_string, ErrorStatus::INVALID_TIME_STRING, error_status);
return RationalTime::_invalid_time;
}
prev_parse_end = nullptr;
if (radix < 2 && val >= 60.0)
{
set_error(time_string, ErrorStatus::INVALID_TIME_STRING, error_status);
return RationalTime::_invalid_time;
}
accumulator += val * power[radix];
}
++radix;
if (radix == sizeof(power) / sizeof(power[0]))
{
set_error(time_string, ErrorStatus::INVALID_TIME_STRING, error_status);
return RationalTime::_invalid_time;
}
}
else if (current < prev_parse_end &&
(*current < '0' || *current > '9') &&
*current != '.')
{
set_error(time_string, ErrorStatus::INVALID_TIME_STRING, error_status);
return RationalTime::_invalid_time;
}

try
{
hours = std::stod(fields[0]);
minutes = std::stod(fields[1]);
seconds = std::stod(fields[2]);
}
catch (std::exception const& e)
{
if (error_status)
if (start == current)
{
*error_status = ErrorStatus(
ErrorStatus::INVALID_TIME_STRING,
string_printf(
"Input time string '%s' is an invalid time string",
time_string.c_str()));
if (prev_parse_end)
{
double val = 0.0;
if (!parseFloat(start, prev_parse_end + 1, true, &val))
{
set_error(time_string, ErrorStatus::INVALID_TIME_STRING, error_status);
return RationalTime::_invalid_time;
}
accumulator += val * power[radix];
}
break;
}
--current;
if (!prev_parse_end)
{
prev_parse_end = current;
}
return RationalTime::_invalid_time;
}

return from_seconds(seconds + minutes * 60 + hours * 60 * 60)
.rescaled_to(rate);
return from_seconds(accumulator).rescaled_to(rate);
}

std::string
Expand Down
13 changes: 11 additions & 2 deletions src/opentime/rationalTime.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ class RationalTime
std::string const& timecode,
double rate,
ErrorStatus* error_status = nullptr);

// parse a string in the form
// hours:minutes:seconds
// which may have a leading negative sign. seconds may have up to
// microsecond precision.
static RationalTime from_time_string(
std::string const& time_string,
double rate,
Expand Down Expand Up @@ -154,9 +159,13 @@ class RationalTime
return to_timecode(_rate, IsDropFrameRate::InferFromRate, error_status);
}

// produce a string in the form
// hours:minutes:seconds
// which may have a leading negative sign. seconds may have up to
// microsecond precision.
std::string to_time_string() const;

RationalTime const& operator+=(RationalTime other) noexcept
RationalTime const& operator+=(RationalTime other) noexcept
{
if (_rate < other._rate)
{
Expand All @@ -170,7 +179,7 @@ class RationalTime
return *this;
}

RationalTime const& operator-=(RationalTime other) noexcept
RationalTime const& operator-=(RationalTime other) noexcept
{
if (_rate < other._rate)
{
Expand Down
69 changes: 69 additions & 0 deletions tests/test_opentime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,75 @@ main(int argc, char** argv)
assertFalse(t1 != t3);
});

tests.add_test("test_from_time_string", [] {
std::string time_string = "0:12:04";
auto t = otime::RationalTime(24 * (12 * 60 + 4), 24);
auto time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
});

tests.add_test("test_from_time_string24", [] {
std::string time_string = "00:00:00.041667";
auto t = otime::RationalTime(1, 24);
auto time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "00:00:01";
t = otime::RationalTime(24, 24);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "00:01:00";
t = otime::RationalTime(60 * 24, 24);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "01:00:00";
t = otime::RationalTime(60 * 60 * 24, 24);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "24:00:00";
t = otime::RationalTime(24 * 60 * 60 * 24, 24);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "23:59:59.92";
t = otime::RationalTime((23 * 60 * 60 + 59 * 60 + 59.92) * 24, 24);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
});

tests.add_test("test_from_time_string25", [] {
std::string time_string = "0:12:04.929792";
auto t = otime::RationalTime((12 * 60 + 4.929792) * 25, 25);
auto time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "00:00:01";
t = otime::RationalTime(25, 25);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "0:1";
t = otime::RationalTime(25, 25);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "1";
t = otime::RationalTime(25, 25);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "00:01:00";
t = otime::RationalTime(60 * 25, 25);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "01:00:00";
t = otime::RationalTime(60 * 60 * 25, 25);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "24:00:00";
t = otime::RationalTime(24 * 60 * 60 * 25, 25);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
time_string = "23:59:59.92";
t = otime::RationalTime((23 * 60 * 60 + 59 * 60 + 59.92) * 25, 25);
time_obj = otime::RationalTime::from_time_string(time_string, 24);
assertTrue(t.almost_equal(time_obj, 0.001));
});

tests.run(argc, argv);
return 0;
}
14 changes: 13 additions & 1 deletion tests/test_opentime.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ def test_create(self):
self.assertIsNotNone(t)
self.assertEqual(t.value, t_val)

t_val = -30.2
t = otio.opentime.RationalTime(t_val)
self.assertIsNotNone(t)
self.assertEqual(t.value, t_val)

t = otio.opentime.RationalTime()
self.assertEqual(t.value, 0)
self.assertEqual(t.rate, 1.0)
Expand Down Expand Up @@ -73,7 +78,6 @@ def test_deepcopy(self):
self.assertEqual(t2, otio.opentime.RationalTime(18, 24))

def test_base_conversion(self):

# from a number
t = otio.opentime.RationalTime(10, 24)
with self.assertRaises(TypeError):
Expand All @@ -93,6 +97,14 @@ def test_time_timecode_convert(self):
t = otio.opentime.from_timecode(timecode, 24)
self.assertEqual(timecode, otio.opentime.to_timecode(t))

def test_negative_timecode(self):
with self.assertRaises(ValueError):
otio.opentime.from_timecode('-01:00:13:13', 24)

def test_bogus_timecode(self):
with self.assertRaises(ValueError):
otio.opentime.from_timecode('pink elephants', 13)

def test_time_timecode_convert_bad_rate(self):
with self.assertRaises(ValueError) as exception_manager:
otio.opentime.from_timecode('01:00:13:24', 24)
Expand Down

0 comments on commit 78941dd

Please sign in to comment.