jamulussoftware / jamulussoftware/jamulus

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

Open
#3,923 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
1.1k
Forks
248
Avg merge
2d 3h
Merged PRs (30d)
9

Description

**🤖 AI:** Splitting the jitter-buffer hysteresis item out of [#3916](https://github.com/jamulussoftware/jamulus/issues/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`](https://github.com/jamulussoftware/jamulus/blob/11a28d57e33506fc84cb9b5ee02097931c1e35a3/src/buffer.cpp#L683) asks for hysteresis against the previous decision:

```cpp
// 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](https://github.com/jamulussoftware/jamulus/blob/11a28d57e33506fc84cb9b5ee02097931c1e35a3/src/buffer.cpp#L519-L521):

```cpp
iCurAutoBufferSizeSetting = 6;
dCurIIRFilterResult = iCurAutoBufferSizeSetting;
iCurDecidedResult = iCurAutoBufferSizeSetting;
```

An auto resize calls [`SetSockBufNumFrames ( SockBuf.GetAutoSetting(), true )`](https://github.com/jamulussoftware/jamulus/blob/11a28d57e33506fc84cb9b5ee02097931c1e35a3/src/channel.cpp#L737), which re-`Init`s 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`](https://github.com/jamulussoftware/jamulus/blob/11a28d57e33506fc84cb9b5ee02097931c1e35a3/src/util.h#L1213) 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`](https://github.com/jamulussoftware/jamulus/blob/11a28d57e33506fc84cb9b5ee02097931c1e35a3/src/buffer.h#L57-L58) 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](https://github.com/jamulussoftware/jamulus/blob/11a28d57e33506fc84cb9b5ee02097931c1e35a3/src/buffer.cpp#L599-L607) in exactly the regime where that criterion is asking for a large buffer.

### Since when, and why no patch is attached

[`93e12245`](https://github.com/jamulussoftware/jamulus/commit/93e12245fed6807660d5b5f3a07baa391b5ed1e2) ("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](https://github.com/jamulussoftware/jamulus/commit/b41570a0df5a94a6bc6e87d6a2ce1380c645d34a) 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](https://github.com/jamulussoftware/jamulus/issues/545) territory: which of the three is intended — the frozen anchor as shipped, plain rounding ([proposed on #3916](https://github.com/jamulussoftware/jamulus/issues/3916#issuecomment-5379549780)), 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.*

Contributor guide

Open the contributing guide

Research direction

Read src/buffer.cpp at UpdateAutoSetting and Init(), src/channel.cpp at SetSockBufNumFrames, and MathUtils::DecideWithHysteresis in src/util.h. First resolve which behavior is intended: the frozen anchor, plain rounding, or a live previous-decision anchor. Done means the project maintainers choose one behavior and the corresponding implementation and regression coverage are agreed.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
audio-video-rtc, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.