-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Add canPlayType method to player #2709
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -424,6 +424,19 @@ class Tech extends Component { | |
*/ | ||
setPoster() {} | ||
|
||
/* | ||
* Check if the tech can support the given type | ||
* | ||
* The base tech does not support any type, but source handlers might | ||
* overwrite this. | ||
* | ||
* @param {String} type The mimetype to check | ||
* @return {String} 'probably', 'maybe', or '' (empty string) | ||
*/ | ||
canPlayType() { | ||
return ''; | ||
} | ||
|
||
} | ||
|
||
/* | ||
|
@@ -498,6 +511,26 @@ Tech.withSourceHandlers = function(_Tech){ | |
handlers.splice(index, 0, handler); | ||
}; | ||
|
||
/* | ||
* Check if the tech can support the given type | ||
* @param {String} type The mimetype to check | ||
* @return {String} 'probably', 'maybe', or '' (empty string) | ||
*/ | ||
_Tech.canPlayType = function(type){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means that only sourcehandlers will get the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah totally overlooked that; will add an empty function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should return an empty string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's what I mean |
||
let handlers = _Tech.sourceHandlers || []; | ||
let can; | ||
|
||
for (let i = 0; i < handlers.length; i++) { | ||
can = handlers[i].canPlayType(type); | ||
|
||
if (can) { | ||
return can; | ||
} | ||
} | ||
|
||
return ''; | ||
}; | ||
|
||
/* | ||
* Return the first source handler that supports the source | ||
* TODO: Answer question: should 'probably' be prioritized over 'maybe' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the tech's
canPlayType
check whether it is supported itself?I guess the downside is that you won't know whether it can't because it isn't supported or because it can't play that type if you ask the tech directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also kinda copied this from
selectSource
, to keep them "in sync".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping them in sync might not be a bad idea. Not certain which I like more.
Do you have a strong preference, @nickygerritsen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No not really, although maybe when we want to add support for #2705 that it would be better to add the
isSupported
call inside the tech. Otherwise we need to duplicate that again there?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, thinking about it some more, I think I like keeping the
isSupported
out ofcanPlayType
. MakescanPlayType
super simple and do exactly one thing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK then an update is incoming in 3...