From 56c2f227ffc93f1152f7b17a93649367a6cf435e Mon Sep 17 00:00:00 2001
From: Aaron Vaage <vaage@google.com>
Date: Tue, 24 Apr 2018 10:42:10 -0700
Subject: [PATCH] Skip Style and Region Blocks

To ensure that we can parse content with style and region blocks,
this change updates the parser to skip those blocks so that we
can still parse the cues from a file.

Full style and region support will be added later this year.

Issue #380

Change-Id: I11b8fd862a108c27a5c67b15d4703532b44a1214
---
 .../media/formats/webvtt/webvtt_parser.cc     | 44 +++++++++++++++++++
 .../formats/webvtt/webvtt_parser_unittest.cc  | 32 ++++++++++++--
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/packager/media/formats/webvtt/webvtt_parser.cc b/packager/media/formats/webvtt/webvtt_parser.cc
index 02d03ef37e..71c9dfcc26 100644
--- a/packager/media/formats/webvtt/webvtt_parser.cc
+++ b/packager/media/formats/webvtt/webvtt_parser.cc
@@ -59,6 +59,22 @@ bool IsLikelyCueTiming(const std::string& line) {
 bool MaybeCueId(const std::string& line) {
   return line.find("-->") == std::string::npos;
 }
+
+// Check to see if the block is likely a style block. Style blocks are
+// identified as any block that starts with a line that only contains
+// "STYLE".
+// SOURCE: https://w3c.github.io/webvtt/#styling
+bool IsLikelyStyle(const std::string& line) {
+  return base::TrimWhitespaceASCII(line, base::TRIM_TRAILING) == "STYLE";
+}
+
+// Check to see if the block is likely a region block. Region blocks are
+// identified as any block that starts with a line that only contains
+// "REGION".
+// SOURCE: https://w3c.github.io/webvtt/#webvtt-region
+bool IsLikelyRegion(const std::string& line) {
+  return base::TrimWhitespaceASCII(line, base::TRIM_TRAILING) == "REGION";
+}
 }  // namespace
 
 WebVttParser::WebVttParser(std::unique_ptr<FileReader> source,
@@ -113,6 +129,8 @@ bool WebVttParser::Parse() {
     return false;
   }
 
+  bool saw_cue = false;
+
   while (reader_.Next(&block) && keep_reading_) {
     // NOTE
     if (IsLikelyNote(block[0])) {
@@ -120,15 +138,41 @@ bool WebVttParser::Parse() {
       continue;
     }
 
+    // STYLE
+    if (IsLikelyStyle(block[0])) {
+      if (saw_cue) {
+        LOG(ERROR)
+            << "Found style block after seeing cue. Ignoring style block";
+      } else {
+        LOG(WARNING) << "Missing support for style blocks. Skipping block:\n"
+                     << BlockToString(block.data(), block.size());
+      }
+      continue;
+    }
+
+    // REGION
+    if (IsLikelyRegion(block[0])) {
+      if (saw_cue) {
+        LOG(ERROR)
+            << "Found region block after seeing cue. Ignoring region block";
+      } else {
+        LOG(WARNING) << "Missing support for region blocks. Skipping block:\n"
+                     << BlockToString(block.data(), block.size());
+      }
+      continue;
+    }
+
     // CUE with ID
     if (block.size() > 2 && MaybeCueId(block[0]) &&
         IsLikelyCueTiming(block[1]) && ParseCueWithId(block)) {
+      saw_cue = true;
       continue;
     }
 
     // CUE with no ID
     if (block.size() > 1 && IsLikelyCueTiming(block[0]) &&
         ParseCueWithNoId(block)) {
+      saw_cue = true;
       continue;
     }
 
diff --git a/packager/media/formats/webvtt/webvtt_parser_unittest.cc b/packager/media/formats/webvtt/webvtt_parser_unittest.cc
index 12347ae23a..26a547348e 100644
--- a/packager/media/formats/webvtt/webvtt_parser_unittest.cc
+++ b/packager/media/formats/webvtt/webvtt_parser_unittest.cc
@@ -123,9 +123,35 @@ TEST_F(WebVttParserTest, FailToParseHeaderNotOneLine) {
   ASSERT_NE(Status::OK, parser_->Run());
 }
 
-// TODO: Add style blocks support to WebVttParser.
-// This test is disabled until WebVTT parses STYLE blocks.
-TEST_F(WebVttParserTest, DISABLED_ParseStyleBlocks) {
+// Right now we don't support region blocks, but for now make sure that we don't
+// die if we see a region block.
+TEST_F(WebVttParserTest, ParserDoesNotDieOnRegionBlock) {
+  const char* text =
+      "WEBVTT\n"
+      "\n"
+      "REGION\n"
+      "id:fred\n"
+      "width:40%\n"
+      "lines:3\n"
+      "regionanchor:0%,100%\n"
+      "viewportanchor:10%,90%\n"
+      "scroll:up";
+
+  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());
+}
+
+// Right now we don't support style blocks, but for now make sure that we don't
+// die if we see a style block.
+TEST_F(WebVttParserTest, ParserDoesNotDieOnStyleBlock) {
   const char* text =
       "WEBVTT\n"
       "\n"