jfujita / videojs-http-source-selector

VideoJS plugin that leverages videojs-contrib-quality-levels plugin to offer manual user-select able level selection options for adaptive http streams.
MIT License
62 stars 49 forks source link

Console error when selecting bitrate (race condition?) #11

Closed stevendesu closed 5 years ago

stevendesu commented 5 years ago

Sometimes when selecting a bitrate I received a console error that JavaScript could not access the property qualityLevels of null.

Tracing it I noticed that this occurred inside of the SourceMenuItem.prototype.handleClick method. At the top of the for-loop this.player() would return a player instance. At the bottom of the for-loop (after one or two iterations) this.player() would return null.

Given that all the loop is doing is calling this.player().qualityLevels()[i].enabled = <value>, this tells me that one of two things is happening:

  1. There is some asynchronous logic involved here and this.player() is getting updated between iterations of the loop (race condition)
  2. The setter for enabled is producing unexpected side effects

I'm not sure why the issue was occurring, but found it could be fixed by caching qualityLevels like so:

var qualityLevels = this.player().qualityLevels();
for (var i = 0; i < qualityLevels.length; i++) {
  //If this is the Auto option, enable all renditions for adaptive selection
  if (this.options_.index == qualityLevels.length) {
    qualityLevels[i].enabled = true;
  } else if (i == this.options_.index) {
    qualityLevels[i].enabled = true;
  } else {
    qualityLevels[i].enabled = false;
  }
}
jfujita commented 5 years ago

Thanks for the suggestion, your solution was already merged in from a prior PR: #2 along with other optimizations. Built artifacts should be available on npm

handleClick() {
    var selected = this.options_;
    console.log("Changing quality to:", selected.label);

    this.selected_ = true;
    this.selected(true);

    var levels = this.player().qualityLevels();
    for(var i = 0; i < levels.length; i++) {
      if (selected.index == levels.length) {
        // If this is the Auto option, enable all renditions for adaptive selection
        levels[i].enabled = true;
      } else if (selected.index == i) {
        levels[i].enabled = true;
      } else {
        levels[i].enabled = false;
      }
    }
  }