Skip to content

Fix integer overflow in MP4 timestamp calculations (GPAC path)#2205

Closed
Apprentice2907 wants to merge 5 commits intoCCExtractor:masterfrom
Apprentice2907:Fixes-tx3g-subtitle-timestamps-broken
Closed

Fix integer overflow in MP4 timestamp calculations (GPAC path)#2205
Apprentice2907 wants to merge 5 commits intoCCExtractor:masterfrom
Apprentice2907:Fixes-tx3g-subtitle-timestamps-broken

Conversation

@Apprentice2907
Copy link
Contributor

In raising this pull request, I confirm the following (please check boxes):

Reason for this PR:

  • This PR adds new functionality.
  • This PR fixes a bug that I have personally experienced or that a real user has reported and for which a sample exists.
  • This PR is porting code from C to Rust.

Sanity check:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • If the PR adds new functionality, I've added it to the changelog. If it's just a bug fix, I have NOT added it to the changelog.
  • I am NOT adding new C code unless it's to fix an existing, reproducible bug.

Description

This PR fixes a potential integer overflow in MP4 timestamp calculations in the GPAC path.

Previously timestamps were computed using expressions like:

(DTS + CTS_Offset) * MPEG_CLOCK_FREQ / timescale

Since DTS and CTS_Offset are stored in 32-bit integers, the multiplication could overflow before the result was converted to a larger type. This could lead to incorrect subtitle timestamps in some cases.

This patch ensures that the arithmetic is performed using 64-bit integers by casting operands to int64_t before the calculation.

Affected code paths include:

  • process_avc_sample
  • process_hevc_sample
  • process_xdvb_track
  • process_vobsub_track
  • processmp4 sample processing loop

This change does not alter logic or functionality other than preventing overflow during timestamp calculation.


Repro instructions

  1. Use an MP4 file containing subtitle tracks (e.g. tx3g or CC tracks).
  2. Run CCExtractor on the file:

@cfsmp3
Copy link
Contributor

cfsmp3 commented Mar 18, 2026

Closing this PR. A few issues:

  1. This doesn't build. Please make sure your code compiles before submitting a PR.

  2. Don't mix unrelated changes in the same PR. This claims to be about MP4 timestamp overflow, but it also includes CEA-608 decoder changes, teletext sentence merging, a JSON report format feature, and whitespace edits. Each of those should be a separate PR.

  3. Don't submit PRs until they are ready for review. A PR should be a finished, tested piece of work.

This is the same feedback we gave on #2108 and #2106. Please take it into account going forward.

@cfsmp3 cfsmp3 closed this Mar 18, 2026
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.

2 participants