Skip to content

Commit ce32f85

Browse files
committed
fix(linux): bound the window crop wait by time, not by frame count
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.
1 parent 5bb1b3f commit ce32f85

1 file changed

Lines changed: 60 additions & 44 deletions

File tree

  • electron/native/pipewire-capture/src

‎electron/native/pipewire-capture/src/main.rs‎

Lines changed: 60 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,17 @@ struct MicrophoneRequest {
9393
}
9494

9595
const DEFAULT_AUDIO_BITRATE: i64 = 128_000;
96-
/// How many frames a window stream may deliver without a crop rectangle before
97-
/// the helper gives up waiting and records the whole stream.
96+
/// How long a window stream may go without a crop rectangle before the helper
97+
/// gives up waiting and records the whole stream, starting from the last frame.
9898
///
9999
/// mutter records one frame synchronously when the stream is enabled, which can
100-
/// land before the picked window is mapped and therefore carry an empty rect. A
101-
/// handful of frames is ~100 ms at 60 fps — long enough to skip that, short
102-
/// enough that a compositor which never sends a crop still starts recording.
103-
const MAX_FRAMES_AWAITING_CROP: u32 = 8;
100+
/// land before the picked window is mapped and therefore carry an empty rect;
101+
/// its real crop follows within a few frames. KWin never fills the crop at all
102+
/// (it sizes a window stream to the window) and sends one frame when the stream
103+
/// starts, then only when the window repaints. That is why this is a duration
104+
/// and not a frame count: counting frames waited forever on a still window,
105+
/// whose only frame had been thrown away, and the start timed out.
106+
const MAX_CROP_WAIT: Duration = Duration::from_millis(500);
104107
/// How much audio may queue before the oldest is discarded. Generous: the drain
105108
/// runs every loop tick, so reaching this means the encoder stopped entirely.
106109
const AUDIO_RING_SECONDS: usize = 2;
@@ -608,18 +611,10 @@ struct SharedMemoryFallback {
608611
source_kind: Option<portal::SourceKind>,
609612
}
610613

611-
/// Whether the negotiated buffers carry a SPA_META_VideoCrop block, read from
612-
/// the shim's "Header:12,VideoCrop:16" list.
613-
///
614-
/// Only then is waiting for a window's crop worth anything. mutter negotiates
615-
/// the block and fills it a few frames late; KWin never negotiates it — it sizes
616-
/// a window stream to the window itself — and sends one frame when the stream
617-
/// starts, then only when the window repaints. Waiting on KWin therefore threw
618-
/// away the one frame a still window ever sends, and the start timed out.
619-
fn negotiates_video_crop(metas: &str) -> bool {
620-
metas
621-
.split(',')
622-
.any(|meta| meta.split(':').next() == Some("VideoCrop"))
614+
/// Whether a crop-less window frame is held back rather than opening the encoder.
615+
/// See MAX_CROP_WAIT.
616+
fn holds_for_crop(expecting_crop: bool, waited: Duration) -> bool {
617+
expecting_crop && waited < MAX_CROP_WAIT
623618
}
624619

625620
fn should_retry_shared_memory(
@@ -661,11 +656,11 @@ fn run<W: Write>(
661656
// What the portal granted, kept past the `StreamInfo` that is consumed at
662657
// Format: only a window stream is expected to carry a crop.
663658
let mut granted_kind: Option<portal::SourceKind> = None;
664-
// Frames skipped while waiting for a window's first usable crop rectangle.
665-
let mut frames_awaiting_crop: u32 = 0;
666-
// Set from the first `BufferInfo`, which the shim sends before that same
667-
// buffer's frame. See `negotiates_video_crop`.
668-
let mut crop_meta_negotiated = false;
659+
// While a window's first usable crop rectangle is awaited: when the wait
660+
// began, and the latest crop-less frame, which starts the recording if the
661+
// wait runs out. See MAX_CROP_WAIT.
662+
let mut crop_wait_since: Option<Instant> = None;
663+
let mut held_frame: Option<shim::Frame> = None;
669664
// A crop change is reported once, not once per frame.
670665
let mut crop_change_reported = false;
671666
// A `deferStart` grant held while the caller runs its countdown.
@@ -758,6 +753,15 @@ fn run<W: Write>(
758753
}
759754
}
760755

756+
// A window still waiting for its crop when the wait runs out starts from
757+
// the frame it holds. Replayed through FrameReady so the encoder opens on
758+
// the one path; KWin may never send another frame to trigger it.
759+
if held_frame.is_some()
760+
&& crop_wait_since.is_some_and(|since| since.elapsed() >= MAX_CROP_WAIT)
761+
{
762+
let _ = sender.send(Message::Stream(StreamEvent::FrameReady));
763+
}
764+
761765
// Return PipeWire buffers whose dmabuf imports completed last iteration
762766
// (or that were superseded on the capture thread). This MUST run on this
763767
// loop, not the PipeWire thread — `Session::requeue` takes the thread-loop
@@ -821,7 +825,18 @@ fn run<W: Write>(
821825
// `take` can legitimately return None: several FrameReady
822826
// notifications can arrive for frames that superseded each other
823827
// in the mailbox before the loop got here.
824-
if let Some(frame) = mailbox.take() {
828+
// A fresh frame supersedes the one held for a crop; with none,
829+
// this is the replay of the held frame once the wait ran out.
830+
let next = match mailbox.take() {
831+
Some(frame) => {
832+
if let Some(stale) = held_frame.take() {
833+
mailbox.recycle(stale.pixels);
834+
}
835+
Some(frame)
836+
}
837+
None => held_frame.take(),
838+
};
839+
if let Some(frame) = next {
825840
// Opening the encoder is deferred to here because only a
826841
// frame carries the crop, and the crop — not the negotiated
827842
// format — is the size of a window.
@@ -835,24 +850,22 @@ fn run<W: Write>(
835850
// the encoder to that frame would pin a window
836851
// recording at monitor size for its whole duration,
837852
// which is the very bug being fixed, only intermittent.
838-
// A compositor that never negotiated the crop meta
839-
// (KWin) is not waited on at all.
840-
let expecting_crop = granted_kind == Some(portal::SourceKind::Window)
841-
&& crop_meta_negotiated
842-
&& !frame.has_crop;
843-
if expecting_crop && frames_awaiting_crop < MAX_FRAMES_AWAITING_CROP {
844-
frames_awaiting_crop += 1;
845-
mailbox.recycle(frame.pixels);
853+
let expecting_crop =
854+
granted_kind == Some(portal::SourceKind::Window) && !frame.has_crop;
855+
let waited = crop_wait_since.get_or_insert_with(Instant::now).elapsed();
856+
if holds_for_crop(expecting_crop, waited) {
857+
held_frame = Some(frame);
846858
continue;
847859
}
848860
if expecting_crop {
849861
let _ = emitter.emit(&Event::Warning {
850862
code: "window-crop-missing".to_owned(),
851863
message: format!(
852864
"a window was granted but no crop rectangle arrived in \
853-
{frames_awaiting_crop} frames; recording the full \
854-
{}x{} stream instead",
855-
frame.width, frame.height
865+
{} ms; recording the full {}x{} stream instead",
866+
waited.as_millis(),
867+
frame.width,
868+
frame.height
856869
),
857870
});
858871
}
@@ -886,7 +899,7 @@ fn run<W: Write>(
886899
("encodedWidth", width.into()),
887900
("encodedHeight", height.into()),
888901
("hasCrop", frame.has_crop.into()),
889-
("framesAwaited", frames_awaiting_crop.into()),
902+
("cropWaitMs", (waited.as_millis() as u64).into()),
890903
]),
891904
});
892905

@@ -1180,7 +1193,6 @@ fn run<W: Write>(
11801193
// bearing field: a missing Cursor next to a present Header means
11811194
// our ParamMeta was sent and lost the size intersection, which is
11821195
// otherwise indistinguishable from never having sent it.
1183-
crop_meta_negotiated = negotiates_video_crop(&metas);
11841196
let _ = emitter.emit(&Event::Debug {
11851197
code: "buffer-info".to_owned(),
11861198
data: json_map([
@@ -1862,15 +1874,19 @@ mod microphone_resolution_tests {
18621874

18631875
#[cfg(test)]
18641876
mod capture_fallback_tests {
1865-
use super::{negotiates_video_crop, should_retry_shared_memory, stream_error_is_fatal};
1877+
use super::{
1878+
holds_for_crop, should_retry_shared_memory, stream_error_is_fatal, MAX_CROP_WAIT,
1879+
};
1880+
use std::time::Duration;
18661881

18671882
#[test]
1868-
fn only_a_negotiated_video_crop_block_is_worth_waiting_for() {
1869-
// mutter: the block is there, its rect may arrive a few frames late.
1870-
assert!(negotiates_video_crop("Header:12,VideoCrop:16,Cursor:589872"));
1871-
// KWin: no block, so waiting would discard a still window's only frame.
1872-
assert!(!negotiates_video_crop("Header:12,VideoDamage:256,Cursor:589872"));
1873-
assert!(!negotiates_video_crop(""));
1883+
fn a_crop_is_awaited_for_a_bounded_time_not_a_frame_count() {
1884+
// mutter: the crop follows the first frame within a few frames.
1885+
assert!(holds_for_crop(true, Duration::ZERO));
1886+
// KWin: a still window sends one frame and never a crop, so the wait has
1887+
// to end on time alone and start from that frame.
1888+
assert!(!holds_for_crop(true, MAX_CROP_WAIT));
1889+
assert!(!holds_for_crop(false, Duration::ZERO));
18741890
}
18751891

18761892
#[test]

0 commit comments

Comments
 (0)