From 26d39db0e73532ef5dc2f7327d048e0ce3a70c3f Mon Sep 17 00:00:00 2001 From: Aaron Vaage Date: Wed, 31 May 2017 12:23:44 -0700 Subject: [PATCH] Moved Timestamp Parsing For Easier Testing Moved all the timestamp parsing code into its own class with its own unit tests. This is to make testing easier. Change-Id: I624472baba51dfa1254a9f4bae55ebb79e310855 --- packager/media/formats/webvtt/webvtt.gyp | 3 + .../formats/webvtt/webvtt_media_parser.cc | 81 +--------------- .../media/formats/webvtt/webvtt_timestamp.cc | 74 ++++++++++++++ .../media/formats/webvtt/webvtt_timestamp.h | 25 +++++ .../webvtt/webvtt_timestamp_unittest.cc | 96 +++++++++++++++++++ 5 files changed, 201 insertions(+), 78 deletions(-) create mode 100644 packager/media/formats/webvtt/webvtt_timestamp.cc create mode 100644 packager/media/formats/webvtt/webvtt_timestamp.h create mode 100644 packager/media/formats/webvtt/webvtt_timestamp_unittest.cc diff --git a/packager/media/formats/webvtt/webvtt.gyp b/packager/media/formats/webvtt/webvtt.gyp index c909fb1564..ba9a760f9b 100644 --- a/packager/media/formats/webvtt/webvtt.gyp +++ b/packager/media/formats/webvtt/webvtt.gyp @@ -19,6 +19,8 @@ 'webvtt_media_parser.h', 'webvtt_sample_converter.cc', 'webvtt_sample_converter.h', + 'webvtt_timestamp.cc', + 'webvtt_timestamp.h', ], 'dependencies': [ '../../../base/base.gyp:base', @@ -32,6 +34,7 @@ 'sources': [ 'webvtt_media_parser_unittest.cc', 'webvtt_sample_converter_unittest.cc', + 'webvtt_timestamp_unittest.cc', ], 'dependencies': [ '../../../testing/gmock.gyp:gmock', diff --git a/packager/media/formats/webvtt/webvtt_media_parser.cc b/packager/media/formats/webvtt/webvtt_media_parser.cc index 67a5780708..a18b6c12e3 100644 --- a/packager/media/formats/webvtt/webvtt_media_parser.cc +++ b/packager/media/formats/webvtt/webvtt_media_parser.cc @@ -16,6 +16,7 @@ #include "packager/media/base/macros.h" #include "packager/media/base/media_sample.h" #include "packager/media/base/text_stream_info.h" +#include "packager/media/formats/webvtt/webvtt_timestamp.h" namespace shaka { namespace media { @@ -68,82 +69,6 @@ bool ReadLine(std::string* data, std::string* line) { return true; } -bool TimestampToMilliseconds(const std::string& original_str, - uint64_t* time_ms) { - const size_t kMinimalHoursLength = 2; - const size_t kMinutesLength = 2; - const size_t kSecondsLength = 2; - const size_t kMillisecondsLength = 3; - - // +2 for a colon and a dot for splitting minutes and seconds AND seconds and - // milliseconds, respectively. - const size_t kMinimalLength = - kMinutesLength + kSecondsLength + kMillisecondsLength + 2; - - base::StringPiece str(original_str); - if (str.size() < kMinimalLength) - return false; - - int hours = 0; - int minutes = 0; - int seconds = 0; - int milliseconds = 0; - - size_t str_index = 0; - if (str.size() > kMinimalLength) { - // Check if hours is in the right format, if so get the number. - // -1 for excluding colon for splitting hours and minutes. - const size_t hours_length = str.size() - kMinimalLength - 1; - if (hours_length < kMinimalHoursLength) - return false; - if (!base::StringToInt(str.substr(0, hours_length), &hours)) - return false; - str_index += hours_length; - - if (str[str_index] != ':') - return false; - ++str_index; - } - - DCHECK_EQ(str.size() - str_index, kMinimalLength); - - if (!base::StringToInt(str.substr(str_index, kMinutesLength), &minutes)) - return false; - if (minutes < 0 || minutes > 60) - return false; - - str_index += kMinutesLength; - if (str[str_index] != ':') - return false; - ++str_index; - - if (!base::StringToInt(str.substr(str_index, kSecondsLength), &seconds)) - return false; - if (seconds < 0 || seconds > 60) - return false; - - str_index += kSecondsLength; - if (str[str_index] != '.') - return false; - ++str_index; - - if (!base::StringToInt(str.substr(str_index, kMillisecondsLength), - &milliseconds)) { - return false; - } - str_index += kMillisecondsLength; - - if (milliseconds < 0 || milliseconds > 999) - return false; - - DCHECK_EQ(str.size(), str_index); - *time_ms = milliseconds + - seconds * 1000 + - minutes * 60 * 1000 + - hours * 60 * 60 * 1000; - return true; -} - // Clears |settings| and 0s |start_time| and |duration| regardless of the // parsing result. bool ParseTimingAndSettingsLine(const std::string& line, @@ -168,14 +93,14 @@ bool ParseTimingAndSettingsLine(const std::string& line, } const std::string& start_time_str = entries[0]; - if (!TimestampToMilliseconds(start_time_str, start_time)) { + if (!WebVttTimestampParse(start_time_str, start_time)) { LOG(ERROR) << "Failed to parse " << start_time_str << " in " << line; return false; } const std::string& end_time_str = entries[2]; uint64_t end_time = 0; - if (!TimestampToMilliseconds(end_time_str, &end_time)) { + if (!WebVttTimestampParse(end_time_str, &end_time)) { LOG(ERROR) << "Failed to parse " << end_time_str << " in " << line; return false; } diff --git a/packager/media/formats/webvtt/webvtt_timestamp.cc b/packager/media/formats/webvtt/webvtt_timestamp.cc new file mode 100644 index 0000000000..2f2810e2cf --- /dev/null +++ b/packager/media/formats/webvtt/webvtt_timestamp.cc @@ -0,0 +1,74 @@ +// Copyright 2017 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#include "packager/media/formats/webvtt/webvtt_timestamp.h" + +#include + +#include "packager/base/logging.h" +#include "packager/base/strings/string_number_conversions.h" + +namespace shaka { +namespace media { +namespace { + +bool GetTotalMilliseconds(uint64_t hours, + uint64_t minutes, + uint64_t seconds, + uint64_t ms, + uint64_t* out) { + DCHECK(out); + if (minutes > 59 || seconds > 59 || ms > 999) { + VLOG(1) << "Hours:" << hours + << " Minutes:" << minutes + << " Seconds:" << seconds + << " MS:" << ms + << " shoud have never made it to GetTotalMilliseconds"; + return false; + } + *out = 60 * 60 * 1000 * hours + + 60 * 1000 * minutes + + 1000 * seconds + + ms; + return true; +} +} // namespace + +bool WebVttTimestampParse(const base::StringPiece& source, uint64_t* out) { + DCHECK(out); + + if (source.length() < 9) { + LOG(WARNING) << "Timestamp '" << source << "' is mal-formed"; + return false; + } + + const size_t minutes_begin = source.length() - 9; + const size_t seconds_begin = source.length() - 6; + const size_t milliseconds_begin = source.length() - 3; + + uint64_t hours = 0; + uint64_t minutes = 0; + uint64_t seconds = 0; + uint64_t ms = 0; + + const bool has_hours = minutes_begin >= 3 && + source[minutes_begin-1] == ':' && + base::StringToUint64(source.substr(0, minutes_begin-1), &hours); + + if ((minutes_begin == 0 || has_hours) && + source[seconds_begin-1] == ':' && + source[milliseconds_begin-1] == '.' && + base::StringToUint64(source.substr(minutes_begin, 2), &minutes) && + base::StringToUint64(source.substr(seconds_begin, 2), &seconds) && + base::StringToUint64(source.substr(milliseconds_begin, 3), &ms)) { + return GetTotalMilliseconds(hours, minutes, seconds, ms, out); + } + + LOG(WARNING) << "Timestamp '" << source << "' is mal-formed"; + return false; +} +} // namespace media +} // namespace shaka diff --git a/packager/media/formats/webvtt/webvtt_timestamp.h b/packager/media/formats/webvtt/webvtt_timestamp.h new file mode 100644 index 0000000000..3bdc65f434 --- /dev/null +++ b/packager/media/formats/webvtt/webvtt_timestamp.h @@ -0,0 +1,25 @@ +// Copyright 2017 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#ifndef PACKAGER_MEDIA_FORMATS_WEBVTT_TIMESTAMP_H_ +#define PACKAGER_MEDIA_FORMATS_WEBVTT_TIMESTAMP_H_ + +#include + +#include + +#include "packager/base/strings/string_piece.h" + +namespace shaka { +namespace media { +// Parse a timestamp into milliseconds using the two patterns defined by WebVtt: +// LONG : ##:##:##.### (long can have 2 or more hour digits) +// SHORT : ##:##:### +bool WebVttTimestampParse(const base::StringPiece& source, uint64_t* out); +} // namespace media +} // namespace shaka + +#endif // PACKAGER_MEDIA_FORMATS_WEBVTT_TIMESTAMP_H_ diff --git a/packager/media/formats/webvtt/webvtt_timestamp_unittest.cc b/packager/media/formats/webvtt/webvtt_timestamp_unittest.cc new file mode 100644 index 0000000000..5cb855df51 --- /dev/null +++ b/packager/media/formats/webvtt/webvtt_timestamp_unittest.cc @@ -0,0 +1,96 @@ +// Copyright 2017 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#include + +#include "packager/media/formats/webvtt/webvtt_timestamp.h" + +namespace shaka { +namespace media { + +TEST(WebVttTimestampTest, TooShort) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("00.000", &ms)); +} + +TEST(WebVttTimestampTest, RightLengthButMeaningless) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("ABCDEFGHI", &ms)); +} + +TEST(WebVttTimestampTest, ParseHours) { + uint64_t ms; + EXPECT_TRUE(WebVttTimestampParse("12:00:00.000", &ms)); + EXPECT_EQ(43200000u, ms); +} + +TEST(WebVttTimestampTest, ParseLongHours) { + uint64_t ms; + EXPECT_TRUE(WebVttTimestampParse("120:00:00.000", &ms)); + EXPECT_EQ(432000000u, ms); +} + +TEST(WebVttTimestampTest, ParseMinutes) { + uint64_t ms; + EXPECT_TRUE(WebVttTimestampParse("00:12:00.000", &ms)); + EXPECT_EQ(720000u, ms); +} + +TEST(WebVttTimestampTest, ParseSeconds) { + uint64_t ms; + EXPECT_TRUE(WebVttTimestampParse("00:00:12.000", &ms)); + EXPECT_EQ(12000u, ms); +} + +TEST(WebVttTimestampTest, ParseMs) { + uint64_t ms; + EXPECT_TRUE(WebVttTimestampParse("00:00:00.123", &ms)); + EXPECT_EQ(123u, ms); +} + +TEST(WebVttTimestampTest, ParseNoHours) { + uint64_t ms; + EXPECT_TRUE(WebVttTimestampParse("12:00.000", &ms)); + EXPECT_EQ(720000u, ms); +} + +TEST(WebVttTimestampTest, FailWithShortHours) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("1:00:00.000", &ms)); +} + +TEST(WebVttTimestampTest, FailWithShortMinutes) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("00:1:00.000", &ms)); +} + +TEST(WebVttTimestampTest, FailWithShortSeconds) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("00:1.000", &ms)); +} + +TEST(WebVttTimestampTest, FailWithShortMs) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("00:00.01", &ms)); +} + +TEST(WebVttTimestampTest, FailWithNonDigit) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("00:0A:00.000", &ms)); +} + +TEST(WebVttTimestampTest, FailWithInvalidMinutes) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("00:79:00.000", &ms)); +} + +TEST(WebVttTimestampTest, FailWithInvalidSeconds) { + uint64_t ms; + EXPECT_FALSE(WebVttTimestampParse("00:00:79.000", &ms)); +} + +} // namespace media +} // namespace shaka