fix: timeline/preview sync for speed-edited clips (clip offsets, mapping, zoom split)#439
Closed
ExtraBinoss wants to merge 10 commits intowebadderallorg:mainfrom
Closed
fix: timeline/preview sync for speed-edited clips (clip offsets, mapping, zoom split)#439ExtraBinoss wants to merge 10 commits intowebadderallorg:mainfrom
ExtraBinoss wants to merge 10 commits intowebadderallorg:mainfrom
Conversation
add: VideoEditor fix when a clip is shorter than an other, it would skip it fix: zoom will also be cut according to split/speed of a clip
…onflicts in types.ts)
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR refactors clip timing throughout the video editor to be source-based rather than timeline-only. It introduces ChangesSource-Based Clip Timing & Timeline Mapping
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ExtraBinoss
changed the title
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/video-editor/types.ts (1)
237-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp timeline gaps to source boundaries, not timeline boundaries.
When no clip contains
timeMs, this function still falls back to the nearest timeline edge. For any clip wheresourceStartMs !== startMs, seeking in dead space before/after a clip returns the wrong source offset and can jump preview playback into the wrong part of the recording. Use source-space boundaries for the fallback too.Suggested fix
- return clampToNearestClipBoundary(roundedTimeMs, sortedClips, "timeline"); + return clampToNearestClipBoundary(roundedTimeMs, sortedClips, "source");// Also update `clampToNearestClipBoundary` so its source branch uses: [getClipSourceStartMs(clip), getClipSourceEndMs(clip)]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/types.ts` around lines 237 - 259, The fallback currently clamps empty-space seeks to the nearest timeline boundary, which yields incorrect source offsets when a clip's sourceStartMs differs from startMs; update the final return to call clampToNearestClipBoundary(roundedTimeMs, sortedClips, "source") (and modify clampToNearestClipBoundary so its "source" branch uses [getClipSourceStartMs(clip), getClipSourceEndMs(clip)]), ensuring normalizeSortedClipRegions, getClipSourceStartMs, getClipSourceEndMs, getSafeClipSpeed and the sortedClips/roundedTimeMs logic produce source-space clamping rather than timeline-space.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/video-editor/timeline/AudioWaveform.tsx`:
- Line 3: The waveform remapping currently computes sourceTimeMs anchored to
activeClip.startMs and therefore ignores the clip's sourceStartMs, causing bars
to sample the wrong source segment; update the mapping logic (where sourceTimeMs
is computed and in the same remap used around the waveform rendering code and
the block that mirrors lines ~97-101) to include clip.sourceStartMs so
sourceTimeMs = anchor + (time - clip.startMs) + clip.sourceStartMs (or
equivalent: compute offsets entirely in source-space using sourceStartMs),
ensuring all uses of sourceTimeMs and the remap function reference the
clip.sourceStartMs anchor (e.g., in the functions/variables named sourceTimeMs,
activeClip.startMs, sourceStartMs, and the waveform remap logic).
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 1097-1120: totalMs currently uses region.endMs (which for
trimRegions is in source-space) and then forces Math.max(sourceDurationMs,
maxRegionEndMs), causing the timeline to never shrink; change the totalMs
calculation to use timeline-space end times for all region types (convert
trimRegions.endMs from source→timeline or call the existing helper that computes
a region's timeline end) when building allRegionEnds for zoomRegions,
trimRegions, clipRegions, speedRegions, annotationRegions and audioRegions, and
only fall back to sourceDurationMs when there are no timeline-space region ends
at all (i.e., when allRegionEnds is empty).
---
Outside diff comments:
In `@src/components/video-editor/types.ts`:
- Around line 237-259: The fallback currently clamps empty-space seeks to the
nearest timeline boundary, which yields incorrect source offsets when a clip's
sourceStartMs differs from startMs; update the final return to call
clampToNearestClipBoundary(roundedTimeMs, sortedClips, "source") (and modify
clampToNearestClipBoundary so its "source" branch uses
[getClipSourceStartMs(clip), getClipSourceEndMs(clip)]), ensuring
normalizeSortedClipRegions, getClipSourceStartMs, getClipSourceEndMs,
getSafeClipSpeed and the sortedClips/roundedTimeMs logic produce source-space
clamping rather than timeline-space.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: afdb209c-5513-42b8-9cfe-8d61a744fe34
📒 Files selected for processing (6)
src/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/AudioWaveform.tsxsrc/components/video-editor/timeline/KeyframeMarkers.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/videoEventHandlers.ts
💤 Files with no reviewable changes (1)
- src/components/video-editor/timeline/KeyframeMarkers.tsx
There was a problem hiding this comment.
Pull request overview
This PR aims to fix timeline/playback inconsistencies after clip speed edits by separating “source time” from “timeline time” (via a sourceStartMs anchor), updating the mapping helpers, and remapping dependent regions (zoom/annotation/audio/speed) so UI + preview stay aligned.
Changes:
- Introduces
sourceStartMsonClipRegionand updates source↔timeline mapping utilities to use it, with additional boundary handling. - Improves timeline edit behavior: propagates timeline offsets after speed changes, splits zoom regions on clip split, remaps zoom timing on speed changes, and clamps seeks more defensively.
- Updates timeline UI behavior: timeline duration/scale derived from edited layout, wheel panning is RAF-batched, and audio waveform is made clip-aware.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/video-editor/videoPlayback/videoEventHandlers.ts | Adds guard to prevent trim-boundary seek loops. |
| src/components/video-editor/VideoEditor.tsx | Uses source-anchored clip speed regions, remaps regions on speed edits/split, clamps seek, and displays timeline-based duration. |
| src/components/video-editor/types.ts | Adds sourceStartMs + mapping updates (timeline↔source), plus clip→trim conversion updates. |
| src/components/video-editor/timeline/TimelineEditor.tsx | Passes clip regions down for waveform rendering, derives total timeline bounds from regions, and batches wheel pan updates. |
| src/components/video-editor/timeline/KeyframeMarkers.tsx | Removes marker left-transition styling while not dragging. |
| src/components/video-editor/timeline/AudioWaveform.tsx | Makes waveform rendering aware of clip regions and speed mapping. |
…, it will also match the duration correctly
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 1093-1118: The totalMs computed in TimelineEditor is
timeline-space but the trim normalization later is incorrectly clamping
trimRegions against totalMs (timeline end), which will shorten valid
source-space trims after speed edits; change the normalization logic that clamps
trimRegions to use sourceDurationMs (the source-space upper bound computed with
Math.round(videoDuration * 1000)) instead of totalMs so trims are validated
against the original source duration; locate the totalMs and sourceDurationMs
variables and the trim normalization code that references totalMs (e.g., the
normalize pass that adjusts trimRegions) and replace its upper-bound check to
compare/round against sourceDurationMs.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 4323-4329: The audioOffset calculation uses raw source time and
jumps across trimmed cuts; replace it with a timeline-aware elapsed time:
compute timelineElapsedMs = currentTimelineTimeMs - region.startMs (this is
timeline-relative and already excludes skipped source segments) and set
audioOffset = timelineElapsedMs / 1000 (adding any region-specific source offset
if the AudioRegion model exposes one). Change the code that computes audioOffset
(currently using currentTimeMs - regionSourceStartMs and regionSourceStartMs
from mapTimelineTimeToSourceTime) to use currentTimelineTimeMs and
region.startMs so playback stays in sync across trimmed cuts.
- Around line 3773-3784: The code is incorrectly remapping speedRegions when
applying timeline shifts; remove speedRegions from the remap (do not call
remapTimelineTime for speedRegions) so only timeline-anchored regions
(annotationRegions, audioRegions) are transformed. In the block that runs when
deltaMs !== 0 (where remapTimelineTime is defined and
setAnnotationRegions/setAudioRegions/setSpeedRegions are called), stop calling
setSpeedRegions(remapRegions) and instead keep speedRegions unchanged (or
reapply previous state) so legacy speed effects remain in source-time
coordinates used by the runtime check (currentTimeMs >= region.startMs &&
currentTimeMs < region.endMs).
- Around line 4954-4981: The export error payload is leaking local filesystem
paths via audioPath and sourceAudioFallbackPaths in the console.error call;
update the payload construction (the console.error call around the audioContext
object and the audioRegions.map) to redact local paths by replacing audioPath
with either the filename basename or null and convert sourceAudioFallbackPaths
to either an array of basenames or a count (and similarly map
sourceAudioFallbackStartDelayMsByPath keys to basenames or remove exact paths),
while preserving IDs, startMs/endMs/trackIndex/volume and effectiveSpeedRegions
for debugging; ensure no full absolute paths are emitted in the console.error
payload.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b16f5f22-0e4c-444c-962a-bfeac1f993b2
📒 Files selected for processing (5)
src/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/AudioWaveform.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.test.tssrc/components/video-editor/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/video-editor/types.ts
This was referenced May 8, 2026
…o fix/timeline-slow-clip-visibility
Contributor
Author
|
Close as #434 fixes it |
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
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
Description
This PR fixes timeline/playback inconsistencies when changing clip speed, especially with adjacent clips.
Description
gaps and skip loops.
duration
Why
Previously, slowing down a clip and placing another clip after it could cause:
This PR makes timeline edits and preview playback consistent under speed changes.
Type of Change
Related Issue(s)
[Bug]: Slow-speed clips are not fully visible on the timeline #434
Video:
speed_fix_recordly.webm
Testing Guide
See video for how to test it
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Style
Tests
Chores