TTML: Ignore regions that don't declare origin and extent

Issue: #2953 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159218386
google · Jun 30, 2017 · 92206b9 · 92206b9
1 parent e618080
commit 92206b9
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 10 deletions.
diff --git a/...ry/core/src/androidTest/java/com/google/android/exoplayer2/text/ttml/TtmlDecoderTest.java b/...ry/core/src/androidTest/java/com/google/android/exoplayer2/text/ttml/TtmlDecoderTest.java
@@ -179,9 +179,13 @@ public void testMultipleRegions() throws IOException, SubtitleDecoderException {
     assertEquals(1, output.size());
     ttmlCue = output.get(0);
     assertEquals("dolor", ttmlCue.text.toString());
-    assertEquals(10f / 100f, ttmlCue.position);
-    assertEquals(80f / 100f, ttmlCue.line);
-    assertEquals(1f, ttmlCue.size);
+    assertEquals(Cue.DIMEN_UNSET, ttmlCue.position);
+    assertEquals(Cue.DIMEN_UNSET, ttmlCue.line);
+    assertEquals(Cue.DIMEN_UNSET, ttmlCue.size);
+    // TODO: Should be as below, once https://github.com/google/ExoPlayer/issues/2953 is fixed.
+    // assertEquals(10f / 100f, ttmlCue.position);
+    // assertEquals(80f / 100f, ttmlCue.line);
+    // assertEquals(1f, ttmlCue.size);
 
     output = subtitle.getCues(21000000);
     assertEquals(1, output.size());

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/ttml/TtmlDecoder.java b/library/core/src/main/java/com/google/android/exoplayer2/text/ttml/TtmlDecoder.java
@@ -222,9 +222,9 @@ private Map<String, TtmlStyle> parseHeader(XmlPullParser xmlParser,
   /**
    * Parses a region declaration.
    * <p>
-   * If the region defines an origin and/or extent, it is required that they're defined as
-   * percentages of the viewport. Region declarations that define origin and/or extent in other
-   * formats are unsupported, and null is returned.
+   * If the region defines an origin and extent, it is required that they're defined as percentages
+   * of the viewport. Region declarations that define origin and extent in other formats are
+   * unsupported, and null is returned.
    */
   private TtmlRegion parseRegionAttributes(XmlPullParser xmlParser) {
     String regionId = XmlPullParserUtil.getAttributeValue(xmlParser, TtmlNode.ATTR_ID);
@@ -250,9 +250,13 @@ private TtmlRegion parseRegionAttributes(XmlPullParser xmlParser) {
         return null;
       }
     } else {
+      Log.w(TAG, "Ignoring region without an origin");
+      return null;
+      // TODO: Should default to top left as below in this case, but need to fix
+      // https://github.com/google/ExoPlayer/issues/2953 first.
       // Origin is omitted. Default to top left.
-      position = 0;
-      line = 0;
+      // position = 0;
+      // line = 0;
     }
 
     float width;
@@ -273,9 +277,13 @@ private TtmlRegion parseRegionAttributes(XmlPullParser xmlParser) {
         return null;
       }
     } else {
+      Log.w(TAG, "Ignoring region without an extent");
+      return null;
+      // TODO: Should default to extent of parent as below in this case, but need to fix
+      // https://github.com/google/ExoPlayer/issues/2953 first.
       // Extent is omitted. Default to extent of parent.
-      width = 1;
-      height = 1;
+      // width = 1;
+      // height = 1;
     }
 
     @Cue.AnchorType int lineAnchor = Cue.ANCHOR_TYPE_START;