[fix] drop-frame timecode emits illegal values at minute boundaries#139
Open
xianjianlf2 wants to merge 1 commit into
Open
[fix] drop-frame timecode emits illegal values at minute boundaries#139xianjianlf2 wants to merge 1 commit into
xianjianlf2 wants to merge 1 commit into
Conversation
formatTimecode's drop-frame branch partitioned the frame count by the raw fps*600 (10 min) and fps*60 (1 min), but drop-frame counts real, post-drop frames: 17982 per 10 min and 1798 per min at 30. Using the un-adjusted divisors under-counted the accumulated drops, so the first frames of minutes 1-9 (11-19, ...) emitted illegal values like 00;01;00;00 instead of 00;01;00;02. DaVinci/Premiere relink-by-source-timecode disagreed near minute marks. Subtract drop*9 (per 10 min) and drop (per min) from the divisors. Verified against a brute-force reference: 0 mismatches across a full 30 min at 30 and 60 fps; the previously passing 42966 -> 00;23;53;18 case is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TimLai666
added a commit
to TimLai666/fronda
that referenced
this pull request
Jul 10, 2026
…ruption bugs, palmier-io#264 overflow ceiling, palmier-io#36/palmier-io#268 ports, palmier-io#139/palmier-io#212 verified TDD per item (RED repro first, then fix): palmier-io#124 (BUG-FIXED): place_clips overwriting a linked V+A pair cleared only the target track — the audio partner's range stayed (double playback) and new linked audio was pushed onto a spurious extra track. Now a video-track overwrite clears the same range on linked-partner tracks (mirrors Swift PR 4b776e1's video-gated semantics); audio-track placement stays track-local. palmier-io#263 (BUG-FIXED): compute_ripple_delete partner propagation only scanned the anchor track, so linked partners of clips on cleared sync-locked tracks were left behind on lock-off tracks (desync). Propagation is now a worklist fixpoint across ALL cleared tracks; palmier-io#227 sync-follower and palmier-io#207 ignore-set semantics unchanged. palmier-io#264/palmier-io#265 (BUG-FIXED): near-i64::MAX frame/duration args crashed the process (debug 'attempt to add with overflow' in insert/move/apply_layout/add_texts; set_clip_properties silently stored poisoned values). Shared MAX_TOOL_FRAME (1e9, upstream maxToolFrame) + require_frame_in_bounds in mutation.rs, wired into both the mutation validators and the executor's live arg parsing (resolve_placement, insert_clips frame incl. >=0 guard, move_clips toFrame, split_clips atFrame/frames, set_clip_properties duration/trims, apply_layout, add_texts). ripple_delete_ranges stays clamp-tolerant like upstream. palmier-io#139 (VERIFIED-OK): format_timecode already uses drop-adjusted divisors (1798/17982); frame 1800 @29.97DF = 00;01;00;02 pre-existing test confirmed, 10-minute-boundary assertions added (17982 -> 00;10;00;00). palmier-io#212 (VERIFIED-OK + guard): speed 0.1 legal end-to-end, duration math pinned (150f @1.0 -> 1500f @0.1). Fixed the live executor silently storing speed <= 0 — now rejects 'speed must be > 0' like Swift. palmier-io#36 (PORTED): AnthropicConfig::from_env honours ANTHROPIC_BASE_URL (trimmed, blank falls back to the public API); chat_view uses it; URL construction unit-tested via pure resolve_base_url. palmier-io#268 (PORTED): sonnet5-family requests carry output_config.effort=low (shape from upstream AgentClientTypes.swift), applied in build_agent_request AND the live run_agent_turn body; snapshot + ScriptedTransport tests. Gates (exit code 0): cargo test --workspace; cargo test -p fronda-app-shell-gpui --features desktop-app; cargo check -p fronda-app-shell-gpui --features desktop-app --bin fronda.
TimLai666
added a commit
to TimLai666/fronda
that referenced
this pull request
Jul 10, 2026
All 8 items verdicted: palmier-io#124 stranded audio + palmier-io#263 ripple fixpoint BUG-FIXED (TDD, upstream-diff-matched), palmier-io#139 drop-frame + palmier-io#212 speed VERIFIED-OK (with boundary pins + a speed<=0 guard), palmier-io#264 overflow ceiling + palmier-io#36 base URL + palmier-io#268 sonnet5 effort:low PORTED. 19 new tests. Residuals flagged: mutation.rs is a dormant validation layer (live path never calls it - palmier-io#144's checks included), and cmd_add_texts ignores startFrame (chip task_14403ebd).
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
1 participant
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.
Problem
XMLExporter.formatTimecode(extracted + unit-tested in #136) computes drop-frame timecode wrong at minute boundaries.The drop-frame branch partitions the frame count by the raw
fps*600(10 min) andfps*60(1 min). But drop-frame counts the real, post-drop frames: at 30 DF that's17982per 10 min and1798per min, not18000/1800. With the un-adjusted divisors the accumulated drops are under-counted, so the first frames of minutes 1–9 (11–19, …) emit values that can never legally occur in drop-frame, e.g. real frame1800→00;01;00;00instead of00;01;00;02.Impact: the
<timecode>written into XMEML disagrees with the source file near minute marks, so DaVinci/Premiere relink-by-source-timecode is off — the same class of issue #136 set out to fix.The existing test only checked frame
42966, which happens to land where the error cancels, so it slipped through.Fix
Subtract the per-period drop counts from the divisors:
Verification
Brute-forced against a reference that skips the dropped frame numbers, over a full 30 minutes:
00;01;00;00)00;01;00;00)The previously passing
42966 → 00;23;53;18case is unchanged. Added a regression test (dropFrameSkipsTwoFramesAtEachNonTenthMinute) covering minute 1/2 boundaries, the minute-10 (no-drop) case, and 59.94.Note: I couldn't run
swift testlocally (SwiftPM failed to fetch the Sentry/Sparkle binary artifacts — a keychain/cache issue unrelated to this change), so the math is validated via the standalone brute-force run above. CI should confirm.🤖 Generated with Claude Code
Note
Medium Risk
Changes exported source timecode in XMEML, which affects NLE relink accuracy, but the scope is limited to drop-frame math and is covered by new tests.
Overview
Drop-frame SMPTE strings in exported XMEML were wrong at the start of most minutes because
formatTimecodepartitioned the real frame count using rawfps×60/fps×600windows instead of the post-drop lengths (e.g. 1798 and 17982 at 30 DF). That under-counted skipped frames and could emit illegal values such as00;01;00;00when the source should read00;01;00;02, breaking relink-by-source-timecode in DaVinci/Premiere near minute marks.The fix uses
per10Min = fps×600 − drop×9andperMin = fps×60 − dropwhen accumulating drops. A new unit test covers minute 1/2, the no-drop minute 10 case, and 59.94 (60 DF).Reviewed by Cursor Bugbot for commit 691a224. Bugbot is set up for automated code reviews on this repo. Configure here.