mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-10-19 16:17:03 +00:00
fix(drm): drop a frame equal to the previous one, as the other capturers do (#16450)
* fix(drm): drop a frame equal to the previous one, as the other capturers do The producer grabs on a timer, so a still screen reached the encoder as the same frame every tick and the client decoded about 30 frames per second of nothing new (rustdesk#16449). The x11, pipewire and quartz capturers drop a repeat with would_block_if_equal; the drm capturer now does the same. A regression test checks that a repeat is not encoded again and that the next new frame is. The frame helper of the tests now differs per call, and the 180 test forgets the frame its last case repeats on purpose. * fix(drm): a frame whose encode failed is not dropped as a repeat The capturer marked a frame as seen when it returned it, so when its encode failed and the screen then stayed still, every later grab compared equal to it and it was never sent: the client kept the previous image, and a hardware encoder never reached its failure count to switch away. handle_one_frame now also says whether the frame was encoded, and when it was not the loop calls TraitCapturer::forget_last_frame(), a new method whose default does nothing. The DRM capturer then returns the next repeat instead of dropping it. The DRM capturer also stops keeping a copy of the last frame: it compares the new buffer with the one it returned last before replacing it, and a turned frame is turned into a spare buffer first. * fix(linux): x11 and pipewire do not drop a failed frame as a repeat either Both capturers drop a frame equal to the one they returned last, so they had the same hole as the drm one: a frame whose encode failed was not sent again while the screen stayed still. They now forget it when the loop says so. * fix(linux): every video run starts by forgetting the last frame A pipewire recorder outlives a restarted run while another display keeps the session open, and so did the frame it returned last. A run restarted by an encoder switch or a new subscriber then dropped the first repeat and sent nothing until the screen changed. The capturers of the other paths are built fresh for each run, so for them the call does nothing. * fix(drm): keep the repeat drop in the drm capturer, with one repeat a second The shared part of the three commits before is reverted: TraitCapturer, the video_service loop, x11 and pipewire are as on master again. So is the restructure of frame() from the second commit: the drop is again the one of the first commit, would_block_if_equal on the frame returned last, as the other capturers do. A repeat is dropped until a second has passed since frame() last returned a frame, so a frame whose encode failed reaches the encoder again a second later even if the screen stays still. The tests check the drop, the repeat a second later, and that the second starts over after it. * fix(drm): a frame is a repeat only when its format matches too The same bytes as BGRA and as RGBA are two pictures when red and blue differ, and the convert path can return either, so a change of format clears the saved frame and the new one goes out. The 180 test no longer checks a turned repeat: nothing reported asked for it. * fix(drm): compare a frame with cur before replacing it, without a copy saved_raw_data copied every changed frame (8 MB at 1080p). The previous frame is still in cur when a new one arrives, so the bytes and the format are compared with it before the swap; a rotated output turns the frame into a spare buffer first. The 180 test checks the turned repeat again, since that path compares on its own now.
This commit is contained in:
Mariano Abad
@@ -24,6 +24,9 @@ const HANDSHAKE_WAIT_MS: u64 = DRM_CONNECT_TIMEOUT_MS + DISPLAY_LIST_TIMEOUT_MS
|
||||
/// Only the header read rechecks `stop`, so bound the body read here rather than relying on
|
||||
/// `next_raw_into`'s own cap.
|
||||
const BODY_READ_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
/// A repeat of the frame returned last goes out anyway once this much time has passed since
|
||||
/// then: a frame whose encode failed reaches the encoder again, even if nothing moves.
|
||||
const REPEAT_EVERY: Duration = Duration::from_secs(1);
|
||||
|
||||
struct FrameSlot {
|
||||
// Row stride is `pixels.len() / height`, possibly padded; the format and the plane rotation
|
||||
@@ -103,6 +106,10 @@ pub struct IpcDrmCapturer {
|
||||
cur_h: usize,
|
||||
cur_fmt: Pixfmt,
|
||||
got_frame: bool,
|
||||
// A rotated output turns each frame into here first, to compare it with `cur`.
|
||||
spare: Vec<u8>,
|
||||
// When frame() last returned a frame; see REPEAT_EVERY.
|
||||
returned_at: Instant,
|
||||
}
|
||||
|
||||
/// A list index is NOT an identity: `drm_enumerate_all_displays` concatenates per-card lists.
|
||||
@@ -441,6 +448,8 @@ impl IpcDrmCapturer {
|
||||
cur_h: 0,
|
||||
cur_fmt: Pixfmt::BGRA,
|
||||
got_frame: false,
|
||||
spare: Vec::new(),
|
||||
returned_at: Instant::now(),
|
||||
},
|
||||
displays,
|
||||
wire_idx,
|
||||
@@ -486,7 +495,7 @@ impl Drop for IpcDrmCapturer {
|
||||
impl TraitCapturer for IpcDrmCapturer {
|
||||
fn frame<'a>(&'a mut self, timeout: Duration) -> io::Result<Frame<'a>> {
|
||||
let deadline = Instant::now() + timeout;
|
||||
{
|
||||
let repeat = {
|
||||
let mut slot = self.shared.slot.lock().unwrap();
|
||||
loop {
|
||||
if slot.latest.is_some() || slot.ended.is_some() {
|
||||
@@ -541,9 +550,14 @@ impl TraitCapturer for IpcDrmCapturer {
|
||||
),
|
||||
));
|
||||
}
|
||||
if t == 0 {
|
||||
// The producer grabs on a timer, so a still screen arrives as the same bytes every
|
||||
// tick. `cur` still holds the previous frame; the same bytes in another format are
|
||||
// another picture.
|
||||
let repeat = if t == 0 {
|
||||
let repeat = fmt == self.cur_fmt && buf == self.cur;
|
||||
let previous = std::mem::replace(&mut self.cur, buf);
|
||||
self.shared.slot.lock().unwrap().recycle(previous);
|
||||
repeat
|
||||
} else if !matches!(fmt, Pixfmt::BGRA | Pixfmt::RGBA) {
|
||||
// Unreachable with today's producers (the convert path emits 4-byte pixels
|
||||
// and the CPU path hardcodes BGRA); kept so a future non-4-byte producer
|
||||
@@ -560,9 +574,12 @@ impl TraitCapturer for IpcDrmCapturer {
|
||||
),
|
||||
));
|
||||
} else {
|
||||
unrotate_bgra(&buf, w, h, t, &mut self.cur);
|
||||
unrotate_bgra(&buf, w, h, t, &mut self.spare);
|
||||
let repeat = fmt == self.cur_fmt && self.spare == self.cur;
|
||||
std::mem::swap(&mut self.cur, &mut self.spare);
|
||||
self.shared.slot.lock().unwrap().recycle(buf);
|
||||
}
|
||||
repeat
|
||||
};
|
||||
self.cur_w = fw;
|
||||
self.cur_h = fh;
|
||||
self.cur_fmt = fmt;
|
||||
@@ -579,6 +596,7 @@ impl TraitCapturer for IpcDrmCapturer {
|
||||
}
|
||||
}
|
||||
}
|
||||
repeat
|
||||
} else {
|
||||
let err = slot
|
||||
.ended
|
||||
@@ -589,7 +607,11 @@ impl TraitCapturer for IpcDrmCapturer {
|
||||
}
|
||||
return Err(io::Error::new(io::ErrorKind::Other, err));
|
||||
}
|
||||
};
|
||||
if repeat && self.returned_at.elapsed() < REPEAT_EVERY {
|
||||
return Err(io::ErrorKind::WouldBlock.into());
|
||||
}
|
||||
self.returned_at = Instant::now();
|
||||
Ok(Frame::PixelBuffer(PixelBuffer::new(
|
||||
&self.cur,
|
||||
self.cur_fmt,
|
||||
@@ -2401,6 +2423,8 @@ mod drm_capturer_tests {
|
||||
cur_h: 0,
|
||||
cur_fmt: Pixfmt::BGRA,
|
||||
got_frame: false,
|
||||
spare: Vec::new(),
|
||||
returned_at: Instant::now(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2931,6 +2955,12 @@ mod drm_capturer_tests {
|
||||
Ok(_) => panic!("expected a pixel-buffer frame"),
|
||||
Err(err) => panic!("expected a delivered frame, got {err}"),
|
||||
}
|
||||
// The same scanout again: once turned it repeats the frame above, so it is dropped.
|
||||
put_frame_with(&c, w, h, Some(0x1), &src);
|
||||
assert!(matches!(
|
||||
c.frame(Duration::from_millis(50)),
|
||||
Err(e) if e.kind() == io::ErrorKind::WouldBlock
|
||||
));
|
||||
// Hardware rotation (i915 + mutter): the scanout is already upright and is left alone.
|
||||
put_frame_with(&c, w, h, Some(0x4), &src);
|
||||
match c.frame(Duration::from_millis(50)) {
|
||||
@@ -2941,16 +2971,67 @@ mod drm_capturer_tests {
|
||||
Err(err) => panic!("expected a delivered frame, got {err}"),
|
||||
}
|
||||
// A producer that cannot say (pre-0.5.8 library) keeps the old rule: 180 left alone.
|
||||
put_frame_with(&c, w, h, None, &src);
|
||||
// A new picture: `src` left alone is the frame above and would be dropped as a repeat.
|
||||
let (other, _, _) = px_frame(&[&[7, 8, 9], &[10, 11, 12]], 0);
|
||||
put_frame_with(&c, w, h, None, &other);
|
||||
match c.frame(Duration::from_millis(50)) {
|
||||
Ok(Frame::PixelBuffer(pb)) => {
|
||||
assert_eq!(labels_of(pb.data(), w, h), vec![vec![1, 2, 3], vec![4, 5, 6]]);
|
||||
assert_eq!(labels_of(pb.data(), w, h), vec![vec![7, 8, 9], vec![10, 11, 12]]);
|
||||
}
|
||||
Ok(_) => panic!("expected a pixel-buffer frame"),
|
||||
Err(err) => panic!("expected a delivered frame, got {err}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_frame_equal_to_the_last_one_goes_out_once_a_second() {
|
||||
let (still, w, h) = px_frame(&[&[1, 2, 3], &[4, 5, 6]], 0);
|
||||
let (moved, _, _) = px_frame(&[&[1, 2, 3], &[4, 5, 7]], 0);
|
||||
let mut c = capturer_with(Some((w, h)));
|
||||
put_frame_with(&c, w, h, None, &still);
|
||||
assert!(c.frame(Duration::from_millis(50)).is_ok());
|
||||
put_frame_with(&c, w, h, None, &still);
|
||||
assert!(matches!(
|
||||
c.frame(Duration::from_millis(50)),
|
||||
Err(e) if e.kind() == io::ErrorKind::WouldBlock
|
||||
));
|
||||
// A second later it goes out again, in case its encode failed, and the second starts over.
|
||||
std::thread::sleep(REPEAT_EVERY);
|
||||
put_frame_with(&c, w, h, None, &still);
|
||||
assert!(c.frame(Duration::from_millis(50)).is_ok());
|
||||
put_frame_with(&c, w, h, None, &still);
|
||||
assert!(matches!(
|
||||
c.frame(Duration::from_millis(50)),
|
||||
Err(e) if e.kind() == io::ErrorKind::WouldBlock
|
||||
));
|
||||
put_frame_with(&c, w, h, None, &moved);
|
||||
assert!(c.frame(Duration::from_millis(50)).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_same_bytes_in_another_format_are_not_a_repeat() {
|
||||
// Blue as BGRA, red as RGBA.
|
||||
let px = [255u8, 0, 0, 255];
|
||||
let put = |c: &IpcDrmCapturer, fmt| {
|
||||
c.shared
|
||||
.slot
|
||||
.lock()
|
||||
.unwrap()
|
||||
.publish(1, 1, fmt, None, px.to_vec());
|
||||
c.shared.cv.notify_one();
|
||||
};
|
||||
let mut c = capturer_with(Some((1, 1)));
|
||||
put(&c, Pixfmt::BGRA);
|
||||
assert!(c.frame(Duration::from_millis(50)).is_ok());
|
||||
put(&c, Pixfmt::RGBA);
|
||||
assert!(c.frame(Duration::from_millis(50)).is_ok());
|
||||
put(&c, Pixfmt::RGBA);
|
||||
assert!(matches!(
|
||||
c.frame(Duration::from_millis(50)),
|
||||
Err(e) if e.kind() == io::ErrorKind::WouldBlock
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_plane_that_rotated_90_in_hardware_hands_over_a_portrait_scanout() {
|
||||
use scrap::TraitPixelBuffer;
|
||||
@@ -3062,9 +3143,12 @@ mod drm_capturer_tests {
|
||||
}
|
||||
|
||||
fn put_frame_rot(c: &IpcDrmCapturer, w: usize, h: usize, plane_rotation: Option<u32>) {
|
||||
// Each call differs in its first pixel: frame() drops a frame equal to the last one.
|
||||
static SEQ: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0);
|
||||
let mut buf = c.shared.slot.lock().unwrap().take_free().unwrap_or_default();
|
||||
buf.clear();
|
||||
buf.resize(w * h * 4, 0);
|
||||
buf[..4].copy_from_slice(&SEQ.fetch_add(1, Ordering::Relaxed).to_le_bytes());
|
||||
let mut slot = c.shared.slot.lock().unwrap();
|
||||
slot.publish(w, h, Pixfmt::BGRA, plane_rotation, buf);
|
||||
c.shared.cv.notify_one();
|
||||
|
||||
Reference in New Issue
Block a user