Handle negative ctts sample_offset values in mp4 parsing#790
Merged
Conversation
ptpt
approved these changes
Jan 9, 2026
Member
ptpt
left a comment
ptpt
left a comment
There was a problem hiding this comment.
Some minor comments about naming and readability. Thanks for providing the test and the sample video
|
|
||
| def _convert_to_signed_offset(offset: int) -> int: | ||
| """Convert unsigned 32-bit offset to signed if high bit is set.""" | ||
| if (offset & 0x80000000) == 0: |
Member
There was a problem hiding this comment.
1 << 31 is easier to understand than 0x80000000 IMO, or add it as a comment
| if (offset & 0x80000000) == 0: | ||
| return offset | ||
| else: | ||
| return offset - 0x100000000 |
| from . import construct_mp4_parser as cparser, simple_mp4_parser as sparser | ||
|
|
||
|
|
||
| def _convert_to_signed_offset(offset: int) -> int: |
Member
There was a problem hiding this comment.
Naming can be generalized as _convert_to_int32(n: int) -> int
| # ctts version to 0 instead of 1 even when using signed offsets. | ||
| # Leigitimate positive values are relatively small so we can assume the value is signed. | ||
| composition_offsets.append( | ||
| _convert_to_signed_offset(entry.sample_offset) |
Member
There was a problem hiding this comment.
Use entry["sample_offset"] to be consistent?
They are same, but looks in this context dict access is preferred
Member
|
@caglarpir A more clean solution IMO is use signed int32 instead of unit32 in CTTS's construct parser directly: So a negative CTTS offset can be also correctly constructed when we build the mp4 data before uploading |
caglarpir
force-pushed
the
support-h265
branch
2 times, most recently
from
2d97f5c to
878ce18
Compare
Summary:
Some encodings like H.264 and H.265 support negative offsets.
We cannot rely on the version field since some encoders incorrectly set
ctts version to 0 instead of 1 even when using signed offsets.
Leigitimate positive values are relatively small so we can assume the value is signed.
The unsigned to signed conversion could have been gated based on the encoding. However I chose not to since large positive values exceeding 2^31 is unrealistic.
For reference the stsd and ctts boxes from a video:
stsd box: Container:
size = 233
type = b'stsd' (total 4)
data = Container:
version = 0
flags = 0
entries = ListContainer:
Container:
format = b'hvc1' (total 4)
ctts box Container:
size = 128480
type = b'ctts' (total 4)
data = Container:
version = 0
flags = 0
entries = ListContainer:
Container:
sample_count = 1
sample_offset = 0
Container:
sample_count = 1
sample_offset = 60
Container:
sample_count = 1
sample_offset = 0
Container:
sample_count = 1
sample_offset = 4294967256
Container:
sample_count = 1
sample_offset = 4294967276
caglarpir
force-pushed
the
support-h265
branch
from
878ce18 to
0f8e800
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some encodings like H.264 and H.265 support negative offsets.
We cannot rely on the version field since some encoders incorrectly set
ctts version to 0 instead of 1 even when using signed offsets.
Legitimate positive values are relatively small so we can assume the value is signed.
The unsigned to signed conversion could have been gated based on the encoding. However I chose not to since large positive values exceeding 2^31 is unrealistic.
For reference the stsd and ctts boxes from a video: