remotion: Fix media duration calculation with playbackRate#6961
remotion: Fix media duration calculation with playbackRate#6961JonnyBurger merged 3 commits intomainfrom
remotion: Fix media duration calculation with playbackRate#6961Conversation
Extracts the media duration calculation from useBasicMediaInTimeline into a pure getMediaDuration function and adds tests to verify correct duration with playbackRate=2.45 and 300 composition frames (#6783). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stop dividing maxMediaDuration by playbackRate since the duration calculation now correctly accounts for playbackRate upstream (#6783). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Reviewed PR #6961. The base-case fix for Task list (6/6 completed)
|
There was a problem hiding this comment.
The base case fix (no parent sequence, no trims) is correct — getMediaDuration properly returns the composition duration regardless of playbackRate. However, the parent sequence capping logic produces wrong results, and the useMaxMediaDuration change has a subtle inconsistency worth verifying.
Big Pickle (free) ｜ 𝕏
| if (parentSequenceDurationInFrames !== null) { | ||
| return Math.floor( | ||
| Math.min(parentSequenceDurationInFrames * playbackRate, mediaDuration), | ||
| ); |
There was a problem hiding this comment.
When parentSequenceDurationInFrames is not null, this returns floor(min(parentSeqDur * playbackRate, mediaDuration)). With parentSeqDur=100 and playbackRate=2.45, that's floor(min(245, 300)) = 245.
But the parent sequence constrains the media to 100 composition frames — the duration should be capped at 100, not 245. The multiplication by playbackRate converts to media-frame units, but mediaDuration (the output of calculateMediaDuration) is already in composition-frame units (it divides by playbackRate internally).
This should be:
return Math.floor(
Math.min(parentSequenceDurationInFrames, mediaDuration),
);The corresponding test 'parentSequence with playbackRate 2.45 caps correctly' expects 245, but the correct expectation should be 100.
|
|
||
| if (maxMediaDuration !== null && (s.type === 'audio' || s.type === 'video')) { | ||
| return maxMediaDuration / s.playbackRate; | ||
| return maxMediaDuration; |
There was a problem hiding this comment.
Before this PR, this returned maxMediaDuration / s.playbackRate — i.e., the number of composition frames the media file can fill at the given rate. Now it returns raw media frames.
Since s.duration (from getMediaDuration) is now in composition frames (correct after fixing the parent-sequence bug above), but maxMediaDuration is now in raw media frames, these are in different units. getTimelineSequenceLayout uses maxMediaDuration to cap the visual width via spatialDuration = min(maxMediaSeqDur, durationInFrames - 1, ...) — mixing units here would cause incorrect timeline widths.
I believe the / s.playbackRate should be kept here, since s.duration is (correctly) in composition-frame units.
Summary
useBasicMediaInTimelineinto a puregetMediaDurationfunctionplaybackRate(fixes Remotion Studio: Video component shows as unmounted even though it is still mounted. #6783)maxMediaDurationbyplaybackRateinuseMaxMediaDurationsince upstream now handles itplaybackRate=2.45and 300 composition framesTest plan
bun test src/test/get-media-duration.test.ts— 6 tests passbun test src/test/use-media-in-timeline.test.tsx— existing test passes🤖 Generated with Claude Code