From 00cc3c9accc353efbeefc23695873ab68314f3eb Mon Sep 17 00:00:00 2001 From: Brad Stein Date: Sat, 2 May 2026 01:00:57 -0300 Subject: [PATCH] media: remove stale upstream video delay --- AGENTS.md | 1 + Cargo.lock | 6 +- client/Cargo.toml | 2 +- common/Cargo.toml | 2 +- scripts/install/server.sh | 26 ++++++- server/Cargo.toml | 2 +- server/src/calibration.rs | 67 ++++++++++++++----- server/src/upstream_media_runtime.rs | 24 +++++-- .../upstream_media_runtime/tests/config.rs | 2 +- .../upstream_media_runtime/tests/planning.rs | 27 ++++++++ .../tests/server_install_script_contract.rs | 18 +++-- 11 files changed, 143 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 98c94dd..c1238bd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -225,3 +225,4 @@ Context: 0.16.x proved that queue tweaks and static calibration cannot guarantee - 2026-05-01: Patch follow-up models the observed MJPEG/UVC browser egress delta by defaulting video playout offset to `+350ms` and preserving the 1s freshness ceiling. Raw activity-start evidence is now ignored for verdict/calibration when it disagrees with paired pulses that are already failing directly. Existing early-0.17 `audio=0/video=0` factory/env calibration files migrate to the new `video=+350ms` default on load. - 2026-05-01: Release identity cleanup: bumped the patched build to clean semver `0.17.1`; probe attribution now prints `client_version`/`server_version` separately from `client_revision`/`server_revision` and refuses old `client_full_version` output. - 2026-05-01: 0.17.1 mirrored probe failed with video about `1.18-1.31s` behind audio and 761 planner video freezes. Root cause candidate: the client rebaser forced independent camera/mic pipelines onto one first-packet capture base, so a later-starting camera path was timestamped too early and looked permanently behind audio. Patch 0.17.2 anchors each stream to the shared monotonic clock at its own first packet time. +- 2026-05-02: 0.17.2 mirrored probe and Google Meet test showed major improvement but persistent sub-second late video. Root cause follow-up: the temporary `+350ms` factory MJPEG video playout offset matched the observed browser skew and also made the server skew guard freeze video against its own offset. Patch 0.17.3 restores factory video offset to `0ms`, migrates untouched `+350ms` install/calibration defaults back to `0ms`, and makes the skew guard offset-aware for intentional site calibration. diff --git a/Cargo.lock b/Cargo.lock index c8ad9e1..bf80114 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1652,7 +1652,7 @@ checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" [[package]] name = "lesavka_client" -version = "0.17.2" +version = "0.17.3" dependencies = [ "anyhow", "async-stream", @@ -1686,7 +1686,7 @@ dependencies = [ [[package]] name = "lesavka_common" -version = "0.17.2" +version = "0.17.3" dependencies = [ "anyhow", "base64", @@ -1698,7 +1698,7 @@ dependencies = [ [[package]] name = "lesavka_server" -version = "0.17.2" +version = "0.17.3" dependencies = [ "anyhow", "base64", diff --git a/client/Cargo.toml b/client/Cargo.toml index 1b2a57c..7060715 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -4,7 +4,7 @@ path = "src/main.rs" [package] name = "lesavka_client" -version = "0.17.2" +version = "0.17.3" edition = "2024" [dependencies] diff --git a/common/Cargo.toml b/common/Cargo.toml index 01fd51e..2aa6848 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lesavka_common" -version = "0.17.2" +version = "0.17.3" edition = "2024" build = "build.rs" diff --git a/scripts/install/server.sh b/scripts/install/server.sh index bbf0bea..32b24e0 100755 --- a/scripts/install/server.sh +++ b/scripts/install/server.sh @@ -15,10 +15,11 @@ INSTALL_SERVER_BIND_ADDR=${LESAVKA_INSTALL_SERVER_BIND_ADDR:-0.0.0.0:50051} LESAVKA_TLS_DIR=${LESAVKA_TLS_DIR:-/etc/lesavka/pki} LESAVKA_CLIENT_BUNDLE=${LESAVKA_CLIENT_BUNDLE:-/etc/lesavka/lesavka-client-pki.tar.gz} DEFAULT_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=0 -DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=350000 +DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=0 LEGACY_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=-45000 PREVIOUS_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=720000 PREVIOUS_TUNED_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=1260000 +PREVIOUS_DELAYED_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=350000 resolve_upstream_audio_playout_offset_us() { if [[ -n ${LESAVKA_INSTALL_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US:-} ]]; then @@ -41,6 +42,27 @@ resolve_upstream_audio_playout_offset_us() { printf '%s\n' "$DEFAULT_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US" } +resolve_upstream_video_playout_offset_us() { + if [[ -n ${LESAVKA_INSTALL_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US:-} ]]; then + printf '%s\n' "$LESAVKA_INSTALL_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US" + return 0 + fi + + if [[ ${LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US:-} == "$PREVIOUS_DELAYED_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US" ]]; then + echo "⚠️ migrating stale upstream video playout offset to the 0.17 freshness-first planner default." >&2 + echo " Use LESAVKA_INSTALL_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US to intentionally keep an older value." >&2 + printf '%s\n' "$DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US" + return 0 + fi + + if [[ -n ${LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US:-} ]]; then + printf '%s\n' "$LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US" + return 0 + fi + + printf '%s\n' "$DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US" +} + manifest_package_version() { local manifest=$1 [[ -f $manifest ]] || return 1 @@ -980,7 +1002,7 @@ fi printf 'LESAVKA_UPSTREAM_MAX_LIVE_LAG_MS=%s\n' "${LESAVKA_UPSTREAM_MAX_LIVE_LAG_MS:-1000}" printf 'LESAVKA_UPSTREAM_STARTUP_TIMEOUT_MS=%s\n' "${LESAVKA_UPSTREAM_STARTUP_TIMEOUT_MS:-60000}" printf 'LESAVKA_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=%s\n' "$(resolve_upstream_audio_playout_offset_us)" - printf 'LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=%s\n' "${LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US:-$DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US}" + printf 'LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=%s\n' "$(resolve_upstream_video_playout_offset_us)" printf 'LESAVKA_UPSTREAM_PAIR_SLACK_US=%s\n' "${LESAVKA_UPSTREAM_PAIR_SLACK_US:-80000}" printf 'LESAVKA_UPSTREAM_STALE_DROP_MS=%s\n' "${LESAVKA_UPSTREAM_STALE_DROP_MS:-80}" printf 'LESAVKA_SERVER_BIND_ADDR=%s\n' "${INSTALL_SERVER_BIND_ADDR}" diff --git a/server/Cargo.toml b/server/Cargo.toml index dfffdcc..5fdc735 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -10,7 +10,7 @@ bench = false [package] name = "lesavka_server" -version = "0.17.2" +version = "0.17.3" edition = "2024" autobins = false diff --git a/server/src/calibration.rs b/server/src/calibration.rs index 12e774d..d25531f 100644 --- a/server/src/calibration.rs +++ b/server/src/calibration.rs @@ -10,11 +10,12 @@ use lesavka_common::lesavka::{ use crate::upstream_media_runtime::UpstreamMediaRuntime; pub const FACTORY_MJPEG_AUDIO_OFFSET_US: i64 = 0; -pub const FACTORY_MJPEG_VIDEO_OFFSET_US: i64 = 350_000; +pub const FACTORY_MJPEG_VIDEO_OFFSET_US: i64 = 0; const LEGACY_FACTORY_MJPEG_AUDIO_OFFSET_US: i64 = -45_000; const PREVIOUS_FACTORY_MJPEG_AUDIO_OFFSET_US: i64 = 720_000; const PREVIOUS_TUNED_MJPEG_AUDIO_OFFSET_US: i64 = 1_260_000; const PREVIOUS_FACTORY_MJPEG_VIDEO_OFFSET_US: i64 = 0; +const PREVIOUS_DELAYED_FACTORY_MJPEG_VIDEO_OFFSET_US: i64 = 350_000; const PROFILE: &str = "mjpeg"; const FACTORY_CONFIDENCE: &str = "factory"; const OFFSET_LIMIT_US: i64 = 500_000; @@ -251,9 +252,10 @@ fn migrate_legacy_snapshot(mut state: CalibrationSnapshot) -> CalibrationSnapsho | PREVIOUS_TUNED_MJPEG_AUDIO_OFFSET_US ) || clamped_previous_baseline) && state.active_audio_offset_us == state.default_audio_offset_us; - let untouched_legacy_video = state.default_video_offset_us - == PREVIOUS_FACTORY_MJPEG_VIDEO_OFFSET_US - && state.active_video_offset_us == PREVIOUS_FACTORY_MJPEG_VIDEO_OFFSET_US; + let untouched_legacy_video = matches!( + state.default_video_offset_us, + PREVIOUS_FACTORY_MJPEG_VIDEO_OFFSET_US | PREVIOUS_DELAYED_FACTORY_MJPEG_VIDEO_OFFSET_US + ) && state.active_video_offset_us == state.default_video_offset_us; if state.profile == PROFILE && source_allows_migration && confidence_allows_migration @@ -338,7 +340,7 @@ mod tests { || { let state = snapshot_from_env(); assert_eq!(state.default_audio_offset_us, 0); - assert_eq!(state.active_video_offset_us, 350_000); + assert_eq!(state.active_video_offset_us, 0); assert_eq!(state.source, "factory"); }, ); @@ -362,7 +364,7 @@ mod tests { }) .expect("manual adjust applies"); assert_eq!(state.active_audio_offset_us, -5_000); - assert_eq!(runtime.playout_offsets(), (350_000, -5_000)); + assert_eq!(runtime.playout_offsets(), (0, -5_000)); let raw = std::fs::read_to_string(file.path()).expect("persisted calibration"); assert!(raw.contains("active_audio_offset_us=-5000")); }); @@ -447,10 +449,10 @@ mod tests { let state = store.current(); assert_eq!(state.active_audio_offset_us, 0); assert_eq!(state.default_audio_offset_us, 0); - assert_eq!(state.active_video_offset_us, 350_000); - assert_eq!(state.default_video_offset_us, 350_000); + assert_eq!(state.active_video_offset_us, 0); + assert_eq!(state.default_video_offset_us, 0); assert_eq!(state.source, "factory"); - assert_eq!(runtime.playout_offsets(), (350_000, 0)); + assert_eq!(runtime.playout_offsets(), (0, 0)); assert!(state.detail.contains("migrated legacy MJPEG")); }); } @@ -479,11 +481,42 @@ mod tests { let state = store.current(); assert_eq!(state.active_audio_offset_us, 0); assert_eq!(state.default_audio_offset_us, 0); - assert_eq!(state.active_video_offset_us, 350_000); - assert_eq!(state.default_video_offset_us, 350_000); + assert_eq!(state.active_video_offset_us, 0); + assert_eq!(state.default_video_offset_us, 0); assert_eq!(state.source, "factory"); - assert_eq!(runtime.playout_offsets(), (350_000, 0)); - assert!(state.detail.contains("to audio +0.0ms/video +350.0ms")); + assert_eq!(runtime.playout_offsets(), (0, 0)); + assert!(state.detail.contains("to audio +0.0ms/video +0.0ms")); + }); + } + + #[test] + fn load_migrates_delayed_video_factory_mjpeg_baseline() { + let file = NamedTempFile::new().expect("temp calibration file"); + std::fs::write( + file.path(), + r#" + profile="mjpeg" + default_audio_offset_us=0 + default_video_offset_us=350000 + active_audio_offset_us=0 + active_video_offset_us=350000 + source="factory" + confidence="factory" + detail="loaded upstream A/V calibration defaults" + "#, + ) + .expect("delayed video calibration seed"); + let path = file.path().to_string_lossy().to_string(); + temp_env::with_var("LESAVKA_CALIBRATION_PATH", Some(path.as_str()), || { + let runtime = Arc::new(UpstreamMediaRuntime::new()); + let store = CalibrationStore::load(runtime.clone()); + let state = store.current(); + assert_eq!(state.active_audio_offset_us, 0); + assert_eq!(state.default_audio_offset_us, 0); + assert_eq!(state.active_video_offset_us, 0); + assert_eq!(state.default_video_offset_us, 0); + assert_eq!(state.source, "factory"); + assert_eq!(runtime.playout_offsets(), (0, 0)); }); } @@ -511,10 +544,10 @@ mod tests { let state = store.current(); assert_eq!(state.active_audio_offset_us, 0); assert_eq!(state.default_audio_offset_us, 0); - assert_eq!(state.active_video_offset_us, 350_000); - assert_eq!(state.default_video_offset_us, 350_000); + assert_eq!(state.active_video_offset_us, 0); + assert_eq!(state.default_video_offset_us, 0); assert_eq!(state.source, "factory"); - assert_eq!(runtime.playout_offsets(), (350_000, 0)); + assert_eq!(runtime.playout_offsets(), (0, 0)); }); } @@ -571,7 +604,7 @@ mod tests { .expect("blind estimate"); assert_eq!(blind.source, "blind"); assert!(blind.detail.contains("delivery skew 44.0ms")); - assert_eq!(runtime.playout_offsets(), (348_000, 5_000)); + assert_eq!(runtime.playout_offsets(), (-2_000, 5_000)); let manual = store .apply(CalibrationRequest { diff --git a/server/src/upstream_media_runtime.rs b/server/src/upstream_media_runtime.rs index c32b8be..1cd4fc4 100644 --- a/server/src/upstream_media_runtime.rs +++ b/server/src/upstream_media_runtime.rs @@ -96,6 +96,12 @@ impl UpstreamMediaRuntime { microphone_offset_us.saturating_sub(camera_offset_us).max(0) as u64 } + fn audio_ahead_video_allowance_us(&self) -> u64 { + let camera_offset_us = self.camera_playout_offset_us.load(Ordering::Relaxed); + let microphone_offset_us = self.microphone_playout_offset_us.load(Ordering::Relaxed); + camera_offset_us.saturating_sub(microphone_offset_us).max(0) as u64 + } + /// Mark one audio chunk as actually handed to the UAC sink. pub fn mark_audio_presented(&self, local_pts_us: u64) { let mut state = self @@ -455,9 +461,14 @@ impl UpstreamMediaRuntime { local_pts_us = last_pts_us.saturating_add(min_step_us.max(1)); } *last_slot = Some(local_pts_us); + let audio_ahead_video_allowance_us = self.audio_ahead_video_allowance_us(); if kind == UpstreamMediaKind::Camera && state.last_audio_local_pts_us.is_some_and(|audio_pts_us| { - video_is_too_far_behind_audio(local_pts_us, audio_pts_us) + video_is_too_far_behind_audio( + local_pts_us, + audio_pts_us, + audio_ahead_video_allowance_us, + ) }) { state.skew_video_drops = state.skew_video_drops.saturating_add(1); @@ -587,10 +598,15 @@ fn source_lag_for_kind( Duration::from_micros(latest.saturating_sub(remote_pts_us)) } -fn video_is_too_far_behind_audio(video_pts_us: u64, audio_pts_us: u64) -> bool { - let slack_us = upstream_pairing_master_slack() +fn video_is_too_far_behind_audio( + video_pts_us: u64, + audio_pts_us: u64, + audio_ahead_video_allowance_us: u64, +) -> bool { + let slack_us = (upstream_pairing_master_slack() .as_micros() - .min(u64::MAX as u128) as u64; + .min(u64::MAX as u128) as u64) + .saturating_add(audio_ahead_video_allowance_us); video_pts_us.saturating_add(slack_us) < audio_pts_us } diff --git a/server/src/upstream_media_runtime/tests/config.rs b/server/src/upstream_media_runtime/tests/config.rs index 7374124..eb2388e 100644 --- a/server/src/upstream_media_runtime/tests/config.rs +++ b/server/src/upstream_media_runtime/tests/config.rs @@ -74,7 +74,7 @@ fn upstream_playout_offsets_default_to_mjpeg_calibration_and_accept_overrides() ); assert_eq!( super::upstream_playout_offset_us(UpstreamMediaKind::Camera), - 350_000 + 0 ); }); }); diff --git a/server/src/upstream_media_runtime/tests/planning.rs b/server/src/upstream_media_runtime/tests/planning.rs index 242a736..58d5b2b 100644 --- a/server/src/upstream_media_runtime/tests/planning.rs +++ b/server/src/upstream_media_runtime/tests/planning.rs @@ -360,6 +360,33 @@ fn video_too_far_behind_audio_master_is_dropped_and_counted_as_freeze() { }); } +#[test] +#[serial(upstream_media_runtime)] +fn configured_video_delay_does_not_make_the_planner_freeze_video() { + temp_env::with_var("LESAVKA_UPSTREAM_PLAYOUT_DELAY_MS", Some("0"), || { + temp_env::with_var("LESAVKA_UPSTREAM_PAIR_SLACK_US", Some("50000"), || { + let runtime = UpstreamMediaRuntime::new(); + runtime.set_playout_offsets(350_000, 0); + let _camera = runtime.activate_camera(); + let _microphone = runtime.activate_microphone(); + + assert!(matches!( + runtime.plan_video_pts(1_000_000, 16_666), + super::UpstreamPlanDecision::AwaitingPair + )); + let _audio = play(runtime.plan_audio_pts(1_000_000)); + let _video = play(runtime.plan_video_pts(1_000_000, 16_666)); + let _audio_master = play(runtime.plan_audio_pts(1_300_000)); + + let video = play(runtime.plan_video_pts(1_100_000, 16_666)); + assert_eq!(video.local_pts_us, 100_000); + let snapshot = runtime.snapshot(); + assert_eq!(snapshot.skew_video_drops, 0); + assert_eq!(snapshot.video_freezes, 0); + }); + }); +} + #[test] #[serial(upstream_media_runtime)] fn paired_startup_times_out_instead_of_waiting_forever() { diff --git a/testing/tests/server_install_script_contract.rs b/testing/tests/server_install_script_contract.rs index c979f93..6bff9bd 100644 --- a/testing/tests/server_install_script_contract.rs +++ b/testing/tests/server_install_script_contract.rs @@ -55,24 +55,34 @@ fn server_install_pins_hdmi_camera_and_display_defaults() { assert!(SERVER_INSTALL.contains("${LESAVKA_UPSTREAM_MAX_LIVE_LAG_MS:-1000}")); assert!(SERVER_INSTALL.contains("${LESAVKA_UPSTREAM_STARTUP_TIMEOUT_MS:-60000}")); assert!(SERVER_INSTALL.contains("DEFAULT_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=0")); - assert!(SERVER_INSTALL.contains("DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=350000")); + assert!(SERVER_INSTALL.contains("DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=0")); assert!( - SERVER_INSTALL.contains( - "${LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US:-$DEFAULT_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US}" - ) + SERVER_INSTALL.contains("resolve_upstream_video_playout_offset_us"), + "video offset should be resolved through stale-baseline migration logic" ); assert!(SERVER_INSTALL.contains("LEGACY_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=-45000")); assert!( SERVER_INSTALL.contains("PREVIOUS_TUNED_MJPEG_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US=1260000") ); + assert!( + SERVER_INSTALL.contains("PREVIOUS_DELAYED_MJPEG_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US=350000") + ); assert!( SERVER_INSTALL.contains("LESAVKA_INSTALL_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US"), "install-specific offset override should bypass stale ambient runtime env" ); + assert!( + SERVER_INSTALL.contains("LESAVKA_INSTALL_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US"), + "install-specific video offset override should bypass stale ambient runtime env" + ); assert!( SERVER_INSTALL.contains("migrating stale upstream audio playout offset to the 0.17 freshness-first planner default"), "installer should not preserve old MJPEG/UVC sync baselines accidentally" ); + assert!( + SERVER_INSTALL.contains("migrating stale upstream video playout offset to the 0.17 freshness-first planner default"), + "installer should not preserve old video delay baselines accidentally" + ); assert!(SERVER_INSTALL.contains("${LESAVKA_UPSTREAM_PAIR_SLACK_US:-80000}")); assert!(SERVER_INSTALL.contains("${LESAVKA_UPSTREAM_STALE_DROP_MS:-80}")); assert!(SERVER_INSTALL.contains("${LESAVKA_INSTALL_SERVER_BIND_ADDR:-0.0.0.0:50051}"));