Skip to content

feat(time): add frame-grid helpers - #81

Merged
renezander030 merged 2 commits into
renezander030:masterfrom
floze-the-genius:feat/76-frame-grid-helpers
Aug 11, 2026
Merged

renezander030 merged 2 commits into
renezander030:masterfrom
floze-the-genius:feat/76-frame-grid-helpers

Conversation

@floze-the-genius

@floze-the-genius floze-the-genius commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • export framesFor(us, fps) and quantizeToFrame(us, fps) from the public library entry point
  • use nearest-frame rounding with the documented 30 fps fallback while leaving every existing command write path unchanged
  • floor positive sub-half-frame durations at one frame so a real segment cannot collapse to zero; keep zero and negative signed rounding explicit
  • cover the five measured docs: note CapCut's frame quantisation of durations on first open #69 rows, the positive one-frame floor, fractional rates, and invalid/missing rates
  • document the opt-in API and include its reachable type declaration in the package

Closes #76.

Validation

  • RED: the named 1 ms / 16 ms regression failed before the follow-up with 0 !== 1
  • npm test — 665 tests: 654 passed, 11 skipped, 0 failed
  • npm run lint — 125 files checked, no warnings or errors
  • npm pack --dry-run — package includes dist/time.d.ts
  • git diff --check

AI disclosure

OpenAI Codex assisted with implementation and validation. I reviewed the final diff and test results.

Expose framesFor and quantizeToFrame through the public library entry point so callers can opt into CapCut's nearest-frame duration rounding without changing any existing command writes.\n\nCover the measured issue rows, sub-frame behavior, fractional rates, and the documented 30 fps fallback.\n\nCloses renezander030#76.\n\nAI-assisted: OpenAI Codex helped implement and validate this change; the contributor reviewed the final diff.
@renezander030

Copy link
Copy Markdown
Owner

Thanks Floze — this landed exactly where the issue asked: helpers in src/time.ts, re-exported from src/lib.ts, tested against dist/lib.js, dist/time.d.ts added to files, and no existing write path touched. I've approved the workflow run so CI can report.

One thing I'd like pinned down before merge.

The third acceptance criterion asks that sub-frame durations survive rather than collapsing to zero. Math.round gets the 33 ms case right (0.99 frames → 1), but anything under half a frame does collapse:

framesFor(16000, 30)      // 0
quantizeToFrame(16000, 30) // 0
framesFor(1000, 30)        // 0

So a caller who quantises a short segment before writing turns it into a zero-length one. The 33 ms test passes over this because 33 ms happens to sit just under a full frame, not because the floor is handled.

Two ways to go, and I don't mind which:

  1. Floor positive durations at one frame — us > 0 && frames === 0 → 1. Matches "sub-frame durations survive" literally, and a caller quantising a real segment never gets a zero back.
  2. Keep the pure round, but say so in the doc block and the schema section, and add a test asserting quantizeToFrame(1000, 30) === 0 so the behaviour is deliberate rather than incidental.

I lean towards (1), since the helper exists so callers can quantise safely, and returning 0 for a real segment is the one result that is never useful. Either way, a test that names the case.

Two smaller notes, neither blocking:

  • Negative us passes straight through (framesFor(-100000, 30) → -3). Not in the criteria, worth a line in the doc block if you keep it as-is.
  • quantizeToFrame resolves the rate then calls framesFor(us, rate), which resolves it again. Harmless, but calling the inner Math.round((us / US_PER_SEC) * rate) directly reads cleaner.

draftToOtio in src/interchange.ts now duplicates this maths — I'll open a separate issue for that rather than widen this PR.

Floor positive sub-half-frame durations at one frame while preserving zero and signed negative rounding. Document and test the boundary explicitly.

AI-assisted: OpenAI Codex helped implement and validate this maintainer-requested follow-up; the contributor reviewed the final diff.
@floze-the-genius

Copy link
Copy Markdown
Contributor Author

Implemented your preferred option (1) in a917315: any positive duration that would round to zero now floors to one frame, while zero remains zero and negative durations keep their signed nearest-frame result. The named regression covers 1 ms, 16 ms, zero, and a negative value; it failed RED with 0 !== 1 before the fix. I also documented the boundary and used a private pre-resolved-rate helper so quantizeToFrame no longer resolves the rate twice.

Local follow-up validation is green: 665 tests (654 passed, 11 skipped), Biome on 125 files, package dry-run with dist/time.d.ts, and diff-check. The refreshed CI run is waiting for workflow approval.

@renezander030 renezander030 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Exactly what I asked for, and faster than I expected. The floor is in the right place — inside a single private framesAtRate so both exports share it — and the regression test names the case rather than relying on 33 ms happening to sit just under a frame. Documenting the negative-duration behaviour instead of quietly clamping it was the right call too.

Verified locally against the new head: all five #69 rows still land on 83 / 94 / 10 / 26 / 26, 1 ms and 16 ms now floor to one frame at 33333 µs, zero stays zero, and 29.97 / 23.976 stay finite. Full CI green across Node 18/20/22, lint, and platform smoke on all three OSes.

Merging. Thank you for the contribution — a first PR that arrives scoped to the issue, tested the way the repo asks, and turned around on review this cleanly is genuinely rare. Please do pick up another one.

@renezander030
renezander030 merged commit a8d31e3 into renezander030:master Aug 11, 2026
7 checks passed
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.

Frame-grid helper: quantise durations before writing

2 participants