Skip to content

fix(linux): bound the window crop wait by time, not by frame count - #1353

Merged
EtienneLescot merged 1 commit into
mainfrom
fix/linux-kde-window-crop-timeout
Oct 27, 2026
Merged

EtienneLescot merged 1 commit into
mainfrom
fix/linux-kde-window-crop-timeout

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Summary

Follow-up to #1351, which did not fix the reporter's case. On KDE Plasma (Wayland), a still window still failed to record ("the screen share delivered no frame within 10 s"). It only started if the user typed in it during the countdown.

Why #1351 missed. It skipped the crop wait when the buffers did not carry SPA_META_VideoCrop. KWin's buffers do carry the block; KWin just never fills it. The 2.1.2 CI build still logged window-crop-missing … 8 frames once the window repainted.

Fix. The crop wait is now bounded by time, not by frame count.

  • The latest crop-less window frame is held instead of discarded.
  • A frame that brings a crop supersedes it. That is mutter, which sends its crop a few frames late.
  • Otherwise, after 500 ms, the held frame starts the recording at the stream's size, replayed through the same FrameReady path.
  • The negotiated-meta check from fix(linux): start a KDE window recording on its first frame #1351 is removed.

Related issue

Refs #1351. Reported on Discord by a Debian 13 / KDE Plasma 6 / AMD Renoir user.

Type of change

  • Bug fix

Release impact

  • Patch

Desktop impact

  • Linux

Testing

  • Helper tests: cargo test --bin openscreen-pipewire-helper in WSL (rustc 1.98.1): 101 passed, including a_crop_is_awaited_for_a_bounded_time_not_a_frame_count.
  • Real KDE run, pending: a CI build of this branch goes to the reporter before merge.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Window recording now waits briefly for crop information before starting. If crop information does not arrive, recording starts with the full stream instead of remaining stalled. When crop information is available, recording proceeds with the cropped view.

@EtienneLescot
EtienneLescot requested a review from Beetix as a code owner October 19, 2026 16:10
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c425c8b9-78af-4bf1-b81b-5f7d5fa82073
Commits

Reviewing files that changed from the base of the PR and between 8be1b28 and ce32f85.

Files selected for processing: 1
  • electron/native/pipewire-capture/src/main.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

Window crop waiting now uses a 500 ms duration instead of a frame limit or negotiated-metadata check. The capture helper holds crop-less window frames during the wait and replays the latest held frame if the wait expires. Crop-less non-window frames do not wait.

Changes: Window crop wait

Layer / File(s) Summary
Hold and replay frames
electron/native/pipewire-capture/src/main.rs
The helper tracks elapsed wait time and holds a crop-less window frame for up to 500 ms. It replays the held frame when the wait expires, warns when proceeding without a crop, and reports elapsed milliseconds in diagnostics. The test checks the wait duration and verifies that a source not expecting a crop is not held.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

  • getopenscreen/openscreen#1380: Both changes update crop-less window-frame handling in the PipeWire capture helper. This PR replaces the frame-limit fallback with a 500 ms timeout and replays the latest held frame.
Merge Risk: Minimal · up to ce32f

No actionable issue remains from this review; the change is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main Linux fix: replacing the window crop wait frame-count limit with a time limit.
Description check Passed The description follows the repository template. It explains the bug, root cause, fix, related issue, change type, release impact, platform impact, and testing status. It also clearly identifies that …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description follows the repository template. It explains the bug, root cause, fix, related issue, change type, release impact, platform impact, and testing status. It also clearly identifies that real KDE validation is still pending.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

Clippy (1.99.0)

Clippy execution failed

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.

The previous fix (#1351) skipped the crop wait only when the buffers did not carry SPA_META_VideoCrop. KWin's buffers do carry the block; KWin just never fills it. So a still window on KDE still had its one frame thrown away while the helper counted to 8, and the start still timed out after 10 s (confirmed on the reporter's machine with the 2.1.2 CI build).

The helper now keeps the latest crop-less window frame instead of discarding it. A frame that brings a crop (mutter, a few frames late) supersedes it; otherwise, after 500 ms, the held frame starts the recording at the stream's size, replayed through the same FrameReady path. The negotiated-meta check is gone.
@EtienneLescot
EtienneLescot force-pushed the fix/linux-kde-window-crop-timeout branch from 8be1b28 to ce32f85 Compare October 27, 2026 07:53
@EtienneLescot

Copy link
Copy Markdown
Member Author

Measured on real Linux (Ubuntu 24.04, GNOME 46 Wayland, PipeWire 1.0.5, Vivobook). Helper built from this branch (ce32f85) and from main (5bb1b3f), driven through the real portal picker, 60 fps request, 7 s takes, 3 takes each, interleaved.

cargo test --bin openscreen-pipewire-helper: 104 passed, including the new a_crop_is_awaited_for_a_bounded_time_not_a_frame_count.

Still window (no input at all)

branch main
Share → first frame encoded 324 / 335 / 335 ms 322 / 327 / 330 ms
stream-started → capture-started ~19 ms ~19 ms
encoded size 422×322 (crop applied) 422×322
cropWaitMs / window-crop-missing 0 / none n/a / none
frames in 7.08 s, dropped 406, 406, 406 · 0 406, 407, 407 · 0
transcode of the take (export proxy) 0.240 s 0.237 s

Full screen

branch main
Share → first frame encoded 337 / 338 / 335 ms 334 / 328 / 337 ms
frames in ~7.08 s, dropped 405, 406, 406 · 0 407, 406, 407 · 0
transcode of the take (export proxy) 1.757 s 1.765 s

All within ±1.5% of main. On mutter the crop still lands on the first frame, so the window is still recorded at window size, not stream size. No "no frame within 10 s" anywhere.

Not covered: KDE/KWin itself (this machine runs GNOME), so the 500 ms fallback path was not exercised live; the unit test covers it. The diff does not touch the export pipeline, hence the transcode proxy.

🤖 Generated with Claude Code

@EtienneLescot
EtienneLescot merged commit 0abf010 into main Oct 27, 2026
21 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