Skip to content

Commit 8be1b28

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 3620cf8 commit 8be1b28

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;
@@ -603,18 +606,10 @@ struct SharedMemoryFallback {
603606
source_kind: Option<portal::SourceKind>,
604607
}
605608

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

620615
fn should_retry_shared_memory(
@@ -656,11 +651,11 @@ fn run<W: Write>(
656651
// What the portal granted, kept past the `StreamInfo` that is consumed at
657652
// Format: only a window stream is expected to carry a crop.
658653
let mut granted_kind: Option<portal::SourceKind> = None;
659-
// Frames skipped while waiting for a window's first usable crop rectangle.
660-
let mut frames_awaiting_crop: u32 = 0;
661-
// Set from the first `BufferInfo`, which the shim sends before that same
662-
// buffer's frame. See `negotiates_video_crop`.
663-
let mut crop_meta_negotiated = false;
654+
// While a window's first usable crop rectangle is awaited: when the wait
655+
// began, and the latest crop-less frame, which starts the recording if the
656+
// wait runs out. See MAX_CROP_WAIT.
657+
let mut crop_wait_since: Option<Instant> = None;
658+
let mut held_frame: Option<shim::Frame> = None;
664659
// A crop change is reported once, not once per frame.
665660
let mut crop_change_reported = false;
666661
// A `deferStart` grant held while the caller runs its countdown.
@@ -753,6 +748,15 @@ fn run<W: Write>(
753748
}
754749
}
755750

751+
// A window still waiting for its crop when the wait runs out starts from
752+
// the frame it holds. Replayed through FrameReady so the encoder opens on
753+
// the one path; KWin may never send another frame to trigger it.
754+
if held_frame.is_some()
755+
&& crop_wait_since.is_some_and(|since| since.elapsed() >= MAX_CROP_WAIT)
756+
{
757+
let _ = sender.send(Message::Stream(StreamEvent::FrameReady));
758+
}
759+
756760
// Return PipeWire buffers whose dmabuf imports completed last iteration
757761
// (or that were superseded on the capture thread). This MUST run on this
758762
// loop, not the PipeWire thread — `Session::requeue` takes the thread-loop
@@ -816,7 +820,18 @@ fn run<W: Write>(
816820
// `take` can legitimately return None: several FrameReady
817821
// notifications can arrive for frames that superseded each other
818822
// in the mailbox before the loop got here.
819-
if let Some(frame) = mailbox.take() {
823+
// A fresh frame supersedes the one held for a crop; with none,
824+
// this is the replay of the held frame once the wait ran out.
825+
let next = match mailbox.take() {
826+
Some(frame) => {
827+
if let Some(stale) = held_frame.take() {
828+
mailbox.recycle(stale.pixels);
829+
}
830+
Some(frame)
831+
}
832+
None => held_frame.take(),
833+
};
834+
if let Some(frame) = next {
820835
// Opening the encoder is deferred to here because only a
821836
// frame carries the crop, and the crop — not the negotiated
822837
// format — is the size of a window.
@@ -830,24 +845,22 @@ fn run<W: Write>(
830845
// the encoder to that frame would pin a window
831846
// recording at monitor size for its whole duration,
832847
// which is the very bug being fixed, only intermittent.
833-
// A compositor that never negotiated the crop meta
834-
// (KWin) is not waited on at all.
835-
let expecting_crop = granted_kind == Some(portal::SourceKind::Window)
836-
&& crop_meta_negotiated
837-
&& !frame.has_crop;
838-
if expecting_crop && frames_awaiting_crop < MAX_FRAMES_AWAITING_CROP {
839-
frames_awaiting_crop += 1;
840-
mailbox.recycle(frame.pixels);
848+
let expecting_crop =
849+
granted_kind == Some(portal::SourceKind::Window) && !frame.has_crop;
850+
let waited = crop_wait_since.get_or_insert_with(Instant::now).elapsed();
851+
if holds_for_crop(expecting_crop, waited) {
852+
held_frame = Some(frame);
841853
continue;
842854
}
843855
if expecting_crop {
844856
let _ = emitter.emit(&Event::Warning {
845857
code: "window-crop-missing".to_owned(),
846858
message: format!(
847859
"a window was granted but no crop rectangle arrived in \
848-
{frames_awaiting_crop} frames; recording the full \
849-
{}x{} stream instead",
850-
frame.width, frame.height
860+
{} ms; recording the full {}x{} stream instead",
861+
waited.as_millis(),
862+
frame.width,
863+
frame.height
851864
),
852865
});
853866
}
@@ -881,7 +894,7 @@ fn run<W: Write>(
881894
("encodedWidth", width.into()),
882895
("encodedHeight", height.into()),
883896
("hasCrop", frame.has_crop.into()),
884-
("framesAwaited", frames_awaiting_crop.into()),
897+
("cropWaitMs", (waited.as_millis() as u64).into()),
885898
]),
886899
});
887900

@@ -1175,7 +1188,6 @@ fn run<W: Write>(
11751188
// bearing field: a missing Cursor next to a present Header means
11761189
// our ParamMeta was sent and lost the size intersection, which is
11771190
// otherwise indistinguishable from never having sent it.
1178-
crop_meta_negotiated = negotiates_video_crop(&metas);
11791191
let _ = emitter.emit(&Event::Debug {
11801192
code: "buffer-info".to_owned(),
11811193
data: json_map([
@@ -1857,15 +1869,19 @@ mod microphone_resolution_tests {
18571869

18581870
#[cfg(test)]
18591871
mod capture_fallback_tests {
1860-
use super::{negotiates_video_crop, should_retry_shared_memory, stream_error_is_fatal};
1872+
use super::{
1873+
holds_for_crop, should_retry_shared_memory, stream_error_is_fatal, MAX_CROP_WAIT,
1874+
};
1875+
use std::time::Duration;
18611876

18621877
#[test]
1863-
fn only_a_negotiated_video_crop_block_is_worth_waiting_for() {
1864-
// mutter: the block is there, its rect may arrive a few frames late.
1865-
assert!(negotiates_video_crop("Header:12,VideoCrop:16,Cursor:589872"));
1866-
// KWin: no block, so waiting would discard a still window's only frame.
1867-
assert!(!negotiates_video_crop("Header:12,VideoDamage:256,Cursor:589872"));
1868-
assert!(!negotiates_video_crop(""));
1878+
fn a_crop_is_awaited_for_a_bounded_time_not_a_frame_count() {
1879+
// mutter: the crop follows the first frame within a few frames.
1880+
assert!(holds_for_crop(true, Duration::ZERO));
1881+
// KWin: a still window sends one frame and never a crop, so the wait has
1882+
// to end on time alone and start from that frame.
1883+
assert!(!holds_for_crop(true, MAX_CROP_WAIT));
1884+
assert!(!holds_for_crop(false, Duration::ZERO));
18691885
}
18701886

18711887
#[test]

0 commit comments

Comments
 (0)