Skip to content
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

Merged
merged 4 commits into from
Jun 16, 2020

Conversation

brandonocasey
Copy link
Contributor

@brandonocasey brandonocasey commented Jun 11, 2020

Saves us 5732 gzipped, from 77004 min gzipped in master to 71272 min gzipped in this pull request.
See videojs/mux.js#345

@brandonocasey brandonocasey force-pushed the refactor/worker-caption-parser branch 5 times, most recently from a49cbfb to 1acd567 Compare June 12, 2020 18:02
@brandonocasey brandonocasey force-pushed the refactor/worker-caption-parser branch from 1acd567 to 6eeb707 Compare June 12, 2020 18:26
@gkatsev gkatsev added this to the 2.0 milestone Jun 12, 2020
gkatsev
gkatsev previously approved these changes Jun 12, 2020
Copy link
Member

@gkatsev gkatsev left a 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 👍

gesinger
gesinger previously approved these changes Jun 16, 2020
Copy link
Contributor

@gesinger gesinger left a 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

}
finishLoading();
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@brandonocasey brandonocasey dismissed stale reviews from gesinger and gkatsev via 09b692a June 16, 2020 16:32
gesinger
gesinger previously approved these changes Jun 16, 2020
Copy link
Contributor

@gesinger gesinger left a 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

gkatsev
gkatsev previously approved these changes Jun 16, 2020
@brandonocasey brandonocasey dismissed stale reviews from gkatsev and gesinger via 5bcf2ec June 16, 2020 16:46
@brandonocasey brandonocasey merged commit 491d194 into master Jun 16, 2020
@gkatsev gkatsev deleted the refactor/worker-caption-parser branch June 16, 2020 17:13
gkatsev pushed a commit that referenced this pull request Jul 6, 2020
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants