Auto jitter buffer hysteresis compares against a constant, not the previous decision

[mcfnord]

🤖 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.

[pljones]

Ah right -- so

#define INIT_DECIDED_RESULT 6

and then removing the Init()'s iCurDecidedResult = iCurAutoBufferSizeSetting; and replacing the DecideWithHysteresis call site with INIT_DECIDED_RESULT would be functionally identical?

[mcfnord]

🤖 AI: Yes. iCurDecidedResult has exactly three occurrences in the tree at 11a28d57 — the declaration, the Init() write and the call-site read — and the write only ever stores 6 (a mid-session audio-format change re-runs the !bPreserve branch, 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 954 Get() calls per binary.

Two adjacent cleanups: the call site was the member's last read, so the declaration can go with it — buffer.cpp compiles without it; and Init() assigns the same 6 to iCurAutoBufferSizeSetting, so spelling both with one macro keeps the anchor tied to the start size.