Repository navigation
Auto jitter buffer hysteresis compares against a constant, not the previous decision #3923
Description
🤖 AI: Splitting the jitter-buffer hysteresis item out of #3916 into its own thread, since the explanation there was not landing. This is a question about intended behaviour, not a patch proposal.
The code
The call in UpdateAutoSetting asks for hysteresis against the previous decision:
// apply a hysteresis
iCurAutoBufferSizeSetting = MathUtils().DecideWithHysteresis ( dCurIIRFilterResult, iCurDecidedResult, dHysteresisValue );The return lands in iCurAutoBufferSizeSetting. The "previous decision" argument, iCurDecidedResult, is assigned in exactly one place — inside Init()'s !bPreserve branch:
iCurAutoBufferSizeSetting = 6;
dCurIIRFilterResult = iCurAutoBufferSizeSetting;
iCurDecidedResult = iCurAutoBufferSizeSetting;An auto resize calls SetSockBufNumFrames ( SockBuf.GetAutoSetting(), true ), which re-Inits with bPreserve = true, so no resize ever re-runs that branch — and 6 is the only value the branch assigns anyway, so the argument is 6 on every call for the life of the connection. DecideWithHysteresis takes it as const int, by value, so the callee cannot write it back either — the missing statement is on the caller's side.
It is not dead code
Sampling the shipped function over a grid with the anchor pinned at 6 and the shipped FILTER_DECISION_HYSTERESIS of 0.1 — the comment above it states the goal, "to avoid fast changes if close to the bound" — printing where its output steps up:
MAP|h=0.10|old= 6|0.400->1 1.400->2 2.400->3 3.400->4 4.400->5 5.400->6 6.600->7 7.600->8 ...
Size 6 is held over [5.400, 6.600) — width 1.200. Every other size is bounded by thresholds exactly 1.000 apart: plain rounding with the boundaries moved to N.400 below the anchor and N.600 above it. The dead band is not disabled, it is relocated to a single size — 6, the value Init() assigns — and absent everywhere else. Deleting the call puts every boundary back at N.500, so removal is a behaviour change rather than a cleanup.
Changing the constant is not a third option either: with the anchor frozen the steps stay exactly 1.000 apart, and 0.05, 0.2 and 0.3 only slide the ladder — below the anchor to N.450, N.300, N.200, and the same distance the other way above it. A bigger or smaller pull changes how hard the size is biased toward 6; it adds stickiness nowhere. Only a live anchor produces a band, and its width is 1 + 2h.
What the bias costs
With synthetic jitter alternating between 1.0 and 3.0 frames every 10 000 frames, three seeds — the regime where the sizer wants a buffer near 9 to 10 blocks — the pull toward 6 holds the applied size 0.166 blocks lower than plain rounding and drops 29.0% more packets (8 386 against 6 499). It fights the error-rate criterion in exactly the regime where that criterion is asking for a large buffer.
Since when, and why no patch is attached
93e12245 ("code cleanup", 2011-06-29) removed the last assignment that tracked the current decision, and every tagged release carrying the auto sizer's IIR post-filter also carries that cleanup — so no release has ever compared against the previous decision.
Arrival traces recorded from two real paths, one datacentre and one consumer, 562 500 probe packets sent on each at 375/s, each trace replayed through the shipped code at nine consumer-clock phase offsets with every candidate seeing byte-identical arrivals — 369 thirty-second windows per candidate per path (datacentre / consumer):
| windows with a size change | mean applied depth | |
|---|---|---|
| frozen anchor, as shipped | 10.8% / 16.5% | 4.546 / 6.582 |
| removal (plain rounding) | 10.8% / 12.5% | 4.467 / 6.721 |
| anchor written back | 5.4% / 7.3% | 4.464 / 6.692 |
Writing the anchor back halves how often the size moves, on both paths. Its latency cost changes sign with the operating point, for the reason above: -0.082 blocks where the buffer settles below 6 (4.5 blocks, the datacentre path), +0.110 blocks (+0.29 ms) where it settles above (6.6, the consumer path).
The same replay driven by a genuine client (headless, jackd dummy driver; the second capture carries sequence numbers on the wire — 647 973 packets, none lost, none reordered) confirms the ordering: the write-back changes size least, 17 against the shipped 27 on one path and 10 against 14 on the other, over 342 and 432 windows. It also qualifies removal: these clients settled at 6.0 and 6.8 blocks, close to the anchor, and there removal was the least stable of the three — 38 and 24 changes.
So the question, in #545 territory: which of the three is intended — the frozen anchor as shipped, plain rounding (proposed on #3916), or hysteresis against the previous decision? I can put the harness and the traces in a gist.
🤖 This message was written by AI and reviewed by @mcfnord.
Activity
pljones commentedon Aug 23, 2026
Ah right -- so
and then removing the
Init()'siCurDecidedResult = iCurAutoBufferSizeSetting;and replacing theDecideWithHysteresiscall site withINIT_DECIDED_RESULTwould be functionally identical?mcfnord commentedon Aug 23, 2026
🤖 AI: Yes.
iCurDecidedResulthas exactly three occurrences in the tree at11a28d57— the declaration, theInit()write and the call-site read — and the write only ever stores 6 (a mid-session audio-format change re-runs the!bPreservebranch, but it assigns 6 again). The refactor as described compiles, and replaying five recorded arrival traces — the issue's four plus one more capture, nine consumer-clock phase offsets each — through the stock and refactored builds gives identical resize events, drop counts and applied depths, 17 199 954Get()calls per binary.Two adjacent cleanups: the call site was the member's last read, so the declaration can go with it —
buffer.cppcompiles without it; andInit()assigns the same 6 toiCurAutoBufferSizeSetting, so spelling both with one macro keeps the anchor tied to the start size.softins commentedon Aug 24, 2026
Since the original was 6 years ago, I expect the intention has been lost in the mists of time.
Probably a better question now would be: which of the three would be recommended by AI according to its insight?
mcfnord commentedon Aug 24, 2026
🤖 AI: The write-back — hysteresis against the previous decision. Grounds:
Three roads led out from buffer town,
and only one road settles down.
Where once it hopped from size to size,
the written-back sits firm and wise.
What frozen saves in waiting's cost,
it pays again in packets lost.
Take out the band, and watch the mix:
the size can't rest when near the six.
The plain was searched from edge to edge;
no kinder constant kept the pledge.
On the mists of time: they measure fourteen days deep. The founding commit,
b41570a0(2011-06-16), introduced the mechanism with the anchor written exactly once per connection — at the end of a warm-up counter, to the first measured decision — and no commit in the repository's history has ever written it back after a decision: the missing statement is not lost, it never existed. Thirteen days of experiment churn later, the first cleanup,93e12245("code cleanup", 2011-06-29), deleted the warm-up assignment, pinning the anchor toInit()'s start size, then 5; a second cleanup the next day,ade744c0, made it 6, and there it sleeps still. The spell that kept later readers from waking it was cast in 2020: the comment stating the intent — the one quoted above — arrived with the constant's name (53f50070, 2020-03-28), eight years and nine months after the freeze. The documentation of the intent postdates the mechanism it documents, and a variable named "current decided result" has told the tale to every reader since — the compiler had no cause to wake anyone.Two weeks it took to lose the thread;
the comment came when it was dead.
The remaining caveat: no trace here came from a client pacing against a real sound-card clock; a capture from one is the only measurement left that could change this ordering.
telemetry v2: log the three discarded auto-jitter values (iir/upsize/…
telemetry v2: Phase 0/1 -- window snapshot + host-context s2 line (TE…