From ab4cdb4ab7af9dc9e3aafe4b155c031e0ee226bb Mon Sep 17 00:00:00 2001 From: JulianDomingo Date: Fri, 12 Aug 2022 16:57:37 -0700 Subject: [PATCH 1/3] fix(text): Fix missing TTML line breaks and caption rendering timing issues This changes the SimpleTextDisplayer to flatten text payloads only starting at non-container cues. Not doing so poses an issue with 'rolled over' caption lines. E.g., consider: - TTML file N with caption line A from time [1.4, 2.1]. - TTML file N+1 with caption lines A from time [1.4, 2.4] and B from time [2.4, 3.4]. If captions A and B are descendents of two different

elements, Shaka currently flattens the text payload from A and B into a single cue that is rendered onto the display from time [1.4, 3.4]. Therefore, caption A's text payload is incorrectly showing during time [2.4, 3.4].\nThe fix updates the SimpleTextDisplayer append() logic to split up individual non-container cues into their own VTTCue object. This will not be done at the TTMLParser level to ensure proper style inheritance still occurs for the (actual) text cues. Closes #4381 --- lib/text/simple_text_displayer.js | 31 ++++++++++++++++++------- test/text/simple_text_displayer_unit.js | 27 +++++++++++++++++++++ 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/lib/text/simple_text_displayer.js b/lib/text/simple_text_displayer.js index 63604c8fdc..988a718199 100644 --- a/lib/text/simple_text_displayer.js +++ b/lib/text/simple_text_displayer.js @@ -121,16 +121,29 @@ shaka.text.SimpleTextDisplayer = class { // We don't want to modify the array or objects passed in, since we don't // technically own them. So we build a new array and replace certain items // in it if they need to be flattened. - const flattenedCues = cues.map((cue) => { - if (cue.nestedCues.length) { - const flatCue = cue.clone(); - flatCue.nestedCues = []; - flatCue.payload = flattenPayload(cue); - return flatCue; - } else { - return cue; + // We also don't want to flatten the text payloads starting at a container + // element; otherwise, for containers encapsulating multiple caption lines, + // the lines would merge into a single cue. This is undesirable when a + // subset of the captions are outside of the append time window. To fix + // this, we only call flattenPayload() starting at elements marked as + // isContainer = false. + let flattenedCues = []; + const getCuesToFlatten = (cues) => { + for (const cue of cues) { + if (cue.isContainer) { + flattenedCues = + flattenedCues.concat(getCuesToFlatten(cue.nestedCues)); + } else { + // Flatten the payload. + const flatCue = cue.clone(); + flatCue.nestedCues = []; + flatCue.payload = flattenPayload(cue); + flattenedCues = flattenedCues.concat(flatCue); + } } - }); + return flattenedCues; + }; + flattenedCues = getCuesToFlatten(cues); // Convert cues. const textTrackCues = []; diff --git a/test/text/simple_text_displayer_unit.js b/test/text/simple_text_displayer_unit.js index 88c4dc86b2..206387f6e4 100644 --- a/test/text/simple_text_displayer_unit.js +++ b/test/text/simple_text_displayer_unit.js @@ -127,6 +127,33 @@ describe('SimpleTextDisplayer', () => { [shakaCue]); }); + it('flattens nested cue payloads correctly', () => { + const level0ContainerCue = new shaka.text.Cue(10, 30, ''); + level0ContainerCue.isContainer = true; + + const level1NonContainerCueA = new shaka.text.Cue(10, 20, ''); + const level1NonContainerCueB = new shaka.text.Cue(20, 30, ''); + + // Add a trailing whitespace character to get a space-delimited expected + // result. + const cueANestedCue0 = new shaka.text.Cue(10, 20, 'Cue A Test0 '); + const cueANestedCue1 = new shaka.text.Cue(10, 20, 'Cue A Test1'); + const cueBNestedCue0 = new shaka.text.Cue(20, 30, 'Cue B Test0 '); + const cueBNestedCue1 = new shaka.text.Cue(20, 30, 'Cue B Test1'); + + level1NonContainerCueA.nestedCues = [cueANestedCue0, cueANestedCue1]; + level1NonContainerCueB.nestedCues = [cueBNestedCue0, cueBNestedCue1]; + level0ContainerCue.nestedCues = + [level1NonContainerCueA, level1NonContainerCueB]; + + verifyHelper( + [ + {startTime: 10, endTime: 20, text: 'Cue A Test0 Cue A Test1'}, + {startTime: 20, endTime: 30, text: 'Cue B Test0 Cue B Test1'}, + ], + [level0ContainerCue]); + }); + // Regression test for b/159050711 it('maintains the styles of the parent cue', () => { const shakaCue = new shaka.text.Cue(10, 20, ''); From 0dbf2f3f509e3b4b9d6cd54cf717e0e0791251e0 Mon Sep 17 00:00:00 2001 From: JulianDomingo Date: Mon, 15 Aug 2022 18:13:28 -0700 Subject: [PATCH 2/3] Refactored the payload flattening function to stop modifying an 'outer' variable which is also the result. --- lib/text/simple_text_displayer.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/text/simple_text_displayer.js b/lib/text/simple_text_displayer.js index 988a718199..0db8a87463 100644 --- a/lib/text/simple_text_displayer.js +++ b/lib/text/simple_text_displayer.js @@ -127,23 +127,22 @@ shaka.text.SimpleTextDisplayer = class { // subset of the captions are outside of the append time window. To fix // this, we only call flattenPayload() starting at elements marked as // isContainer = false. - let flattenedCues = []; - const getCuesToFlatten = (cues) => { + const getCuesToFlatten = (cues, result) => { for (const cue of cues) { if (cue.isContainer) { - flattenedCues = - flattenedCues.concat(getCuesToFlatten(cue.nestedCues)); + // Recurse to find the actual text payload cues. + getCuesToFlatten(cue.nestedCues, result); } else { // Flatten the payload. const flatCue = cue.clone(); flatCue.nestedCues = []; flatCue.payload = flattenPayload(cue); - flattenedCues = flattenedCues.concat(flatCue); + result.push(flatCue); } } - return flattenedCues; + return result }; - flattenedCues = getCuesToFlatten(cues); + const flattenedCues = getCuesToFlatten(cues, []); // Convert cues. const textTrackCues = []; From 09d852e6cefa84bed1800957da5737893dae1853 Mon Sep 17 00:00:00 2001 From: JulianDomingo Date: Mon, 15 Aug 2022 18:24:25 -0700 Subject: [PATCH 3/3] Fixed linting errors. --- lib/text/simple_text_displayer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/text/simple_text_displayer.js b/lib/text/simple_text_displayer.js index 0db8a87463..e2343caa3f 100644 --- a/lib/text/simple_text_displayer.js +++ b/lib/text/simple_text_displayer.js @@ -140,7 +140,7 @@ shaka.text.SimpleTextDisplayer = class { result.push(flatCue); } } - return result + return result; }; const flattenedCues = getCuesToFlatten(cues, []);