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