Repository navigation
fix(export): put the whole timeline on one frame grid - #1163
Merged
Merged
Conversation
Each speed span rounded its own frame count up, so an export ran up to one frame per span longer than its timeline (+7 frames on the 14-span v2.1.0 QA project). Spans now take their frames from a single programme grid carried across trims and clips, and sample the source where that grid puts them. The progress total mirrors it. Part of #1162.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No blocking issue was identified; the change is mergeable after normal checks. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: using one frame grid across the entire export timeline. |
| Description check | |
The description is mostly complete. It explains the change, links issue #1162, identifies the implementation areas, documents testing results, and states known limitations. It does not include the tem… |
| Linked Issues check | |
Issue #1162 is directly linked and has coding requirements. SpeedSegment now stores cumulative programme time and the first grid frame. source_time samples from that shared grid. `walk_composited_… |
| Out of Scope Changes check | |
The changes stay within issue #1162. The modified compositor, timeline walk, preview clock, progress-count code, tests, and export documentation all implement or verify the shared frame-grid behavior.… |
| Docstring Coverage | |
Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 4 files. (2 skipped: 2 … |
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Clippy (1.98.1)
-
Clippy execution failed
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
2 tasks done
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.
Summary
An export now lasts its timeline's programme length, to one frame, however many trims, regions and clips it has. Each speed span used to round its own frame count up, which added up to a frame per span.
Fixes #1162
Candidate for cherry-pick to release/v2.1.0.
Change
speed_segments_for_window): frameksits at programme timek / fps. Programme time carries across spans, trims and clip junctions, and each span emits the grid frames inside it. The total is⌈(P − 1 ms) × fps⌉for P programme seconds.SpeedSegment::source_time): each span samples the source where the grid puts its frames, instead of restarting its cadence at the span start.walk_composited_timelinecarries programme time from clip to clip. A clip cut short by its decoder resyncs the grid to the real frame counter.ProgrammeClock): it chains the clips the same way, so preview and export still agree at every frame.outputFrameCount.ts): it mirrors the grid, so the UI total equals the encoder's count.export-pipeline.mddescribes the grid.Verification
CLI export, 20 s synthetic source, 60 fps. Counts are video packets.
cargo test -p openscreen-compositor --libpasses (413 tests), and so doesvitest src/lib/exporter. New cases cover the QA project (2445 frames, was 2452) and 200 windows (1750 frames, was 2000), on both the Rust and the TS side.tscconfigs and Biome are clean.Not covered
outputFrameCount.ts.🤖 Generated with Claude Code
Summary by CodeRabbit