fix(text): Fix webvtt offset in sequence mode (#3955)

When running in sequence mode, we ignore the normal timestamps of video and audio segments. This lead to problems in some Apple- encoded webvtt content, which used the X-TIMESTAMP-MAP tag to account for the timestamp offsets in their video. Thus, those subtitles would end up 10 seconds offset. This changes the webvtt parser to ignore the X-TIMESTAMP-MAP when in sequence mode. Issue #2337
shaka-project · Feb 16, 2022 · a4e9267 · a4e9267
1 parent e49c849
commit a4e9267
Show file tree

Hide file tree

Showing 14 changed files with 108 additions and 11 deletions.
diff --git a/externs/shaka/text.js b/externs/shaka/text.js
@@ -426,6 +426,13 @@ shaka.extern.TextParser = class {
    * @exportDoc
    */
   parseMedia(data, timeContext) {}
+
+  /**
+   * Notifies the stream if the manifest is in sequence mode or not.
+   *
+   * @param {boolean} sequenceMode
+   */
+  setSequenceMode(sequenceMode) {}
 };
 
 

diff --git a/lib/media/media_source_engine.js b/lib/media/media_source_engine.js
@@ -332,7 +332,7 @@ shaka.media.MediaSourceEngine = class {
       let mimeType = shaka.util.MimeUtils.getFullType(
           stream.mimeType, stream.codecs);
       if (contentType == ContentType.TEXT) {
-        this.reinitText(mimeType);
+        this.reinitText(mimeType, sequenceMode);
       } else {
         if ((forceTransmuxTS || !MediaSource.isTypeSupported(mimeType)) &&
             shaka.media.Transmuxer.isSupported(mimeType, contentType)) {
@@ -361,12 +361,13 @@ shaka.media.MediaSourceEngine = class {
   /**
    * Reinitialize the TextEngine for a new text type.
    * @param {string} mimeType
+   * @param {boolean} sequenceMode
    */
-  reinitText(mimeType) {
+  reinitText(mimeType, sequenceMode) {
     if (!this.textEngine_) {
       this.textEngine_ = new shaka.text.TextEngine(this.textDisplayer_);
     }
-    this.textEngine_.initParser(mimeType);
+    this.textEngine_.initParser(mimeType, sequenceMode);
   }
 
   /**
@@ -535,7 +536,7 @@ shaka.media.MediaSourceEngine = class {
       // For HLS CEA-608/708 CLOSED-CAPTIONS, text data is embedded in
       // the video stream, so textEngine may not have been initialized.
       if (!this.textEngine_) {
-        this.reinitText('text/vtt');
+        this.reinitText('text/vtt', sequenceMode || false);
       }
 
       if (transmuxedData.metadata) {
@@ -562,7 +563,7 @@ shaka.media.MediaSourceEngine = class {
           contentType, () => this.append_(contentType, transmuxedSegment));
     } else if (hasClosedCaptions) {
       if (!this.textEngine_) {
-        this.reinitText('text/vtt');
+        this.reinitText('text/vtt', sequenceMode || false);
       }
       // If it is the init segment for closed captions, initialize the closed
       // caption parser.

diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js
@@ -234,7 +234,8 @@ shaka.media.StreamingEngine = class {
 
     const mimeType = shaka.util.MimeUtils.getFullType(
         stream.mimeType, stream.codecs);
-    this.playerInterface_.mediaSourceEngine.reinitText(mimeType);
+    this.playerInterface_.mediaSourceEngine.reinitText(
+        mimeType, this.manifest_.sequenceMode);
 
     const textDisplayer =
         this.playerInterface_.mediaSourceEngine.getTextDisplayer();
@@ -428,7 +429,8 @@ shaka.media.StreamingEngine = class {
       // init segment again.
       const fullMimeType = shaka.util.MimeUtils.getFullType(
           stream.mimeType, stream.codecs);
-      this.playerInterface_.mediaSourceEngine.reinitText(fullMimeType);
+      this.playerInterface_.mediaSourceEngine.reinitText(
+          fullMimeType, this.manifest_.sequenceMode);
     }
 
     // Releases the segmentIndex of the old stream.

diff --git a/lib/text/lrc_text_parser.js b/lib/text/lrc_text_parser.js
@@ -28,6 +28,14 @@ shaka.text.LrcTextParser = class {
     goog.asserts.assert(false, 'LRC does not have init segments');
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    // Unused.
+  }
+
   /**
    * @override
    * @export

diff --git a/lib/text/mp4_ttml_parser.js b/lib/text/mp4_ttml_parser.js
@@ -55,6 +55,14 @@ shaka.text.Mp4TtmlParser = class {
     }
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    // Unused.
+  }
+
   /**
    * @override
    * @export

diff --git a/lib/text/mp4_vtt_parser.js b/lib/text/mp4_vtt_parser.js
@@ -84,6 +84,14 @@ shaka.text.Mp4VttParser = class {
     }
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    // Unused.
+  }
+
   /**
    * @override
    * @export

diff --git a/lib/text/sbv_text_parser.js b/lib/text/sbv_text_parser.js
@@ -27,6 +27,14 @@ shaka.text.SbvTextParser = class {
     goog.asserts.assert(false, 'SubViewer does not have init segments');
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    // Unused.
+  }
+
   /**
    * @override
    * @export

diff --git a/lib/text/srt_text_parser.js b/lib/text/srt_text_parser.js
@@ -35,6 +35,14 @@ shaka.text.SrtTextParser = class {
     goog.asserts.assert(false, 'SRT does not have init segments');
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    // Unused.
+  }
+
   /**
    * @override
    * @export

diff --git a/lib/text/ssa_text_parser.js b/lib/text/ssa_text_parser.js
@@ -28,6 +28,14 @@ shaka.text.SsaTextParser = class {
     goog.asserts.assert(false, 'SSA does not have init segments');
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    // Unused.
+  }
+
   /**
    * @override
    * @export

diff --git a/lib/text/text_engine.js b/lib/text/text_engine.js
@@ -7,6 +7,7 @@
 goog.provide('shaka.text.TextEngine');
 
 goog.require('goog.asserts');
+goog.require('shaka.log');
 goog.require('shaka.text.Cue');
 goog.require('shaka.util.BufferUtils');
 goog.require('shaka.util.Functional');
@@ -128,8 +129,9 @@ shaka.text.TextEngine = class {
    * called at least once before appendBuffer.
    *
    * @param {string} mimeType
+   * @param {boolean} sequenceMode
    */
-  initParser(mimeType) {
+  initParser(mimeType, sequenceMode) {
     // No parser for CEA, which is extracted from video and side-loaded
     // into TextEngine and TextDisplayer.
     if (mimeType == shaka.util.MimeUtils.CEA608_CLOSED_CAPTION_MIMETYPE ||
@@ -141,6 +143,12 @@ shaka.text.TextEngine = class {
     goog.asserts.assert(
         factory, 'Text type negotiation should have happened already');
     this.parser_ = shaka.util.Functional.callFactory(factory);
+    if (this.parser_.setSequenceMode) {
+      this.parser_.setSequenceMode(sequenceMode);
+    } else {
+      shaka.log.alwaysWarn(
+          'Text parsers should have a "setSequenceMode" method!');
+    }
   }
 
   /**

diff --git a/lib/text/ttml_text_parser.js b/lib/text/ttml_text_parser.js
@@ -30,6 +30,14 @@ shaka.text.TtmlTextParser = class {
     goog.asserts.assert(false, 'TTML does not have init segments');
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    // Unused.
+  }
+
   /**
    * @override
    * @export

diff --git a/lib/text/vtt_text_parser.js b/lib/text/vtt_text_parser.js
@@ -22,6 +22,12 @@ goog.require('shaka.util.XmlUtils');
  * @export
  */
 shaka.text.VttTextParser = class {
+  /** Constructs a VTT parser. */
+  constructor() {
+    /** @private {boolean} */
+    this.sequenceMode_ = false;
+  }
+
   /**
    * @override
    * @export
@@ -30,6 +36,14 @@ shaka.text.VttTextParser = class {
     goog.asserts.assert(false, 'VTT does not have init segments');
   }
 
+  /**
+   * @override
+   * @export
+   */
+  setSequenceMode(sequenceMode) {
+    this.sequenceMode_ = sequenceMode;
+  }
+
   /**
    * @override
    * @export
@@ -52,7 +66,11 @@ shaka.text.VttTextParser = class {
     // It is no longer closely tied to periods, but the name stuck around.
     let offset = time.periodStart;
 
-    if (blocks[0].includes('X-TIMESTAMP-MAP')) {
+    // Do not honor the 'X-TIMESTAMP-MAP' value when in sequence mode.
+    // That is because it is used mainly (solely?) to account for the timestamp
+    // offset of the video/audio; when in sequence mode, we normalize that
+    // timestamp offset to 0, so we should not account for it.
+    if (blocks[0].includes('X-TIMESTAMP-MAP') && !this.sequenceMode_) {
       // https://bit.ly/2K92l7y
       // The 'X-TIMESTAMP-MAP' header is used in HLS to align text with
       // the rest of the media.

diff --git a/test/media/media_source_engine_unit.js b/test/media/media_source_engine_unit.js
@@ -1120,7 +1120,7 @@ describe('MediaSourceEngine', () => {
     });
 
     it('destroys text engines', async () => {
-      mediaSourceEngine.reinitText('text/vtt');
+      mediaSourceEngine.reinitText('text/vtt', false);
 
       await mediaSourceEngine.destroy();
       expect(mockTextEngine).toBeTruthy();

diff --git a/test/text/text_engine_unit.js b/test/text/text_engine_unit.js
@@ -24,6 +24,9 @@ describe('TextEngine', () => {
   /** @type {!jasmine.Spy} */
   let mockParseInit;
 
+  /** @type {!jasmine.Spy} */
+  let mockSetSequenceMode;
+
   /** @type {!jasmine.Spy} */
   let mockParseMedia;
 
@@ -32,11 +35,13 @@ describe('TextEngine', () => {
 
   beforeEach(() => {
     mockParseInit = jasmine.createSpy('mockParseInit');
+    mockSetSequenceMode = jasmine.createSpy('mockSetSequenceMode');
     mockParseMedia = jasmine.createSpy('mockParseMedia');
     // eslint-disable-next-line no-restricted-syntax
     mockParserPlugIn = function() {
       return {
         parseInit: mockParseInit,
+        setSequenceMode: mockSetSequenceMode,
         parseMedia: mockParseMedia,
       };
     };
@@ -46,7 +51,7 @@ describe('TextEngine', () => {
 
     TextEngine.registerParser(dummyMimeType, mockParserPlugIn);
     textEngine = new TextEngine(mockDisplayer);
-    textEngine.initParser(dummyMimeType);
+    textEngine.initParser(dummyMimeType, false);
   });
 
   afterEach(() => {