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

fix(mp4Inspector): read version 1 sidx boxes #310

Merged
merged 2 commits into from
Nov 24, 2020
Merged

fix(mp4Inspector): read version 1 sidx boxes #310

merged 2 commits into from
Nov 24, 2020

Conversation

ldayananda
Copy link
Contributor

Should resolve #287.

} else {
// read 64 bits
earliestPresentationTime = view.getUint32(i);
earliestPresentationTime *= Math.pow(2, 32);
Copy link
Contributor

Choose a reason for hiding this comment

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

Regular integer in javascript is 52 bits. Might want to use BigInt for 64 bits number.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, browser compatibility doesn't allow us to use BigInt yet. However, since we're working with Uint32 array views, we should still be able to access the full value.

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.

Looks good, but one style suggestion (though not required)

Co-authored-by: Garrett Singer <[email protected]>
@brandonocasey brandonocasey merged commit 0390cbe into master Nov 24, 2020
@brandonocasey
Copy link
Contributor

accidentally merged this into master. I cherry picked this into main and reverted the change in master.

@gkatsev gkatsev deleted the sidx-v1 branch December 1, 2020 18:46
@brandonocasey
Copy link
Contributor

Fixes videojs/http-streaming#909

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.

Incorrectly parsing SIDX box
5 participants