parse ctts atom implemented #379
Conversation
|
I'm not one of the maintainers of this project, so I won't be able to approve the PR. Sorry for any confusion! |
|
I'm not deeply familiar with norms and preferred style in mux.js, and I haven't compared your parser to any spec for the ctts box, but the implementation looks reasonable to me, for what that's worth. |
There was a problem hiding this comment.
Style-wise, it matches existing code. Also, appreciate that there are tests! Appreciate you looking at this @joeyparrish.
I'll have to defer to @gesinger or @brandonocasey to do a deeper review.
|
@gkatsev, if you are happy with the project-specific details, and it's just a question of correctness, I found docs from Apple detailing the ctts box, and the parser matches perfectly as far as I can tell: |
lib/tools/mp4-inspector.js
Outdated
| for (i = 8; entryCount; i += 8, entryCount--) { | ||
| result.compositionOffsets.push({ | ||
| sampleCount: view.getUint32(i), | ||
| sampleOffset: view.getInt32(i + 4) |
There was a problem hiding this comment.
in the spec for mp4 version 0 of this box uses an uint while version 1 uses an int. Would you mind making the change for this case?
There was a problem hiding this comment.
|
@joeyparrish Sorry, i thought you are a maintainer |
lib/tools/mp4-inspector.js
Outdated
| for (i = 8; entryCount; i += 8, entryCount--) { | ||
| result.compositionOffsets.push({ | ||
| sampleCount: view.getUint32(i), | ||
| sampleOffset: view.getUint32(i + 4) |
There was a problem hiding this comment.
The spec is closed/paid for so I can't really link a reference to it. Basically you have to check the box version for this box and use view.getUint32 if it is version 0 and view.getInt32 for version 1. Should be something like this suggestion.
| sampleOffset: view.getUint32(i + 4) | |
| sampleOffset: view[(result.version === 0 ? 'getUint32' :'getInt32')](i + 4) |
|
We definitely value @joeyparrish's opinion! |
Then I've fooled you all!! 😈 |
|
We'll know better next time! |
Implementation of ctts atom parsing.
@gkatsev @gesinger @joeyparrish could you review this PR? I need this atom in my project.