From 984f7be8963a830a3daf7f919b3252971ee8e42d Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 30 Jul 2026 21:11:15 +0200 Subject: [PATCH] fix(android): the presenter paces to the panel's real grid, not the app's down-rated vsync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On-glass (A024, 120 Hz panel, 120 fps session) the first presenter build released only 60/s and the HUD display term hit 40 ms. Root cause, in two layers: Android down-rates a game-category uid's choreographer stream to 60 Hz (frame-rate categories / game default frame rate), and under that override Display.getRefreshRate REPORTS THE OVERRIDE — so the presenter's panel grid read 16.67 ms on an 8.33 ms panel and the subdivision became a no-op, pacing the video at half rate and dropping every other frame. Three-part fix, verified live on the same device: - Kotlin passes the panel rate from the supported-modes TABLE (MainActivity.streamPanelFps — the mode list is not override-filtered) instead of display.refreshRate, and votes the app's render rate up via View.requestedFrameRate = streamHz (API 35+) while streaming. - The native vsync clock LEARNS the panel period from observed timeline spacing (downward-only: the finest spacing SurfaceFlinger ever reports is the true grid) and next_target subdivides the reported timeline onto it — full-rate on down-rated devices, a no-op where callbacks match the panel. - OnFrameRendered display/latch samples get the e2e clamp (0..10 s): a vendor's first callbacks can carry a garbage system_nano (observed: an epoch-sized latch max) that would poison every max it lands in. pf.present gained panelMs next to vsyncMs, and a one-shot cadence diagnostic logs Δ/timelines/spacing/panel on the third tick. After: released=120 displays=120 paced=0, pace p50 <1 ms, latch p50 ~16 ms idle / ~22 ms under game load (2 refresh intervals at 8.33 — the same composited-pipeline law the Apple client measured), HUD display ~17-26 ms vs 40 before. Co-Authored-By: Claude Fable 5 --- .../kotlin/io/unom/punktfunk/MainActivity.kt | 24 +++- .../kotlin/io/unom/punktfunk/StreamScreen.kt | 23 +++- .../io/unom/punktfunk/kit/NativeBridge.kt | 3 + .../android/native/src/decode/async_loop.rs | 10 +- clients/android/native/src/decode/display.rs | 8 +- clients/android/native/src/decode/mod.rs | 4 + .../android/native/src/decode/presenter.rs | 6 +- .../android/native/src/decode/sync_loop.rs | 1 + clients/android/native/src/decode/vsync.rs | 129 ++++++++++++++---- clients/android/native/src/session/planes.rs | 2 + 10 files changed, 176 insertions(+), 34 deletions(-) diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/MainActivity.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/MainActivity.kt index 83834d29..0b4ab965 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/MainActivity.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/MainActivity.kt @@ -471,9 +471,26 @@ class MainActivity : ComponentActivity() { setConsoleHighRefreshRate(false) return } + val target = streamModeFor(hz) ?: return + window.attributes = window.attributes.apply { preferredDisplayModeId = target.modeId } + } + + /** + * The panel refresh rate a [hz] stream runs against — [streamModeFor]'s pick, from the mode + * TABLE rather than `display.refreshRate`. The distinction matters: under a per-uid frame + * rate override (games get a 60 fps default on Android 15+) `refreshRate` reports the + * override, not the panel — observed on-glass as a 120 Hz panel reading back as 60. The + * supported-modes list is not override-filtered. `0` when unresolvable. + */ + fun streamPanelFps(hz: Int): Int = + streamModeFor(hz)?.refreshRate?.let { kotlin.math.round(it).toInt() } ?: 0 + + /** The same-resolution display mode [setStreamDisplayMode] pins for a [hz] stream. */ + private fun streamModeFor(hz: Int): android.view.Display.Mode? { + if (hz <= 0) return null @Suppress("DEPRECATION") val disp = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) display else windowManager.defaultDisplay - val current = disp?.mode ?: return + val current = disp?.mode ?: return null val sameRes = disp.supportedModes.filter { it.physicalWidth == current.physicalWidth && it.physicalHeight == current.physicalHeight } @@ -481,7 +498,7 @@ class MainActivity : ComponentActivity() { val k = (rate / hz).toInt() return if (k >= 2 && kotlin.math.abs(rate - hz * k) < 1f) k else 0 } - val target = sameRes.minWithOrNull( + return sameRes.minWithOrNull( compareBy( { when { @@ -492,8 +509,7 @@ class MainActivity : ComponentActivity() { }, { if (multiple(it.refreshRate) > 0) it.refreshRate else -it.refreshRate }, ), - ) ?: return - window.attributes = window.attributes.apply { preferredDisplayModeId = target.modeId } + ) } override fun dispatchKeyEvent(event: KeyEvent): Boolean { diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt index 00493892..f93c0a01 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt @@ -63,6 +63,7 @@ import io.unom.punktfunk.kit.Sc2Capture import io.unom.punktfunk.kit.VideoDecoders import io.unom.punktfunk.models.ActiveSession import java.util.concurrent.atomic.AtomicBoolean +import kotlin.math.roundToInt import kotlinx.coroutines.delay /** @@ -83,6 +84,9 @@ fun StreamScreen(session: ActiveSession, onDisconnect: () -> Unit) { // requested. val composeView = androidx.compose.ui.platform.LocalView.current val window = activity?.window + // The negotiated stream refresh, known from the handshake (0 = unknown / older native lib) — + // drives the panel mode pin, the render-rate vote, and the presenter's latch grid. + val streamHz = remember(handle) { NativeBridge.nativeVideoSize(handle)?.getOrNull(2) ?: 0 } val controller = remember(window) { window?.let { WindowCompat.getInsetsController(it, it.decorView) } } @@ -343,7 +347,6 @@ fun StreamScreen(session: ActiveSession, onDisconnect: () -> Unit) { // own ANativeWindow_setFrameRate hint still aligns vsync, but it is advisory — some OEM // refresh governors ignore it outright and would leave a 120 Hz session on a 60/90 Hz // panel. TV boxes skip the pin: the native side actively drives the HDMI mode there. - val streamHz = NativeBridge.nativeVideoSize(handle)?.getOrNull(2) ?: 0 if (isTv) { activity?.setConsoleHighRefreshRate(false) // the decoder's HDMI mode switch governs } else { @@ -355,6 +358,15 @@ fun StreamScreen(session: ActiveSession, onDisconnect: () -> Unit) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { composeView.requestUnbufferedDispatch(android.view.InputDevice.SOURCE_CLASS_POINTER) } + // Vote the app's RENDER rate up to the stream's (API 35+). The mode pin above governs the + // panel, but the platform separately down-rates a quiet app's choreographer stream + // (frame-rate categories: a non-animating UI reads as "normal" = 60) — observed on-glass + // as 16.6 ms vsync callbacks on a 120 Hz panel, which would pace the presenter at half + // rate. The native side also subdivides onto the panel grid, so this vote is the belt to + // that braces. Reset to no-preference on the way out. + if (Build.VERSION.SDK_INT >= 35 && streamHz > 0) { + composeView.requestedFrameRate = streamHz.toFloat() + } // Host→client feedback (rumble + DualSense lightbar/LEDs), routed to each controller by pad // index via the router; poll threads stopped + joined before the router is released and the // session closed. "Rumble on this phone" (opt-in) additionally mirrors controller 1's @@ -487,6 +499,9 @@ fun StreamScreen(session: ActiveSession, onDisconnect: () -> Unit) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { composeView.requestUnbufferedDispatch(0) // back to ordinary batched dispatch } + if (Build.VERSION.SDK_INT >= 35) { + composeView.requestedFrameRate = View.REQUESTED_FRAME_RATE_CATEGORY_DEFAULT + } controller?.hide(WindowInsetsCompat.Type.ime()) // drop any keyboard left showing window?.setSoftInputMode(priorSoftInput) if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R && priorCutout != null) { @@ -586,6 +601,12 @@ fun StreamScreen(session: ActiveSession, onDisconnect: () -> Unit) { isTv, initialSettings.presentPriorityWire(), initialSettings.smoothBuffer, + // The panel's own refresh — from the mode TABLE (streamPanelFps), + // because display.refreshRate reports a per-uid override, not the + // panel. Fallback: the (possibly lying) live rate. + activity?.streamPanelFps(streamHz)?.takeIf { it > 0 } + ?: (runCatching { context.display }.getOrNull()?.refreshRate ?: 0f) + .roundToInt(), ) NativeBridge.nativeStartAudio(handle, lowLatencyMode) if (micWanted) NativeBridge.nativeStartMic(handle) diff --git a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt index bf084384..d1394b0f 100644 --- a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt +++ b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/NativeBridge.kt @@ -223,6 +223,9 @@ object NativeBridge { isTv: Boolean, presentPriority: Int, smoothBuffer: Int, + /** The display mode's own refresh rate (0 = unknown) — the latch grid the presenter + * subdivides onto when the platform down-rates the app's choreographer stream. */ + panelFps: Int, ) /** Stop + join the decode thread without closing the session. No-op on `0`. */ diff --git a/clients/android/native/src/decode/async_loop.rs b/clients/android/native/src/decode/async_loop.rs index ed5390ee..bf9bc668 100644 --- a/clients/android/native/src/decode/async_loop.rs +++ b/clients/android/native/src/decode/async_loop.rs @@ -84,6 +84,7 @@ pub(super) fn run_async( is_tv, present_priority, smooth_buffer, + panel_hz, } = opts; boost_thread_priority(); let mode = client.mode(); @@ -387,9 +388,12 @@ pub(super) fn run_async( // lesson). A `None` from start (no choreographer surface) simply leaves ASAP targets. if had_output && vsync.is_none() { if let Some(tx) = vsync_tx.take() { - vsync = VsyncClock::start(Box::new(move || { - let _ = tx.send(DecodeEvent::Vsync); - })); + vsync = VsyncClock::start( + panel_hz, + Box::new(move || { + let _ = tx.send(DecodeEvent::Vsync); + }), + ); if vsync.is_none() { log::info!("decode: no choreographer clock — presenter uses ASAP targets"); } diff --git a/clients/android/native/src/decode/display.rs b/clients/android/native/src/decode/display.rs index adb83c70..b5df8b27 100644 --- a/clients/android/native/src/decode/display.rs +++ b/clients/android/native/src/decode/display.rs @@ -178,8 +178,12 @@ unsafe extern "C" fn on_frame_rendered( } } } - let display_us = paired.map(|(d, _)| ((displayed_ns - d).max(0) / 1000) as u64); - let latch_us = paired.map(|(_, r)| ((displayed_ns - r).max(0) / 1000) as u64); + // Clamped to (0, 10 s) like the e2e sample: a vendor's first render callbacks can carry a + // garbage `system_nano` (observed on-glass: an epoch-sized latch max on the session's first + // window), and one such sample would poison every max/percentile it lands in. + let clamp = |v: i128| (v > 0 && v < 10_000_000_000).then_some((v / 1000) as u64); + let display_us = paired.and_then(|(d, _)| clamp(displayed_ns - d)); + let latch_us = paired.and_then(|(_, r)| clamp(displayed_ns - r)); // Always-on half: the presenter's pf-present line reads these with the HUD off. t.meter.note_latch(latch_us); if !t.stats.enabled() { diff --git a/clients/android/native/src/decode/mod.rs b/clients/android/native/src/decode/mod.rs index 18d411d7..bf4d3d71 100644 --- a/clients/android/native/src/decode/mod.rs +++ b/clients/android/native/src/decode/mod.rs @@ -115,6 +115,10 @@ pub(crate) struct DecodeOptions { /// The smoothness buffer depth (`smooth_buffer` setting): 0 = automatic (2), else 1..=3. /// Only meaningful with `present_priority` = smooth. pub smooth_buffer: i32, + /// The display mode's own refresh rate (Kotlin's `display.refreshRate` at stream start; + /// 0 = unknown) — the latch grid the presenter subdivides onto when the app's choreographer + /// stream is down-rated below the panel (see `vsync.rs`). + pub panel_hz: i32, } /// The decode entry point on the `pf-decode` thread: dispatches to the async or synchronous loop. diff --git a/clients/android/native/src/decode/presenter.rs b/clients/android/native/src/decode/presenter.rs index 1766c8f6..d331c16b 100644 --- a/clients/android/native/src/decode/presenter.rs +++ b/clients/android/native/src/decode/presenter.rs @@ -340,10 +340,13 @@ impl Presenter { let (pace_p50, pace_max) = p50_max_ms(std::mem::take(&mut self.pace_us)); let (latch_p50, latch_max) = p50_max_ms(latch); let period_ms = clock.map(|c| c.period_ns() as f64 / 1e6).unwrap_or(0.0); + let panel_ms = clock + .map(|c| c.panel_period_ns() as f64 / 1e6) + .unwrap_or(0.0); log::info!( target: "pf.present", "released={} displays={} paced={} noBudget={} forced={} qDry={} \ - paceMs p50={:.2} max={:.2} latchMs p50={:.2} max={:.2} vsyncMs={:.2}", + paceMs p50={:.2} max={:.2} latchMs p50={:.2} max={:.2} vsyncMs={:.2} panelMs={:.2}", self.released, displays, self.paced_drops, @@ -355,6 +358,7 @@ impl Presenter { latch_p50, latch_max, period_ms, + panel_ms, ); self.released = 0; self.paced_drops = 0; diff --git a/clients/android/native/src/decode/sync_loop.rs b/clients/android/native/src/decode/sync_loop.rs index 870239e2..400054c4 100644 --- a/clients/android/native/src/decode/sync_loop.rs +++ b/clients/android/native/src/decode/sync_loop.rs @@ -47,6 +47,7 @@ pub(super) fn run_sync( // The timeline presenter lives in the async loop only; this loop IS the escape hatch. present_priority: _, smooth_buffer: _, + panel_hz: _, } = opts; boost_thread_priority(); let mode = client.mode(); diff --git a/clients/android/native/src/decode/vsync.rs b/clients/android/native/src/decode/vsync.rs index 23eb9b69..b1ce680d 100644 --- a/clients/android/native/src/decode/vsync.rs +++ b/clients/android/native/src/decode/vsync.rs @@ -51,7 +51,18 @@ pub(super) struct VsyncShared { /// The latest vsync callback's frame time (0 = no callback yet). last_vsync_ns: AtomicI64, /// Estimated vsync period (EMA over callback deltas / timeline spacing; 0 = unmeasured). + /// + /// ⚠ This is the APP's render rate, not necessarily the panel's: Android down-rates a + /// process's vsync stream (frame-rate categories / per-uid overrides), so a quiet UI can be + /// served 60 Hz callbacks while the panel scans at 120 (observed on-glass, A024). Pacing + /// video to THIS rate would cap the stream — hence `panel_period_ns` + the subdivision in + /// [`Self::next_target`]. period_ns: AtomicI64, + /// The panel's own refresh period (from the display mode Kotlin resolved at stream start; + /// 0 = unknown). The grid SurfaceFlinger actually latches on. + panel_period_ns: AtomicI64, + /// Callback count, for the one-shot cadence diagnostic log. + ticks: std::sync::atomic::AtomicU32, /// The latest callback's upcoming timelines, soonest first. Empty on the 31/32 fallback. timelines: Mutex>, } @@ -62,33 +73,60 @@ impl VsyncShared { self.period_ns.load(Ordering::Relaxed) } + /// The panel's own refresh period (0 = unknown) — for the pf-present line's decomposition. + pub(super) fn panel_period_ns(&self) -> i64 { + self.panel_period_ns.load(Ordering::Relaxed) + } + /// The release target for a frame submitted at `now`: the earliest stored timeline whose /// deadline is still `margin` away, extrapolated forward by whole periods once the stored /// set has aged out (timelines refresh once per vsync callback; a frame can decode anywhere /// inside that window). `None` on the 31/32 fallback — the caller releases ASAP. + /// + /// The picked target is then SUBDIVIDED onto the panel grid: the platform reports timelines + /// at the app's assigned render rate, but the panel latches at its own — when the app is + /// down-rated (60 Hz callbacks on a 120 Hz panel) the reported timelines are a whole panel + /// period apart or more, and pacing to them would cap the video. Pulling the target earlier + /// by whole panel periods (while its deadline still clears the margin) restores the true + /// grid; when callbacks run at the panel rate the pull condition is never true and this is + /// a no-op. pub(super) fn next_target(&self, now_ns: i64, margin_ns: i64) -> Option { - let g = self - .timelines - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - for t in g.iter() { - if t.deadline_ns > now_ns + margin_ns { - return Some(*t); + let mut t = { + let g = self + .timelines + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let found = g + .iter() + .find(|t| t.deadline_ns > now_ns + margin_ns) + .copied(); + match found { + Some(t) => t, + None => { + let last = g.last().copied()?; + let period = self.period_ns(); + if period <= 0 { + return None; + } + // All stored timelines have passed — step the last one forward whole + // periods until its deadline clears `now + margin` again. + let behind = (now_ns + margin_ns).saturating_sub(last.deadline_ns); + let k = behind / period + 1; + FrameTimeline { + expected_present_ns: last.expected_present_ns + k * period, + deadline_ns: last.deadline_ns + k * period, + } + } + } + }; + let panel = self.panel_period_ns.load(Ordering::Relaxed); + if panel > 0 { + while t.deadline_ns - panel > now_ns + margin_ns { + t.deadline_ns -= panel; + t.expected_present_ns -= panel; } } - let last = g.last().copied()?; - let period = self.period_ns(); - if period <= 0 { - return None; - } - // All stored timelines have passed — step the last one forward whole periods until its - // deadline clears `now + margin` again. - let behind = (now_ns + margin_ns).saturating_sub(last.deadline_ns); - let k = behind / period + 1; - Some(FrameTimeline { - expected_present_ns: last.expected_present_ns + k * period, - deadline_ns: last.deadline_ns + k * period, - }) + Some(t) } } @@ -194,6 +232,43 @@ impl CallbackCtx { .shared .last_vsync_ns .swap(frame_time_ns, Ordering::Relaxed); + // Panel-grid learner: timeline spacing is SurfaceFlinger's own grid, and the finest + // spacing ever observed is the panel's true period — trustworthy where the configured + // value is not (under a per-uid frame-rate override, `Display.getRefreshRate` REPORTS + // THE OVERRIDE, observed on-glass: a 120 Hz panel read back as 60 while early timelines + // ran at 8.28 ms). Corrects DOWNWARD only: subdividing onto a finer real grid is always + // valid, widening on a later down-rated window never is. + if timelines.len() >= 2 { + let spacing = timelines[1].expected_present_ns - timelines[0].expected_present_ns; + if (2_000_000..=42_000_000).contains(&spacing) { + let cur = self.shared.panel_period_ns.load(Ordering::Relaxed); + if cur == 0 || spacing < cur - 200_000 { + self.shared + .panel_period_ns + .store(spacing, Ordering::Relaxed); + } + } + } + // One-shot cadence diagnostic (3rd tick, once deltas exist): the callback cadence vs the + // panel period is exactly the down-rating question, and this line answers it on-glass. + if self.shared.ticks.fetch_add(1, Ordering::Relaxed) == 2 { + let spacing = if timelines.len() >= 2 { + timelines[1].expected_present_ns - timelines[0].expected_present_ns + } else { + 0 + }; + log::info!( + "vsync: cadence Δ={:.2}ms timelines={} spacing={:.2}ms panel={:.2}ms", + if prev > 0 { + (frame_time_ns - prev) as f64 / 1e6 + } else { + 0.0 + }, + timelines.len(), + spacing as f64 / 1e6, + self.shared.panel_period_ns.load(Ordering::Relaxed) as f64 / 1e6, + ); + } // Period: prefer timeline spacing (exact, straight from the platform), else the delta of // successive callbacks (jittery — EMA'd), clamped to sane panel rates (24..500 Hz). let mut period = 0i64; @@ -289,15 +364,23 @@ pub(super) struct VsyncClock { impl VsyncClock { /// Spawn the choreographer thread. `on_tick` fires once per vsync ON THAT THREAD — it must /// only do something cheap and `Send` (the decode loop passes an event-channel send). - /// `None` when the platform surface is missing (very old device) — the presenter then runs - /// clock-less (ASAP targets, predicted-latch budget). - pub(super) fn start(on_tick: Box) -> Option { + /// `panel_hz` is the display mode's own refresh rate (0 = unknown), the latch grid that + /// [`VsyncShared::next_target`] subdivides onto. `None` when the platform surface is missing + /// (very old device) — the presenter then runs clock-less (ASAP targets, predicted-latch + /// budget). + pub(super) fn start(panel_hz: i32, on_tick: Box) -> Option { let api = ChoreoApi::resolve()?; let timelines_live = api.post_vsync.is_some(); let shared = Arc::new(VsyncShared { stop: AtomicBool::new(false), last_vsync_ns: AtomicI64::new(0), period_ns: AtomicI64::new(0), + panel_period_ns: AtomicI64::new(if panel_hz > 0 { + 1_000_000_000 / panel_hz as i64 + } else { + 0 + }), + ticks: std::sync::atomic::AtomicU32::new(0), timelines: Mutex::new(Vec::new()), }); let thread_shared = shared.clone(); diff --git a/clients/android/native/src/session/planes.rs b/clients/android/native/src/session/planes.rs index 23d486ce..366dc17a 100644 --- a/clients/android/native/src/session/planes.rs +++ b/clients/android/native/src/session/planes.rs @@ -31,6 +31,7 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeStartVideo( is_tv: jboolean, present_priority: jni::sys::jint, smooth_buffer: jni::sys::jint, + panel_fps: jni::sys::jint, ) { use super::VideoThread; use std::sync::atomic::AtomicBool; @@ -76,6 +77,7 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeStartVideo( is_tv: is_tv != 0, present_priority, smooth_buffer, + panel_hz: panel_fps, }; let join = std::thread::Builder::new() .name("pf-decode".into())