-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: Do not assume 1080p Cast devices, some are 720p #6562
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 |
---|---|---|
|
@@ -611,27 +611,32 @@ shaka.util.Platform = class { | |
|
||
if (Platform.isChromecast()) { | ||
// In our tests, the original Chromecast seems to have trouble decoding | ||
// above 1080p. It would be a waste to select a higher res anyway, given | ||
// above 1080p. It would be a waste to select a higher res anyway, given | ||
// that the device only outputs 1080p to begin with. | ||
// Chromecast has an extension to query the device/display's resolution. | ||
const hasCanDisplayType = window.cast && cast.__platform__ && | ||
cast.__platform__.canDisplayType; | ||
|
||
// Most Chromecasts can do 1080p. | ||
maxResolution.width = 1920; | ||
maxResolution.height = 1080; | ||
// Some hub devices can only do 720p. Default to that. | ||
maxResolution.width = 1280; | ||
maxResolution.height = 720; | ||
|
||
try { | ||
if (hasCanDisplayType && await cast.__platform__.canDisplayType( | ||
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. nit; Up to your preference, just in case you missed it - you might check the presence of 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. I tried it, but I think the extra layer of |
||
'video/mp4; codecs="avc1.640028"; width=3840; height=2160')) { | ||
// The device and display can both do 4k. Assume a 4k limit. | ||
// The device and display can both do 4k. Assume a 4k limit. | ||
maxResolution.width = 3840; | ||
maxResolution.height = 2160; | ||
} else if (hasCanDisplayType && await cast.__platform__.canDisplayType( | ||
'video/mp4; codecs="avc1.640028"; width=1920; height=1080')) { | ||
// Most Chromecasts can do 1080p. | ||
maxResolution.width = 1920; | ||
maxResolution.height = 1080; | ||
} | ||
} catch (error) { | ||
// This shouldn't generally happen. Log the error. | ||
// This shouldn't generally happen. Log the error. | ||
shaka.log.alwaysError('Failed to check canDisplayType:', error); | ||
// Now ignore the error and let the 1080p default stand. | ||
// Now ignore the error and let the 720p default stand. | ||
} | ||
} else if (Platform.isTizen()) { | ||
maxResolution.width = 1920; | ||
|
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.
Just a heads up. The physical resolution of Google Nest Hub is 1024 x 600. But I guess it can play 720p.
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.
It can decode 720p, and with the typical encoding ladder people use, you'd hate to have them play 480p instead.