-
Notifications
You must be signed in to change notification settings - Fork 100
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
EXT-X-START tag support #31
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 the contribution! A couple minor comments, but looks great otherwise.
src/parse-stream.js
Outdated
if (match[1]) { | ||
event.attributes = parseAttributes(match[1]); | ||
|
||
event.attributes['TIME-OFFSET'] = parseFloat(event.attributes['TIME-OFFSET'], 10); |
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.
Does parseFloat take a second parameter?
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 believe it does - sorry about that, I'm used to parseInt
taking a base parameter and just assumed parseFloat
behaved similarly...
}); | ||
parser.push(manifest); | ||
|
||
assert.ok(warning, 'a warning was triggered'); |
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.
Minor, but might be worth testing the warning message.
Tested locally and tests pass after working around rollup issue. |
This should add support for the EXT-X-START tag and its two attributes.