feat: Implement speed changing (with mute) #986
Conversation
WalkthroughAdds per-segment audio mute support and playback speed controls. Extends TimelineSegment with mute flags, updates project config creation, UI to toggle mute and adjust speed, timeline rendering to account for timescale in segment length, and audio engine to mute output when current segment is muted. Adds a segment lookup API. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as Editor UI (ConfigSidebar)
participant TL as Timeline State
participant PR as Project Config
participant AE as Audio Engine
U->>UI: Toggle "Audio" / adjust "Playback Speed"
UI->>TL: Update segment.muteAudio / segment.timescale
TL->>PR: Persist updated TimelineSegment
rect rgba(230,245,255,0.5)
note over AE,PR: During playback/render
AE->>PR: get_segment_at_time(playhead)
PR-->>AE: TimelineSegment { mute_audio, timescale, ... }
AE->>AE: If mute_audio || global mute → silence frame
end
sequenceDiagram
autonumber
participant UI as Timeline View (ClipTrack)
participant TL as Timeline State
UI->>TL: Read segment {start,end,timescale}
TL-->>UI: Segment data
UI->>UI: Compute width = (end-start)/timescale
UI-->>UI: Render clip with adjusted length
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (1)
32-37: Waveform rendering ignores timescale — visual desync when speed ≠ 1.Waveform X mapping uses source seconds; segment width uses timeline seconds (divided by
timescale). Result: waveform stretches/compresses incorrectly for speeds. Adjust X mapping to account fortimescale.-function WaveformCanvas(props: { - systemWaveform?: number[]; - micWaveform?: number[]; - segment: { start: number; end: number }; - secsPerPixel: number; -}) { +function WaveformCanvas(props: { + systemWaveform?: number[]; + micWaveform?: number[]; + segment: { start: number; end: number; timescale?: number }; + secsPerPixel: number; +}) { @@ - const x = (xTime - props.segment.start) / secsPerPixel(); + const x = ((xTime - props.segment.start) / (props.segment.timescale ?? 1)) / secsPerPixel(); @@ - const prevX = (xTime - 0.1 - props.segment.start) / secsPerPixel(); + const prevX = ((xTime - 0.1 - props.segment.start) / (props.segment.timescale ?? 1)) / secsPerPixel(); @@ - ctx.lineTo( - (props.segment.end + 0.3 - props.segment.start) / secsPerPixel(), - h, - ); + ctx.lineTo( + ((props.segment.end + 0.3 - props.segment.start) / (props.segment.timescale ?? 1)) / secsPerPixel(), + h, + );Also applies to: 78-82, 93-95
🧹 Nitpick comments (8)
crates/project/src/configuration.rs (2)
506-517: Handle negative times and reduce duplication with existing lookup.
- Edge case: negative
frame_timecurrently resolves to the first segment. Consider returningNoneearly for< 0.0.- Minor duplication with
get_segment_time; factor a small helper to DRY.Apply:
- pub fn get_segment_at_time(&self, frame_time: f64) -> Option<&TimelineSegment> { - let mut accum_duration = 0.0; + pub fn get_segment_at_time(&self, frame_time: f64) -> Option<&TimelineSegment> { + if frame_time < 0.0 { + return None; + } + let mut accum_duration = 0.0; for segment in self.segments.iter() { if frame_time < accum_duration + segment.duration() { return Some(segment); } accum_duration += segment.duration(); } None }
441-443: Guard against zero/negative timescale in duration.Division by zero yields
infand can skew lookups. Consider clamping to a small epsilon.- pub fn duration(&self) -> f64 { - (self.end - self.start) / self.timescale + pub fn duration(&self) -> f64 { + let ts = if self.timescale <= 0.0 { 1.0 } else { self.timescale }; + (self.end - self.start) / ts }apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (1)
472-474: Show timeline duration instead of source duration (optional).For speed-changed clips, consider displaying timeline duration:
((end - start) / timescale).toFixed(1).- { (segment.end - segment.start).toFixed(1) }s + { ((segment.end - segment.start) / (segment.timescale ?? 1)).toFixed(1) }scrates/editor/src/audio.rs (1)
184-190: Avoid O(n) segment search per audio frame.You already track
cursor.segment_index. Use it to readmute_audiodirectly instead of scanning timeline each render.- let segment_muted = project - .timeline - .as_ref() - .and_then(|t| t.get_segment_at_time(self.elapsed_samples_to_playhead())) - .map(|s| s.mute_audio) - .unwrap_or(false); + let segment_muted = project + .timeline + .as_ref() + .and_then(|t| t.segments.get(self.cursor.segment_index as usize)) + .map(|s| s.mute_audio) + .unwrap_or(false);apps/desktop/src/routes/editor/ConfigSidebar.tsx (4)
52-52: Remove manual lucide icon imports; rely on unplugin-icons auto-imports.Per desktop guidelines, avoid manual icon imports. Let unplugin-icons auto-import these components.
-import IconLucideGauge from "~icons/lucide/gauge"; -... -import IconLucideVolumeX from "~icons/lucide/volume-x"; +// Use <IconLucideGauge /> and <IconLucideVolumeX /> without manual imports.Also applies to: 55-55
2247-2257: Presets look good; consider centralizing MIN/MAX speed constants.You duplicate 0.5 and 5 elsewhere. Define once to keep UI/logic in sync.
const speedPresets = [ @@ { label: "5×", value: 5 }, ]; + +const MIN_SPEED = 0.5; +const MAX_SPEED = 5;
2265-2275: Clamp using shared constants; consider grouping history for preset clicks.Use MIN/MAX constants. For preset clicks, optionally pause/resume history to keep a single entry.
-const handleSpeedChange = (speed: number) => { - // Clamp the speed value to ensure it's within valid bounds - const clampedSpeed = Math.min(Math.max(speed, 0.5), 5); +const handleSpeedChange = (speed: number) => { + const clampedSpeed = Math.min(Math.max(speed, MIN_SPEED), MAX_SPEED); setProject( "timeline", "segments", props.segmentIndex, "timescale", clampedSpeed, ); };
2343-2346: Make preset highlight tolerant to float precision.Strict equality may miss due to rounding.
- currentSpeed() === preset.value + Math.abs(currentSpeed() - preset.value) < 1e-6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/desktop/src-tauri/src/recording.rs(1 hunks)apps/desktop/src/routes/editor/ConfigSidebar.tsx(3 hunks)apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx(1 hunks)apps/desktop/src/utils/tauri.ts(1 hunks)crates/editor/src/audio.rs(1 hunks)crates/project/src/configuration.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file: tauri.ts
Files:
apps/desktop/src/utils/tauri.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsx
apps/desktop/src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types
Files:
apps/desktop/src-tauri/src/recording.rs
🧬 Code graph analysis (2)
crates/project/src/configuration.rs (1)
apps/desktop/src/utils/tauri.ts (1)
TimelineSegment(442-442)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (2)
apps/desktop/src/routes/editor/ui.tsx (3)
Field(25-47)Slider(65-147)Subfield(49-63)apps/desktop/src/components/Toggle.tsx (1)
Toggle(37-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
crates/project/src/configuration.rs (1)
427-429: Good additive field; serde default + camelCase is correct.
mute_audiowith#[serde(default)]integrates safely with older configs and maps tomuteAudioviarename_all = "camelCase". No breaking changes.apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (1)
195-198: Timescale-aware width: LGTM.Using
(end - start) / timescale + prevDuration()fixes segment width on the timeline.apps/desktop/src-tauri/src/recording.rs (1)
1014-1015: Defaulting new field: LGTM.Initializing
mute_audio: falsefor each segment is consistent with the Rust model (#[serde(default)]).apps/desktop/src/utils/tauri.ts (1)
442-442: Auto-generated type updated appropriately.
TimelineSegmentnow includes optionalmuteAudio. Ensure this file remains generated only (no manual edits).crates/editor/src/audio.rs (1)
196-201: Mute logic extension: LGTM.
project.audio.mute || segment_mutedcorrectly applies per-segment muting without changing other gain paths.apps/desktop/src/routes/editor/ConfigSidebar.tsx (2)
2263-2264: LGTM: derived speed getter is straightforward.
2259-2262: BindmuteAudiodirectly to the store; remove local state &anycast.- const [muteAudio, setMuteAudio] = createSignal( - (props.segment as any).muteAudio ?? false, - ); + const segmentMuted = () => props.segment.muteAudio ?? false; - const handleMuteToggle = (muted: boolean) => { - setMuteAudio(muted); - setProject("timeline", "segments", props.segmentIndex, "muteAudio" as any, muted); - }; + const handleMuteToggle = (muted: boolean) => + setProject("timeline", "segments", props.segmentIndex, "muteAudio", muted); - <Field - name="Audio" - icon={<IconLucideVolumeX class="size-4" />} - value={<Toggle checked={muteAudio()} onChange={handleMuteToggle} />} - > + <Field + name="Audio" + icon={<IconLucideVolumeX class="size-4" />} + value={<Toggle checked={segmentMuted()} onChange={handleMuteToggle} />} + >Use the typed
props.segment.muteAudio?: boolean(no cast needed).
| <Field name="Playback Speed" icon={<IconLucideGauge class="size-4" />}> | ||
| <div class="space-y-3"> | ||
| <div class="flex items-center gap-2"> | ||
| <Slider | ||
| class="flex-1" | ||
| minValue={0.5} | ||
| maxValue={5} | ||
| step={0.05} | ||
| value={[currentSpeed()]} | ||
| onChange={(value: number[]) => { | ||
| let speed = value[0]; | ||
| if (speed > 5) { | ||
| speed = speed / 10; | ||
| } | ||
| handleSpeedChange(speed); | ||
| }} | ||
| /> | ||
| <span class="text-sm font-medium text-gray-11 min-w-[3rem] text-right"> | ||
| {currentSpeed().toFixed(2)}× | ||
| </span> | ||
| </div> | ||
| <div class="flex flex-wrap gap-1.5"> | ||
| <For each={speedPresets}> | ||
| {(preset) => ( | ||
| <button | ||
| class={cx( | ||
| "px-2.5 py-1 text-xs rounded-md border transition-colors", | ||
| currentSpeed() === preset.value | ||
| ? "bg-blue-9 text-white border-blue-9" | ||
| : "bg-gray-3 text-gray-11 border-gray-4 hover:border-gray-6", | ||
| )} | ||
| onClick={() => handleSpeedChange(preset.value)} | ||
| > | ||
| {preset.label} | ||
| </button> | ||
| )} | ||
| </For> | ||
| </div> | ||
| </div> | ||
| </Field> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unreachable divide-by-10 path; rely on clamp. Add tooltip, reuse MIN/MAX.
Slider already has max=5; dividing by 10 is surprising and can mask bugs.
<Slider
class="flex-1"
- minValue={0.5}
- maxValue={5}
+ minValue={MIN_SPEED}
+ maxValue={MAX_SPEED}
step={0.05}
value={[currentSpeed()]}
- onChange={(value: number[]) => {
- let speed = value[0];
- if (speed > 5) {
- speed = speed / 10;
- }
- handleSpeedChange(speed);
- }}
+ formatTooltip={(v) => `${v.toFixed(2)}×`}
+ onChange={(value: number[]) => {
+ handleSpeedChange(value[0]);
+ }}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Field name="Playback Speed" icon={<IconLucideGauge class="size-4" />}> | |
| <div class="space-y-3"> | |
| <div class="flex items-center gap-2"> | |
| <Slider | |
| class="flex-1" | |
| minValue={0.5} | |
| maxValue={5} | |
| step={0.05} | |
| value={[currentSpeed()]} | |
| onChange={(value: number[]) => { | |
| let speed = value[0]; | |
| if (speed > 5) { | |
| speed = speed / 10; | |
| } | |
| handleSpeedChange(speed); | |
| }} | |
| /> | |
| <span class="text-sm font-medium text-gray-11 min-w-[3rem] text-right"> | |
| {currentSpeed().toFixed(2)}× | |
| </span> | |
| </div> | |
| <div class="flex flex-wrap gap-1.5"> | |
| <For each={speedPresets}> | |
| {(preset) => ( | |
| <button | |
| class={cx( | |
| "px-2.5 py-1 text-xs rounded-md border transition-colors", | |
| currentSpeed() === preset.value | |
| ? "bg-blue-9 text-white border-blue-9" | |
| : "bg-gray-3 text-gray-11 border-gray-4 hover:border-gray-6", | |
| )} | |
| onClick={() => handleSpeedChange(preset.value)} | |
| > | |
| {preset.label} | |
| </button> | |
| )} | |
| </For> | |
| </div> | |
| </div> | |
| </Field> | |
| <Slider | |
| class="flex-1" | |
| minValue={MIN_SPEED} | |
| maxValue={MAX_SPEED} | |
| step={0.05} | |
| value={[currentSpeed()]} | |
| formatTooltip={(v) => `${v.toFixed(2)}×`} | |
| onChange={(value: number[]) => { | |
| handleSpeedChange(value[0]); | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/ConfigSidebar.tsx around lines 2317 to 2357,
the Slider onChange handler contains an unreachable divide-by-10 branch and uses
hard-coded min/max values; remove the conditional that divides speed by 10,
instead clamp the selected value to the established MIN and MAX playback speed
constants (reuse the existing minValue/maxValue or extract constants if not
present), add a tooltip on the speed display/button to show the exact numeric
value, and ensure handleSpeedChange is called with the clamped value only.
This PR adds per-segment audio mute and playback speed controls to the timeline editor, allowing users to individually adjust speed and mute audio for each segment.
Demo:
https://cap.link/jg1p9a7zgwdysek
Summary by CodeRabbit