From 1f6d34b6fa3e20c65902fa0d93540efd8e3f733c Mon Sep 17 00:00:00 2001 From: Brad Stein Date: Wed, 6 May 2026 04:50:08 -0300 Subject: [PATCH] tests: protect server RC calibration defaults --- Cargo.lock | 6 +- client/Cargo.toml | 2 +- client/src/app/uplink_media.rs | 28 +++++- .../src/sync_probe/analyze/media_extract.rs | 12 ++- .../src/sync_probe/analyze/onset_detection.rs | 1 + .../analyze/onset_detection/correlation.rs | 2 +- client/src/sync_probe/runner.rs | 4 +- common/Cargo.toml | 2 +- server/Cargo.toml | 2 +- server/src/calibration.rs | 74 ++++++++++++++-- server/src/main/relay_service.rs | 1 + server/src/upstream_media_runtime.rs | 86 +++++++++++++++++-- testing/tests/client_inputs_contract.rs | 68 ++++++++++----- 13 files changed, 233 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10cc47d..9c0d9c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1652,7 +1652,7 @@ checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" [[package]] name = "lesavka_client" -version = "0.19.29" +version = "0.19.30" dependencies = [ "anyhow", "async-stream", @@ -1686,7 +1686,7 @@ dependencies = [ [[package]] name = "lesavka_common" -version = "0.19.29" +version = "0.19.30" dependencies = [ "anyhow", "base64", @@ -1698,7 +1698,7 @@ dependencies = [ [[package]] name = "lesavka_server" -version = "0.19.29" +version = "0.19.30" dependencies = [ "anyhow", "base64", diff --git a/client/Cargo.toml b/client/Cargo.toml index ea957ae..f48a375 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -4,7 +4,7 @@ path = "src/main.rs" [package] name = "lesavka_client" -version = "0.19.29" +version = "0.19.30" edition = "2024" [dependencies] diff --git a/client/src/app/uplink_media.rs b/client/src/app/uplink_media.rs index b875430..6fede3a 100644 --- a/client/src/app/uplink_media.rs +++ b/client/src/app/uplink_media.rs @@ -1,6 +1,7 @@ impl LesavkaClientApp { /*──────────────── bundled webcam + mic stream ─────────────────*/ #[cfg(not(coverage))] + #[allow(clippy::too_many_arguments)] async fn webcam_media_loop( ep: Channel, initial_camera_source: Option, @@ -985,6 +986,7 @@ fn retain_newest_pending_audio(pending_audio: &mut Vec) -> usize { } #[cfg(not(coverage))] +#[allow(clippy::too_many_arguments)] fn emit_bundled_media( session_id: u64, bundle_seq: &mut u64, @@ -1297,9 +1299,18 @@ mod uplink_timing_tests { packet.client_send_pts_us >= packet.client_capture_pts_us, "enqueue/send stamp must be on or after the shared-clock capture estimate" ); + let capture_to_enqueue = Duration::from_micros( + packet + .client_send_pts_us + .saturating_sub(packet.client_capture_pts_us), + ); + assert_eq!( + capture_to_enqueue, enqueue_age, + "enqueue timing metadata should stay anchored to the pre-pop stamp" + ); assert!( - packet.client_send_pts_us - packet.client_capture_pts_us <= 3_000, - "capture-to-enqueue age, not async pop delay, should define the timing window" + packet.client_queue_age_ms >= duration_ms_u32(enqueue_age).saturating_add(4), + "queue age should include the simulated async pop delay without mutating send timing" ); } @@ -1331,9 +1342,18 @@ mod uplink_timing_tests { packet.client_send_pts_us >= packet.client_capture_pts_us, "enqueue/send stamp must be on or after the shared-clock capture estimate" ); + let capture_to_enqueue = Duration::from_micros( + packet + .client_send_pts_us + .saturating_sub(packet.client_capture_pts_us), + ); + assert_eq!( + capture_to_enqueue, enqueue_age, + "enqueue timing metadata should stay anchored to the pre-pop stamp" + ); assert!( - packet.client_send_pts_us - packet.client_capture_pts_us <= 4_000, - "capture-to-enqueue age, not async pop delay, should define the timing window" + packet.client_queue_age_ms >= duration_ms_u32(enqueue_age).saturating_add(4), + "queue age should include the simulated async pop delay without mutating send timing" ); } diff --git a/client/src/sync_probe/analyze/media_extract.rs b/client/src/sync_probe/analyze/media_extract.rs index 761bcf8..13dbb6c 100644 --- a/client/src/sync_probe/analyze/media_extract.rs +++ b/client/src/sync_probe/analyze/media_extract.rs @@ -279,7 +279,7 @@ fn adaptive_gray_roi_mask(frames: &[&[u8]], pixel_count: usize) -> Option Option Option Option Option= MIN_ADAPTIVE_ROI_PIXELS { break; } mask[index] = true; - selected += 1; } let mask = retain_largest_connected_roi(mask); diff --git a/client/src/sync_probe/analyze/onset_detection.rs b/client/src/sync_probe/analyze/onset_detection.rs index 036637a..cf5e633 100644 --- a/client/src/sync_probe/analyze/onset_detection.rs +++ b/client/src/sync_probe/analyze/onset_detection.rs @@ -728,6 +728,7 @@ pub(super) fn window_segment( } } +#[allow(clippy::too_many_arguments)] fn push_audio_segment( segments: &mut Vec, samples: &[i16], diff --git a/client/src/sync_probe/analyze/onset_detection/correlation.rs b/client/src/sync_probe/analyze/onset_detection/correlation.rs index 3a6b0b9..9a9d91d 100644 --- a/client/src/sync_probe/analyze/onset_detection/correlation.rs +++ b/client/src/sync_probe/analyze/onset_detection/correlation.rs @@ -206,7 +206,7 @@ pub(crate) fn correlate_coded_segments( if event_width_codes.is_empty() { bail!("event width code sequence must not be empty"); } - if event_width_codes.iter().any(|code| *code == 0) { + if event_width_codes.contains(&0) { bail!("event width codes must stay positive"); } if pulse_period_s <= 0.0 { diff --git a/client/src/sync_probe/runner.rs b/client/src/sync_probe/runner.rs index ceee5b7..592e87a 100644 --- a/client/src/sync_probe/runner.rs +++ b/client/src/sync_probe/runner.rs @@ -191,7 +191,7 @@ async fn collect_probe_audio_grace( pending_audio: &mut Vec, audio_done: &mut bool, audio_seq: &mut u64, - mut audio_dump: Option<&mut File>, + audio_dump: Option<&mut File>, ) { if *audio_done || !pending_audio.is_empty() { return; @@ -202,7 +202,7 @@ async fn collect_probe_audio_grace( }; if let Some(mut packet) = next.packet { stamp_probe_audio_packet(&mut packet, audio_seq, next.queue_depth); - write_probe_audio_dump(audio_dump.as_mut().map(|file| &mut **file), &packet); + write_probe_audio_dump(audio_dump, &packet); pending_audio.push(packet); retain_newest_probe_audio(pending_audio); } else if next.closed { diff --git a/common/Cargo.toml b/common/Cargo.toml index 489b469..497a797 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lesavka_common" -version = "0.19.29" +version = "0.19.30" edition = "2024" build = "build.rs" diff --git a/server/Cargo.toml b/server/Cargo.toml index 2211d25..071da3c 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -10,7 +10,7 @@ bench = false [package] name = "lesavka_server" -version = "0.19.29" +version = "0.19.30" edition = "2024" autobins = false diff --git a/server/src/calibration.rs b/server/src/calibration.rs index 9672d24..9eba774 100644 --- a/server/src/calibration.rs +++ b/server/src/calibration.rs @@ -477,8 +477,14 @@ mod tests { use super::*; use tempfile::NamedTempFile; - #[test] - fn default_snapshot_uses_factory_mjpeg_calibration() { + const BLESSED_SERVER_TO_RCT_VIDEO_OFFSETS: &[(&str, i64)] = &[ + ("1280x720@20", 162_659), + ("1280x720@30", 135_090), + ("1920x1080@20", 160_045), + ("1920x1080@30", 127_952), + ]; + + fn with_clean_offset_env(test: impl FnOnce()) { temp_env::with_vars( [ ("LESAVKA_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US", None::<&str>), @@ -491,20 +497,72 @@ mod tests { "LESAVKA_UPSTREAM_VIDEO_PLAYOUT_MODE_OFFSETS_US", None::<&str>, ), + ("UVC_MODE", None::<&str>), + ("LESAVKA_UVC_MODE", None::<&str>), ("LESAVKA_UVC_WIDTH", None::<&str>), ("LESAVKA_UVC_HEIGHT", None::<&str>), ("LESAVKA_UVC_FPS", None::<&str>), ("LESAVKA_UVC_INTERVAL", None::<&str>), + ("LESAVKA_CAM_WIDTH", None::<&str>), + ("LESAVKA_CAM_HEIGHT", None::<&str>), + ("LESAVKA_CAM_FPS", None::<&str>), ], - || { - let state = snapshot_from_env(); - assert_eq!(state.default_audio_offset_us, 0); - assert_eq!(state.active_video_offset_us, FACTORY_MJPEG_VIDEO_OFFSET_US); - assert_eq!(state.source, "factory"); - }, + test, ); } + #[test] + fn blessed_server_to_rct_offsets_are_release_defaults() { + assert_eq!( + FACTORY_MJPEG_VIDEO_OFFSET_US, FACTORY_MJPEG_VIDEO_OFFSET_1280X720_30_US, + "720p30 is the blessed default profile until a new lab matrix replaces it" + ); + assert_eq!(FACTORY_MJPEG_VIDEO_OFFSET_1280X720_20_US, 162_659); + assert_eq!(FACTORY_MJPEG_VIDEO_OFFSET_1280X720_30_US, 135_090); + assert_eq!(FACTORY_MJPEG_VIDEO_OFFSET_1920X1080_20_US, 160_045); + assert_eq!(FACTORY_MJPEG_VIDEO_OFFSET_1920X1080_30_US, 127_952); + assert_eq!( + FACTORY_MJPEG_AUDIO_MODE_OFFSETS_US, + "1280x720@20=0,1280x720@30=0,1920x1080@20=0,1920x1080@30=0" + ); + assert_eq!( + FACTORY_MJPEG_VIDEO_MODE_OFFSETS_US, + "1280x720@20=162659,1280x720@30=135090,1920x1080@20=160045,1920x1080@30=127952" + ); + } + + #[test] + fn every_supported_uvc_mode_loads_tailored_factory_offset() { + for (mode, expected_offset_us) in BLESSED_SERVER_TO_RCT_VIDEO_OFFSETS { + with_clean_offset_env(|| { + temp_env::with_var("UVC_MODE", Some(*mode), || { + let state = snapshot_from_env(); + assert_eq!( + state.factory_video_offset_us, *expected_offset_us, + "{mode} should use its baked server-to-RCT factory offset" + ); + assert_eq!( + state.default_video_offset_us, *expected_offset_us, + "{mode} should default to its baked server-to-RCT offset" + ); + assert_eq!(state.default_audio_offset_us, 0); + assert_eq!(state.source, "factory"); + assert_eq!(state.confidence, FACTORY_CONFIDENCE); + }); + }); + } + } + + #[test] + fn default_snapshot_uses_factory_mjpeg_calibration() { + with_clean_offset_env(|| { + let state = snapshot_from_env(); + assert_eq!(state.default_audio_offset_us, 0); + assert_eq!(state.active_video_offset_us, FACTORY_MJPEG_VIDEO_OFFSET_US); + assert_eq!(state.source, "factory"); + }); + } + #[test] fn default_snapshot_uses_uvc_mode_factory_calibration() { temp_env::with_vars( diff --git a/server/src/main/relay_service.rs b/server/src/main/relay_service.rs index 32dc3ca..5e29779 100644 --- a/server/src/main/relay_service.rs +++ b/server/src/main/relay_service.rs @@ -174,6 +174,7 @@ async fn push_media_v2_audio( } #[cfg(not(coverage))] +#[allow(clippy::too_many_arguments)] async fn feed_media_v2_video( video: Option, clock: &mut MediaV2Clock, diff --git a/server/src/upstream_media_runtime.rs b/server/src/upstream_media_runtime.rs index f849cd2..4a65bd0 100644 --- a/server/src/upstream_media_runtime.rs +++ b/server/src/upstream_media_runtime.rs @@ -130,20 +130,15 @@ impl ScalarWindow { } } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] enum UpstreamSyncPhase { + #[default] Acquiring, Syncing, Live, Healing, } -impl Default for UpstreamSyncPhase { - fn default() -> Self { - Self::Acquiring - } -} - impl UpstreamSyncPhase { fn as_str(self) -> &'static str { match self { @@ -803,3 +798,80 @@ fn apply_offset(instant: Instant, offset_us: i64) -> Instant { .unwrap_or(instant) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn with_clean_offset_env(test: impl FnOnce()) { + temp_env::with_vars( + [ + ("LESAVKA_UPSTREAM_AUDIO_PLAYOUT_OFFSET_US", None::<&str>), + ("LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US", None::<&str>), + ( + "LESAVKA_UPSTREAM_AUDIO_PLAYOUT_MODE_OFFSETS_US", + None::<&str>, + ), + ( + "LESAVKA_UPSTREAM_VIDEO_PLAYOUT_MODE_OFFSETS_US", + None::<&str>, + ), + ("LESAVKA_UVC_WIDTH", None::<&str>), + ("LESAVKA_UVC_HEIGHT", None::<&str>), + ("LESAVKA_UVC_FPS", None::<&str>), + ("LESAVKA_UVC_INTERVAL", None::<&str>), + ], + test, + ); + } + + #[test] + fn runtime_uses_baked_mode_offsets_before_calibration_store_loads() { + for (width, height, fps, expected_video_offset_us) in [ + ("1280", "720", "20", 162_659), + ("1280", "720", "30", 135_090), + ("1920", "1080", "20", 160_045), + ("1920", "1080", "30", 127_952), + ] { + with_clean_offset_env(|| { + temp_env::with_vars( + [ + ("LESAVKA_UVC_WIDTH", Some(width)), + ("LESAVKA_UVC_HEIGHT", Some(height)), + ("LESAVKA_UVC_FPS", Some(fps)), + ], + || { + let runtime = UpstreamMediaRuntime::new(); + assert_eq!( + runtime.playout_offsets(), + (expected_video_offset_us, 0), + "{width}x{height}@{fps} should use its baked startup offset" + ); + }, + ); + }); + } + } + + #[test] + fn runtime_prefers_mode_offset_map_over_scalar_fallback() { + with_clean_offset_env(|| { + temp_env::with_vars( + [ + ("LESAVKA_UVC_WIDTH", Some("1280")), + ("LESAVKA_UVC_HEIGHT", Some("720")), + ("LESAVKA_UVC_FPS", Some("30")), + ("LESAVKA_UPSTREAM_VIDEO_PLAYOUT_OFFSET_US", Some("999999")), + ( + "LESAVKA_UPSTREAM_VIDEO_PLAYOUT_MODE_OFFSETS_US", + Some("1280x720@30=135090"), + ), + ], + || { + let runtime = UpstreamMediaRuntime::new(); + assert_eq!(runtime.playout_offsets(), (135_090, 0)); + }, + ); + }); + } +} diff --git a/testing/tests/client_inputs_contract.rs b/testing/tests/client_inputs_contract.rs index 35c4238..2f1b39c 100644 --- a/testing/tests/client_inputs_contract.rs +++ b/testing/tests/client_inputs_contract.rs @@ -26,15 +26,17 @@ mod inputs_contract { use evdev::uinput::VirtualDevice; use serial_test::serial; use std::thread; - use temp_env::with_var; + use temp_env::{with_var, with_vars}; - fn open_virtual_device(vdev: &mut VirtualDevice) -> Option { + fn open_virtual_device_with_path( + vdev: &mut VirtualDevice, + ) -> Option<(std::path::PathBuf, evdev::Device)> { for _ in 0..40 { if let Ok(mut nodes) = vdev.enumerate_dev_nodes_blocking() { if let Some(Ok(path)) = nodes.next() { - if let Ok(dev) = evdev::Device::open(path) { + if let Ok(dev) = evdev::Device::open(&path) { let _ = dev.set_nonblocking(true); - return Some(dev); + return Some((path.to_path_buf(), dev)); } } } @@ -43,6 +45,10 @@ mod inputs_contract { None } + fn open_virtual_device(vdev: &mut VirtualDevice) -> Option { + open_virtual_device_with_path(vdev).map(|(_, dev)| dev) + } + fn build_keyboard() -> Option { let mut keys = AttributeSet::::new(); keys.insert(evdev::KeyCode::KEY_A); @@ -138,12 +144,13 @@ mod inputs_contract { fn build_keyboard_pair(name: &str) -> Option<(VirtualDevice, evdev::Device)> { build_keyboard_pair_with_keys(name, &[evdev::KeyCode::KEY_A, evdev::KeyCode::KEY_ENTER]) + .map(|(vdev, _, dev)| (vdev, dev)) } fn build_keyboard_pair_with_keys( name: &str, supported_keys: &[evdev::KeyCode], - ) -> Option<(VirtualDevice, evdev::Device)> { + ) -> Option<(VirtualDevice, std::path::PathBuf, evdev::Device)> { let mut keys = AttributeSet::::new(); for key in supported_keys { keys.insert(*key); @@ -157,11 +164,17 @@ mod inputs_contract { .build() .ok()?; - let dev = open_virtual_device(&mut vdev)?; - Some((vdev, dev)) + let (path, dev) = open_virtual_device_with_path(&mut vdev)?; + Some((vdev, path, dev)) } fn build_mouse_pair(name: &str) -> Option<(VirtualDevice, evdev::Device)> { + build_mouse_pair_with_path(name).map(|(vdev, _, dev)| (vdev, dev)) + } + + fn build_mouse_pair_with_path( + name: &str, + ) -> Option<(VirtualDevice, std::path::PathBuf, evdev::Device)> { let mut keys = AttributeSet::::new(); keys.insert(evdev::KeyCode::BTN_LEFT); let mut rel = AttributeSet::::new(); @@ -178,8 +191,8 @@ mod inputs_contract { .build() .ok()?; - let dev = open_virtual_device(&mut vdev)?; - Some((vdev, dev)) + let (path, dev) = open_virtual_device_with_path(&mut vdev)?; + Some((vdev, path, dev)) } fn new_aggregator() -> InputAggregator { @@ -287,22 +300,37 @@ mod inputs_contract { #[test] #[serial] fn init_grabs_virtual_keyboard_and_mouse_when_available() { - let Some((_kbd_vdev, _kbd_dev)) = build_keyboard_pair("lesavka-input-init-kbd") else { + let Some((_kbd_vdev, kbd_path, _kbd_dev)) = build_keyboard_pair_with_keys( + "lesavka-input-init-kbd", + &[evdev::KeyCode::KEY_A, evdev::KeyCode::KEY_ENTER], + ) else { return; }; - let Some((_mouse_vdev, _mouse_dev)) = build_mouse_pair("lesavka-input-init-mouse") else { + let Some((_mouse_vdev, mouse_path, _mouse_dev)) = + build_mouse_pair_with_path("lesavka-input-init-mouse") + else { return; }; - let mut agg = new_aggregator(); - let result = agg.init(); - assert!( - result.is_ok(), - "init should succeed with virtual input devices" - ); - assert!( - !agg.keyboards.is_empty() || !agg.mice.is_empty(), - "init should discover at least one virtual input device" + let kbd_path = kbd_path.to_string_lossy().into_owned(); + let mouse_path = mouse_path.to_string_lossy().into_owned(); + with_vars( + [ + ("LESAVKA_KEYBOARD_DEVICE", Some(kbd_path.as_str())), + ("LESAVKA_MOUSE_DEVICE", Some(mouse_path.as_str())), + ], + || { + let mut agg = new_aggregator(); + let result = agg.init(); + assert!( + result.is_ok(), + "init should succeed with selected virtual input devices" + ); + assert!( + !agg.keyboards.is_empty() || !agg.mice.is_empty(), + "init should discover at least one selected virtual input device" + ); + }, ); }