diff --git a/packager/media/formats/webvtt/webvtt_parser.cc b/packager/media/formats/webvtt/webvtt_parser.cc index e0315f0b56..b4a8df88eb 100644 --- a/packager/media/formats/webvtt/webvtt_parser.cc +++ b/packager/media/formats/webvtt/webvtt_parser.cc @@ -19,6 +19,20 @@ namespace shaka { namespace media { namespace { +std::string BlockToString(const std::string* block, size_t size) { + std::string out = " --- BLOCK START ---\n"; + + for (size_t i = 0; i < size; i++) { + out.append(" "); + out.append(block[i]); + out.append("\n"); + } + + out.append(" --- BLOCK END ---"); + + return out; +} + // Comments are just blocks that are preceded by a blank line, start with the // word "NOTE" (followed by a space or newline), and end at the first blank // line. @@ -117,12 +131,8 @@ bool WebVttParser::Parse() { continue; } - LOG(ERROR) << "Failed to determine block classification:"; - LOG(ERROR) << " --- BLOCK START ---"; - for (const std::string& line : block) { - LOG(ERROR) << " " << line; - } - LOG(ERROR) << " --- BLOCK END ---"; + LOG(ERROR) << "Failed to determine block classification:\n" + << BlockToString(block.data(), block.size()); return false; } @@ -130,34 +140,65 @@ bool WebVttParser::Parse() { } bool WebVttParser::ParseCueWithNoId(const std::vector& block) { - return ParseCue("", block.data(), block.size()); + const Status status = ParseCue("", block.data(), block.size()); + + if (!status.ok()) { + LOG(ERROR) << "Failed to parse cue: " << status.error_message(); + } + + return status.ok(); } bool WebVttParser::ParseCueWithId(const std::vector& block) { - return ParseCue(block[0], block.data() + 1, block.size() - 1); + const Status status = ParseCue(block[0], block.data() + 1, block.size() - 1); + + if (!status.ok()) { + LOG(ERROR) << "Failed to parse cue: " << status.error_message(); + } + + return status.ok(); } -bool WebVttParser::ParseCue(const std::string& id, - const std::string* block, - size_t block_size) { - std::shared_ptr sample(new TextSample); - sample->set_id(id); - +Status WebVttParser::ParseCue(const std::string& id, + const std::string* block, + size_t block_size) { const std::vector time_and_style = base::SplitString( block[0], " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); - uint64_t start_time; - uint64_t end_time; - if (time_and_style.size() >= 3 && time_and_style[1] == "-->" && + uint64_t start_time = 0; + uint64_t end_time = 0; + + const bool parsed_time = + time_and_style.size() >= 3 && time_and_style[1] == "-->" && WebVttTimestampToMs(time_and_style[0], &start_time) && - WebVttTimestampToMs(time_and_style[2], &end_time)) { - sample->SetTime(start_time, end_time); - } else { - LOG(ERROR) << "Could not parse start time, -->, and end time from " - << block[0]; - return false; + WebVttTimestampToMs(time_and_style[2], &end_time); + + if (!parsed_time) { + return Status( + error::INTERNAL_ERROR, + "Could not parse start time, -->, and end time from " + block[0]); } + // According to the WebVTT spec + // (https://www.w3.org/TR/webvtt1/#webvtt-cue-timings) end time must be + // greater than the start time of the cue. Since we are seeing content with + // zero-duration cues in the field, we are going to drop the cue instead of + // failing to package. + // + // Print a warning so that those packaging content can know that their + // content is not spec compliant. + if (start_time == end_time) { + LOG(WARNING) << "WebVTT input is not spec compliant." + " Skipping zero-duration cue\n" + << BlockToString(block, block_size); + + return Status::OK; + } + + std::shared_ptr sample = std::make_shared(); + sample->set_id(id); + sample->SetTime(start_time, end_time); + // The rest of time_and_style are the style tokens. for (size_t i = 3; i < time_and_style.size(); i++) { sample->AppendStyle(time_and_style[i]); @@ -168,14 +209,7 @@ bool WebVttParser::ParseCue(const std::string& id, sample->AppendPayload(block[i]); } - const Status send_result = DispatchTextSample(0, sample); - - if (send_result != Status::OK) { - LOG(ERROR) << "Failed to send text sample down stream:" - << send_result.error_message(); - } - - return send_result == Status::OK; + return DispatchTextSample(0, sample); } Status WebVttParser::DispatchTextStreamInfo() { diff --git a/packager/media/formats/webvtt/webvtt_parser.h b/packager/media/formats/webvtt/webvtt_parser.h index 16351fcee2..b57eb1a859 100644 --- a/packager/media/formats/webvtt/webvtt_parser.h +++ b/packager/media/formats/webvtt/webvtt_parser.h @@ -35,9 +35,9 @@ class WebVttParser : public OriginHandler { bool Parse(); bool ParseCueWithNoId(const std::vector& block); bool ParseCueWithId(const std::vector& block); - bool ParseCue(const std::string& id, - const std::string* block, - size_t block_size); + Status ParseCue(const std::string& id, + const std::string* block, + size_t block_size); Status DispatchTextStreamInfo(); diff --git a/packager/media/formats/webvtt/webvtt_parser_unittest.cc b/packager/media/formats/webvtt/webvtt_parser_unittest.cc index 64aa5029cb..12347ae23a 100644 --- a/packager/media/formats/webvtt/webvtt_parser_unittest.cc +++ b/packager/media/formats/webvtt/webvtt_parser_unittest.cc @@ -147,6 +147,25 @@ TEST_F(WebVttParserTest, DISABLED_ParseStyleBlocks) { ASSERT_OK(parser_->Run()); } +TEST_F(WebVttParserTest, IngoresZeroDurationCues) { + const char* text = + "WEBVTT\n" + "\n" + "00:01:00.000 --> 00:01:00.000\n" + "This subtitle would never show\n"; + + ASSERT_NO_FATAL_FAILURE(SetUpAndInitializeGraph(text)); + + { + testing::InSequence s; + EXPECT_CALL(*Output(kOutputIndex), + OnProcess(IsStreamInfo(kStreamIndex, kTimeScale, !kEncrypted))); + EXPECT_CALL(*Output(kOutputIndex), OnFlush(kStreamIndex)); + } + + ASSERT_OK(parser_->Run()); +} + TEST_F(WebVttParserTest, ParseOneCue) { const char* text = "WEBVTT\n"