fix: clamp clip duration to actual video length to prevent freeze frames#198
Merged
SecurityQQ merged 2 commits intomainfrom Apr 6, 2026
Merged
fix: clamp clip duration to actual video length to prevent freeze frames#198SecurityQQ merged 2 commits intomainfrom
SecurityQQ merged 2 commits intomainfrom
Conversation
Always probe video duration in processClips and clamp both clip.duration and layer.cutTo to the actual source length. Prevents freeze frames and xfade offset misalignment when speech.duration or cutTo exceeds the generated video's real duration (e.g. lipsync models returning slightly shorter videos than the input audio).
Contributor
📝 Walkthroughwalkthroughupdated editly provider and related renderers to propagate and respect source video durations: video layers now carry changes
estimated code review effort🎯 3 (moderate) | ⏱️ ~20 minutes possibly related issues
poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai-sdk/providers/editly/index.ts`:
- Around line 102-124: The code can produce negative effectiveDuration when
videoLayer.cutFrom > videoDuration; clamp cutFrom similarly to cutTo before
computing effectiveDuration by bounding videoLayer.cutFrom to the range [0,
videoDuration] (use the value returned by getVideoDuration) so cutFrom =
Math.min(Math.max(videoLayer.cutFrom ?? 0, 0), videoDuration); then compute
cutTo (as already done), set videoLayer.cutTo = cutTo, and calculate
effectiveDuration = cutTo - cutFrom; this ensures the logic in the video branch
(layer.type === "video", VideoLayer, getVideoDuration, videoLayer.cutFrom,
cutTo, effectiveDuration, clip.duration, duration) never yields a negative
duration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0090c4ef-ae14-45da-b255-b8690d7b6eaf
📒 Files selected for processing (1)
src/ai-sdk/providers/editly/index.ts
Comment on lines
+102
to
124
| if (layer.type === "video") { | ||
| const videoLayer = layer as VideoLayer; | ||
| const videoDuration = await getVideoDuration(videoLayer.path, backend); | ||
| const cutFrom = videoLayer.cutFrom ?? 0; | ||
| const cutTo = videoLayer.cutTo ?? videoDuration; | ||
| duration = cutTo - cutFrom; | ||
| const cutTo = Math.min( | ||
| videoLayer.cutTo ?? videoDuration, | ||
| videoDuration, | ||
| ); | ||
|
|
||
| // Clamp the layer's cutTo so the FFmpeg trim filter also respects | ||
| // the actual source duration (prevents freeze frames at the tail) | ||
| videoLayer.cutTo = cutTo; | ||
|
|
||
| const effectiveDuration = cutTo - cutFrom; | ||
| if (!clip.duration) { | ||
| // No explicit duration — derive from the video layer | ||
| duration = effectiveDuration; | ||
| } else if (effectiveDuration < duration) { | ||
| // Explicit duration exceeds actual video length — clamp to avoid | ||
| // freeze frames and xfade offset misalignment | ||
| duration = effectiveDuration; | ||
| } | ||
| break; |
Contributor
There was a problem hiding this comment.
critical: cutFrom > cutTo can produce negative duration meow
when cutFrom exceeds the video's actual length (or cutTo after clamping), effectiveDuration goes negative. example: cutFrom=5, videoDuration=3 → cutTo=3 → effectiveDuration=-2.
this breaks everything downstream since clip.duration becomes negative.
🛡️ proposed fix: clamp cutFrom or add safeguard
option 1 (recommended): clamp cutFrom to video duration
if (layer.type === "video") {
const videoLayer = layer as VideoLayer;
const videoDuration = await getVideoDuration(videoLayer.path, backend);
- const cutFrom = videoLayer.cutFrom ?? 0;
+ const cutFrom = Math.min(videoLayer.cutFrom ?? 0, videoDuration);
const cutTo = Math.min(
videoLayer.cutTo ?? videoDuration,
videoDuration,
);option 2: add validation and throw
const cutTo = Math.min(
videoLayer.cutTo ?? videoDuration,
videoDuration,
);
+
+ if (cutFrom >= videoDuration) {
+ throw new Error(
+ `cutFrom (${cutFrom}s) must be less than video duration (${videoDuration}s) for ${videoLayer.path}`
+ );
+ }
// Clamp the layer's cutTo so the FFmpeg trim filter also respectsoption 3: add safeguard (defensive)
- const effectiveDuration = cutTo - cutFrom;
+ const effectiveDuration = Math.max(0, cutTo - cutFrom);recommend option 1 for silent correction matching the cutTo behavior, or option 2 if you want to surface configuration errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai-sdk/providers/editly/index.ts` around lines 102 - 124, The code can
produce negative effectiveDuration when videoLayer.cutFrom > videoDuration;
clamp cutFrom similarly to cutTo before computing effectiveDuration by bounding
videoLayer.cutFrom to the range [0, videoDuration] (use the value returned by
getVideoDuration) so cutFrom = Math.min(Math.max(videoLayer.cutFrom ?? 0, 0),
videoDuration); then compute cutTo (as already done), set videoLayer.cutTo =
cutTo, and calculate effectiveDuration = cutTo - cutFrom; this ensures the logic
in the video branch (layer.type === "video", VideoLayer, getVideoDuration,
videoLayer.cutFrom, cutTo, effectiveDuration, clip.duration, duration) never
yields a negative duration.
Instead of always calling ffprobe (HTTP round-trip for remote URLs), propagate the requested generation duration through File metadata and VideoLayer.sourceDuration. processClips() uses this as a fast path and falls back to ffprobe only when sourceDuration is not available. Chain: renderVideo() → File.metadata.duration → renderClipLayers() → VideoLayer.sourceDuration → processClips()
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/react/renderers/video.ts (1)
169-174:⚠️ Potential issue | 🔴 Criticaldon’t persist requested duration as source duration.
line 173 writes
props.duration(requested hint) into metadata as if it were actual media length. that can bypass probing downstream and trim past real eof, bringing back freeze frames/xfade drift.proposed fix
- const file = File.fromGenerated({ + const generatedDuration = + typeof (video as { duration?: number }).duration === "number" && + Number.isFinite((video as { duration?: number }).duration) && + (video as { duration?: number }).duration > 0 + ? (video as { duration?: number }).duration + : undefined; + + const file = File.fromGenerated({ uint8Array: video.uint8Array, mediaType, url: (video as { url?: string }).url, }).withMetadata({ type: "video", model: modelId, prompt: promptText, - duration: props.duration, + ...(generatedDuration !== undefined + ? { duration: generatedDuration } + : {}), });Based on learnings: Ensure that when inserting generated clips into an original video, the sum of all video segment durations equals the duration of the audio segment being used to maintain audio/video synchronization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/renderers/video.ts` around lines 169 - 174, The metadata currently writes the requested hint props.duration into withMetadata (see the withMetadata({...}) call that includes type: "video", model: modelId, prompt: promptText, duration: props.duration); replace that by either omitting the duration field or populating it with the actual resolved media duration (e.g., use the clip/media object’s real duration value such as clip.duration or the probed source duration) so downstream code probes real length instead of trusting the requested hint; ensure when inserting generated clips you compute/adjust clip durations so their sum equals the audio segment duration to preserve A/V sync rather than writing props.duration into metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/react/renderers/video.ts`:
- Around line 169-174: The metadata currently writes the requested hint
props.duration into withMetadata (see the withMetadata({...}) call that includes
type: "video", model: modelId, prompt: promptText, duration: props.duration);
replace that by either omitting the duration field or populating it with the
actual resolved media duration (e.g., use the clip/media object’s real duration
value such as clip.duration or the probed source duration) so downstream code
probes real length instead of trusting the requested hint; ensure when inserting
generated clips you compute/adjust clip durations so their sum equals the audio
segment duration to preserve A/V sync rather than writing props.duration into
metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1ab9fae-b3e8-4743-94d0-9f17fb6ce123
📒 Files selected for processing (5)
src/ai-sdk/file.tssrc/ai-sdk/providers/editly/index.tssrc/ai-sdk/providers/editly/types.tssrc/react/renderers/clip.tssrc/react/renderers/video.ts
✅ Files skipped from review due to trivial changes (1)
- src/ai-sdk/providers/editly/index.ts
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
processClips(previously only probed when no explicitclip.durationwas set)clip.durationto the actual video length when the video is shorter than declared — prevents xfade offset misalignmentvideoLayer.cutToto source duration — prevents FFmpegtrimfilter from reading past the end, which caused freeze framesProblem
When
speech.duration(from ElevenLabs TTS) was used as the clip duration for lipsync videos (veed-fabric-1.0), the generated video was often slightly shorter than the input audio. The old code skipped video probing whenclip.durationwas explicitly set, so the composition assumed frames existed that didn't — causing frozen frames at clip boundaries and xfade glitches.Same issue with
cutFrom/cutToexceeding source length (e.g.cutTo=3.3on a 3.0s Kling clip).Fix