From 81c1bbcc974b0bd17746a7d74619bfc1404d56e3 Mon Sep 17 00:00:00 2001 From: Ad1th <90973387+Ad1th@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:15:49 +0530 Subject: [PATCH] perf(renderd-host): sharpen a still screen at native size, and fix software colour maths Move the encoder up to the display's native size once the screen has been still, then back to the link's size on motion. Decode the software path as BT.709 limited range instead of full-range BT.601, and pin the capture to sRGB/BT.709. --- CHANGELOG.md | 21 ++ Cargo.lock | 1 + crates/renderd-host/src/capture.rs | 15 +- crates/renderd-host/src/encode.rs | 99 ++++++- crates/renderd-host/src/refine.rs | 269 +++++++++++++++++- crates/renderd-sc-sys/Cargo.toml | 3 + .../renderd-sc-sys/examples/colour_probe.rs | 19 ++ crates/renderd-sc-sys/src/stream.rs | 59 ++++ crates/renderd-viewer/src/renderer.rs | 55 ++-- 9 files changed, 507 insertions(+), 34 deletions(-) create mode 100644 crates/renderd-sc-sys/examples/colour_probe.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 36eb06d..1c42107 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Performance +- **Sharpen on idle.** On a slow link the encoder runs below the display's resolution, and a + still screen stayed soft however long it sat there: no refinement pass restores pixels that were + never encoded. Once the screen has been still for 500 ms the host now moves the encoder up to + the display's native size for one full-resolution keyframe plus two refinement passes, and drops + back to the link's size when the screen moves again (four captures within 300 ms, so a blinking + cursor does not undo it; 2 s cooldown before sharpening again). Skipped when the keyframe would + take over 1.2 s to drain at the current bitrate. ([`host::refine`](crates/renderd-host/src/refine.rs), + [`EncodePipeline::sharpen`](crates/renderd-host/src/encode.rs)) + +### Fixed +- **Software renderer used the wrong colour maths.** The Windows software path (and the fallback + when the D3D11 presenter cannot start) converted NV12 as full-range BT.601, but the stream is + BT.709 limited range. Every black came out grey (16 instead of 0), every white dim (235 instead + of 255) and hues shifted, which reads as washed-out text. It now converts BT.709 limited range, + matching the D3D11 presenter. +- **Capture colour space is pinned.** `ScreenCaptureKit` is asked for sRGB with the BT.709 matrix + instead of inheriting the display's colour space, so a wide-gamut Mac cannot send Display P3 + values the viewer reads as sRGB. The capture's colour tags are logged for the first frames, and + `cargo run -p renderd-sc-sys --example colour_probe` prints them. + --- ## [1.0.0] — 2026-10-05 diff --git a/Cargo.lock b/Cargo.lock index 179539b..e252f51 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2457,6 +2457,7 @@ dependencies = [ "renderd-vt-sys", "thiserror 2.0.19", "tracing", + "tracing-subscriber", ] [[package]] diff --git a/crates/renderd-host/src/capture.rs b/crates/renderd-host/src/capture.rs index 3e228c3..5839261 100644 --- a/crates/renderd-host/src/capture.rs +++ b/crates/renderd-host/src/capture.rs @@ -125,9 +125,10 @@ impl CapturePipeline { "Capture target selected" ); - let refiner = Arc::new(crate::refine::StaticRefiner::start(Arc::clone( - &encode_pipeline, - ))); + let refiner = Arc::new(crate::refine::StaticRefiner::start( + Arc::clone(&encode_pipeline), + (width, height), + )); let refiner_ref = Arc::clone(&refiner); let pipeline_ref = encode_pipeline; @@ -147,14 +148,16 @@ impl CapturePipeline { "CapturePipeline: forwarding first captured frame to VideoToolbox encoder" ); } + // Whether it is encoded or skipped for a deep queue, this is + // now the screen: the refiner makes sure the viewer ends up + // with it, and, before the frame is encoded, takes the encoder + // back to the link's size if a sharpened still has ended. + refiner_ref.on_capture(&frame.surface, frame.pts_ns); if let Err(e) = pipeline_ref.encode_surface(&frame.surface, frame.pts_ns) { if count % 300 == 1 { tracing::warn!("encode_surface failed: {e}"); } } - // Encoded or skipped for a deep queue, this is now the - // screen; the refiner makes sure the viewer ends up with it. - refiner_ref.on_capture(&frame.surface, frame.pts_ns); }, ) .map_err(|e| HostError::Initialization(format!("ScreenStream creation failed: {e}")))?; diff --git a/crates/renderd-host/src/encode.rs b/crates/renderd-host/src/encode.rs index cca868f..b01dbdc 100644 --- a/crates/renderd-host/src/encode.rs +++ b/crates/renderd-host/src/encode.rs @@ -224,6 +224,9 @@ pub struct EncodePipeline { /// Presentation timestamp of the last surface submitted, in nanoseconds. last_pts_ns: AtomicI64, params: std::sync::Mutex>, + /// The size the link calls for, while the encoder is temporarily running at + /// a larger one to sharpen a still screen (see [`EncodePipeline::sharpen`]). + base_size: std::sync::Mutex>, #[cfg(target_os = "macos")] session: std::sync::Mutex>, } @@ -262,6 +265,7 @@ impl EncodePipeline { link: Arc::new(LinkPressure::new()), last_pts_ns: AtomicI64::new(i64::MIN), params: std::sync::Mutex::new(None), + base_size: std::sync::Mutex::new(None), #[cfg(target_os = "macos")] session: std::sync::Mutex::new(None), } @@ -359,6 +363,70 @@ impl EncodePipeline { /// configured or the new session cannot be created; the old one keeps /// running in that case. pub fn reconfigure(&self, width: u32, height: u32) -> Result { + // A size the link asked for replaces whatever sharpening was in use. + if let Ok(mut base) = self.base_size.lock() { + *base = None; + } + self.resize(width, height, "Encoder resized to follow the link") + } + + /// Runs the encoder at `native` until [`restore_size`](Self::restore_size), + /// remembering the size the link called for. Returns `false` if it already + /// runs at that size. + /// + /// The viewer follows the stream's size, so this is how a screen that has + /// stopped changing gets the full-resolution picture the link could not + /// afford while it was moving. + /// + /// # Errors + /// + /// As [`reconfigure`](Self::reconfigure). + pub fn sharpen(&self, native: (u32, u32)) -> Result { + let current = self + .encoded_size() + .ok_or_else(|| HostError::Initialization("encoder was never configured".into()))?; + if current == native || self.is_sharpened() { + return Ok(false); + } + if let Ok(mut base) = self.base_size.lock() { + *base = Some(current); + } + match self.resize(native.0, native.1, "Encoder sharpened for a still screen") { + Ok(changed) => Ok(changed), + Err(e) => { + if let Ok(mut base) = self.base_size.lock() { + *base = None; + } + Err(e) + } + } + } + + /// Whether the encoder is running at a sharpened size. + #[must_use] + pub fn is_sharpened(&self) -> bool { + self.base_size.lock().is_ok_and(|b| b.is_some()) + } + + /// Returns to the size the link calls for after [`sharpen`](Self::sharpen). + /// Returns `false` if the encoder was not sharpened. + /// + /// # Errors + /// + /// As [`reconfigure`](Self::reconfigure); the sharpened size stays in use + /// if the new session cannot be created. + pub fn restore_size(&self) -> Result { + let Some(base) = self.base_size.lock().ok().and_then(|b| *b) else { + return Ok(false); + }; + let changed = self.resize(base.0, base.1, "Encoder back to the link's size")?; + if let Ok(mut b) = self.base_size.lock() { + *b = None; + } + Ok(changed) + } + + fn resize(&self, width: u32, height: u32, why: &str) -> Result { let mut params = self .params .lock() @@ -372,7 +440,7 @@ impl EncodePipeline { params.height = height; self.keyframes.reset(); self.start_session(¶ms, self.current_bitrate().max(1))?; - tracing::info!(width, height, "Encoder resized to follow the link"); + tracing::info!(width, height, "{why}"); if let Ok(mut guard) = self.params.lock() { *guard = Some(params); } @@ -795,6 +863,35 @@ mod tests { assert_eq!(pipeline.encoded_size(), None); } + #[test] + fn test_sharpen_runs_at_native_and_restores_the_links_size() { + let pipeline = EncodePipeline::new(); + assert!(pipeline.sharpen((1920, 1080)).is_err()); + assert!(!pipeline.restore_size().unwrap()); + if pipeline.init(960, 544, 4_000, "hevc", 60).is_err() { + return; // no hardware encoder here + } + assert!(pipeline.sharpen((1920, 1080)).unwrap()); + assert!(pipeline.is_sharpened()); + assert_eq!(pipeline.encoded_size(), Some((1920, 1080))); + assert!( + !pipeline.sharpen((1920, 1080)).unwrap(), + "already sharpened" + ); + + assert!(pipeline.restore_size().unwrap()); + assert!(!pipeline.is_sharpened()); + assert_eq!(pipeline.encoded_size(), Some((960, 544))); + + // A size the link asks for wins over a sharpened one. + assert!(pipeline.sharpen((1920, 1080)).unwrap()); + assert!(pipeline.reconfigure(1280, 720).unwrap()); + assert!(!pipeline.is_sharpened()); + assert!(!pipeline.restore_size().unwrap()); + assert_eq!(pipeline.encoded_size(), Some((1280, 720))); + pipeline.shutdown(); + } + #[test] fn test_monotonic_pts_never_goes_backwards() { let pipeline = EncodePipeline::new(); diff --git a/crates/renderd-host/src/refine.rs b/crates/renderd-host/src/refine.rs index 3f5e6eb..bc3e1e8 100644 --- a/crates/renderd-host/src/refine.rs +++ b/crates/renderd-host/src/refine.rs @@ -23,6 +23,14 @@ //! ends on the current screen; the next spends the now-idle budget on the //! detail the motion frames could not afford. A still screen costs nothing //! once the passes are done. +//! +//! On a slow link the encoder also runs below the display's resolution (see +//! [`crate::scale`]), and no number of passes brings back pixels that were +//! never encoded: text stays soft however long the screen is still. So once +//! the screen has settled, the refiner moves the encoder up to the display's +//! native size for one full-resolution keyframe and a couple of refinement +//! passes ([`plan_sharpen`]), and drops back to the link's size as soon as the +//! screen moves again ([`MotionDetector`]). use std::time::{Duration, Instant}; @@ -48,6 +56,105 @@ pub const MAX_ATTEMPTS: u32 = 12; /// real frames, and a pass would only queue behind them. pub const MAX_QUEUE: Duration = Duration::from_millis(10); +/// Stillness before the encoder is moved up to the display's native size. +/// +/// Long enough that typing and cursor movement do not trigger it, short +/// enough to feel like the picture sharpening as soon as you stop. +pub const SHARPEN_AFTER: Duration = Duration::from_millis(500); + +/// Minimum time at the link's size after moving, before sharpening again. +pub const SHARPEN_COOLDOWN: Duration = Duration::from_secs(2); + +/// Longest a full-resolution keyframe may take to drain at the current +/// bitrate. A text-heavy desktop keyframe measures about half a bit per pixel. +pub const MAX_SHARPEN_DRAIN: Duration = Duration::from_millis(1200); + +/// Captures within [`MOTION_WINDOW`] that count as the screen moving again. +pub const MOTION_FRAMES: usize = 4; + +/// See [`MOTION_FRAMES`]. +pub const MOTION_WINDOW: Duration = Duration::from_millis(300); + +/// Approximate size of a full-resolution desktop keyframe, in bits per pixel. +const KEYFRAME_BITS_PER_PIXEL: f64 = 0.5; + +/// Whether a native-size keyframe can drain in a tolerable time at +/// `bitrate_kbps`. +#[must_use] +pub fn sharpen_affordable(native: (u32, u32), bitrate_kbps: u32) -> bool { + if bitrate_kbps == 0 { + return false; + } + let bits = f64::from(native.0) * f64::from(native.1) * KEYFRAME_BITS_PER_PIXEL; + bits / (f64::from(bitrate_kbps) * 1_000.0) <= MAX_SHARPEN_DRAIN.as_secs_f64() +} + +/// What the refiner should do about sharpening. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SharpenPlan { + /// Sharpen now. + Now, + /// Sharpen at this time if nothing changes. + At(Instant), + /// Nothing to do until the screen changes. + Never, +} + +/// Decides whether and when to move the encoder up to `native`. +/// +/// `encoded` is the size the encoder runs at, `captured` when the screen last +/// changed and `last_restore` when the encoder last went back down. +#[must_use] +pub fn plan_sharpen( + now: Instant, + encoded: (u32, u32), + native: (u32, u32), + bitrate_kbps: u32, + captured: Instant, + last_restore: Option, +) -> SharpenPlan { + if encoded.0 >= native.0 && encoded.1 >= native.1 { + return SharpenPlan::Never; + } + if !sharpen_affordable(native, bitrate_kbps) { + return SharpenPlan::Never; + } + let mut ready = captured + SHARPEN_AFTER; + if let Some(at) = last_restore { + ready = ready.max(at + SHARPEN_COOLDOWN); + } + if now >= ready { + SharpenPlan::Now + } else { + SharpenPlan::At(ready) + } +} + +/// Tells a screen that is moving from one that merely changed. +/// +/// A blinking cursor or a clock produces an isolated capture now and then; +/// scrolling, dragging and video produce a run of them. Only a run is allowed +/// to undo a sharpened picture. +#[derive(Debug, Default)] +pub struct MotionDetector { + recent: std::collections::VecDeque, +} + +impl MotionDetector { + /// Records a capture at `now`; returns whether the screen is moving. + pub fn on_capture(&mut self, now: Instant) -> bool { + self.recent.push_back(now); + while self + .recent + .front() + .is_some_and(|&t| now.saturating_duration_since(t) > MOTION_WINDOW) + { + self.recent.pop_front(); + } + self.recent.len() >= MOTION_FRAMES + } +} + /// When the next re-encode pass is due. #[derive(Debug, Default)] pub struct RefineSchedule { @@ -106,7 +213,7 @@ mod macos { use renderd_vt_sys::{HeldSurface, IoSurface}; - use super::{RefineSchedule, MAX_QUEUE}; + use super::{plan_sharpen, MotionDetector, RefineSchedule, SharpenPlan, MAX_QUEUE}; use crate::encode::{EncodePipeline, Submitted}; /// The newest captured surface, held so the capture pool leaves it alone. @@ -122,6 +229,13 @@ mod macos { schedule: RefineSchedule, /// The encoder's output count when the last pass was submitted. pending: Option, + motion: MotionDetector, + /// The display's size: what a sharpened picture is encoded at. + native: (u32, u32), + /// Whether the current still screen has already been sharpened. + sharpen_done: bool, + /// When the encoder last went back down from a sharpened size. + last_restore: Option, stop: bool, } @@ -134,6 +248,7 @@ mod macos { /// Re-encodes the newest captured surface a few times once the screen goes /// still (see the module documentation). pub struct StaticRefiner { + encode: Arc, shared: Arc, thread: Mutex>>, } @@ -145,22 +260,34 @@ mod macos { } impl StaticRefiner { - /// Starts the refiner thread feeding `encode`. + /// Starts the refiner thread feeding `encode`. `native` is the size of + /// the captured surfaces, which a sharpened picture is encoded at. #[must_use] - pub fn start(encode: Arc) -> Self { + pub fn start(encode: Arc, native: (u32, u32)) -> Self { let shared = Arc::new(Shared::default()); + shared + .state + .lock() + .unwrap_or_else(PoisonError::into_inner) + .native = native; let worker = Arc::clone(&shared); + let worker_encode = Arc::clone(&encode); let thread = std::thread::Builder::new() .name("renderd-refine".into()) - .spawn(move || run(&worker, &encode)) + .spawn(move || run(&worker, &worker_encode)) .ok(); Self { + encode, shared, thread: Mutex::new(thread), } } /// Records a freshly captured surface, whether or not it was encoded. + /// + /// Call this *before* submitting the surface: if the screen has started + /// moving again after a sharpened still, the encoder goes back to the + /// link's size here, so that the frame is encoded at it. pub fn on_capture(&self, surface: &IoSurface, pts_ns: i64) { let now = Instant::now(); let latest = Latest { @@ -176,7 +303,18 @@ mod macos { state.latest = Some(latest); state.schedule.on_capture(now); state.pending = None; + state.sharpen_done = false; + let moving = state.motion.on_capture(now); + let restore = moving && self.encode.is_sharpened(); + if restore { + state.last_restore = Some(now); + } drop(state); + if restore { + if let Err(e) = self.encode.restore_size() { + tracing::warn!("could not leave the sharpened size: {e}"); + } + } self.shared.wake.notify_one(); } @@ -209,6 +347,24 @@ mod macos { } } + /// Whether the settled screen should be re-encoded at the display's size. + fn sharpen_wanted(state: &State, encode: &EncodePipeline, now: Instant) -> SharpenPlan { + let (Some(latest), Some(encoded)) = (state.latest.as_ref(), encode.encoded_size()) else { + return SharpenPlan::Never; + }; + if state.sharpen_done || encode.is_sharpened() || state.native == (0, 0) { + return SharpenPlan::Never; + } + plan_sharpen( + now, + encoded, + state.native, + encode.current_bitrate(), + latest.captured, + state.last_restore, + ) + } + fn run(shared: &Shared, encode: &EncodePipeline) { let mut passes: u64 = 0; let mut state = shared.state.lock().unwrap_or_else(PoisonError::into_inner); @@ -225,6 +381,37 @@ mod macos { let now = Instant::now(); match state.schedule.next_at() { None => { + match sharpen_wanted(&state, encode, now) { + SharpenPlan::Now => { + state.sharpen_done = true; + let native = state.native; + drop(state); + let landed = match encode.sharpen(native) { + Ok(changed) => changed, + Err(e) => { + tracing::debug!("sharpen failed: {e}"); + false + } + }; + state = shared.state.lock().unwrap_or_else(PoisonError::into_inner); + if landed { + // The new session starts with a keyframe of the + // still screen, then refines it like any other. + state.schedule.on_capture(Instant::now()); + state.pending = None; + } + continue; + } + SharpenPlan::At(at) => { + state = shared + .wake + .wait_timeout(state, at.saturating_duration_since(now)) + .unwrap_or_else(PoisonError::into_inner) + .0; + continue; + } + SharpenPlan::Never => {} + } // Every pass ran: hand the surface back to the capture pool. state.latest = None; state = shared @@ -343,4 +530,78 @@ mod tests { at += PASS_SPACING; } } + + fn at(base: Instant, ms: u64) -> Instant { + base + Duration::from_millis(ms) + } + + #[test] + fn test_sharpen_waits_for_stillness_then_goes() { + let t0 = Instant::now(); + let low = (960, 544); + let native = (1920, 1080); + assert_eq!( + plan_sharpen(at(t0, 100), low, native, 3_000, t0, None), + SharpenPlan::At(t0 + SHARPEN_AFTER) + ); + assert_eq!( + plan_sharpen(at(t0, 600), low, native, 3_000, t0, None), + SharpenPlan::Now + ); + } + + #[test] + fn test_sharpen_not_needed_at_native_size() { + let t0 = Instant::now(); + assert_eq!( + plan_sharpen(at(t0, 900), (1920, 1080), (1920, 1080), 3_000, t0, None), + SharpenPlan::Never + ); + } + + #[test] + fn test_sharpen_skipped_when_the_keyframe_would_clog_the_link() { + // ~1 Mbit of keyframe on a 500 kbps link is two seconds of queue. + assert!(!sharpen_affordable((1920, 1080), 500)); + assert!(sharpen_affordable((1920, 1080), 1_500)); + assert!(sharpen_affordable((1920, 1080), 3_000)); + assert!(!sharpen_affordable((1920, 1080), 0)); + } + + #[test] + fn test_sharpen_cools_down_after_returning_to_the_link_size() { + let t0 = Instant::now(); + let restored = at(t0, 1_000); + assert_eq!( + plan_sharpen( + at(t0, 1_700), + (960, 544), + (1920, 1080), + 3_000, + at(t0, 1_000), + Some(restored) + ), + SharpenPlan::At(restored + SHARPEN_COOLDOWN) + ); + } + + #[test] + fn test_isolated_captures_are_not_motion() { + let mut motion = MotionDetector::default(); + let t0 = Instant::now(); + // A blinking cursor: one capture every half second. + for i in 0..10 { + assert!(!motion.on_capture(at(t0, i * 500))); + } + } + + #[test] + fn test_a_run_of_captures_is_motion() { + let mut motion = MotionDetector::default(); + let t0 = Instant::now(); + assert!(!motion.on_capture(at(t0, 0))); + assert!(!motion.on_capture(at(t0, 16))); + assert!(!motion.on_capture(at(t0, 33))); + assert!(motion.on_capture(at(t0, 50)), "four within 300 ms"); + } } diff --git a/crates/renderd-sc-sys/Cargo.toml b/crates/renderd-sc-sys/Cargo.toml index 6e3e29d..62edc50 100644 --- a/crates/renderd-sc-sys/Cargo.toml +++ b/crates/renderd-sc-sys/Cargo.toml @@ -28,3 +28,6 @@ pedantic = { level = "warn", priority = -1 } nursery = { level = "warn", priority = -1 } module_name_repetitions = "allow" missing_errors_doc = "allow" + +[dev-dependencies] +tracing-subscriber = { workspace = true } diff --git a/crates/renderd-sc-sys/examples/colour_probe.rs b/crates/renderd-sc-sys/examples/colour_probe.rs new file mode 100644 index 0000000..141b95c --- /dev/null +++ b/crates/renderd-sc-sys/examples/colour_probe.rs @@ -0,0 +1,19 @@ +//! Captures a few frames of the main display and logs how they are tagged. +//! Needs Screen Recording permission for the terminal running it. +#![cfg(target_os = "macos")] + +use std::time::Duration; + +use renderd_sc_sys::{ContentFilter, ScreenStream}; + +fn main() { + tracing_subscriber::fmt() + .with_max_level(tracing::Level::INFO) + .init(); + let filter = ContentFilter::main_display().expect("main display"); + let stream = + ScreenStream::with_dimensions(&filter, filter.width(), filter.height(), 30, |_| {}) + .expect("stream"); + stream.start().expect("start"); + std::thread::sleep(Duration::from_secs(2)); +} diff --git a/crates/renderd-sc-sys/src/stream.rs b/crates/renderd-sc-sys/src/stream.rs index 835d4dc..a745a1d 100644 --- a/crates/renderd-sc-sys/src/stream.rs +++ b/crates/renderd-sc-sys/src/stream.rs @@ -51,6 +51,32 @@ const fn to_raw_cmtime(t: CMTime) -> RawCMTime { } } +/// A `CFStringRef` as an Objective-C argument: the runtime checks the type +/// encoding, and a bare `*const c_void` does not match `^{__CFString=}`. +#[repr(transparent)] +#[derive(Clone, Copy)] +struct CfString(CFTypeRef); + +// SAFETY: transparent over a pointer, encoded as the pointer-to-struct type +// Objective-C uses for `CFStringRef`. +unsafe impl Encode for CfString { + const ENCODING: Encoding = Encoding::Pointer(&Encoding::Struct("__CFString", &[])); +} + +#[link(name = "CoreGraphics", kind = "framework")] +extern "C" { + static kCGColorSpaceSRGB: CFTypeRef; + static kCGDisplayStreamYCbCrMatrix_ITU_R_709_2: CFTypeRef; + static kCVImageBufferYCbCrMatrixKey: CFTypeRef; + static kCVImageBufferColorPrimariesKey: CFTypeRef; + static kCVImageBufferTransferFunctionKey: CFTypeRef; + fn CVBufferCopyAttachment( + buffer: *const std::ffi::c_void, + key: CFTypeRef, + attachment_mode: *mut u32, + ) -> CFTypeRef; +} + extern "C" { fn CMSampleBufferGetImageBuffer(sbuf: CFTypeRef) -> *const std::ffi::c_void; fn CVPixelBufferGetIOSurface( @@ -87,6 +113,21 @@ extern "C" { ) -> i32; } +/// The colour tag `key` carries on `image_buf`, if it is a string. +/// +/// # Safety +/// `image_buf` must be a valid `CVImageBufferRef`. +unsafe fn colour_tag(image_buf: *const std::ffi::c_void, key: CFTypeRef) -> String { + use core_foundation::base::TCFType; + use core_foundation::string::{CFString, CFStringRef}; + + let value = CVBufferCopyAttachment(image_buf, key, std::ptr::null_mut()); + if value.is_null() { + return "none".into(); + } + CFString::wrap_under_create_rule(value as CFStringRef).to_string() +} + fn inspect_captured_image_buf(image_buf: *const std::ffi::c_void, count: u64) { if count > 5 || image_buf.is_null() { return; @@ -128,8 +169,15 @@ fn inspect_captured_image_buf(image_buf: *const std::ffi::c_void, count: u64) { let _ = CVPixelBufferUnlockBaseAddress(image_buf, 1); } + let matrix = colour_tag(image_buf, kCVImageBufferYCbCrMatrixKey); + let primaries = colour_tag(image_buf, kCVImageBufferColorPrimariesKey); + let transfer = colour_tag(image_buf, kCVImageBufferTransferFunctionKey); + tracing::info!( count = count, + colour_matrix = %matrix, + colour_primaries = %primaries, + colour_transfer = %transfer, width = w, height = h, format_fourcc = format!("{:#x}", fmt), @@ -338,6 +386,17 @@ impl ScreenStream { // '420v' bi-planar YCbCr 4:2:0 video range NV12 pixel format config.setPixelFormat(0x3432_3076); + + // Capture in sRGB with the BT.709 matrix, which is what the viewer + // converts with. Left alone, ScreenCaptureKit hands over the + // display's own colour space; on a wide-gamut Mac that is Display P3, + // and P3 values read as sRGB look flat and dull. The tags travel in + // the encoded stream, so the decoder knows what it was given too. + let _: () = msg_send![&config, setColorSpaceName: CfString(kCGColorSpaceSRGB)]; + let _: () = msg_send![ + &config, + setColorMatrix: CfString(kCGDisplayStreamYCbCrMatrix_ITU_R_709_2) + ]; } // SAFETY: allocate RenderdStreamOutput delegate with callback ivar. diff --git a/crates/renderd-viewer/src/renderer.rs b/crates/renderd-viewer/src/renderer.rs index 20a5199..8740eae 100644 --- a/crates/renderd-viewer/src/renderer.rs +++ b/crates/renderd-viewer/src/renderer.rs @@ -179,16 +179,20 @@ fn scale_nv12( } } -// BT.601 full-range YUV->RGB coefficients in 16.16 fixed point, shared by both the -// per-pixel and per-pixel-pair conversion below. -/// 1.402 << 16 -const YUV_R_CR: i32 = 91_881; -/// 0.344136 << 16 -const YUV_G_CB: i32 = 22_554; -/// 0.714136 << 16 -const YUV_G_CR: i32 = 46_802; -/// 1.772 << 16 -const YUV_B_CB: i32 = 116_130; +// BT.709 limited-range (16-235) YUV->RGB coefficients in 16.16 fixed point, shared by +// both the per-pixel and per-pixel-pair conversion below. This is how the host tags +// its stream (`ScreenCaptureKit` `420v` capture, BT.709 matrix); decoding it as +// full-range BT.601 lifted every black to grey, dimmed every white and shifted hues. +/// 255/219 << 16: stretches limited-range luma to full range. +const YUV_Y_SCALE: i32 = 76_309; +/// 1.792741 << 16 +const YUV_R_CR: i32 = 117_489; +/// 0.213249 << 16 +const YUV_G_CB: i32 = 13_975; +/// 0.532909 << 16 +const YUV_G_CR: i32 = 34_925; +/// 2.112402 << 16 +const YUV_B_CB: i32 = 138_438; /// 0.5 in 16.16 fixed point — added before the final `>> 16` so the divide rounds /// to the nearest integer instead of always truncating toward zero. A plain /// right-shift is a floor, which biased every reconstructed pixel darker by up to @@ -202,14 +206,14 @@ const YUV_HALF: i32 = 1 << 15; /// Returns 1 if the resulting pixel has any colour, 0 if it is pure black; the caller /// uses this to report how much of a frame was non-blank. /// -/// Uses BT.601 full-range coefficients in 16.16 fixed point. This runs once per pixel +/// Uses BT.709 limited-range coefficients in 16.16 fixed point. This runs once per pixel /// per frame — over two million times per frame at 1080p — so the scalar float form it /// replaces dominated the frame budget on the software path. Where two horizontally /// adjacent pixels share one chroma sample (true for every pair in 4:2:0), prefer /// [`nv12_pair_to_bgra`], which computes the chroma terms once instead of twice. #[inline] fn nv12_to_bgra(luma: u8, chroma_b: u8, chroma_r: u8, out: &mut u32) -> u8 { - let luma = i32::from(luma) << 16; + let luma = (i32::from(luma) - 16) * YUV_Y_SCALE; let chroma_b = i32::from(chroma_b) - 128; let chroma_r = i32::from(chroma_r) - 128; @@ -226,7 +230,7 @@ fn nv12_to_bgra(luma: u8, chroma_b: u8, chroma_r: u8, out: &mut u32) -> u8 { /// Converts two horizontally adjacent NV12 pixels that share one chroma sample — /// true for every pixel pair in 4:2:0 — to two `0RGB` words in one call. /// -/// The three chroma-derived terms in the BT.601 matrix depend only on `(chroma_b, +/// The three chroma-derived terms in the BT.709 matrix depend only on `(chroma_b, /// chroma_r)`, which is identical for both pixels of the pair; computing them once /// here instead of once per pixel removes three of the six fixed-point /// multiplications from what is the hottest loop in the software render path — @@ -248,7 +252,7 @@ fn nv12_pair_to_bgra( let b_term = YUV_B_CB * chroma_b + YUV_HALF; for (luma, out) in [(luma0, out0), (luma1, out1)] { - let luma = i32::from(luma) << 16; + let luma = (i32::from(luma) - 16) * YUV_Y_SCALE; let red = ((luma + r_term) >> 16).clamp(0, 255); let green = ((luma + g_term) >> 16).clamp(0, 255); let blue = ((luma + b_term) >> 16).clamp(0, 255); @@ -614,7 +618,7 @@ mod tests { assert!(!renderer.is_initialized()); } - /// The fixed-point BT.601 conversion must agree with the float form it replaced. + /// The fixed-point BT.709 limited-range conversion must agree with the float form. #[test] fn test_nv12_to_bgra_matches_float_reference() { #[allow( @@ -623,13 +627,13 @@ mod tests { clippy::suboptimal_flops )] fn reference(luma: u8, chroma_b: u8, chroma_r: u8) -> (u32, u32, u32) { - let luma = f32::from(luma); + let luma = (f32::from(luma) - 16.0) * (255.0 / 219.0); let chroma_b = f32::from(chroma_b) - 128.0; let chroma_r = f32::from(chroma_r) - 128.0; ( - (luma + 1.402 * chroma_r).clamp(0.0, 255.0) as u32, - (luma - 0.344_136 * chroma_b - 0.714_136 * chroma_r).clamp(0.0, 255.0) as u32, - (luma + 1.772 * chroma_b).clamp(0.0, 255.0) as u32, + (luma + 1.792_741 * chroma_r).clamp(0.0, 255.0) as u32, + (luma - 0.213_249 * chroma_b - 0.532_909 * chroma_r).clamp(0.0, 255.0) as u32, + (luma + 2.112_402 * chroma_b).clamp(0.0, 255.0) as u32, ) } @@ -652,15 +656,20 @@ mod tests { } } - /// Neutral chroma with zero luma is black and reports as blank. + /// Limited-range black (16) and white (235) must land on 0 and 255, not on + /// grey: that was the whole point of leaving full-range BT.601. #[test] - fn test_nv12_to_bgra_black_is_reported_blank() { + fn test_nv12_to_bgra_limited_range_black_and_white() { let mut out = 0u32; - assert_eq!(nv12_to_bgra(0, 128, 128, &mut out), 0); + assert_eq!(nv12_to_bgra(16, 128, 128, &mut out), 0); assert_eq!(out & 0x00FF_FFFF, 0); - assert_eq!(nv12_to_bgra(255, 128, 128, &mut out), 1); + assert_eq!(nv12_to_bgra(235, 128, 128, &mut out), 1); assert_eq!(out & 0x00FF_FFFF, 0x00FF_FFFF); + + // Below black clamps instead of wrapping. + assert_eq!(nv12_to_bgra(0, 128, 128, &mut out), 0); + assert_eq!(out & 0x00FF_FFFF, 0); } /// The pair fast path must produce byte-identical output to the per-pixel