Conversation
|
Cursor Agent can help with this pull request. Just |
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
…ithub.com/CapSoftware/Cap into cursor/playback-performance-and-sync-dec3
…ithub.com/CapSoftware/Cap into cursor/playback-performance-and-sync-dec3
…ithub.com/CapSoftware/Cap into cursor/playback-performance-and-sync-dec3
…ithub.com/CapSoftware/Cap into cursor/playback-performance-and-sync-dec3
…ithub.com/CapSoftware/Cap into cursor/playback-performance-and-sync-dec3
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
Co-authored-by: Richie McIlroy <richiemcilroy@users.noreply.github.com>
…ithub.com/CapSoftware/Cap into cursor/playback-performance-and-sync-dec3
| } | ||
| } | ||
|
|
||
| if buffered_wait_prefetch_changed { |
There was a problem hiding this comment.
Small perf nit: if a seek came in during the wait loop, you continue right after this and the seek handler clears prefetch_buffer anyway, so you can bail before trimming.
| if buffered_wait_prefetch_changed { | |
| if seek_rx.has_changed().unwrap_or(false) { | |
| continue; | |
| } | |
| if buffered_wait_prefetch_changed { | |
| trim_prefetch_buffer(&mut prefetch_buffer, frame_number); | |
| } |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| frameNumber === pendingSeekFrame || | ||
| frameNumber === inFlightSeekFrame || | ||
| frameNumber === lastCompletedSeekFrame | ||
| ) { |
There was a problem hiding this comment.
Seek dedupe blocks valid repeat seeks
Medium Severity
scheduleSeek drops requests when frameNumber === lastCompletedSeekFrame. Because lastCompletedSeekFrame is never cleared on normal playback progress, a later seek back to a previously completed frame can be ignored even after the playhead moved away. This makes timeline scrubbing intermittently no-op in Timeline/index.tsx.
Additional Locations (1)
| *current_frame = frame_number; | ||
| true | ||
| } | ||
| }); |
There was a problem hiding this comment.
Repeated same-frame seeks are ignored
Medium Severity
PlaybackHandle::seek uses seek_tx.send_if_modified, so a seek to the same frame as the last requested seek is dropped. Because seek_tx is never updated as playback advances, later valid seeks back to that frame can be ignored even after the playhead moved away.
| if (zoomRafId !== null) cancelAnimationFrame(zoomRafId); | ||
| if (scrollRafId !== null) cancelAnimationFrame(scrollRafId); | ||
| if (seekRafId !== null) cancelAnimationFrame(seekRafId); | ||
| }); |
There was a problem hiding this comment.
Unmount doesn’t cancel pending seek pipeline
Low Severity
onCleanup only cancels current RAF IDs, but it does not invalidate flushPendingSeek while an async commands.seekTo is in flight. After unmount, the finally block can schedule another RAF and continue issuing seeks from a disposed Timeline instance.
Additional Locations (1)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| if let Some(handle) = playback_handle { | ||
| handle.seek(frame_number); | ||
| } |
There was a problem hiding this comment.
Seek dedupe uses stale state
Medium Severity
seek_to and set_playhead_position now skip handle.seek(...) when state.playhead_position already equals frame_number. But playhead_position is not advanced by playback frames, so it can be stale. This drops valid seeks to a previously requested frame while playback has moved, making scrubbing/rewind requests intermittently ignored.
Additional Locations (1)
| } | ||
| } | ||
|
|
||
| if let Err(error) = fs::write(&combined_path, &combined_data) { |
There was a problem hiding this comment.
Fragment merge can exhaust memory
Medium Severity
get_fragmented_video_duration reads init.mp4 and every .m4s segment into one in-memory Vec before writing a temp file. Large fragmented recordings can allocate very large buffers and terminate the benchmark process with OOM, since there is no size guard or streaming append path.
| prefetched.segment_index, | ||
| )) | ||
| } else { | ||
| let is_in_flight = main_in_flight | ||
| let in_flight_key = (seek_generation, frame_number); | ||
| let is_in_flight = playback_prefetch_in_flight | ||
| .read() | ||
| .map(|guard| guard.contains(&frame_number)) | ||
| .unwrap_or(false); | ||
| .map(|guard| guard.contains(&in_flight_key)) | ||
| .unwrap_or(false) | ||
| || playback_decode_in_flight | ||
| .read() | ||
| .map(|guard| guard.contains(&in_flight_key)) |
There was a problem hiding this comment.
both playback_prefetch_in_flight and playback_decode_in_flight locks checked separately with .unwrap_or(false) fallback - if lock acquisition fails, returns false and playback starts redundant decode even though frame may already be in-flight in prefetch task
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/playback.rs
Line: 795:805
Comment:
both `playback_prefetch_in_flight` and `playback_decode_in_flight` locks checked separately with `.unwrap_or(false)` fallback - if lock acquisition fails, returns `false` and playback starts redundant decode even though frame may already be in-flight in prefetch task
How can I resolve this? If you propose a fix, please make it concise.| function scheduleSeek(frameNumber: number) { | ||
| if ( | ||
| frameNumber === pendingSeekFrame || | ||
| frameNumber === inFlightSeekFrame || | ||
| frameNumber === lastCompletedSeekFrame | ||
| ) { | ||
| return; | ||
| } | ||
| pendingSeekFrame = frameNumber; | ||
| if (seekRafId === null) { | ||
| seekRafId = requestAnimationFrame(flushPendingSeek); | ||
| } | ||
| } |
There was a problem hiding this comment.
deduplicates seeks against lastCompletedSeekFrame which is only set on success (line 311) - if seekTo throws (line 313), lastCompletedSeekFrame stays stale, and retrying the same frame later gets incorrectly rejected as duplicate on line 284
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Timeline/index.tsx
Line: 280:292
Comment:
deduplicates seeks against `lastCompletedSeekFrame` which is only set on success (line 311) - if `seekTo` throws (line 313), `lastCompletedSeekFrame` stays stale, and retrying the same frame later gets incorrectly rejected as duplicate on line 284
How can I resolve this? If you propose a fix, please make it concise.| pub fn reset(&mut self) { | ||
| *self = Self::new(self.output).unwrap(); | ||
| if let Ok(resampler) = Self::new(self.output) { | ||
| *self = resampler; | ||
| } | ||
| } |
There was a problem hiding this comment.
Self::new() can fail and return Err, but error is silently ignored - resampler stays in old state, causing audio sync drift or corruption after seek when sample rate/channel config changes
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/audio.rs
Line: 439:443
Comment:
`Self::new()` can fail and return `Err`, but error is silently ignored - resampler stays in old state, causing audio sync drift or corruption after seek when sample rate/channel config changes
How can I resolve this? If you propose a fix, please make it concise.| scheduled_in_flight_frames.insert(frame_num); | ||
| if let Ok(mut in_flight_guard) = prefetch_in_flight.write() { | ||
| in_flight_guard.insert(frame_num); | ||
| in_flight_guard.insert((generation, frame_num)); | ||
| } |
There was a problem hiding this comment.
tracked in both local scheduled_in_flight_frames HashSet (line 413) and shared prefetch_in_flight RwLock (line 415) - consider using only the local set to reduce contention, since prefetch task already removes from shared set on completion
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/playback.rs
Line: 413:416
Comment:
tracked in both local `scheduled_in_flight_frames` HashSet (line 413) and shared `prefetch_in_flight` RwLock (line 415) - consider using only the local set to reduce contention, since prefetch task already removes from shared set on completion
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| let state_changed = { | ||
| let state = editor_instance.state.lock().await; | ||
| state.playhead_position != frame_number | ||
| }; | ||
|
|
||
| if state_changed { | ||
| editor_instance | ||
| .modify_and_emit_state(|state| { | ||
| state.playhead_position = frame_number; | ||
| }) | ||
| .await; | ||
| } |
There was a problem hiding this comment.
locks state twice (lines 1919, 1932) - second lock held during seek() call which may briefly block - consider reading both playhead_position and playback_task in single lock scope to reduce hold time
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/lib.rs
Line: 1918:1929
Comment:
locks `state` twice (lines 1919, 1932) - second lock held during `seek()` call which may briefly block - consider reading both `playhead_position` and `playback_task` in single lock scope to reduce hold time
How can I resolve this? If you propose a fix, please make it concise.| @@ -724,10 +1093,29 @@ impl Playback { | |||
| { | |||
There was a problem hiding this comment.
dynamic_skip_threshold reduced progressively during sustained lag (late_streak) - this aggressively drops frames when decoder is slow, but may skip over keyframes needed for next GOP, potentially causing further decode delays instead of recovery
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/playback.rs
Line: 1073:1093
Comment:
`dynamic_skip_threshold` reduced progressively during sustained lag (`late_streak`) - this aggressively drops frames when decoder is slow, but may skip over keyframes needed for next GOP, potentially causing further decode delays instead of recovery
How can I resolve this? If you propose a fix, please make it concise.

Refactor audio playback to a low-latency streaming path, optimize seeking logic, and reduce decoder initialization overhead.
This addresses user reports of audio lag and delayed startup, improves scrubbing responsiveness, and aims for smoother 60fps playback with better A/V synchronization across all supported platforms.
Note
High Risk
Touches core playback/audio timing, seek handling, and prefetch concurrency; regressions could manifest as A/V desync, stalls, or incorrect frames under rapid seeking across platforms.
Overview
Editor playback now supports live seeking while playing: Tauri
seek_to/set_playhead_positiononly emit state on real changes and forward seeks to the activePlaybackHandle, while the timeline UI batches/coaleces drag seeks viarequestAnimationFrameto avoid command storms.Playback runtime is refactored to add a seek channel with generation-based invalidation, keyed
BTreeMapprefetch buffering, dynamic FPS-scaled prefetch/timeout/skip tuning, and added startup/seek telemetry; audio playback defaults to the streaming buffer path with prerendered mode gated byCAP_AUDIO_PRERENDER_PLAYBACK, andAudioPlaybackBufferis enabled cross-platform (incl. Windows).Benchmarking is expanded with new scrub + startup metrics, optional JSON output for
playback-test-runneranddecode-benchmark(incl. fragmented input support), plus new docs/runbook and scripts to aggregate, validate, finalize, publish, analyze, and baseline-compare playback benchmark matrix results.Written by Cursor Bugbot for commit 0cbac24. This will update automatically on new commits. Configure here.
Greptile Overview
Greptile Summary
Refactors playback to use live seeking without stop/restart, switching from VecDeque to BTreeMap prefetch buffer with generation-based concurrency control to discard stale frames after seeks. Audio switches to low-latency streaming by default (prerender mode behind
CAP_AUDIO_PRERENDER_PLAYBACKenv var). Adds dynamic prefetch window tuning based on FPS, seek coalescing in Timeline UI via RAF batching, and telemetry for first-render and seek-settle latency.Key improvements:
PlaybackHandle.seek()updates running loop via watch channelscheduleSeek+ RAF to reduce IPC churnIssues found:
is_in_flightcheck (playback.rs:795) silently returnfalse, triggering redundant decodelastCompletedSeekFrameonly updates on successreset()(audio.rs:441) swallows errors, leaving stale state that causes sync driftConfidence Score: 2/5
crates/editor/src/playback.rs(concurrency/race conditions),apps/desktop/src/routes/editor/Timeline/index.tsx(seek retry logic), andcrates/editor/src/audio.rs(error handling)Important Files Changed
set_playhead_smoothdrift tolerance and saferreset()error handling; low-risk audio changesseek_toandset_playhead_positionto avoid no-op state emissions and forward seeks to active PlaybackHandleSequence Diagram
sequenceDiagram participant UI as Timeline UI participant Tauri as Tauri Commands participant PH as PlaybackHandle participant PL as Playback Loop participant PF as Prefetch Task participant AU as Audio Thread UI->>Tauri: seekTo(frame) Tauri->>Tauri: Check state changed alt State Changed Tauri->>Tauri: Emit state update Tauri->>PH: seek(frame) PH->>PL: seek_rx.send(frame) PL->>PL: Increment seek_generation PL->>PL: Clear prefetch_buffer & frame_cache PL->>PF: Send new generation via seek_generation_tx PL->>AU: Update audio_playhead_tx PF->>PF: Drop stale frames (old generation) PF->>PF: Reset prefetch from new frame PF->>PL: Send new frames with generation tag PL->>PL: Filter by generation, render frames end Note over UI,AU: Live seeking without stop/restart UI->>UI: scheduleSeek coalescing UI->>Tauri: Batched IPC call Note over PL,PF: Generation-aware concurrency prevents stale framesLast reviewed commit: 0cbac24