fix(state_portal): use webrtc_thread.is_some() for PTS mode gating (really fixes #24)

Previous commit 079611a claimed to fix #24 but the gating condition was
wrong, making the entire fix dead code:

  src/state_portal.rs:467
  - let pts = if self.webrtc.is_some() {        // ALWAYS false here
  + let pts = if self.webrtc_thread.is_some() {  // correct lifecycle check

Why self.webrtc was wrong:

  WebRtcState lifecycle in Portal path:
  1. StatePortal::new() sets self.webrtc = Some(...) if args.port > 0
  2. First frame arrives -> WaitingForFormat branch
  3. state_portal.rs:274 does self.webrtc.take() and moves WebRtcState
     into the webrtc thread
  4. Subsequent frames -> Streaming branch -> handle_pw_frame
  5. By this point self.webrtc is None

So my gating check 'if self.webrtc.is_some()' at handle_pw_frame ALWAYS
returned false, and compute_capture_pts was NEVER called. Confirmed by
debug instrumentation showing 0 invocations across a 174s WebRTC session.

Net effect: #24's PTS fix was completely inert. RTP timestamps were
still computed from sequential frame counter (old broken behavior).
User reports of 'latency got worse' were due to other test conditions,
not the dead code.

The correct check is self.webrtc_thread.is_some() because:
- webrtc_thread is set AFTER WebRtcState is moved into it (line 301)
- webrtc_thread stays Some for the entire WebRTC session
- webrtc_thread is None for MP4 mode (no thread spawned)

So this check correctly distinguishes WebRTC mode from MP4 mode at the
point where PTS is computed for each frame in handle_pw_frame.

Lesson learned:
- Oracle review (rounds 1 and 2) verified the design and code structure
  but did not catch the lifecycle issue because they reasoned about the
  code statically.
- Runtime verification via debug instrumentation was needed to confirm
  the function was never called.
- This is why the user ran the test BEFORE I committed - their feedback
  that latency got worse was the canary that exposed the dead code.

Verification plan (next user test):
- Run with --stats and confirm rtp= field in write_h264 debug logs
  shows VARIABLE jumps (not uniform 3000 increments)
- During static periods (capture_fps < 5), rtp should jump by 30000+
- During active periods (capture_fps > 30), rtp increments may still
  look sequential due to 1/fps time_base quantization (acceptable)
- Browser jitter buffer should stabilize at < 500ms
This commit is contained in:
dailz
2026-06-20 22:15:50 +08:00
parent 079611acfc
commit 1e792f191c
+3 -2
View File
@@ -464,7 +464,7 @@ impl StatePortal {
// damage-driven variable fps — issue #24). MP4: keep sequential counter; // damage-driven variable fps — issue #24). MP4: keep sequential counter;
// file output doesn't need real-time PTS and changing it would alter // file output doesn't need real-time PTS and changing it would alter
// playback speed during static periods. // playback speed during static periods.
let pts = if self.webrtc.is_some() { let pts = if self.webrtc_thread.is_some() {
self.compute_capture_pts(frame.pts) self.compute_capture_pts(frame.pts)
} else { } else {
self.frames_encoded as i64 self.frames_encoded as i64
@@ -579,7 +579,8 @@ impl StatePortal {
0 0
}; };
let ticks_i128 = (relative_ns.saturating_mul(i128::from(self.args.fps))) / NS_PER_SEC; let ticks_i128 = (relative_ns.saturating_mul(i128::from(self.args.fps))) / NS_PER_SEC;
let mut pts = i64::try_from(ticks_i128).unwrap_or(i64::MAX); let computed_pts = i64::try_from(ticks_i128).unwrap_or(i64::MAX);
let mut pts = computed_pts;
if let Some(last) = self.last_pts_emitted { if let Some(last) = self.last_pts_emitted {
if pts <= last { if pts <= last {