fix: on dispose, don't call abort on SourceBuffer until after remove(…

…) has finished
videojs · Apr 3, 2019 · 3806750 · 3806750
1 parent fbd615c
commit 3806750
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 3 deletions.
diff --git a/src/mse/virtual-source-buffer.js b/src/mse/virtual-source-buffer.js
@@ -311,7 +311,7 @@ export default class VirtualSourceBuffer extends videojs.EventTarget {
             if (t === 'audio' && this.audioDisabled_) {
               return true;
             }
-            // if the other type if updating we don't trigger
+            // if the other type is updating we don't trigger
             if (type !== t &&
                 this[`${t}Buffer_`] &&
                 this[`${t}Buffer_`].updating) {

diff --git a/src/source-updater.js b/src/source-updater.js
@@ -71,6 +71,7 @@ export default class SourceUpdater {
       let pendingCallback = this.pendingCallback_;
 
       this.pendingCallback_ = null;
+      this.sourceBuffer_.removing = false;
 
       this.logger_(`buffered [${printableRange(this.buffered())}]`);
 
@@ -149,6 +150,7 @@ export default class SourceUpdater {
     if (this.processedAppend_) {
       this.queueCallback_(() => {
         this.logger_(`remove [${start} => ${end}]`);
+        this.sourceBuffer_.removing = true;
         this.sourceBuffer_.remove(start, end);
       }, done);
     }
@@ -208,9 +210,18 @@ export default class SourceUpdater {
    * dispose of the source updater and the underlying sourceBuffer
    */
   dispose() {
+    const disposeFn = () => {
+      if (this.sourceBuffer_ && this.mediaSource.readyState === 'open') {
+        this.sourceBuffer_.abort();
+      }
+      this.sourceBuffer_.removeEventListener('updateend', disposeFn);
+    };
+
     this.sourceBuffer_.removeEventListener('updateend', this.onUpdateendCallback_);
-    if (this.sourceBuffer_ && this.mediaSource.readyState === 'open') {
-      this.sourceBuffer_.abort();
+    if (this.sourceBuffer_.removing) {
+      this.sourceBuffer_.addEventListener('updateend', disposeFn);
+    } else {
+      disposeFn();
     }
   }
 }
diff --git a/test/source-updater.test.js b/test/source-updater.test.js
@@ -260,3 +260,25 @@ QUnit.test('supports timestampOffset', function(assert) {
   sourceBuffer.trigger('updateend');
   assert.equal(sourceBuffer.timestampOffset, 14, 'applied the update');
 });
+
+QUnit.test('abort on dispose waits until after a remove has finished', function(assert) {
+  let updater = new SourceUpdater(this.mediaSource, 'video/mp2t');
+  let sourceBuffer;
+
+  this.mediaSource.trigger('sourceopen');
+  updater.appendBuffer({
+    bytes: new Uint8Array([0])
+  }, () => {});
+
+  sourceBuffer = this.mediaSource.sourceBuffers[0];
+  sourceBuffer.trigger('updateend');
+  updater.remove(0, 10);
+  updater.dispose();
+
+  assert.deepEqual(sourceBuffer.updates_[1].remove, [0, 10], 'remove called');
+  assert.equal(sourceBuffer.updates_.length, 2, 'abort not called before updateend');
+
+  sourceBuffer.trigger('updateend');
+
+  assert.ok(sourceBuffer.updates_[2].abort, 'aborted the source buffer');
+});