fix(webrtc): propagate real capture PTS through WebRTC channel (closes #24)

Root cause (3-stage bug found via Oracle round 1+2 review):

Browser WebRTC clients accumulated 2-3 seconds jitter buffer under
damage-driven variable frame rate (KWin Portal/PipeWire). User moved
mouse, saw action 2-3 seconds later on client.

Three coordinated bugs formed a chain that defeated any single-point fix:

1. state_portal.rs:455 used sequential frame counter as PTS instead of
   real capture time. (Portal path only — wlr-screencopy already correct.)

2. avhw.rs Channel output sent only Vec<u8>, DISCARDING AVPacket PTS.
   Even with correct encoder PTS, timing metadata was thrown away.

3. webrtc.rs:695 computed RTP timestamp as 'frame_number * 90000 / fps'
   from a counter, ignoring any real PTS. Browser saw uniform 33ms RTP
   spacing regardless of actual 1.6-57fps variable delivery, growing
   jitter buffer to compensate for perceived 'network jitter'.

Fix (7-step ordered implementation per Oracle round 2):

1. EncodedH264Frame struct in avhw.rs carries data + pts_ticks
2. FrameOutput::Channel type changed from Sender<Vec<u8>> to
   Sender<EncodedH264Frame>; both new_webrtc signatures updated
3. Channel drain in avhw.rs extracts pkt.pts(), normalizes via
   'p - start_ts' (mirrors existing Muxer branch logic). Drops
   packets with missing PTS instead of silently emitting zero.
4. write_h264_frame signature: frame_number:u64 -> pts_ticks:i64
   (both WebRtcState and WebRtcInner layers). Extracted pure function
   rtp_timestamp_from_pts_ticks(pts_ticks, fps) with 5 unit tests
   covering zero, one-frame, one-second, negative clamp, fps=0.
5. state_portal.rs receiver loop consumes EncodedH264Frame, passes
   .data and .pts_ticks to write_h264_frame.
6. state.rs (wlr-screencopy) receiver loop updated for compile
   compatibility — its existing real-time PTS computation at state.rs:606
   was already correct, now properly propagates through new channel type.
7. state_portal.rs:467 PTS computation GATED on output mode:
   - WebRTC branch: compute_capture_pts() uses PipeWire's ns timestamp
     (or Instant fallback), normalizes to first-frame-origin, converts
     to encoder time_base units with i128 intermediate math, enforces
     monotonicity via safe checked_add pattern.
   - MP4 branch: KEEPS self.frames_encoded as i64 (sequential counter).
     File output does not need real-time PTS; changing it would alter
     file playback speed during static periods.

Oracle round 2 critical revisions incorporated:

- Single-point PTS normalization (only in avhw Channel drain), NOT at
  source. Avoids double-subtraction with existing Muxer logic.
- MP4 path explicitly preserved — real PTS only applied to WebRTC branch.
- Safe Rust monotonicity guard (no unsafe pointer tricks).
- i64::try_from(ticks_i128) instead of broken i128::try_from(...).unwrap_or(i64::MAX).
- pkt.pts() missing -> log + drop, not silent unwrap_or(0).
- Updates span 4 files (avhw.rs, webrtc.rs, state_portal.rs, state.rs)
  because channel type change ripples through both Portal and wlr paths.

Verification expectations:

- Browser jitter buffer should stabilize at 100-500ms (typical) instead
  of growing to 2-3 seconds under damage-driven delivery.
- chrome://webrtc-internals: jitterBufferDelay / jitterBufferEmittedCount
  ratio should drop significantly.
- Server-side metrics (output_bps, frame rate, IDR size) unchanged.
- MP4 file output (--output mode) behavior unchanged.

Out of scope (deferred):

- frame_age metric fix (Oracle: separate commit to isolate behavioral
  change from observability change)
- VFR encoder redesign (1/fps time_base sufficient for this fix)
- MP4 VFR recording (semantic change, separate decision)
- Existing client jitter buffers may not auto-shrink; reconnect may be
  required for users with already-accumulated latency

Tests:
- cargo build --release: clean, 0 new warnings (19 pre-existing)
- cargo test: 96 lib + 96 bin + 3 integration, 0 failed
- 5 new RTP unit tests covering edge cases
- SAFETY comments preserved verbatim
- 4 files changed, +157/-27 lines
This commit is contained in:
dailz
2026-06-20 21:53:27 +08:00
parent 2f0b858920
commit 079611acfc
4 changed files with 157 additions and 27 deletions
+49 -8
View File
@@ -334,9 +334,9 @@ impl WebRtcState {
self.poll_rtc()
}
pub fn write_h264_frame(&mut self, data: &[u8], frame_number: u64, fps: u32) -> Result<()> {
pub fn write_h264_frame(&mut self, data: &[u8], pts_ticks: i64, fps: u32) -> Result<()> {
let should_destroy = if let Some(inner) = self.inner.as_mut() {
inner.write_h264_frame(data, frame_number, fps)?
inner.write_h264_frame(data, pts_ticks, fps)?
} else {
false
};
@@ -655,7 +655,7 @@ impl WebRtcInner {
Ok(())
}
fn write_h264_frame(&mut self, data: &[u8], frame_number: u64, fps: u32) -> Result<bool> {
fn write_h264_frame(&mut self, data: &[u8], pts_ticks: i64, fps: u32) -> Result<bool> {
if !self.connected {
return Ok(false);
}
@@ -690,11 +690,9 @@ impl WebRtcInner {
self.need_keyframe = false;
}
let ticks_per_second = 90_000u64;
let fps = fps.max(1) as u64;
let rtp_timestamp = frame_number.saturating_mul(ticks_per_second) / fps;
self.rtp_clock = rtp_timestamp as u32;
let rtp_time = MediaTime::new(rtp_timestamp, Frequency::NINETY_KHZ);
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 writer = match self.rtc.writer(mid) {
Some(w) => w,
@@ -724,6 +722,19 @@ impl WebRtcInner {
}
}
/// Convert PTS in encoder time_base units (1/fps) to RTP timestamp (90kHz clock).
///
/// 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
}
// ── 工具函数 ──────────────────────────────────────────────────────────────
/// 从 HTTP 请求中提取 body(在 \r\n\r\n 之后)
@@ -866,4 +877,34 @@ mod tests {
let bps = default.as_u64();
assert_eq!(bps, 5_000_000);
}
// ── RTP timestamp conversion (issue #24) ──
#[test]
fn rtp_timestamp_zero_pts() {
assert_eq!(rtp_timestamp_from_pts_ticks(0, 30), 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);
}
#[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);
}
#[test]
fn rtp_timestamp_negative_clamps_to_zero() {
assert_eq!(rtp_timestamp_from_pts_ticks(-5, 30), 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);
}
}