-
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
Check the output of mediacapabilities with MediaSource #3897
Conversation
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.
Thanks for identifying the root of this issue and for sending a fix!
lib/util/stream_utils.js
Outdated
return false; | ||
} | ||
} | ||
videoCodecs = shaka.util.StreamUtils.patchVp9(videoCodecs); |
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.
For multiplexed streams, this was already done in the branch above. Should this section be in an "else" branch for non-multiplexed only?
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.
The If only applies to audio content, the else is not necessary because it is always needed for video, anyway I have seen that something was being done wrong and duplicated.
lib/util/stream_utils.js
Outdated
if (audio) { | ||
const codecs = | ||
(audio.codecs.toLowerCase() == 'ac-3' && | ||
shaka.util.Platform.isTizen()) ? 'ec-3' : audio.codecs; |
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.
I assume this Tizen-specific logic must already exist in another location, so I think it would be best to encapsulate it in a method somewhere.
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.
Done!
The proposed solution checks that all variants, apart from being checked in mediaCapabilities.decodingInfo, are also checked with MediaSource.isTypeSupported There are some browser implementations that return a true mediaCapabilities.decodingInfo for unknown mimetypes and codecs and that can cause an application-level problem. For example, EDGE on Windows 10 returns true to HEVC even if you don't have the extension installed to support HEVC, instead MediaSource.isTypeSupported does take this into account Fixes: #3860
The proposed solution checks that all variants, apart from being checked in mediaCapabilities.decodingInfo, are also checked with MediaSource.isTypeSupported There are some browser implementations that return a true mediaCapabilities.decodingInfo for unknown mimetypes and codecs and that can cause an application-level problem. For example, EDGE on Windows 10 returns true to HEVC even if you don't have the extension installed to support HEVC, instead MediaSource.isTypeSupported does take this into account Fixes: #3860
The proposed solution checks that all variants, apart from being checked in mediaCapabilities.decodingInfo, are also checked with MediaSource.isTypeSupported There are some browser implementations that return a true mediaCapabilities.decodingInfo for unknown mimetypes and codecs and that can cause an application-level problem. For example, EDGE on Windows 10 returns true to HEVC even if you don't have the extension installed to support HEVC, instead MediaSource.isTypeSupported does take this into account Fixes: #3860
Fixes: #3860
The proposed solution checks that all variants, apart from being checked in
mediaCapabilities.decodingInfo
, are also checked withMediaSource.isTypeSupported
There are some browser implementations that return a true
mediaCapabilities.decodingInfo
for unknown mimetypes and codecs and that can cause an application-level problem. For example, EDGE on Windows 10 returns true to HEVC even if you don't have the extension installed to support HEVC, insteadMediaSource.isTypeSupported
does take this into account