Add clip speed editing #632
Conversation
EricWiener
force-pushed
the
ewiener/clip-speed-up
branch
from
b1b37c9 to
77372d7
Compare
|
Speed changes are only implemented in the video renderer at the moment, audio rendering will need to be modified to respect the |
|
Caution Review failedThe pull request is closed. WalkthroughRenames Segment → SegmentMedia and segments → segment_medias across crates; replaces TimelineSegment.recording_segment with recording_clip; adds per-segment timescale state + UI/action; threads clip_index/timescale through audio, playback, and rendering paths; updates frame/decoder lookups and minor desktop UI/CI/icon typings. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as ConfigSidebar\n(Speed Control)
participant Actions as projectActions\n.setClipSegmentTimescale
participant Project as ProjectState
participant ClipTrack as ClipTrack UI
User->>UI: choose multiplier (e.g., 2x)
UI->>Actions: setClipSegmentTimescale(index, timescale)
activate Actions
Actions->>Project: update segment.timescale, start, end
Actions->>Project: propagate length diffs to zoomSegments
deactivate Actions
Note over Project: segment.timescale ≠ 1
Project->>ClipTrack: state update
alt timescale == 1
ClipTrack->>ClipTrack: render WaveformCanvas
else timescale != 1
ClipTrack->>ClipTrack: hide waveform
ClipTrack->>ClipTrack: show indicator (⏩ x{timescale})
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/desktop/src-tauri/src/lib.rs(2 hunks)apps/desktop/src-tauri/src/recording.rs(1 hunks)apps/desktop/src/routes/editor/ConfigSidebar.tsx(1 hunks)apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx(4 hunks)apps/desktop/src/routes/editor/context.ts(1 hunks)crates/editor/src/audio.rs(6 hunks)crates/editor/src/editor_instance.rs(9 hunks)crates/editor/src/lib.rs(1 hunks)crates/editor/src/playback.rs(5 hunks)crates/editor/src/segments.rs(1 hunks)crates/export/src/lib.rs(2 hunks)crates/project/src/configuration.rs(3 hunks)crates/rendering/src/lib.rs(3 hunks)packages/ui-solid/src/auto-imports.d.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/editor/src/segments.rscrates/editor/src/lib.rsapps/desktop/src-tauri/src/lib.rscrates/editor/src/audio.rscrates/rendering/src/lib.rsapps/desktop/src-tauri/src/recording.rscrates/export/src/lib.rscrates/project/src/configuration.rscrates/editor/src/editor_instance.rscrates/editor/src/playback.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/editor/src/segments.rscrates/editor/src/lib.rscrates/editor/src/audio.rscrates/rendering/src/lib.rscrates/export/src/lib.rscrates/project/src/configuration.rscrates/editor/src/editor_instance.rscrates/editor/src/playback.rs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/context.tspackages/ui-solid/src/auto-imports.d.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/context.tspackages/ui-solid/src/auto-imports.d.ts
🧬 Code graph analysis (6)
crates/editor/src/lib.rs (1)
crates/editor/src/editor_instance.rs (1)
create_segments(291-375)
crates/editor/src/audio.rs (1)
crates/editor/src/playback.rs (1)
start(50-171)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
apps/desktop/src/routes/editor/ui.tsx (1)
Field(25-47)
crates/project/src/configuration.rs (1)
apps/desktop/src/utils/tauri.ts (1)
TimelineSegment(475-475)
crates/editor/src/editor_instance.rs (1)
crates/editor/src/segments.rs (1)
segments(7-36)
crates/editor/src/playback.rs (1)
crates/editor/src/segments.rs (1)
get_audio_segments(6-37)
🔇 Additional comments (16)
packages/ui-solid/src/auto-imports.d.ts (1)
1-113: Auto-generated file - changes look appropriate.This file is auto-generated by unplugin-auto-import. The icon additions (e.g.,
IconLucideFastForward,IconLucideRabbit,IconCapClock) align with the PR's clip speed editing feature.crates/editor/src/audio.rs (6)
22-28: LGTM: Cursor structure updated correctly.The refactoring from
segment_indextoclip_indexwith the addition oftimescaleappropriately supports the new clip-based speed control model.
85-95: LGTM: Initialization values are appropriate.Default values correctly initialize the cursor:
clip_index: 0for the first clip,timescale: 1.0for normal playback speed, andsamples: 0for the starting position.
97-112: LGTM: Playhead positioning correctly integrates clip and timescale data.The method properly extracts
clip_indexandtimescalefrom the segment, with appropriate fallback values when no segment is found.
114-140: LGTM: Cursor adjustment logic correctly updated.The method properly handles clip transitions and timescale changes, with the comparison at line 135 correctly using
clip_indexto detect segment boundaries.
181-181: LGTM: Track access correctly uses clip_index.The array indexing properly references the current clip using
clip_index.
204-212: LGTM: Clip offset resolution correctly uses clip_index.The lookup properly matches clips by
clip_indexto retrieve the appropriate offsets.crates/export/src/lib.rs (1)
4-4: LGTM: Straightforward type rename.The import and field type have been correctly updated from
SegmenttoSegmentMedia, consistent with the broader refactor across the codebase.Also applies to: 127-127
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (3)
225-237: LGTM: Correct timescale integration.The segment duration calculations correctly account for timescale by dividing
(segment.end - segment.start) / segment.timescale, converting recorded time to playback time.
475-482: Good: Conditional waveform rendering.Gating
WaveformCanvasrendering to only show whentimescale === 1is appropriate, as waveforms would be misleading when the clip is sped up or slowed down. This aligns with the note in the UI that speed changes affect audio.
592-596: LGTM: Helpful timescale indicator.The visual indicator showing the timescale multiplier when it's not 1x provides good user feedback about which segments have modified speed.
crates/editor/src/playback.rs (2)
19-19: LGTM: Consistent type and field renames.The changes correctly update:
- Import from
SegmenttoSegmentMedia- Field from
segmentstosegment_medias- Function call to use the new field name
Also applies to: 33-33, 83-83
114-122: Correct: Updated segment lookup to use recording_clip.The segment lookup has been updated to index by
segment.recording_clipinstead of the previous approach, consistent with the field rename throughout the PR.crates/rendering/src/lib.rs (1)
192-192: LGTM: Clear parameter rename and updated segment access.The changes correctly:
- Rename the parameter to
render_segmentsfor clarity- Update segment lookups to use
segment.recording_clipas the index- Pass additional parameters (
needs_camera,offsets) toget_frames- Access cursor data from the render_segment
All changes are consistent with the broader refactor.
Also applies to: 216-226, 232-259
crates/editor/src/editor_instance.rs (1)
29-29: LGTM: Comprehensive and consistent type rename.The changes thoroughly update:
- Field names from
segmentstosegment_medias- Type names from
SegmenttoSegmentMedia- Segment lookups to use
segment.recording_clipindexing- Function signatures and return types
All changes maintain consistency across the public API and internal implementation.
Also applies to: 84-84, 149-149, 210-240, 284-294, 319-319, 364-364
apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
548-551: Audio muting behavior is correctly implemented and matches the UI message.The verification confirms that audio IS automatically muted when timescale changes. In
crates/editor/src/audio.rsat lines 177-179, the audio rendering explicitly returnsNonewhenevertimescale != 1.0, preventing any audio output. This automatic muting prevents the sync issues mentioned in the review (avoiding out-of-speed audio playback). The UI warning accurately describes the implemented behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui-solid/src/auto-imports.d.ts (1)
11-115: Inconsistent quote styles in auto-generated declarations.The newly added icon declarations use double quotes (e.g., lines 11, 13, 17, 23, 33, etc.), while existing declarations use single quotes. Since this file is auto-generated by unplugin-auto-import, the inconsistency may stem from the tool's configuration or from different upstream icon package exports.
Consider verifying the unplugin-auto-import configuration to ensure consistent quote style generation, though this is a low-priority cosmetic issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yml(1 hunks)apps/desktop/src-tauri/src/lib.rs(2 hunks)apps/desktop/src/routes/editor/ConfigSidebar.tsx(1 hunks)packages/ui-solid/src/auto-imports.d.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src-tauri/src/lib.rs
- apps/desktop/src/routes/editor/ConfigSidebar.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/ui-solid/src/auto-imports.d.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/ui-solid/src/auto-imports.d.ts
🔇 Additional comments (2)
.github/workflows/ci.yml (1)
100-108: Good refactor: make clippy runner platform-specific via matrix.Using
${{ matrix.settings.runner }}aligns the clippy job with the build-desktop and rust-cache jobs, ensuring each target runs on its appropriate OS. Previously, clippy would attempt to lint Windows targets on macOS, which is both inefficient and potentially problematic for platform-specific checks.packages/ui-solid/src/auto-imports.d.ts (1)
1-7: Auto-generated file – changes reflect icon usage elsewhere.This file is automatically generated by unplugin-auto-import. The new icon declarations added throughout this file correspond to icons now used in the clip speed editing UI and other features. No manual changes should be made directly to this file.
|
audio doesnt't work in 1x clip, after adding a intermediate sped up clip section |
3 participants
Example usage shown below:
https://github.com/user-attachments/assets/6f0381a2-87d3-4f46-8268-20af74d419fc
Summary by CodeRabbit
New Features
UI