Skip to content

Add drop-frame timecode support to SCCWriter#378

Open
OlteanuRares wants to merge 2 commits into
mainfrom
OCTO-11497
Open

Add drop-frame timecode support to SCCWriter#378
OlteanuRares wants to merge 2 commits into
mainfrom
OCTO-11497

Conversation

@OlteanuRares

Copy link
Copy Markdown
Contributor

Summary

Adds drop-frame timecode support to SCCWriter via a new drop_frame=False parameter. Reimplements the work from community PR #354 (by @foxPrateek, closed due to inactivity) with all review feedback addressed.

Changes

  • New drop_frame flag on SCCWriter — when True, outputs DF timestamps with semicolon separators using the standard 10-minute block method
  • Refactored timing pipeline: exact token counting, monotonic frame deduplication, caption splitting at 80 tokens
  • Extracted SCC_TOKENS_PER_CAPTION_MAX, _SCC_PREFIX, _SCC_SUFFIX as named constants
  • Added _format_timestamp dispatcher, _format_timestamp_df, _format_timestamp_ndf, _microseconds_to_frame
  • CI: guard coverage-parsing step on test success
  • 20 new tests covering DF/NDF formatting, monotonic ordering, backshift, split, round-trip

Backward compatibility

SCCWriter() with no args produces identical output to before. Default is NDF.

Test plan

  • All 318 tests pass
  • DF math verified at minute boundaries (frame skip at non-10th, no skip at 10th)
  • Frame values stay 0–29 in both modes
  • Round-trip SRT→SCC→SCCReader works for both modes

  Implement drop-frame (DF) timecode formatting for the SCC writer,
  controlled by a new  parameter. When enabled,
  timestamps use the standard 10-minute block method (FRAMES_PER_10MIN=17982)
  with frame-skip at non-10th minute boundaries and semicolon separators.

  Also refactors the writer's timing pipeline:
  - Extract SCC_TOKENS_PER_CAPTION_MAX (80) as a named constant
  - Fix token counting (split() instead of len/5 heuristic)
  - Add monotonic frame deduplication pass (PASS 3)
  - Split captions exceeding 80 tokens into multiple lines
  - Guard CI coverage step against test failures

  Based on the work by @foxPrateek in PR #354, reimplemented with review
  feedback addressed: drop_frame=False default, math.floor consistently,
  PAC range(2) preserved, _format_timestamp dispatcher, named constants,
  helpers outside loops, no unused imports.

  Co-Authored-By: foxPrateek <foxPrateek@users.noreply.github.com>
@OlteanuRares OlteanuRares requested a review from a team as a code owner June 26, 2026 08:48
@github-actions

Copy link
Copy Markdown

🟡 PR Compliance Review

Risk Level: MEDIUM

  • Compliance Issues: 0 (0 critical)
  • Regressions: 2

REVIEW REQUIRED - Address issues before merging

Full report available in workflow artifacts

@github-actions

Copy link
Copy Markdown

🟡 PR Compliance Review

Risk Level: MEDIUM

  • Compliance Issues: 0 (0 critical)
  • Regressions: 2

REVIEW REQUIRED - Address issues before merging

Full report available in workflow artifacts

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.

1 participant