fix: store transmuxQueue and currentTransmux on transmuxer instead of globally (#1045)

Prevents separate videos on the page or separate transmuxers from bottlenecking one another.

Author: brandonocasey

src/segment-loader.js

@@ -7,7 +7,6 @@ import Config from './config';
 import window from 'global/window';
 import { initSegmentId, segmentKeyId } from './bin-utils';
 import { mediaSegmentRequest, REQUEST_ERRORS } from './media-segment-request';
-import TransmuxWorker from 'worker!./transmuxer-worker.js';
 import segmentTransmuxer from './segment-transmuxer';
 import { TIME_FUDGE_FACTOR, timeUntilRebuffer as timeUntilRebuffer_ } from './ranges';
 import { minRebufferMaxBandwidthSelector } from './playlist-selectors';
@@ -530,7 +529,6 @@ export default class SegmentLoader extends videojs.EventTarget {
     };
 
     this.transmuxer_ = this.createTransmuxer_();
-
     this.triggerSyncInfoUpdate_ = () => this.trigger('syncinfoupdate');
     this.syncController_.on('syncinfoupdate', this.triggerSyncInfoUpdate_);
 
@@ -591,20 +589,13 @@ export default class SegmentLoader extends videojs.EventTarget {
   }
 
   createTransmuxer_() {
-    const transmuxer = new TransmuxWorker();
-
-    transmuxer.postMessage({
-      action: 'init',
-      options: {
-        parse708captions: this.parse708captions_,
-        remux: false,
-        alignGopsAtEnd: this.safeAppend_,
-        keepOriginalTimestamps: true,
-        handlePartialData: this.handlePartialData_
-      }
+    return segmentTransmuxer.createTransmuxer({
+      remux: false,
+      alignGopsAtEnd: this.safeAppend_,
+      keepOriginalTimestamps: true,
+      handlePartialData: this.handlePartialData_,
+      parse708captions: this.parse708captions_
     });
-
-    return transmuxer;
   }
 
   /**
@@ -632,9 +623,6 @@ export default class SegmentLoader extends videojs.EventTarget {
     this.abort_();
     if (this.transmuxer_) {
       this.transmuxer_.terminate();
-      // Although it isn't an instance of a class, the segment transmuxer must still be
-      // cleaned up.
-      segmentTransmuxer.dispose();
     }
     this.resetStats_();
 

src/segment-transmuxer.js

@@ -1,5 +1,4 @@
-const transmuxQueue = [];
-let currentTransmux;
+import TransmuxWorker from 'worker!./transmuxer-worker.js';
 
 export const handleData_ = (event, transmuxedData, callback) => {
   const {
@@ -66,30 +65,31 @@ export const handleGopInfo_ = (event, transmuxedData) => {
   transmuxedData.gopInfo = event.data.gopInfo;
 };
 
-export const processTransmux = ({
-  transmuxer,
-  bytes,
-  audioAppendStart,
-  gopsToAlignWith,
-  isPartial,
-  remux,
-  onData,
-  onTrackInfo,
-  onAudioTimingInfo,
-  onVideoTimingInfo,
-  onVideoSegmentTimingInfo,
-  onAudioSegmentTimingInfo,
-  onId3,
-  onCaptions,
-  onDone
-}) => {
+export const processTransmux = (options) => {
+  const {
+    transmuxer,
+    bytes,
+    audioAppendStart,
+    gopsToAlignWith,
+    isPartial,
+    remux,
+    onData,
+    onTrackInfo,
+    onAudioTimingInfo,
+    onVideoTimingInfo,
+    onVideoSegmentTimingInfo,
+    onAudioSegmentTimingInfo,
+    onId3,
+    onCaptions,
+    onDone
+  } = options;
   const transmuxedData = {
     isPartial,
     buffer: []
   };
 
   const handleMessage = (event) => {
-    if (!currentTransmux) {
+    if (transmuxer.currentTransmux !== options) {
       // disposed
       return;
     }
@@ -134,7 +134,7 @@ export const processTransmux = ({
     });
 
     /* eslint-disable no-use-before-define */
-    dequeue();
+    dequeue(transmuxer);
     /* eslint-enable */
   };
 
@@ -187,30 +187,30 @@ export const processTransmux = ({
   transmuxer.postMessage({ action: isPartial ? 'partialFlush' : 'flush' });
 };
 
-export const dequeue = () => {
-  currentTransmux = null;
-  if (transmuxQueue.length) {
-    currentTransmux = transmuxQueue.shift();
-    if (typeof currentTransmux === 'function') {
-      currentTransmux();
+export const dequeue = (transmuxer) => {
+  transmuxer.currentTransmux = null;
+  if (transmuxer.transmuxQueue.length) {
+    transmuxer.currentTransmux = transmuxer.transmuxQueue.shift();
+    if (typeof transmuxer.currentTransmux === 'function') {
+      transmuxer.currentTransmux();
     } else {
-      processTransmux(currentTransmux);
+      processTransmux(transmuxer.currentTransmux);
     }
   }
 };
 
 export const processAction = (transmuxer, action) => {
   transmuxer.postMessage({ action });
-  dequeue();
+  dequeue(transmuxer);
 };
 
 export const enqueueAction = (action, transmuxer) => {
-  if (!currentTransmux) {
-    currentTransmux = action;
+  if (!transmuxer.currentTransmux) {
+    transmuxer.currentTransmux = action;
     processAction(transmuxer, action);
     return;
   }
-  transmuxQueue.push(processAction.bind(null, transmuxer, action));
+  transmuxer.transmuxQueue.push(processAction.bind(null, transmuxer, action));
 };
 
 export const reset = (transmuxer) => {
@@ -222,23 +222,35 @@ export const endTimeline = (transmuxer) => {
 };
 
 export const transmux = (options) => {
-  if (!currentTransmux) {
-    currentTransmux = options;
+  if (!options.transmuxer.currentTransmux) {
+    options.transmuxer.currentTransmux = options;
     processTransmux(options);
     return;
   }
-  transmuxQueue.push(options);
+  options.transmuxer.transmuxQueue.push(options);
 };
 
-export const dispose = () => {
-  // clear out module-level references
-  currentTransmux = null;
-  transmuxQueue.length = 0;
+export const createTransmuxer = (options) => {
+  const transmuxer = new TransmuxWorker();
+
+  transmuxer.currentTransmux = null;
+  transmuxer.transmuxQueue = [];
+  const term = transmuxer.terminate;
+
+  transmuxer.terminate = () => {
+    transmuxer.currentTransmux = null;
+    transmuxer.transmuxQueue.length = 0;
+    return term.call(transmuxer);
+  };
+
+  transmuxer.postMessage({action: 'init', options});
+
+  return transmuxer;
 };
 
 export default {
   reset,
-  dispose,
   endTimeline,
-  transmux
+  transmux,
+  createTransmuxer
 };

test/media-segment-request.test.js

@@ -8,9 +8,8 @@ import {
   standardXHRResponse,
   downloadProgress
 } from './test-helpers';
-import TransmuxWorker from 'worker!../src/transmuxer-worker.js';
+import {createTransmuxer as createTransmuxer_} from '../src/segment-transmuxer.js';
 import Decrypter from 'worker!../src/decrypter-worker.js';
-import {dispose as segmentTransmuxerDispose} from '../src/segment-transmuxer.js';
 import {
   aacWithoutId3 as aacWithoutId3Segment,
   aacWithId3 as aacWithId3Segment,
@@ -71,18 +70,11 @@ const sharedHooks = {
     };
 
     this.createTransmuxer = (isPartial) => {
-      const transmuxer = new TransmuxWorker();
-
-      transmuxer.postMessage({
-        action: 'init',
-        options: {
-          remux: false,
-          keepOriginalTimestamps: true,
-          handlePartialData: isPartial
-        }
+      return createTransmuxer_({
+        remux: false,
+        keepOriginalTimestamps: true,
+        handlePartialData: isPartial
       });
-
-      return transmuxer;
     };
   },
   afterEach(assert) {
@@ -92,9 +84,6 @@ const sharedHooks = {
     if (this.transmuxer) {
       this.transmuxer.terminate();
     }
-
-    // clear current transmux on segment transmuxer
-    segmentTransmuxerDispose();
   }
 
 };

test/segment-loader.test.js

@@ -9,7 +9,6 @@ import {
   mediaDuration,
   getTroublesomeSegmentDurationMessage
 } from '../src/segment-loader';
-import segmentTransmuxer from '../src/segment-transmuxer';
 import videojs from 'video.js';
 import mp4probe from 'mux.js/lib/mp4/probe';
 import {
@@ -2381,29 +2380,23 @@ QUnit.module('SegmentLoader', function(hooks) {
     QUnit.test('dispose cleans up transmuxer', function(assert) {
       return setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {
         loader.playlist(playlistWithDuration(20));
+        const transmuxer = loader.transmuxer_;
 
-        const origTransmuxerTerminate =
-          loader.transmuxer_.terminate.bind(loader.transmuxer_);
+        const origTransmuxerTerminate = transmuxer.terminate.bind(transmuxer);
         let transmuxerTerminateCount = 0;
-        const origSegmentTransmuxerDispose =
-          segmentTransmuxer.dispose.bind(segmentTransmuxer);
-        let segmentTransmuxerDisposeCalls = 0;
 
-        loader.transmuxer_.terminate = () => {
+        transmuxer.terminate = () => {
           transmuxerTerminateCount++;
           origTransmuxerTerminate();
         };
-        segmentTransmuxer.dispose = () => {
-          origSegmentTransmuxerDispose();
-          segmentTransmuxerDisposeCalls++;
-        };
 
         loader.load();
         this.clock.tick(1);
         loader.dispose();
 
         assert.equal(transmuxerTerminateCount, 1, 'terminated transmuxer');
-        assert.equal(segmentTransmuxerDisposeCalls, 1, 'disposed segment transmuxer');
+        assert.ok(!transmuxer.currentTransmux, 'no current transmux');
+        assert.equal(transmuxer.transmuxQueue.length, 0, 'no queue');
       });
     });
 

test/segment-transmuxer.test.js

@@ -1,6 +1,5 @@
 import QUnit from 'qunit';
 import sinon from 'sinon';
-import TransmuxWorker from 'worker!../src/transmuxer-worker.js';
 import {
   muxed as muxedSegment,
   caption as captionSegment,
@@ -17,32 +16,27 @@ import {
   handleGopInfo_,
   handleDone_,
   handleData_,
-  dispose
+  createTransmuxer as createTransmuxer_
 } from '../src/segment-transmuxer';
 // needed for plugin registration
 import '../src/videojs-http-streaming';
 
 const noop = () => {};
 
 const createTransmuxer = (isPartial) => {
-  const transmuxer = new TransmuxWorker();
-
-  transmuxer.postMessage({
-    action: 'init',
-    options: {
-      remux: false,
-      keepOriginalTimestamps: true,
-      handlePartialData: isPartial
-    }
+  return createTransmuxer_({
+    remux: false,
+    keepOriginalTimestamps: true,
+    handlePartialData: isPartial
   });
-
-  return transmuxer;
 };
 
 const mockTransmuxer = (isPartial) => {
   const transmuxer = {
     postMessage(event) {},
-    terminate() {}
+    terminate() {},
+    currentTransmux: null,
+    transmuxQueue: []
   };
 
   return transmuxer;
@@ -54,7 +48,6 @@ QUnit.module('Segment Transmuxer', {
     assert.timeout(5000);
   },
   afterEach(assert) {
-    dispose();
     if (this.transmuxer) {
       this.transmuxer.terminate();
     }
@@ -240,7 +233,7 @@ QUnit.test('dequeues and processes action on dequeue()', function(assert) {
     'the transmux() posted `flush` to the transmuxer'
   );
 
-  dequeue();
+  dequeue(this.transmuxer);
   assert.deepEqual(this.transmuxer.postMessage.callCount, 2, 'two actions processed');
   assert.deepEqual(
     this.transmuxer.postMessage.args[1][0],

test/transmuxer-worker.test.js

@@ -1,5 +1,5 @@
 import QUnit from 'qunit';
-import TransmuxWorker from 'worker!../src/transmuxer-worker.js';
+import {createTransmuxer as createTransmuxer_} from '../src/segment-transmuxer.js';
 import {
   mp4Captions as mp4CaptionsSegment,
   muxed as muxedSegment,
@@ -10,18 +10,11 @@ import {
 import '../src/videojs-http-streaming';
 
 const createTransmuxer = (isPartial) => {
-  const transmuxer = new TransmuxWorker();
-
-  transmuxer.postMessage({
-    action: 'init',
-    options: {
-      remux: false,
-      keepOriginalTimestamps: true,
-      handlePartialData: isPartial
-    }
+  return createTransmuxer_({
+    remux: false,
+    keepOriginalTimestamps: true,
+    handlePartialData: isPartial
   });
-
-  return transmuxer;
 };
 
 // The final done message from the Transmux worker