Skip to content

Conversation

p-delorme
Copy link
Contributor

@p-delorme p-delorme commented Oct 7, 2025

Addresses several issues related to Direct3D capture on Windows:

  • Ensures the GraphicsCaptureItem is created on the capture thread to avoid COM threading problems.
  • Propagates encoder setup errors back to the main thread.
  • Improves error handling and logging in the capture thread.
  • Shares the D3D device with the encoder to avoid device mismatch.

Summary by CodeRabbit

  • Bug Fixes

    • Resolved Windows screen capture threading issues by creating capture items on the capture thread.
    • Improved error propagation when encoder readiness signaling or setup fails, and when the encoder thread ends unexpectedly.
  • Refactor

    • Replaced capture_item with display_id in VideoSourceConfig and updated setup/init to construct capture items on the capture thread.
  • Chores

    • Expanded error-level and trace logging across capture initialization, start/stop control, and encoder lifecycle for clearer diagnostics.

Addresses several issues related to Direct3D capture on Windows:

- Ensures the GraphicsCaptureItem is created on the capture thread
  to avoid COM threading problems.
- Propagates encoder setup errors back to the main thread.
- Improves error handling and logging in the capture thread.
- Shares the D3D device with the encoder to avoid device mismatch.
Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

Walkthrough

Updates improve Windows recording components. Output pipeline now logs encoder readiness/setup failures as errors and propagates thread termination errors. Screen capture refactors to use DisplayId, creating GraphicsCaptureItem on the capture thread, updating setup/start flows, and expanding logging and error propagation.

Changes

Cohort / File(s) Summary
Output pipeline: logging and error propagation
crates/recording/src/output_pipeline/win.rs
Upgraded log levels to error! on readiness/send failures; added explicit error logging on encoder setup failure; propagated early encoder-thread termination via mapped anyhow errors and ??.
Screen capture refactor: DisplayId and capture-thread initialization
crates/recording/src/sources/screen_capture/windows.rs
Replaced GraphicsCaptureItem with DisplayId in VideoSourceConfig; deferred item creation to capture thread using Display::from_id; reworked setup/start sequencing, readiness/error signaling, and tracing; minor import/order adjustments.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App
  participant VideoSource as VideoSource::setup
  participant CapThread as Capture Thread
  participant WinAPI as Windows GraphicsCapture/Direct3D

  App->>VideoSource: setup(VideoSourceConfig{ display_id, ... })
  VideoSource->>CapThread: spawn + init channels
  Note over CapThread: Create GraphicsCaptureItem on capture thread
  CapThread->>WinAPI: Display::from_id(display_id)
  alt item created
    CapThread->>WinAPI: Create GraphicsCapture capturer + D3D11 resources
    CapThread-->>VideoSource: send ready()
    CapThread->>CapThread: start capture loop
  else error creating item/init
    CapThread-->>VideoSource: send error()
    CapThread-->>CapThread: terminate thread
  end

  Note over VideoSource: Await ready/error
  VideoSource-->>App: return Ok/Err

  rect rgba(255,230,230,0.4)
  Note right of VideoSource: Output pipeline change
  App->>VideoSource: start encoder
  VideoSource-->>App: error! log on readiness/send failure<br/>propagate thread termination errors
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

A rabbit taps keys with a flick of an ear,
Threads spin up swiftly, the capture runs clear.
From IDs to items, on the right thread we hop,
Errors now shout, no whispers that stop.
Frames in the meadow, logs bright as the sun—
Encode, capture, nibble—another run done! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
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.
✅ 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 clearly summarizes the core purpose of the pull request by stating it fixes Direct3D capture issues on Windows, which aligns with the implemented changes to COM threading, error handling, and D3D device usage.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 99770b9 and af1f612.

📒 Files selected for processing (2)
  • crates/recording/src/output_pipeline/win.rs (2 hunks)
  • crates/recording/src/sources/screen_capture/windows.rs (7 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/output_pipeline/win.rs
  • crates/recording/src/sources/screen_capture/windows.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/output_pipeline/win.rs
  • crates/recording/src/sources/screen_capture/windows.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/output_pipeline/win.rs
  • crates/recording/src/sources/screen_capture/windows.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/output_pipeline/win.rs
  • crates/recording/src/sources/screen_capture/windows.rs
🧬 Code graph analysis (1)
crates/recording/src/sources/screen_capture/windows.rs (1)
crates/recording/src/sources/screen_capture/mod.rs (2)
  • display (66-72)
  • d3d_device (408-410)
🔇 Additional comments (3)
crates/recording/src/output_pipeline/win.rs (3)

115-118: LGTM! Appropriate elevation to error level.

The change from info! to error! correctly reflects the severity of the receiver being dropped, which indicates the main thread terminated or stopped waiting for setup completion.


121-125: LGTM! Improved diagnostics for setup failures.

Explicit error logging before signaling failure ensures diagnostic information is captured even if the channel send fails, improving troubleshooting capabilities.


189-191: LGTM! Correct error propagation pattern.

The double-question mark pattern properly handles both channel errors (thread termination) and encoder setup errors, ensuring failures are propagated to the caller with clear error context.


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 92b587f into CapSoftware:main Oct 7, 2025
10 of 11 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.

2 participants