Skip to content

sync engine: bound timeline drift, preserve OWD across NTP rebuilds#948

Open
frostbyte73 wants to merge 2 commits into
mainfrom
sync-engine-bounded-drift
Open

sync engine: bound timeline drift, preserve OWD across NTP rebuilds#948
frostbyte73 wants to merge 2 commits into
mainfrom
sync-engine-bounded-drift

Conversation

@frostbyte73

Copy link
Copy Markdown
Member

Summary

A small fraction of canary sessions (~0.2% of room_composite and participant recordings; two extreme sessions in the last 48h) dropped thousands of seconds of audio — one 8.4h session ended up ~97% silent. Two distinct failure modes cause it, both traceable to timeline anchoring in the sync engine.

Root causes

Publishers with sample-clock skew (consumer USB audio, mobile, headsets: ±500ppm is common). wallClockPTSForRTPLocked computed rtpDerived = lastWallPTS + rtpDelta and wrote the result back into lastWallPTS each packet — self-referential. Publisher-vs-wall skew accumulated up to the 5s sanity band before being corrected, and once it crossed the 3s force-correction threshold, force-correction fired on every subsequent packet, snapping each to deadline - maxDelay/2. Successive slow-arriving packets collapsed onto the same corrected PTS → downstream audiomixer dropped the collisions.

Publishers with unstable RTCP SR timing. NtpEstimator.resetLocked reset owdEstimator on every outlier-triggered rebuild, forcing OWD to re-converge from scratch. sessionPTS = ntpTime + OWD - sessionStart jumped by the reconvergence delta at each rebuild (39 rebuilds in one 3h session). The drift signal fed to the tempo controller (wallPTS - sessionPTS) became a burst pattern; the audio pacer chased moving targets, and the resulting pitch changes pushed audio into the mixer at rates that overran its alignment threshold.

Changes

  1. wallClockPTSForRTPLocked slews rtpDerived toward wallElapsed by α=0.01 per packet inside the sanity band. Steady-state |drift| = R·δ·(1-α)/α ≈ 10ms for R=20ms, δ=±500ppm — well below force-correction. Preserves jitter immunity (receivedAt noise stays filtered).
  2. NtpEstimator.resetLocked no longer resets owdEstimator. Outlier-triggered rebuilds are residual-pattern changes, not network topology changes; OWD's min-tracking absorbs sender-clock steps via its own convergence. The public Reset() (called via SessionTimeline.ResetTrack on true stream discontinuity) still resets OWD.
  3. Force-correction now resets lastTimelyPacket when it fires so a residual transient can't cascade into per-packet re-firing.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread pkg/synchronizer/ntpestimator.go
"behindBy", limit-oldPTS,
)
// Reset timeliness clock so a lingering transient deficit doesn't fire force-correction on every subsequent packet, collapsing them into the same newPTS at the mixer.
st.lastTimelyPacket = time.Now()

@milos-lk milos-lk Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is one scenario worth zooming in - what happens on the packets after a force-correction, when the track is still behind.. which is the case we kept seeing in cases where carriers don't signal loss correctly (sip) - i.e when the deficit is structural rather than transient.

The jump raises lastPTSAdjusted but the raw baselines (lastWallPTS/lastSlewPTS/sessionOffset) still reflect the behind timeline, so the next packet computes a behind PTS again. With the timer now reset, the monotonicity floor becomes the only thing advancing output - +1ms per 20ms frame, until the timer expires and a multi-second jump fires again.

return wallElapsed

// Slew rtpDerived toward wallElapsed each packet so publisher sample-clock skew stops accumulating across the sanity band; see wallSlewAlpha for the steady-state math.
absorbed := time.Duration(float64(diff) * wallSlewAlpha)

@milos-lk milos-lk Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm - I have a feeling this creates a closed loop with the tempo controller. Let me check it.. The SR drift path reads the slewed baseline, so a delayed packet now enters the tempo loop as phantom drift: tempo books a permanent correction (corrected), the slew then drains the same disturbance back out of the sensor, effective flips sign, and the controller fires counter-corrections undoing its own work, audible ±5% warble from one late packet. Need to check more

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asked claude to check it - I think the graph it generated might be helpful:

@milos-lk milos-lk self-assigned this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants