Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
calesthio
left a comment
There was a problem hiding this comment.
Thank you for investigating the slowed-clip black tail. I verified the underlying issue in the pinned Remotion 4.0.484 source: its internal trim Sequence uses unscaled source frames, so this is a real problem. The proposed workaround still needs to preserve the caller's explicit source-out boundary.
[P2] Do not discard trimAfterSeconds for every non-1 playback rate. The enclosing Sequence only knows durationSeconds; it does not enforce the source trim. For a direct cinematic scene with trimBeforeSeconds=0, trimAfterSeconds=2, playbackRate=0.5 and durationSeconds=6, this change allows source frames after second 2 to render during the final two timeline seconds. With playbackRate=2 it similarly removes an explicit source bound even though the reported defect concerns slowed playback. Please reconcile the source interval and playback rate (or reject inconsistent scene duration) so the fix cannot show footage outside the selected cut. Add a behavior test with distinguishable source regions; the new source-string assertion only checks that the trim is omitted and cannot catch this regression.
Validation: 12 focused transition/adapter tests passed in an isolated worktree integrated with main 63d1087a; no full Remotion/browser render was run. Source semantics checked against https://raw.githubusercontent.com/remotion-dev/remotion/v4.0.484/packages/core/src/video/OffthreadVideo.tsx and https://www.remotion.dev/docs/timing .
…r's source-out Remotion's OffthreadVideo enforces trimAfter through an internal Sequence bounded by (trimAfter - trimBefore) timeline frames without scaling for playbackRate. Dropping trimAfter for non-1 rates (the previous workaround) let footage past the caller's explicit trimAfterSeconds render; passing it through unscaled cut slowed clips early. resolveTrimAfterFrames now scales the source interval into timeline frames (trimBefore + interval / playbackRate), so the bound expires exactly when playback reaches trimAfterSeconds at any rate. Replaces the source-string omission contract test with one covering the scaling behaviour for distinguishable source regions.
|
Thanks for verifying the underlying Remotion behaviour and for the concrete counterexample. You were right: dropping Fixed in 7e61b47 by reconciling the source interval with the playback rate instead of dropping the trim:
@calesthio could you take another look? |
Summary
Fixes an issue in
CinematicRenderer.tsxwhere video clips rendered withplaybackRate < 1(slowed down) stop playback prematurely and display a black background for the remainder of the scene (#855).Related issue
Closes #855
Changes
remotion-composer/src/CinematicRenderer.tsx, omittrimAfteron<OffthreadVideo>whenscene.playbackRateis customized (!= 1). Remotion's<OffthreadVideo>bounds its internal sequence bytrimAfterin unscaled frames, which cuts off slowed clips early. The enclosing<Sequence>inCinematicRendereralready precisely bounds the scene duration toMath.round(scene.durationSeconds * FPS).test_cinematic_custom_playback_rate_omits_trim_aftertotests/contracts/test_remotion_video_transition_contract.py.Testing
npx tsc --noEmitinremotion-composerto ensure clean TypeScript compilation.pytest tests/contracts/test_remotion_video_transition_contract.py tests/tools/test_cinematic_remotion_adapter.py(12 passed).pytest tests/contracts/(1271 passed, 7 skipped).Checklist
make test-contracts/make test) where applicable.