Skip to content

Conversation

Brendonovich
Copy link
Member

@Brendonovich Brendonovich commented Oct 2, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved audio/video sync in recordings by aligning video frames to the initial capture timestamp.
    • Ensured consistent frame timing with correct presentation timestamps, reducing drift and lip‑sync issues.
    • More reliable start of recordings, preventing delayed or missing first frames in some Direct3D capture scenarios.

Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Walkthrough

Implements per-frame initialization and synchronization for FFmpeg encoding in the Direct3D video capture path: captures the first frame’s timestamp, shares it via a one-shot channel, computes elapsed time for PTS via encoder.get_pts, converts frames to FFmpeg format, sets pts, and queues frames through the encoder.

Changes

Cohort / File(s) Summary of changes
D3D capture FFmpeg sync and PTS flow
crates/recording/src/capture_pipeline.rs
Added first_timestamp tracking and one-shot sender for first frame timestamp; replaced _unix_time with timestamp; computed elapsed for PTS via encoder.get_pts; converted frames to FFmpeg format (FrameAsFfmpeg), set pts, and queued via encoder.queue_frame; adjusted control flow to initialize on first frame and synchronize with audio path.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant D3D as D3D Capture
  participant VidRx as Video Frame Handler
  participant Sync as First-Frame Sync (one-shot)
  participant FFConv as FrameAsFfmpeg
  participant Enc as Encoder
  participant Out as Output/Muxer

  D3D->>VidRx: Receive frame (timestamp)
  alt First frame
    VidRx->>Sync: Send initial timestamp
    Note right of VidRx: Store first_timestamp
  end
  VidRx->>VidRx: elapsed = timestamp - first_timestamp
  VidRx->>FFConv: Convert frame to FFmpeg frame
  FFConv-->>VidRx: ff_frame
  VidRx->>Enc: pts = Enc.get_pts(elapsed)
  VidRx->>Enc: queue_frame(ff_frame with pts, &mut output)
  Enc->>Out: Write encoded packet(s)
  Out-->>Enc: Ack/bytes written
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • set pts in ffmpeg screen encoder #1020 — Implements H264Encoder::get_pts and time_base adjustments used by the new PTS computation in this PR; directly tied to how pts is derived and applied.

Poem

A tick, a tock, the frames align,
I twitch my ears at perfect time.
First stamp shared, the chorus starts,
Video, audio—synchronized hearts.
Hops through PTS, carrots encoded—
A bunny’s cut, cleanly uploaded. 🥕🎬

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “Fix windows software encoding” is related to the changeset but is too generic to convey the primary modifications, which focus on initializing and synchronizing per-frame timestamps in the FFmpeg-based Direct3D capture pipeline. It does not specify the introduction of first_timestamp, channel-based timestamp propagation, or PTS calculation adjustments that form the core of the update. As a result, the title lacks the specificity needed for a clear understanding of the main change. Please revise the title to succinctly describe the key update, for example “Synchronize FFmpeg frame timestamps in Windows Direct3D capture pipeline,” so that it clearly reflects the introduction of initial timestamp tracking and PTS adjustments.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch windows-instant-mode-software-enc-fix

📜 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 c1e481a and 3834ab3.

📒 Files selected for processing (1)
  • crates/recording/src/capture_pipeline.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

For desktop IPC, use tauri_specta derive/macros on Rust types/events; do not hand-roll bindings

Files:

  • crates/recording/src/capture_pipeline.rs
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • crates/recording/src/capture_pipeline.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and 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/recording/src/capture_pipeline.rs
crates/*/src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Rust crates should place tests within the src/ and/or a sibling tests/ directory for each crate inside crates/*.

Files:

  • crates/recording/src/capture_pipeline.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). (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/recording/src/capture_pipeline.rs (7)

729-730: LGTM!

The initialization of first_timestamp and first_frame_tx follows the same pattern used in the studio mode pipeline for consistency.


735-735: LGTM!

Correctly changed from the unused _unix_time variable to timestamp, which is now properly utilized for PTS calculations.


748-748: LGTM!

The initialization of first_timestamp using get_or_insert ensures it's set once on the first frame, matching the pattern used in studio mode.


750-752: LGTM!

The first frame timestamp is correctly sent via the one-shot channel to synchronize audio encoding, matching the pattern used in the native encoder path.


754-756: LGTM!

The frame conversion to FFmpeg format is correctly implemented with appropriate error handling, matching the pattern used in studio mode.


758-760: LGTM!

The elapsed time calculation and PTS assignment are correctly implemented, ensuring frame timing starts from zero and aligns with the encoder's expectations.


762-762: LGTM!

The frame is correctly queued with the FFmpeg encoder after PTS has been set, completing the encoding pipeline as expected.


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.

@Brendonovich Brendonovich merged commit 1a665ae into main Oct 2, 2025
15 checks passed
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.

1 participant