From 4bf22e2359a840fe15c482de5e4671cfb72c14f0 Mon Sep 17 00:00:00 2001
From: Joey Parrish <joeyparrish@google.com>
Date: Wed, 12 Aug 2020 12:43:20 -0700
Subject: [PATCH] fix(VTT): Fix timing of VTT in HLS without map header

When there is no X-TIMESTAMP-MAP header, the HLS spec states that the
client should map 0 to 0, effectively.  That means we were wrong to
offset the cues by the segment time.

Since DASH IOP states that segmented text should be packaged in MP4,
this should not affect compliant DASH content.

Closes #2714

Change-Id: Iddb00f2fd1afeb4f0f2c99f92f65e5db0e3a84f1
---
 lib/text/vtt_text_parser.js       |  4 +++-
 test/text/cue_integration.js      |  2 +-
 test/text/vtt_text_parser_unit.js | 21 +++++----------------
 3 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/lib/text/vtt_text_parser.js b/lib/text/vtt_text_parser.js
index a6ff63c66e..cc3c08a784 100644
--- a/lib/text/vtt_text_parser.js
+++ b/lib/text/vtt_text_parser.js
@@ -48,7 +48,9 @@ shaka.text.VttTextParser = class {
           shaka.util.Error.Code.INVALID_TEXT_HEADER);
     }
 
-    let offset = time.segmentStart;
+    // NOTE: "periodStart" is the timestamp offset applied via TextEngine.
+    // It is no longer closely tied to periods, but the name stuck around.
+    let offset = time.periodStart;
 
     if (blocks[0].includes('X-TIMESTAMP-MAP')) {
       // https://bit.ly/2K92l7y
diff --git a/test/text/cue_integration.js b/test/text/cue_integration.js
index 0ffd7049cc..7845d714f1 100644
--- a/test/text/cue_integration.js
+++ b/test/text/cue_integration.js
@@ -19,7 +19,7 @@ describe('Cue', () => {
         'WEBVTT\n\n' +
         '00:00:20.000 --> 00:00:40.000\n' +
         'Test',
-        {periodStart: 0, segmentStart: 7, segmentEnd: 0});
+        {periodStart: 7, segmentStart: 10, segmentEnd: 60});
     expect(cues.length).toBe(1);
     expect(cues[0].startTime).toBe(27);
     expect(cues[0].endTime).toBe(47);
diff --git a/test/text/vtt_text_parser_unit.js b/test/text/vtt_text_parser_unit.js
index e459bee3bb..dee1c01bae 100644
--- a/test/text/vtt_text_parser_unit.js
+++ b/test/text/vtt_text_parser_unit.js
@@ -83,17 +83,6 @@ describe('VttTextParser', () => {
         {periodStart: 0, segmentStart: 0, segmentEnd: 0});
   });
 
-  it('ignores offset', () => {
-    verifyHelper(
-        [
-          {startTime: 20, endTime: 40, payload: 'Test'},
-        ],
-        'WEBVTT\n\n' +
-        '00:00:20.000 --> 00:00:40.000\n' +
-        'Test',
-        {periodStart: 0, segmentStart: 0, segmentEnd: 0});
-  });
-
   it('supports cues with no settings', () => {
     verifyHelper(
         [
@@ -407,12 +396,12 @@ describe('VttTextParser', () => {
         {periodStart: 0, segmentStart: 0, segmentEnd: 0});
   });
 
-  it('uses segment time', () => {
+  it('uses time offset from periodStart, not segmentStart', () => {
     verifyHelper(
         [
           {
-            startTime: 40, // Note these are 20s off of the cue
-            endTime: 60,   // because using relative timestamps
+            startTime: 70,
+            endTime: 80,
             payload: 'Test',
             textAlign: 'center',
             size: 56,
@@ -420,9 +409,9 @@ describe('VttTextParser', () => {
           },
         ],
         'WEBVTT\n\n' +
-        '0:00:20.000 --> 0:00:40.000 align:center size:56% vertical:lr\n' +
+        '0:00:10.000 --> 0:00:20.000 align:center size:56% vertical:lr\n' +
         'Test',
-        {periodStart: 0, segmentStart: 20, segmentEnd: 0});
+        {periodStart: 60, segmentStart: 80, segmentEnd: 100});
   });