Skip to content

Conversation

@richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Oct 29, 2025

Cancel audio pipelines (mic, system audio) when video pipeline completes to prevent tracks from outliving video and causing timeline stretching.

Previously, audio tracks continued recording for ~18-19s after video ended, causing the editor timeline to stretch to ~29s while videos were only ~9-9.4s.

Summary by CodeRabbit

  • Bug Fixes

    • Recording startup errors now surface an error dialog and halt recording cleanly to avoid confusing partial runs.
  • Improvements

    • Recording pipelines support explicit cancellation for more reliable shutdown.
    • Non-video audio pipelines stop promptly when video recording finishes.
    • Audio processing no longer injects artificial silence, improving audio continuity.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

Walkthrough

Adds a pipeline cancellation token, centralizes actor-spawn error handling with user-facing UI cleanup on failure, removes wall-clock-based silence injection from the audio mixer, and cancels non-video pipelines when screen recording finishes.

Changes

Cohort / File(s) Summary
Recording startup / UI error flow
apps/desktop/src-tauri/src/recording.rs
Reworked start_recording spawn result handling to explicitly match the nested Result (Ok(Ok(rx)) success); unified error branch now emits failure events, shows an error dialog (anchored to recording controls if present), calls handle_recording_end, and returns early on any spawn error.
OutputPipeline cancellation
crates/recording/src/output_pipeline/core.rs
Added private cancel_token: CancellationToken to OutputPipeline and public methods cancel_token(&self) -> CancellationToken and cancel(&self); wired initialization to clone/propagate the existing stop token.
Audio mixer timing
crates/recording/src/sources/audio_mixer.rs
Removed wall-clock-based gap detection and silence-frame injection; gaps are no longer filled proactively and are bridged only when new frames arrive. Made Instant import conditional for macOS/windows.
Coordinated pipeline shutdown
crates/recording/src/studio_recording.rs
Added a watcher that, upon screen/video completion, obtains cancel tokens for non-video pipelines (microphone, camera, system audio) and calls cancel() to stop them when the screen_done future resolves.

Sequence Diagram(s)

sequenceDiagram
    participant VideoRecorder
    participant Watcher
    participant AudioPipeline
    participant CameraPipeline
    participant SystemAudioPipeline

    Note over VideoRecorder: Recording in progress
    VideoRecorder->>VideoRecorder: Record video
    par Parallel pipelines
        AudioPipeline->>AudioPipeline: Process audio frames
        CameraPipeline->>CameraPipeline: Process camera frames
        SystemAudioPipeline->>SystemAudioPipeline: Process system audio
    end

    rect rgb(220,235,255)
        Note over VideoRecorder,Watcher: Video completes
        VideoRecorder->>Watcher: screen_done future resolves
        Watcher->>AudioPipeline: cancel()  -- uses cancel_token
        Watcher->>CameraPipeline: cancel() -- uses cancel_token
        Watcher->>SystemAudioPipeline: cancel() -- uses cancel_token
    end

    AudioPipeline->>AudioPipeline: Shutdown
    CameraPipeline->>CameraPipeline: Shutdown
    SystemAudioPipeline->>SystemAudioPipeline: Shutdown
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • apps/desktop/src-tauri/src/recording.rs: ensure all spawn-result branches are covered and UI/dialog anchoring is correct without leaking state.
    • crates/recording/src/output_pipeline/core.rs: verify cancel_token initialization semantics relative to stop_token and guard/drop behavior.
    • crates/recording/src/sources/audio_mixer.rs: confirm removing silence injection doesn't introduce underrun or downstream timing issues.
    • crates/recording/src/studio_recording.rs: check for race conditions between natural pipeline shutdown and explicit cancel() calls.

Possibly related PRs

Poem

🐰 a rabbit celebrates
I hopped through code with nimble ears,
Silences gone, no phantom years,
Tokens cancel with gentle taps,
Errors shown and cleaned from maps,
Pipelines rest while applause appears. 🎥🍃

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% 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 "synchronize audio pipeline stop with video completion" directly and clearly captures the primary objective of this pull request. According to the PR summary, the main goal is to cancel audio pipelines (microphone and system audio) when the video pipeline completes, which is precisely what the title describes. The title is specific, concise, and avoids vague terminology—a developer scanning the repository history would immediately understand that this PR addresses timing synchronization between audio and video pipeline termination. The supporting changes across multiple files (error handling improvements, cancellation mechanism addition, and silence injection removal) all work toward enabling this core functionality described in the title.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch more-audio-fixes

📜 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 30705e8 and 21f7536.

📒 Files selected for processing (1)
  • crates/recording/src/sources/audio_mixer.rs (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/recording/src/sources/audio_mixer.rs
⏰ 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). (2)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)

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 richiemcilroy merged commit 4886f65 into main Oct 29, 2025
15 of 16 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 30, 2025
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