-
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(hls): Support playing media playlists directly #4080
Conversation
demo/custom.js
Outdated
* @return {!Element} div | ||
* @private | ||
*/ | ||
makeAssetDialogContentsMediaPlaylist_(assetInProgress, inputsToCheck) { |
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'm concerned that this extra tab may make the dialog too crowded. (I haven't cherry-picked this to look at the layout yet.)
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.
After discussing this in person, we decided to keep the separate tab but rename it to be a more general-purpose "HLS" tab.
externs/shaka/player.js
Outdated
* directly. | ||
* You can use the <code>shaka.util.MimeUtils.getFullType()</code> utility to | ||
* format this value. | ||
* <i>Defaults to <code>'video/mp4; codecs="avc1.42E01E"'</code>.</i> |
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 think we should assume muxed content with AAC audio by default. (Add ", mp4a.40.2")
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.
most of the content is TS, I think that the default should be TS instead of MP4
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.
lib/hls/hls_parser.js
Outdated
// Parsing a media playlist results in a single-variant stream. | ||
if (playlist.type == shaka.hls.PlaylistType.MEDIA) { | ||
// Get necessary info for this stream, from the config. These are things | ||
// we would normally be finding from the master playlist (e.g. from values |
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.
nit: "would normally find"
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.
- Fixed typos. - Improved description in externs. - Added audio to default media playlist codecs. - Renamed some demo strings.
7ee8248
to
61c0e50
Compare
You have a new test that needs to be updated with the new default codec string. Looks good otherwise, though. |
Closes #3536