-
Notifications
You must be signed in to change notification settings - Fork 18
Description
I want to try other playback implementations (like hls.js) to explore features in some TV models, but the strategy picker code is restricted to the current implemented strategies.
So, instead of opening a PR adding support for N playback implementations, it's healthier to support custom playback strategies. Following the contributing guidelines, I opened this issue to discuss it 🙃
One suggestion is to map a new playback strategy called customstrategy at PlaybackStrategy and add an option to propagate the path to the StrategyPicker to do something like:
function StrategyPicker(customStrategyPath) {
return new Promise((resolve, reject) => {
if (window.bigscreenPlayer.playbackStrategy === PlaybackStrategy.MSE) {
return import("./msestrategy")
.then(({ default: MSEStrategy }) => resolve(MSEStrategy))
.catch((reason) => {
const error = new Error(isError(reason) ? reason.message : undefined)
error.name = "StrategyDynamicLoadError"
reject(error)
})
} else if (window.bigscreenPlayer.playbackStrategy === PlaybackStrategy.CUSTOM && customStrategyPath !== "") {
return import(customStrategyPath)
.then(({ default: CustomStrategy }) => resolve(CustomStrategy))
.catch((reason) => {
const error = new Error(isError(reason) ? reason.message : undefined)
error.name = "StrategyDynamicLoadError"
reject(error)
})
} else if (window.bigscreenPlayer.playbackStrategy === PlaybackStrategy.BASIC) {
return resolve(BasicStrategy)
}
return resolve(NativeStrategy)
})
}Another possibility is to force the custom playback strategy export one expected name (like CustomStrategy), avoiding the new option.
Those suggestions have the drawback of only supporting one instance of a custom playback strategy, but resolve my initial tests 🙃