search

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

Button does not appear if first video lacks multiple qualities #10

Closed stevendesu closed 4 years ago

stevendesu commented 5 years ago

I'm playing a pre-roll ad (using the IMA plugin) and then loading HLS live streaming content.

When the ad plays, SourceMenuButton.prototype.update gets called, which calls MenuButton.prototype.update, which sees that the number of items (0) is less than or equal to the threshold (0) and hides the button.

When the ad finishes and the content plays, SourceMenuButton.prototype.update is not called, and so the button is never displayed.

I'm not sure what the "appropriate" solution is to this since I don't have a firm grasp of VideoJS plugin development and best practice, but as a hack it can be solved by adding the following line to the constructor for SourceMenuButton:

player.on(["loadedmetadata"], this.update.bind(this));
stevendesu commented 5 years ago

Looking at the src/ directory (I forked the code to make my own version with my hack) I noticed the following comment:

    // Bind update to qualityLevels changes
    //qualityLevels.on(['change', 'addqualitylevel'], videojs.bind( this, this.update) );

Any reason this is commented out? This looks like it will do exactly what I want.

jfujita commented 5 years ago

@stevendesu I'm going to investigate this. I'm not sure if binding to addqualitylevel events might have some unintended consequences for things like livestreams or streams with changing sources.

jfujita commented 5 years ago

@stevendesu Your fix appears to effectively refresh the quality level options when changing video sources. Create a pull-request and I'll test and credit you with the fix.

stevendesu commented 4 years ago

After several months I finally got around to making this pull request, and I noticed that it looks like it was already handled by @leonklingele on October 7th 👍 (https://github.com/jfujita/videojs-http-source-selector/commit/12b35df9#diff-1d835642c7b9aa8015728c4667206a43R30)

I'll go ahead and close this issue, then.