fix(text): Fix caption overlap.

This changes the TTML parser to not allow cue regions to be inherited to the children of the element the region was originally assigned on, except for the purposes of styles (colors, etc). To allow regions on elements "above" the cues in TTML, such as the <body> or <div> elements, this also changes the TTML parser to render the full structure of the TTML file as a tree of cues. The end result will be a single cue representing the <body>, with children representing the <div> elements inside it, and those <divs> will have children that represent the actual cues. Now that our text displayer can intelligently update child cues as they enter or leave the display window, this approach should be possible. Closes #3850 Closes #3741 Change-Id: Ia8d750daa06920610c04e9b26e29d2d304eaf8a9
shaka-project · Jan 20, 2022 · bf67d87 · bf67d87
1 parent 3a22cb3
commit bf67d87
Show file tree

Hide file tree

Showing 7 changed files with 534 additions and 286 deletions.
diff --git a/externs/shaka/text.js b/externs/shaka/text.js
@@ -363,6 +363,16 @@ shaka.extern.Cue = class {
      */
     this.nestedCues;
 
+    /**
+     * If true, this represents a container element that is "above" the main
+     * cues. For example, the <body> and <div> tags that contain the <p> tags
+     * in a TTML file. This controls the flow of the final cues; any nested cues
+     * within an "isContainer" cue will be laid out as separate lines.
+     * @type {boolean}
+     * @exportDoc
+     */
+    this.isContainer;
+
     /**
      * Whether or not the cue only acts as a line break between two nested cues.
      * Should only appear in nested cues.

diff --git a/lib/text/cue.js b/lib/text/cue.js
@@ -222,6 +222,12 @@ shaka.text.Cue = class {
      */
     this.nestedCues = [];
 
+    /**
+     * @override
+     * @exportInterface
+     */
+    this.isContainer = false;
+
     /**
      * @override
      * @exportInterface

diff --git a/lib/text/ttml_text_parser.js b/lib/text/ttml_text_parser.js
@@ -147,28 +147,15 @@ shaka.text.TtmlTextParser = class {
               shaka.util.Error.Code.INVALID_TEXT_CUE,
               '<span> can only be inside <p> in TTML');
         }
+      }
 
-        const pChildren = XmlUtils.findChildren(div, 'p');
-        if (pChildren && pChildren.length) {
-          for (const p of pChildren) {
-            const cue = TtmlTextParser.parseCue_(
-                p, time.periodStart, rateInfo, metadataElements, styles,
-                regionElements, cueRegions, whitespaceTrim,
-                cellResolutionInfo, /* parentCueElement= */ null);
-            if (cue) {
-              cues.push(cue);
-            }
-          }
-        } else {
-          // Only used for parsing the background image
-          const cue = TtmlTextParser.parseCue_(
-              div, time.periodStart, rateInfo, metadataElements, styles,
-              regionElements, cueRegions, whitespaceTrim,
-              cellResolutionInfo, /* parentCueElement= */ null);
-          if (cue) {
-            cues.push(cue);
-          }
-        }
+      const cue = TtmlTextParser.parseCue_(
+          body, time.periodStart, rateInfo, metadataElements, styles,
+          regionElements, cueRegions, whitespaceTrim,
+          cellResolutionInfo, /* parentCueElement= */ null,
+          /* isContent= */ false);
+      if (cue) {
+        cues.push(cue);
       }
     }
 
@@ -188,23 +175,30 @@ shaka.text.TtmlTextParser = class {
    * @param {boolean} whitespaceTrim
    * @param {?{columns: number, rows: number}} cellResolution
    * @param {?Element} parentCueElement
+   * @param {boolean} isContent
    * @return {shaka.text.Cue}
    * @private
    */
   static parseCue_(
       cueNode, offset, rateInfo, metadataElements, styles, regionElements,
-      cueRegions, whitespaceTrim, cellResolution, parentCueElement) {
+      cueRegions, whitespaceTrim, cellResolution, parentCueElement, isContent) {
     /** @type {Element} */
     let cueElement;
     /** @type {Element} */
-    let parentElement = /** @type {Element} */(cueNode.parentNode);
+    let parentElement = /** @type {Element} */ (cueNode.parentNode);
 
     if (cueNode.nodeType == Node.COMMENT_NODE) {
       // The comments do not contain information that interests us here.
       return null;
     }
 
     if (cueNode.nodeType == Node.TEXT_NODE) {
+      if (!isContent) {
+        // Ignore text elements outside the content. For example, whitespace
+        // on the same lexical level as the <p> elements, in a document with
+        // xml:space="preserve", should not be renderer.
+        return null;
+      }
       // This should generate an "anonymous span" according to the TTML spec.
       // So pretend the element was a <span>.  parentElement was set above, so
       // we should still be able to correctly traverse up for timing
@@ -219,6 +213,21 @@ shaka.text.TtmlTextParser = class {
     }
     goog.asserts.assert(cueElement, 'cueElement should be non-null!');
 
+    let imageElement = null;
+    for (const nameSpace of shaka.text.TtmlTextParser.smpteNsList_) {
+      imageElement = shaka.text.TtmlTextParser.getElementsFromCollection_(
+          cueElement, 'backgroundImage', metadataElements, '#',
+          nameSpace)[0];
+      if (imageElement) {
+        break;
+      }
+    }
+
+    const parentIsContent = isContent;
+    if (cueNode.nodeName == 'p' || imageElement) {
+      isContent = true;
+    }
+
     const spaceStyle = cueElement.getAttribute('xml:space') ||
         (whitespaceTrim ? 'default' : 'preserve');
 
@@ -245,6 +254,7 @@ shaka.text.TtmlTextParser = class {
             localWhitespaceTrim,
             cellResolution,
             cueElement,
+            isContent,
         );
 
         // This node may or may not generate a nested cue.
@@ -254,7 +264,7 @@ shaka.text.TtmlTextParser = class {
       }
     }
 
-    const isNested = /** @type {boolean} */(parentCueElement != null);
+    const isNested = /** @type {boolean} */ (parentCueElement != null);
 
     // In this regex, "\S" means "non-whitespace character".
     const hasTextContent = /\S/.test(cueElement.textContent);
@@ -336,30 +346,30 @@ shaka.text.TtmlTextParser = class {
     const cue = new shaka.text.Cue(start, end, payload);
     cue.nestedCues = nestedCues;
 
+    if (!isContent) {
+      // If this is not a <p> element or a <div> with images, and it has no
+      // parent that was a <p> element, then it's part of the outer containers
+      // (e.g. the <body> or a normal <div> element within it).
+      cue.isContainer = true;
+    }
+
     if (cellResolution) {
       cue.cellResolution = cellResolution;
     }
 
     // Get other properties if available.
     const regionElement = shaka.text.TtmlTextParser.getElementsFromCollection_(
         cueElement, 'region', regionElements, /* prefix= */ '')[0];
-    if (regionElement && regionElement.getAttribute('xml:id')) {
-      const regionId = regionElement.getAttribute('xml:id');
-      cue.region = cueRegions.filter((region) => region.id == regionId)[0];
-    }
-
-    let imageElement = null;
-    for (const nameSpace of shaka.text.TtmlTextParser.smpteNsList_) {
-      imageElement = shaka.text.TtmlTextParser.getElementsFromCollection_(
-          cueElement, 'backgroundImage', metadataElements, '#',
-          nameSpace)[0];
-      if (imageElement) {
-        break;
+    // Do not actually apply that region unless it is non-inherited, though.
+    // This makes it so that, if a parent element has a region, the children
+    // don't also all independently apply the positioning of that region.
+    if (cueElement.hasAttribute('region')) {
+      if (regionElement && regionElement.getAttribute('xml:id')) {
+        const regionId = regionElement.getAttribute('xml:id');
+        cue.region = cueRegions.filter((region) => region.id == regionId)[0];
       }
     }
 
-    const isLeaf = nestedCues.length == 0;
-
     let regionElementForStyle = regionElement;
     if (parentCueElement && isNested && !cueElement.getAttribute('region') &&
       !cueElement.getAttribute('style')) {
@@ -375,8 +385,8 @@ shaka.text.TtmlTextParser = class {
         regionElementForStyle,
         imageElement,
         styles,
-        isNested,
-        isLeaf);
+        /** isNested= */ parentIsContent, // "nested in a <div>" doesn't count.
+        /** isLeaf= */ (nestedCues.length == 0));
 
     return cue;
   }

diff --git a/lib/text/ui_text_displayer.js b/lib/text/ui_text_displayer.js
@@ -456,7 +456,7 @@ shaka.text.UITextDisplayer = class {
     // The displayAlign attribute specifies the vertical alignment of the
     // captions inside the text container. Before means at the top of the
     // text container, and after means at the bottom.
-    if (isNested) {
+    if (isNested && !parents[parents.length - 1].isContainer) {
       style.display = 'inline';
     } else {
       style.display = 'flex';

diff --git a/test/test/util/ttml_utils.js b/test/test/util/ttml_utils.js
@@ -0,0 +1,81 @@
+/*! @license
+ * Shaka Player
+ * Copyright 2016 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+goog.provide('shaka.test.TtmlUtils');
+
+shaka.test.TtmlUtils = class {
+  /**
+   * @param {!Array} expectedCues
+   * @param {!Array} actualCues
+   * @param {!Object} bodyProperties
+   * @param {Object=} divProperties
+   */
+  static verifyHelper(expectedCues, actualCues, bodyProperties, divProperties) {
+    const mapExpected = (cue) => {
+      if (cue.region) {
+        cue.region = jasmine.objectContaining(cue.region);
+      }
+
+      if (cue.nestedCues && (cue.nestedCues instanceof Array)) {
+        cue.nestedCues = cue.nestedCues.map(mapExpected);
+      }
+
+      if (cue.isContainer == undefined) {
+        // If not specified to be true, check for isContainer to be false.
+        cue.isContainer = false;
+      }
+
+      return jasmine.objectContaining(cue);
+    };
+
+    /**
+     * @param {!Object} properties
+     * @return {!shaka.extern.Cue}
+     */
+    const makeContainer = (properties) => {
+      const region = {
+        id: '',
+        viewportAnchorX: 0,
+        viewportAnchorY: 0,
+        regionAnchorX: 0,
+        regionAnchorY: 0,
+        width: 100,
+        height: 100,
+        widthUnits: shaka.text.CueRegion.units.PERCENTAGE,
+        heightUnits: shaka.text.CueRegion.units.PERCENTAGE,
+        viewportAnchorUnits: shaka.text.CueRegion.units.PERCENTAGE,
+        scroll: '',
+      };
+      const containerCue = /** @type {!shaka.extern.Cue} */ ({
+        region,
+        nestedCues: jasmine.any(Object),
+        payload: '',
+        startTime: 0,
+        endTime: Infinity,
+        isContainer: true,
+      });
+      Object.assign(containerCue, properties);
+      return mapExpected(containerCue);
+    };
+
+    if (expectedCues.length == 0 && !divProperties) {
+      expect(actualCues.length).toBe(0);
+    } else {
+      // Body.
+      expect(actualCues.length).toBe(1);
+      const body = actualCues[0];
+      expect(body).toEqual(makeContainer(bodyProperties));
+
+      // Div.
+      expect(body.nestedCues.length).toBe(1);
+      const div = body.nestedCues[0];
+      expect(div).toEqual(makeContainer(divProperties || bodyProperties));
+
+      // Cues.
+      expect(div.nestedCues).toEqual(expectedCues.map(mapExpected));
+    }
+  }
+};
diff --git a/test/text/mp4_ttml_parser_unit.js b/test/text/mp4_ttml_parser_unit.js
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+goog.require('shaka.test.TtmlUtils');
 goog.require('shaka.test.Util');
 goog.require('shaka.text.Mp4TtmlParser');
 goog.require('shaka.util.BufferUtils');
@@ -48,7 +49,14 @@ describe('Mp4TtmlParser', () => {
     parser.parseInit(ttmlInitSegment);
     const time = {periodStart: 0, segmentStart: 0, segmentEnd: 0};
     const ret = parser.parseMedia(ttmlSegmentMultipleMDAT, time);
-    expect(ret.length).toBe(20);
+    // Bodies.
+    expect(ret.length).toBe(2);
+    // Divs.
+    expect(ret[0].nestedCues.length).toBe(1);
+    expect(ret[1].nestedCues.length).toBe(1);
+    // Cues.
+    expect(ret[0].nestedCues[0].nestedCues.length).toBe(10);
+    expect(ret[1].nestedCues[0].nestedCues.length).toBe(10);
   });
 
   it('accounts for offset', () => {
@@ -159,22 +167,7 @@ describe('Mp4TtmlParser', () => {
     parser.parseInit(ttmlInitSegment);
     const time = {periodStart: 0, segmentStart: 0, segmentEnd: 0};
     const result = parser.parseMedia(ttmlSegment, time);
-    verifyHelper(cues, result);
+    shaka.test.TtmlUtils.verifyHelper(
+        cues, result, {startTime: 23, endTime: 53.5});
   });
-
-  function verifyHelper(/** !Array */ expected, /** !Array */ actual) {
-    const mapExpected = (cue) => {
-      if (cue.region) {
-        cue.region = jasmine.objectContaining(cue.region);
-      }
-
-      if (cue.nestedCues) {
-        cue.nestedCues = cue.nestedCues.map(mapExpected);
-      }
-
-      return jasmine.objectContaining(cue);
-    };
-
-    expect(actual).toEqual(expected.map(mapExpected));
-  }
 });