You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(linux): bound the window crop wait by time, not by frame count - #1353
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.
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.
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.
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.
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: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
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 checks: 4 passed
Check name
Status
Explanation
Title check
The title clearly and concisely describes the main Linux fix: replacing the window crop wait frame-count limit with a time limit.
Description check
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
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 loggedwindow-crop-missing … 8 framesonce the window repainted.Fix. The crop wait is now bounded by time, not by frame count.
FrameReadypath.Related issue
Refs #1351. Reported on Discord by a Debian 13 / KDE Plasma 6 / AMD Renoir user.
Type of change
Release impact
Desktop impact
Testing
cargo test --bin openscreen-pipewire-helperin WSL (rustc 1.98.1): 101 passed, includinga_crop_is_awaited_for_a_bounded_time_not_a_frame_count.🤖 Generated with Claude Code
Summary by CodeRabbit