Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dorska/quality selector #556

Merged
merged 6 commits into from
Apr 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
"victory": "^0.25.6",
"video.js": "^5.20.1",
"videojs-contrib-hls": "^5.8.3",
"videojs-contrib-quality-levels": "^2.0.4",
"videojs-resolution-switcher": "^0.4.2",
"videojs-youtube": "^2.4.1",
"webpack": "^3.2.0",
Expand Down
29 changes: 17 additions & 12 deletions static/js/components/VideoPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const makeConfigForVideo = (
]
: video.sources,
plugins: {
hlsQualitySelector: {},
videoJsResolutionSwitcher: {
default: "high",
dynamicLabel: true
Expand Down Expand Up @@ -294,17 +295,21 @@ class VideoPlayer extends React.Component<*, void> {
if (this.player.tech_.currentTime() < 10) {
return _.last(playlists)
}
// Return playlist with highest bandwidth <= system bandwidth
return _.last(
R.filter(
rep =>
rep.attributes.BANDWIDTH <=
_.max([
this.player.tech_.hls.systemBandwidth,
playlists[0].attributes.BANDWIDTH
]),
playlists
)
// Return active playlist with highest bandwidth <= system bandwidth,
// or first active playlist otherwise.
const activePlaylists = R.filter((rep) => !rep.disabled, playlists)
return (
_.last(
R.filter(
((rep) => {
return rep.attributes.BANDWIDTH <= _.max([
this.player.tech_.hls.systemBandwidth,
playlists[0].attributes.BANDWIDTH
])
}),
activePlaylists
)
) || activePlaylists[0]
)
}

Expand All @@ -314,14 +319,14 @@ class VideoPlayer extends React.Component<*, void> {
const cropVideo = this.cropVideo
const createEventHandler = this.createEventHandler
const toggleFullscreen = this.toggleFullscreen
const selectPlaylist = this.selectPlaylist
if (video.multiangle) {
videojs.getComponent(
"FullscreenToggle"
).prototype.handleClick = toggleFullscreen
}
const useYouTube = video.is_public && video.youtube_id !== null
this.lastMinuteTracked = null
const selectPlaylist = this.selectPlaylist.bind(this)
this.player = videojs(
this.videoNode,
makeConfigForVideo(video, useYouTube, embed),
Expand Down
78 changes: 68 additions & 10 deletions static/js/components/VideoPlayer_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe("VideoPlayer", () => {
},
playbackRates: [0.5, 0.75, 1.0, 1.25, 1.5, 2.0, 4.0],
plugins: {
hlsQualitySelector: {},
videoJsResolutionSwitcher: {
default: "high",
dynamicLabel: true
Expand Down Expand Up @@ -146,7 +147,7 @@ describe("VideoPlayer", () => {
})
;[false, true].forEach(function(embed) {
it("video element is rendered with the correct style attributes", () => {
const wrapper = renderPlayer({embed}).find("VideoPlayer")
const wrapper = renderPlayer({ embed }).find("VideoPlayer")
const videoProps = wrapper.find("video").props()
assert.equal(
videoProps.className,
Expand Down Expand Up @@ -321,9 +322,44 @@ describe("VideoPlayer", () => {
}
})
})
;[5, 15].forEach(videoTime => {
[1000, 2000, 3000, 4000].forEach(bandwidth => {
it(`Returns correct playlist if elapsed time is ${videoTime} secs, bandwidth is ${bandwidth}`, () => {

describe("selectPlaylist", () => {
describe("when elapsed time is < 10 seconds", () => {
[1000, 2000, 3000, 4000].forEach(bandwidth => {
it(`Returns correct playlist if bandwidth is ${bandwidth}`, () => {
const videoTime = 5
playerStub.tech_ = {
currentTime: sandbox.stub().returns(videoTime),
hls: {
selectPlaylist: sandbox.stub(),
playlists: {
master: {
playlists: [
{ attributes: { BANDWIDTH: 900 } },
{ attributes: { BANDWIDTH: 1900 } },
{ attributes: { BANDWIDTH: 2900 } },
{ attributes: { BANDWIDTH: 3900 } }
]
}
},
systemBandwidth: bandwidth
}
}
const wrapper = renderPlayer().find("VideoPlayer")
wrapper.instance().player = playerStub
const bestPlayList = wrapper.instance().selectPlaylist()
assert.equal(
bestPlayList.attributes.BANDWIDTH,
videoTime < 10 ? 3900 : bandwidth - 100
)
})
})
})

describe("when elapsed time is > 10 secs", () => {
const videoTime = 11

it("selects highest active playlist <= system bandwidth", () => {
playerStub.tech_ = {
currentTime: sandbox.stub().returns(videoTime),
hls: {
Expand All @@ -332,25 +368,47 @@ describe("VideoPlayer", () => {
master: {
playlists: [
{ attributes: { BANDWIDTH: 900 } },
{ attributes: { BANDWIDTH: 1900 } },
{ attributes: { BANDWIDTH: 1900 }, disabled: true },
{ attributes: { BANDWIDTH: 2900 } },
{ attributes: { BANDWIDTH: 3900 } }
]
}
},
systemBandwidth: bandwidth
systemBandwidth: 2000,
}
}
const wrapper = renderPlayer().find("VideoPlayer")
wrapper.instance().player = playerStub
const bestPlayList = wrapper.instance().selectPlaylist()
assert.equal(
bestPlayList.attributes.BANDWIDTH,
videoTime < 10 ? 3900 : bandwidth - 100
)
assert.equal(bestPlayList.attributes.BANDWIDTH, 900)
})

it("selects lowest playlist if no active playlist <= system bandwidth", () => {
playerStub.tech_ = {
currentTime: sandbox.stub().returns(videoTime),
hls: {
selectPlaylist: sandbox.stub(),
playlists: {
master: {
playlists: [
{ attributes: { BANDWIDTH: 900 }, disabled: true},
{ attributes: { BANDWIDTH: 1900 }, disabled: true },
{ attributes: { BANDWIDTH: 2900 } },
{ attributes: { BANDWIDTH: 3900 } }
]
}
},
systemBandwidth: 2000,
}
}
const wrapper = renderPlayer().find("VideoPlayer")
wrapper.instance().player = playerStub
const bestPlayList = wrapper.instance().selectPlaylist()
assert.equal(bestPlayList.attributes.BANDWIDTH, 2900)
})
})
})

;[false, true].forEach(function(isPublic) {
["asdJ4y", null].forEach(function(youtubeId) {
it(`checkYouTube ${expect(
Expand Down
5 changes: 5 additions & 0 deletions static/js/lib/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@ import type { Video, VideoFile } from "../flow/videoTypes"

import _videojs from "video.js"
import { makeVideoFileName, makeVideoFileUrl } from "./urls"
import hlsQualitySelector from "./videojs_hls_quality_selector"

// For this to work properly videojs must be available as a global
global.videojs = _videojs
require("videojs-contrib-hls")
require("videojs-contrib-quality-levels")
require("videojs-resolution-switcher")
require("videojs-youtube")

_videojs.plugin("hlsQualitySelector", hlsQualitySelector)

// export here to allow mocking of videojs function
export const videojs = _videojs

Expand Down
135 changes: 135 additions & 0 deletions static/js/lib/videojs_hls_quality_selector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/* Adapted from https://github.com/chrisboustead/videojs-hls-quality-selector .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to bring in/adapt some of the unit tests for this plugin too, to improve coverage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure on whether this would be worth it. The original plugin just tests that it can register against videojs:
https://github.com/chrisboustead/videojs-hls-quality-selector/blob/master/test/plugin.test.js

We could write tests, but I think it would mostly look like a lot of mocking for videojs functions.

Do you think it's worth pursuing? Either way is fine with me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok with me to skip it if it won't add much value.

* We use our own version rather than the original because
* the original structures its code in a way that makes it difficult to
* register the plugin;
* the original will register the plugin against its own videojs instance,
* from its own node_modules, rather than our videojs instance.
* This happens regardless of whether we 'require' or 'import' it.
* So, we use our own version instead.
*/

import videojs from "video.js"

const hlsQualitySelector = function() {
const player = this
player.ready(() => {
player.hlsQualitySelector = new HlsQualitySelectorPlugin(player)
})
}

class HlsQualitySelectorPlugin {
constructor(player) {
this.player = player
this.keyedMenuItems = {}
this.KEY_FOR_AUTO = "__AUTO__"
if (this.player.qualityLevels && this.getHls()) {
this.createQualityMenu()
this.bindPlayerEvents()
}
}

getHls() {
return this.player.tech({ IWillNotUseThisInPlugins: true }).hls
}

bindPlayerEvents() {
this.player
.qualityLevels()
.on("addqualitylevel", this.onAddQualityLevel.bind(this))
}

createQualityMenu() {
const player = this.player
const videoJsButtonClass = videojs.getComponent("MenuButton")
const concreteButtonClass = videojs.extend(videoJsButtonClass, {
constructor: function() {
videoJsButtonClass.call(this, player, {
title: player.localize("Quality")
})
},
createItems: function() {
return []
}
})

this._qualityMenuButton = new concreteButtonClass()

const placementIndex = player.controlBar.children().length - 2
const concreteButtonInstance = player.controlBar.addChild(
this._qualityMenuButton,
{ componentClass: "qualitySelector" },
placementIndex
)
concreteButtonInstance.addClass("vjs-quality-selector")
concreteButtonInstance.addClass("vjs-icon-hd")
concreteButtonInstance.removeClass("vjs-hidden")
}

createQualityMenuItem(item) {
const player = this.player
const videoJsMenuItemClass = videojs.getComponent("MenuItem")
const concreteMenuItemClass = videojs.extend(videoJsMenuItemClass, {
constructor: function() {
videoJsMenuItemClass.call(this, player, {
label: item.label,
selectable: true,
selected: item.selected || false
})
this.key = item.key
},
handleClick: () => {
this.selectQualityLevel({ key: item.key })
this._qualityMenuButton.unpressButton()
}
})
return new concreteMenuItemClass()
}

onAddQualityLevel() {
const player = this.player
const qualityList = player.qualityLevels()
const levels = qualityList.levels_ || []

const menuItems = []
for (let i = 0; i < levels.length; ++i) {
const menuItem = this.createQualityMenuItem.call(this, {
key: levels[i].id,
label: `${levels[i].height}p`,
value: levels[i].height
})
menuItems.push(menuItem)
}
menuItems.push(
this.createQualityMenuItem.call(this, {
key: this.KEY_FOR_AUTO,
label: "Auto",
value: "auto",
selected: true
})
)

if (this._qualityMenuButton) {
this._qualityMenuButton.createItems = () => menuItems
this._qualityMenuButton.update()
}

this.keyedMenuItems = {}
for (const menuItem of menuItems) {
this.keyedMenuItems[menuItem.key] = menuItem
}
}

selectQualityLevel({ key }) {
for (let i = 0; i < this._qualityMenuButton.items.length; ++i) {
this._qualityMenuButton.items[i].selected(false)
}
const qualityList = this.player.qualityLevels()
for (let i = 0; i < qualityList.length; ++i) {
const quality = qualityList[i]
quality.enabled = quality.id === key || key === this.KEY_FOR_AUTO
}
this.keyedMenuItems[key].selected(true)
}
}

export default hlsQualitySelector
1 change: 1 addition & 0 deletions webpack.config.shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ module.exports = {
extensions: ['.js', '.jsx'],
alias: {
'videojs-contrib-hls': path.resolve(__dirname, 'node_modules/videojs-contrib-hls/dist/videojs-contrib-hls.js'),
'videojs-contrib-quality-levels': path.resolve(__dirname, 'node_modules/videojs-contrib-quality-levels/dist/videojs-contrib-quality-levels.js'),
}
},
performance: {
Expand Down
Loading