Add speed audio modes for retimed clip segments#2016
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
| let requested_source_sample = requested_source_sample.clamp( | ||
| key.segment_start_samples as f64, | ||
| key.segment_end_samples as f64, | ||
| ); |
There was a problem hiding this comment.
f64::clamp will panic if segment_end_samples < segment_start_samples (e.g. if a segment gets corrupted / has end < start). Might be worth guarding here so audio rendering can fail soft instead of crashing.
| let requested_source_sample = requested_source_sample.clamp( | |
| key.segment_start_samples as f64, | |
| key.segment_end_samples as f64, | |
| ); | |
| if key.segment_end_samples <= key.segment_start_samples { | |
| return Err(ffmpeg::Error::InvalidData); | |
| } | |
| let requested_source_sample = requested_source_sample.clamp( | |
| key.segment_start_samples as f64, | |
| key.segment_end_samples as f64, | |
| ); |
| while self.pending.len() / 2 < target_samples { | ||
| if self.input_samples < self.segment_end_samples { | ||
| self.feed_input(data, project)?; | ||
| } else if !self.flushed { | ||
| let mut source = self.graph.get("in").ok_or(ffmpeg::Error::InvalidData)?; | ||
| source.source().flush()?; | ||
| self.flushed = true; | ||
| } else { | ||
| break; | ||
| } | ||
| self.drain_output()?; | ||
| } | ||
|
|
||
| let discard = self.discard_output_samples.min(self.pending.len() / 2); | ||
| for _ in 0..discard * 2 { | ||
| self.pending.pop_front(); | ||
| } | ||
| self.discard_output_samples -= discard; | ||
|
|
||
| let available_output = out.len().saturating_sub(out_offset) / 2; | ||
| let written = samples.min(available_output).min(self.pending.len() / 2); | ||
| for target in &mut out[out_offset..out_offset + written * 2] { | ||
| *target = self.pending.pop_front().unwrap(); | ||
| } | ||
| self.expected_source_sample = requested_source_sample + samples as f64 * self.timescale; |
There was a problem hiding this comment.
Seek latency on first call with large preroll
When a new SpeedAudioProcessor is created at a position far from the segment start, discard_output_samples can reach SPEED_AUDIO_PREROLL_SAMPLES (2 400) output frames. The inner while loop then pumps SPEED_AUDIO_INPUT_BLOCK_SAMPLES (4 096) source frames per iteration until the pending queue covers both discard and samples. For MaintainPitch at 0.25× speed each source block only produces ~1 024 output frames, so a cold seek may require ≥3 pump iterations on the first call — this is bounded and acceptable. Worth keeping in mind if users report seek latency at extreme slow-motion values.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/audio.rs
Line: 784-808
Comment:
**Seek latency on first call with large preroll**
When a new `SpeedAudioProcessor` is created at a position far from the segment start, `discard_output_samples` can reach `SPEED_AUDIO_PREROLL_SAMPLES` (2 400) output frames. The inner `while` loop then pumps `SPEED_AUDIO_INPUT_BLOCK_SAMPLES` (4 096) source frames per iteration until the pending queue covers both `discard` and `samples`. For `MaintainPitch` at 0.25× speed each source block only produces ~1 024 output frames, so a cold seek may require ≥3 pump iterations on the first call — this is bounded and acceptable. Worth keeping in mind if users report seek latency at extreme slow-motion values.
How can I resolve this? If you propose a fix, please make it concise.|
hey @greptileai, please re-review the PR |
Clip segments with non-1x speed no longer always mute audio. Each segment can use Mute (default), Maintain pitch, or Match speed (pitch follows playback speed).
ClipSpeedAudioModeto the project schema with backward-compatible serde defaultsLegacy projects default to mute, preserving existing behavior.
Greptile Summary
This PR lifts the blanket audio mute for retimed clip segments and replaces it with three user-selectable modes — Mute (default, preserves existing behavior), Maintain pitch (pitch-corrected via an FFmpeg
atempochain), and Match speed (pitch follows rate viaasetrate+aresample). The schema change is fully backward-compatible (Noneserialises as absent; legacy files default to Mute). A 2-slot LRUSpeedAudioProcessorcache wraps per-segment FFmpeg filter graphs with preroll/discard seek support and a contiguity check for gapless playback.configuration.rs,tauri.ts): addsClipSpeedAudioModeenum and optionalspeed_audio_modefield toTimelineSegment; all new literal sites across recording, import, export, and benchmark code are initialised toNone.audio.rs):render_speed_audio_chunkreplaces the old silent-bypass; theSpeedAudioProcessorfeeds raw audio data into an FFmpeg filter graph and drains output into aVecDeque, with preroll to reduce seek latency;set_playheadclears both slots.ConfigSidebar.tsx,context.ts): adds the audio-mode radio group below the speed picker, gated ontimescale !== 1; the mode is persisted to the project store without requiring a seek.Confidence Score: 5/5
Safe to merge; new speed-audio paths are additive and isolated, legacy projects default to silent behavior, and the schema change is backward-compatible.
FFmpeg filter dispatch, LRU processor cache, preroll/discard seek logic, and serialization round-trip are all covered by new tests. No existing audio rendering path is broken.
No files require special attention; the two suggestions in audio.rs are minor quality improvements, not blockers.
Important Files Changed
Reviews (2): Last reviewed commit: "fix: harden speed audio filter construct..." | Re-trigger Greptile