WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@pmartinonales
Copy link

@pmartinonales pmartinonales commented Dec 10, 2025

Fix: Split action respects segment timescale (speed)

Problem

#1445 When using the split tool (scissors) on the timeline, if a previous segment had a modified playback speed (timescale), the split would occur at the wrong position instead of where the user clicked.

Root Cause

The code was mixing two different time representations:

Time Space Formula Description
Timeline time (end - start) / timescale Visual duration on the timeline
Recording time end - start Raw source video duration

The splitClipSegment function searched for the target segment using recording time durations instead of timeline time, causing incorrect segment detection when earlier segments had modified speeds.

Example

Consider this timeline:

  • Segment 0: 100 frames at 2x speed → 50s visible on timeline
  • Segment 1: 100 frames at 1x speed → 100s visible on timeline

When clicking to split at 25% into Segment 1 (which is at 75s into the timeline):

Before Fix After Fix
Result ❌ Splits Segment 0 at frame 75 ✅ Splits Segment 1 at frame 25

Changes

apps/desktop/src/routes/editor/context.ts

  • Account for timescale when calculating segment duration during search
  • Convert timeline time back to recording time when setting the split position

apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx

  • Calculate splitTime in timeline time (divide by timescale) to be consistent with prevDuration()

Testing

  • Split works correctly when all segments are at 1x speed
  • Split works correctly when previous segments have modified speed (0.5x, 2x, 4x, etc.)
  • Split at current playhead position (C key) works correctly with modified speeds
  • Click-to-split in split mode works correctly with modified speeds

Summary by CodeRabbit

Bug Fixes

  • Fixed clip splitting in the timeline editor to ensure clips are split at the correct position, improving accuracy when working with clips that have varying playback rates.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

Two files modified to account for segment timescale in clip-splitting logic. The splitTime calculation in ClipTrack now divides by timescale. In context.ts, duration calculations and segment boundaries are adjusted using timescale normalization through a new splitPositionInRecording variable.

Changes

Cohort / File(s) Summary
Timescale-Aware Clip Splitting
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx, apps/desktop/src/routes/editor/context.ts
Updated clip-splitting calculations to account for segment timescale. splitTime now normalized by dividing by seg.timescale in ClipTrack. In context.ts, duration calculations use timescale normalization and a new splitPositionInRecording variable controls segment boundary adjustments during splits.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify timescale calculations are correct and consistently applied across both files
  • Confirm the splitPositionInRecording formula accurately represents the split position in the original recording timeline
  • Validate that segment duration and boundary adjustments properly account for playback speed

Possibly related issues

Possibly related PRs

Suggested labels

Desktop

Poem

🐰 A bunny's clips now split just right,
With timescale speeds held tight,
Each segment dances at its beat,
This bugfix makes the story complete!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: split action respects segment timescale (speed)' accurately describes the main change: fixing the split tool to properly handle segment timescales/speeds.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a89670 and 259beaf.

📒 Files selected for processing (2)
  • apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (1 hunks)
  • apps/desktop/src/routes/editor/context.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use React Query hooks with Server Actions for mutations and perform precise cache updates using setQueryData/setQueriesData instead of broad invalidations

Files:

  • apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets

**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome using pnpm format
Use Biome for code formatting and linting; run pnpm format regularly
Use kebab-case for file names (e.g., user-menu.tsx); use PascalCase for components

Files:

  • apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
  • apps/desktop/src/routes/editor/context.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention

Files:

  • apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
  • apps/desktop/src/routes/editor/context.ts
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (AGENTS.md)

Never add comments to code (//, /* */, ///, //!, #, etc.); code must be self-explanatory through naming, types, and structure

Files:

  • apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
  • apps/desktop/src/routes/editor/context.ts
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use TanStack Query v5 for all client-side server state and data fetching in TypeScript files

Files:

  • apps/desktop/src/routes/editor/context.ts
apps/desktop/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

apps/desktop/**/*.ts: Use @tanstack/solid-query for server state management in SolidJS components
Use generated commands and events from tauri_specta for IPC; never manually construct IPC calls
Listen directly to generated events from tauri_specta and use typed event interfaces

Files:

  • apps/desktop/src/routes/editor/context.ts
🧬 Code graph analysis (1)
apps/desktop/src/routes/editor/context.ts (3)
crates/project/src/configuration.rs (2)
  • duration (535-537)
  • duration (728-730)
crates/rendering/src/project_recordings.rs (2)
  • duration (178-180)
  • duration (196-207)
crates/editor/src/segments.rs (1)
  • segments (7-36)
🔇 Additional comments (2)
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (1)

419-428: splitTime now correctly uses timeline time, aligning with segmentOffsets and splitClipSegment

Using (fraction * (seg.end - seg.start)) / seg.timescale ensures splitTime is in timeline seconds, consistent with prevDuration() (which is based on (end - start) / timescale) and with splitClipSegment’s timescale-aware search logic. This fixes the previous mixing of recording time and timeline time when earlier segments had non‑1x speeds.

apps/desktop/src/routes/editor/context.ts (1)

196-228: splitClipSegment’s timescale-aware search and split position conversion are consistent

Computing duration as (segment.end - segment.start) / segment.timescale makes the segment search operate in timeline seconds, matching how offsets and totalDuration are computed. Then splitPositionInRecording = searchTime * segment.timescale correctly converts the local timeline offset back into recording time before updating start/end, so the two resulting segments preserve overall timeline duration even when timescale !== 1. This aligns with the updated splitTime calculation in ClipTrack.tsx and should resolve the mis-split bug when earlier segments have modified speeds, assuming all callers pass time in timeline seconds.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@richiemcilroy
Copy link
Member

Amazing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants