-
Notifications
You must be signed in to change notification settings - Fork 428
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
refactor: Add caption parser to the webworker to save bytes and offlo… #863
Conversation
a49cbfb
to
1acd567
Compare
1acd567
to
6eeb707
Compare
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.
Captions continue to work 👍
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 comment, but looks good
src/media-segment-request.js
Outdated
} | ||
finishLoading(); |
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.
We can save some indentation if we check for the negative case before and finishLoading()
and return
there.
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
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.
Approved once we change the mux.js branch
CaptionParser can return null during parse, we used to handle this case but broke when we switched to the web worker caption parser in #863.
Saves us 5732 gzipped, from 77004 min gzipped in master to 71272 min gzipped in this pull request.
See videojs/mux.js#345