-
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): Show WebVTT subtitles with X-TIMESTAMP-MAP in segments mode #5643
Conversation
lib/text/vtt_text_parser.js
Outdated
@@ -72,8 +84,8 @@ shaka.text.VttTextParser = class { | |||
// HLS content, which should use X-TIMESTAMP-MAP and periodStart instead. | |||
let offset = time.vttOffset; | |||
|
|||
// Only use 'X-TIMESTAMP-MAP' in sequence mode, as that is currently | |||
// shorthand for HLS. Note that an offset based on the first video | |||
// Only use 'X-TIMESTAMP-MAP' in sequence mode or HLS. |
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 don't think this comment is accurate. It makes it sound like we are using X-TIMESTAMP-MAP
in all HLS, but in reality we still don't parse the tag in segment mode HLS.
We are merely taking the presence of an X-TIMESTAMP-MAP
tag as a signal that the offset is to be calculated differently in segment mode HLS, correct?
It might be better to have a short explanation inside the new clause below, and just remove the mention of HLS from this comment entirely.
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.
Changed!
Incremental code coverage: 98.21% |
No description provided.