Skip to content

Commit

Permalink
fix(text): Fix TTML render timing and line break issues (#4407)
Browse files Browse the repository at this point in the history
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 descendants of two different `<p>` 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]`.

The 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 
`TTMLTextParser` level to ensure proper style inheritance still occurs for the 
(actual) text cues.

Closes #4381
  • Loading branch information
JulianDomingo authored and joeyparrish committed Aug 16, 2022
1 parent f8cd4a5 commit 1e68c6e
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 9 deletions.
30 changes: 21 additions & 9 deletions lib/text/simple_text_displayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,28 @@ 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.
const getCuesToFlatten = (cues, result) => {
for (const cue of cues) {
if (cue.isContainer) {
// 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);
result.push(flatCue);
}
}
});
return result;
};
const flattenedCues = getCuesToFlatten(cues, []);

// Convert cues.
const textTrackCues = [];
Expand Down
27 changes: 27 additions & 0 deletions test/text/simple_text_displayer_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, '');
Expand Down

0 comments on commit 1e68c6e

Please sign in to comment.