From 9faf84801a1cf8da5b2261def2b24985f8d07366 Mon Sep 17 00:00:00 2001
From: Joey Parrish
Date: Thu, 8 Dec 2022 00:05:08 -0800
Subject: [PATCH] fix(TTML): Fix duplicate cues overlapping segment boundaries
(#4798)
Closes #4631
---
lib/text/ttml_text_parser.js | 17 ++-
test/text/mp4_ttml_parser_unit.js | 10 +-
test/text/ttml_text_parser_unit.js | 205 +++++++++++++++++------------
3 files changed, 136 insertions(+), 96 deletions(-)
diff --git a/lib/text/ttml_text_parser.js b/lib/text/ttml_text_parser.js
index f8eefb1d02..7bb70fe92c 100644
--- a/lib/text/ttml_text_parser.js
+++ b/lib/text/ttml_text_parser.js
@@ -136,7 +136,7 @@ shaka.text.TtmlTextParser = class {
}
const cue = TtmlTextParser.parseCue_(
- body, time.periodStart, rateInfo, metadataElements, styles,
+ body, time, rateInfo, metadataElements, styles,
regionElements, cueRegions, whitespaceTrim,
cellResolutionInfo, /* parentCueElement= */ null,
/* isContent= */ false);
@@ -157,7 +157,7 @@ shaka.text.TtmlTextParser = class {
* Parses a TTML node into a Cue.
*
* @param {!Node} cueNode
- * @param {number} offset
+ * @param {shaka.extern.TextParser.TimeContext} timeContext
* @param {!shaka.text.TtmlTextParser.RateInfo_} rateInfo
* @param {!Array.} metadataElements
* @param {!Array.} styles
@@ -171,7 +171,7 @@ shaka.text.TtmlTextParser = class {
* @private
*/
static parseCue_(
- cueNode, offset, rateInfo, metadataElements, styles, regionElements,
+ cueNode, timeContext, rateInfo, metadataElements, styles, regionElements,
cueRegions, whitespaceTrim, cellResolution, parentCueElement, isContent) {
/** @type {Element} */
let cueElement;
@@ -236,7 +236,7 @@ shaka.text.TtmlTextParser = class {
for (const childNode of cueElement.childNodes) {
const nestedCue = shaka.text.TtmlTextParser.parseCue_(
childNode,
- offset,
+ timeContext,
rateInfo,
metadataElements,
styles,
@@ -293,15 +293,20 @@ shaka.text.TtmlTextParser = class {
if (start == null) {
start = 0;
}
- start += offset;
+ start += timeContext.periodStart;
// If end is null, that means the duration is effectively infinite.
if (end == null) {
end = Infinity;
} else {
- end += offset;
+ end += timeContext.periodStart;
}
+ // Clip times to segment boundaries.
+ // https://github.com/shaka-project/shaka-player/issues/4631
+ start = Math.max(start, timeContext.segmentStart);
+ end = Math.min(end, timeContext.segmentEnd);
+
if (!hasTimeAttributes && nestedCues.length > 0) {
// If no time is defined for this cue, base the timing information on
// the time of the nested cues. In the case of multiple nested cues with
diff --git a/test/text/mp4_ttml_parser_unit.js b/test/text/mp4_ttml_parser_unit.js
index 3db52df745..a6160617b4 100644
--- a/test/text/mp4_ttml_parser_unit.js
+++ b/test/text/mp4_ttml_parser_unit.js
@@ -41,7 +41,8 @@ describe('Mp4TtmlParser', () => {
it('handles media segments with multiple mdats', () => {
const parser = new shaka.text.Mp4TtmlParser();
parser.parseInit(ttmlInitSegment);
- const time = {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0};
+ const time =
+ {periodStart: 0, segmentStart: 0, segmentEnd: 60, vttOffset: 0};
const ret = parser.parseMedia(ttmlSegmentMultipleMDAT, time);
// Bodies.
expect(ret.length).toBe(2);
@@ -55,9 +56,9 @@ describe('Mp4TtmlParser', () => {
it('accounts for offset', () => {
const time1 =
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0};
+ {periodStart: 0, segmentStart: 0, segmentEnd: 70, vttOffset: 0};
const time2 =
- {periodStart: 7, segmentStart: 0, segmentEnd: 0, vttOffset: 7};
+ {periodStart: 7, segmentStart: 0, segmentEnd: 70, vttOffset: 7};
const parser = new shaka.text.Mp4TtmlParser();
parser.parseInit(ttmlInitSegment);
@@ -161,7 +162,8 @@ describe('Mp4TtmlParser', () => {
];
const parser = new shaka.text.Mp4TtmlParser();
parser.parseInit(ttmlInitSegment);
- const time = {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0};
+ const time =
+ {periodStart: 0, segmentStart: 0, segmentEnd: 60, vttOffset: 0};
const result = parser.parseMedia(ttmlSegment, time);
shaka.test.TtmlUtils.verifyHelper(
cues, result, {startTime: 23, endTime: 53.5});
diff --git a/test/text/ttml_text_parser_unit.js b/test/text/ttml_text_parser_unit.js
index a0b09bb0fb..9e95bf9204 100644
--- a/test/text/ttml_text_parser_unit.js
+++ b/test/text/ttml_text_parser_unit.js
@@ -13,14 +13,14 @@ describe('TtmlTextParser', () => {
it('supports no cues', () => {
verifyHelper([],
'',
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0},
+ {periodStart: 0, segmentStart: 0, segmentEnd: 10, vttOffset: 0},
{});
});
it('supports empty text string', () => {
verifyHelper([],
'',
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0},
+ {periodStart: 0, segmentStart: 0, segmentEnd: 10, vttOffset: 0},
{});
});
@@ -28,7 +28,7 @@ describe('TtmlTextParser', () => {
verifyHelper(
[],
' \r\n
',
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0},
+ {periodStart: 0, segmentStart: 0, segmentEnd: 10, vttOffset: 0},
{});
});
@@ -55,7 +55,7 @@ describe('TtmlTextParser', () => {
},
],
'' + ttBody + '',
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0},
+ {periodStart: 0, segmentStart: 60, segmentEnd: 70, vttOffset: 0},
{startTime: 62.03, endTime: 62.05});
// When xml:space="preserve", take them into account.
verifyHelper(
@@ -81,7 +81,7 @@ describe('TtmlTextParser', () => {
},
],
'' + ttBody + '',
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0},
+ {periodStart: 0, segmentStart: 60, segmentEnd: 70, vttOffset: 0},
{startTime: 62.03, endTime: 62.05});
// The default value for xml:space is "default".
verifyHelper(
@@ -97,7 +97,7 @@ describe('TtmlTextParser', () => {
},
],
'' + ttBody + '',
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0},
+ {periodStart: 0, segmentStart: 60, segmentEnd: 70, vttOffset: 0},
{startTime: 62.03, endTime: 62.05});
// Any other value is rejected as an error.
@@ -128,7 +128,7 @@ describe('TtmlTextParser', () => {
},
],
'' + ttBody + '',
- {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0},
+ {periodStart: 0, segmentStart: 60, segmentEnd: 70, vttOffset: 0},
{startTime: 62.03, endTime: 62.05});
});
@@ -198,7 +198,7 @@ describe('TtmlTextParser', () => {
'First cue
Second cue' +
'
' +
'' +
'