fix(webrtc): switch encoder time_base to 90kHz to stop RTP time inflation (closes #25)
Root cause: After #24 fixed PTS propagation, browser jitter buffer still accumulated to 10+ seconds during active mouse movement. User reported: stop moving mouse, client continues showing motion for ~10 seconds. The encoder time_base was 1/fps (33ms granularity at 30fps). When KWin delivers frames at 60fps (16.7ms apart), compute_capture_pts integer math mapped multiple captures to the same tick. The monotonicity guard then bumped them to sequential ticks (0, 1, 2, 3, ...). Result: 60 captures in 1 real second produced 60 sequential RTP timestamps spanning 60 * 33ms = 1.98 seconds of RTP time. Browser played at RTP rate (half real speed), buffer accumulated. Math verification from test8 log: - 2067 frames * 3000 RTP jumps = 68s of active RTP time - 61 frames * 54000 RTP jumps = 37s of static RTP time - Total RTP time 105s vs real time 98.6s (7% inflation in short session; long active sessions amplify to 50%+ inflation matching user-reported 10-second trailing). Fix (per Oracle round review): Change WebRTC encoder time_base from 1/fps to 1/90000 (90kHz). This matches the RTP video clock directly, providing 11us PTS granularity. Captures 16.7ms apart now produce distinct ticks (~1500 each), no quantization, RTP timestamps accurately reflect real time. Oracle-required revisions incorporated: 1. **set_frame_rate alongside time_base** — libx264 infers fps from time_base when not explicit. With 1/90000 time_base and no explicit framerate, x264 would assume ~90000fps and VBV rate control would break. Setting framerate=fps/1 preserves real frame semantics while using 90kHz PTS precision. 2. **rtp_timestamp_from_pts_ticks returns u64 not u32** — MediaTime::new takes u64. Returning u32 would truncate at 13.25 hours and create backwards MediaTime. str0m handles RTP u32 wrap internally; we feed it full u64. 3. **wlr-screencopy path also updated** — state.rs:605 used fps-based PTS formula. Changed to 90kHz ticks so wlr path matches Portal path unit. Without this, wlr-screencopy users would have wrong PTS after the time_base change. 4. **MP4 path (create_software_h264_muxer) UNCHANGED** — verified at avhw.rs:1693-1789, keeps 1/fps time_base, no set_frame_rate added. File output doesn't need real-time PTS. Implementation: - src/avhw.rs: WEBRTC_RTP_CLOCK_HZ=90_000 const; create_software_h264_encoder uses 1/90000 time_base + explicit framerate - src/state_portal.rs: compute_capture_pts uses WEBRTC_RTP_CLOCK_HZ for tick conversion (was fps multiplier) - src/state.rs: wlr PTS formula uses 90_000 (was fps multiplier) - src/webrtc.rs: rtp_timestamp_from_pts_ticks simplified to identity function (pts_ticks.max(0) as u64), drop fps parameter; write_h264_frame signature drops fps (was only used for rtp conversion); 5 unit tests updated to assert 90kHz identity (0->0, 1500->1500, 90000->90000) Verification expectations: - Active period jitterBufferDelay: 1000+ ms -> < 100 ms - 'Stop mouse, client continues 10s' symptom: should disappear - Static period behavior: unchanged (was already correct) - MP4 file output: unchanged - VBV-constrained IDR sizes: unchanged (framerate explicit preserves rate control semantics) - All prior fixes (#19, #23, #15, #18, #24) preserved Out of scope (Oracle noted, not blocking): - build_swenc_filter_graph still uses 1/fps time_base at avhw.rs:1603/1620 (semantic mismatch but no functional impact since scale_vaapi passes PTS integers through) - Runtime VBV update on bitrate change (separate pre-existing issue) Tests: - cargo build --release: 0 new warnings (23 baseline preserved) - cargo test: 96 lib + 3 integration, 0 failed - 5 rtp_timestamp_* tests updated for 90kHz identity - SAFETY comments preserved verbatim - 4 files changed, +48/-35 lines
This commit is contained in:
+23
-26
@@ -334,9 +334,9 @@ impl WebRtcState {
|
||||
self.poll_rtc()
|
||||
}
|
||||
|
||||
pub fn write_h264_frame(&mut self, data: &[u8], pts_ticks: i64, fps: u32) -> Result<()> {
|
||||
pub fn write_h264_frame(&mut self, data: &[u8], pts_ticks: i64) -> Result<()> {
|
||||
let should_destroy = if let Some(inner) = self.inner.as_mut() {
|
||||
inner.write_h264_frame(data, pts_ticks, fps)?
|
||||
inner.write_h264_frame(data, pts_ticks)?
|
||||
} else {
|
||||
false
|
||||
};
|
||||
@@ -655,7 +655,7 @@ impl WebRtcInner {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_h264_frame(&mut self, data: &[u8], pts_ticks: i64, fps: u32) -> Result<bool> {
|
||||
fn write_h264_frame(&mut self, data: &[u8], pts_ticks: i64) -> Result<bool> {
|
||||
if !self.connected {
|
||||
return Ok(false);
|
||||
}
|
||||
@@ -690,9 +690,9 @@ impl WebRtcInner {
|
||||
self.need_keyframe = false;
|
||||
}
|
||||
|
||||
let rtp_timestamp = rtp_timestamp_from_pts_ticks(pts_ticks, fps);
|
||||
self.rtp_clock = rtp_timestamp;
|
||||
let rtp_time = MediaTime::new(rtp_timestamp as u64, Frequency::NINETY_KHZ);
|
||||
let rtp_timestamp = rtp_timestamp_from_pts_ticks(pts_ticks);
|
||||
self.rtp_clock = rtp_timestamp as u32;
|
||||
let rtp_time = MediaTime::new(rtp_timestamp, Frequency::NINETY_KHZ);
|
||||
|
||||
let writer = match self.rtc.writer(mid) {
|
||||
Some(w) => w,
|
||||
@@ -722,17 +722,13 @@ impl WebRtcInner {
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert PTS in encoder time_base units (1/fps) to RTP timestamp (90kHz clock).
|
||||
/// Convert PTS in 90kHz media-clock ticks to RTP MediaTime ticks (u64).
|
||||
///
|
||||
/// Extracted as a pure function for unit testing. Clamps negative pts_ticks to 0
|
||||
/// (encoder should never emit negative PTS, but defensive). Saturating multiply
|
||||
/// to avoid overflow on long sessions.
|
||||
pub fn rtp_timestamp_from_pts_ticks(pts_ticks: i64, fps: u32) -> u32 {
|
||||
const TICKS_PER_SECOND: u64 = 90_000;
|
||||
let fps_safe = (fps.max(1) as u64).max(1);
|
||||
let pts_u64 = (pts_ticks.max(0) as u64).min(u64::MAX / TICKS_PER_SECOND);
|
||||
let rtp_ts = pts_u64.saturating_mul(TICKS_PER_SECOND) / fps_safe;
|
||||
rtp_ts as u32
|
||||
/// With WebRTC encoder time_base = 1/90000, pts_ticks ARE RTP timestamps.
|
||||
/// No fps-based conversion needed. Returned as u64 to feed MediaTime::new
|
||||
/// without premature 13.25-hour u32 wrap; str0m handles RTP u32 wrap internally.
|
||||
pub fn rtp_timestamp_from_pts_ticks(pts_ticks: i64) -> u64 {
|
||||
pts_ticks.max(0) as u64
|
||||
}
|
||||
|
||||
// ── 工具函数 ──────────────────────────────────────────────────────────────
|
||||
@@ -882,29 +878,30 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn rtp_timestamp_zero_pts() {
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(0, 30), 0);
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(0), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rtp_timestamp_one_frame() {
|
||||
// 1 frame at 30fps = 33ms = 3000 RTP ticks (90kHz / 30)
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(1, 30), 3000);
|
||||
fn rtp_timestamp_one_frame_at_60fps() {
|
||||
// 16.7ms at 90kHz = ~1500 ticks. Real time maps directly to ticks now.
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(1500), 1500);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rtp_timestamp_one_second() {
|
||||
// 30 frames at 30fps = 1 second = 90000 RTP ticks
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(30, 30), 90000);
|
||||
// 1 second at 90kHz = 90000 ticks
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(90_000), 90_000);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rtp_timestamp_negative_clamps_to_zero() {
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(-5, 30), 0);
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(-5), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rtp_timestamp_zero_fps_does_not_panic() {
|
||||
// fps=0 should clamp to 1 internally, not divide by zero
|
||||
let _ = rtp_timestamp_from_pts_ticks(100, 0);
|
||||
fn rtp_timestamp_u64_no_truncation() {
|
||||
// Value above u32::MAX should NOT truncate when feeding MediaTime
|
||||
let large = u32::MAX as i64 + 1000;
|
||||
assert_eq!(rtp_timestamp_from_pts_ticks(large), large as u64);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user