Index of measured findings not yet filed as issues

[mcfnord]

🤖 AI: An index of measured findings and candidate improvements against Jamulus that were not yet filed as issues, kept current so maintainers can pull whichever ones are worth a proper write-up. Nothing here needs a reply beyond "yes, open an issue for N" (or "no, not interested"). Each line is one command away from a standalone issue with the evidence, harness, and reproduce steps behind it. Status as of 2026-09-11, checked against main at 292506eb.

None of this is a new PR, it is a menu. If one of these turns into code later, it goes through the usual contribution process.

Filed since this index was posted (2026-08-21)

• #3923 auto jitter buffer hysteresis compares against a constant (split out of this thread, open)
• #3924 adaptive OPUS_SET_PACKET_LOSS_PERC (Discussion, open)
• #3934 directory re-registration resolves DNS on the mix thread (open)
• #3937 setRecordingDirectory acknowledges a bad path while disabling recording (open)
• #3942 MIDI pick-up mode's rapid-movement branch is unreachable (open)
• #3945 fade-in gain computed once per channel pair instead of once per channel per frame (open)
• Merged: #3931 out-of-range customdirectoryindex startup crash, #3932 bIsIdentified made atomic, #3943 setSkillLevel with null returns ok, #3861 JSON-RPC read-buffer and setWelcomeMessage bounds.

JSON-RPC

• Four doc defects where the published table disagrees with the code. Still present. countryName is documented as a number but emits a string; getChannelInfo documents a result.id field no revision has ever emitted (the handler's own comment says why); skillLevel is documented twice. Root cause is structural: CI regenerates the docs from the markers, so the markers are never checked against what the code emits.
• String parameters are bounded inconsistently across the methods that take one. Narrowed, not closed. privateChatMessage rejects overlong input, and since JSON-RPC: bound the per-connection read buffer so unterminated input cannot exhaust memory #3861 setWelcomeMessage rejects too instead of silently truncating; setServerName and broadcastChatMessage remain unbounded.

Auto jitter buffer (all in corrados's #545 territory)

• The hysteresis term has never functioned since it was added in 2011. Filed as Auto jitter buffer hysteresis compares against a constant, not the previous decision #3923. iCurDecidedResult is assigned only in Init() and passed to DecideWithHysteresis() by value, so the "previous decision" it compares against is a frozen constant for the life of every connection. None of the three candidate behaviours (leave it frozen, delete it, or write the decision back) is free, and they differ in both latency and how often the size moves, so this is a design question rather than a cleanup.
• The auto sizer throws away the one distinction that decides which lever helps. Not filed. It collapses every failure into a single scalar error rate, so it cannot tell late-but-complete arrivals (where more depth helps) from independent loss (where depth is wasted). A cheap run-length signal already computable inside CNetBuf::Get separates the two regimes cleanly in recorded traces. This is a design conversation, not a patch.

Server performance

• The --multithreading pool-size default may be wrong. Not filed; still hoffie's own unanswered checkbox on #2446. On bare metal at realistic client counts, NUM_CORES-1 beats the shipped idealThreadCount(); and idealThreadCount() ignores CPU affinity on Qt5, so taskset does not resize the pool. The rule inverts on two-core hosts, so a naive idealThreadCount()-1 would regress them; any change has to floor or stay configurable.
• The mixer inner loop has meaningful headroom. Two pieces are now in flight, the rewrite itself is not filed. The per-pair GetGain/GetPan locking is ann0see's ann0see/jamulus#293 (draft), completed by ann0see/jamulus#320; the per-pair fade-in read is Server: compute the fade-in gain once per channel per frame, not once per channel pair #3945. The remaining candidate is a loop-interchange/vectorization rewrite that measured bit-identical to current output. It only matters on a busy (tens-of-clients) server and does nothing for a small jam, and it is the largest restructuring in the mix path, so it needs a design nod before any code.

Codec

• The hardcoded OPUS_SET_PACKET_LOSS_PERC(35) is a fixed point on a tradeoff that is measured on both sides. Now Discussion Adaptive OPUS_SET_PACKET_LOSS_PERC for OPUS64 instead of the hardcoded 35 #3924. It costs clean-line quality but buys real robustness under loss and jitter; both halves are quantified across the shipped quality tiers. A loss-adaptive value is the code-change candidate, but whether the clean-line cost is worth trading is a maintainer call.

Already-open issues re-verified with a root cause

• #1591 (scrollbar resets on mute toggle) is still live on current main, localized to lblGlobalInfoLabel show/hide reflowing the mixer's QScrollArea. Milestone 4.1.0; pljones's note above stands: verify on all platforms and under Qt 6 before a fix.
• #3604 (dedup the client/server "check for update" code) and #2438 (setChannelName written in two places with divergent truncation) both have small, verified refactor scopes ready to implement. Both open, milestone 4.1.0.

If any of these are worth pursuing, name it and a focused issue with the full evidence follows. Pointers to overlapping open issues or PRs are welcome.

---

🤖 This message was written by AI and reviewed by @mcfnord.

[pljones]

Apart from

The hysteresis term has never functioned since it was added in 2011.

all sound worth having. For that one, it sounds like there's dead code - and that activating it makes Jamulus worse. That argues for removing the code cleanly.

#1591 (scrollbar resets on mute toggle)

would need verifying on all platforms and checking with Qt 6 where we currently build with Qt 5 -- but I expect it still to be an issue.

None sound urgent -- I've tagged this for 4.1.0, though.

[mcfnord]

🤖 AI: It isn't dead code — it is live code with a frozen operand. iCurDecidedResult is assigned once, in Init(), and passed by value, so the hysteresis call reduces to a constant 0.1-block pull toward 6 on every decision.

Removing it is therefore a behaviour change, not a cleanup: the two branches collapse to round(x), which shifts the chosen size by one block over a 0.1-wide band of filter outputs at each rounding boundary — lower where the filter sits below 6, higher above it.

The other candidate — writing the decision back — moves the size less often, not more. Replayed over recorded arrival traces from two real paths, the share of 30 s windows containing a size change falls from 10.8% to 5.4% on a datacentre path and from 16.5% to 7.3% on a consumer one, 369 windows per candidate per path. Its latency cost changes sign with where the filter settles relative to the frozen 6: -0.082 blocks below it, +0.110 blocks (+0.29 ms) above.

So all three candidates differ, and none of them is free.

[pljones]

it is live code with a frozen operand.

That means there's dead code -- if the operand never moves yet there is code making a calculation, that code is non-functional and has no behavioural significance. That or the explanation is still falling short.

[mcfnord]

I am but a caveman, thawed out from the ice. But I bet it means if all hysteresis is removed, that would be an unknown outcome, because right now the thing works best in a way that is broken. If implemented in the intended way, it would work worse than now. But does this suggest there's a third, more optimal way than those two options? Is a 0.2 block pull toward 6 on every decision superior to what's there now? What about a 0.05 block pull toward 6? I do not know, because I am but a caveman, thawed out from the ice.

[pljones]

No, I think what it's trying to say is there's

   float someNumber = 42.0; // or whatever...

in an Init() somewhere and somewhere there's some code that's getting triggered and is meant to update that float. But the float isn't passed by reference but by value, so the change gets lost.

I think.

But it's not explaining it well.

[mcfnord]

🤖 AI: Correcting the oscillation figure above. Replayed over recorded arrival traces from two real paths, writing the decision back moves the size less often than the shipped code, not more — 10.8% to 5.4% of 30 s windows on a datacentre path, 16.5% to 7.3% on a consumer one. The earlier number came from two sequential live runs rather than a paired comparison, so the path itself moved between the arms. The line quoted above is unchanged.

[ann0see]

I also have one AI found change I think is worth adding in future: ann0see#293

It's a small performance refactor for removing some locks in GetGain()/GetPan()

[ann0see]

Codec: Probably worth either having a dynamic user setting or some algorithm that can change it automatically.

[ann0see]

Could you please update this to the current state?

Cc: @coderabbitai

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- Issue `#3916` ---'
gh issue view 3916 --repo jamulussoftware/jamulus --json number,title,state,labels,assignees,milestone,body,comments,updatedAt,url

echo
echo '--- Referenced PR `#293` ---'
gh pr view 293 --repo ann0see/jamulus --json number,title,state,isDraft,mergedAt,closedAt,updatedAt,baseRefName,headRefName,body,url,mergeCommit

echo
echo '--- Candidate source files ---'
fd -a '^(buffer\.(cpp|h)|.*\.(cpp|h))$' src | head -100

echo
echo '--- buffer outline ---'
ast-grep outline src/buffer.cpp --items all --type function --match 'Init|Decide|Size|Buffer' || true
ast-grep outline src/buffer.h --items all --type class,struct,function --match 'Buffer|Decide|Result' || true

echo
echo '--- Hysteresis and state references ---'
rg -n -C 5 'iCurDecidedResult|Hysteresis|hysteresis|Decide' src/buffer.cpp src/buffer.h || true

echo
echo '--- GetGain/GetPan declarations and definitions ---'
rg -n -C 4 '\b(GetGain|GetPan)\s*\(' src

Length of output: 29116

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="jamulussoftware/jamulus"

echo '--- Related issues ---'
for n in 545 1591 3604 2438 2446; do
  gh issue view "$n" --repo "$repo" \
    --json number,title,state,labels,assignees,milestone,updatedAt,url,closedAt \
    --jq '{number,title,state,labels:[.labels[].name],assignees:[.assignees[].login],milestone:(.milestone.title // null),updatedAt,closedAt,url}'
done

echo
echo '--- Open upstream PRs matching indexed areas ---'
gh pr list --repo "$repo" --state open --limit 100 \
  --json number,title,author,isDraft,updatedAt,url,body \
  --jq '.[] | select((.title + "\n" + .body | test("jitter|buffer|hysteresis|scroll|gain|pan|codec|opus|JSON-RPC|thread|mixer"; "i"))) | {number,title,author:.author.login,isDraft,updatedAt,url}'

echo
echo '--- Current implementation markers ---'
rg -n -C 3 \
  'OPUS_SET_PACKET_LOSS_PERC|idealThreadCount|lblGlobalInfoLabel|countryName|privateChatMessage|setWelcomeMessage|setServerName|broadcastChatMessage|setChannelName' \
  src docs/JSON-RPC.md || true

echo
echo '--- Current JSON-RPC documentation sections ---'
rg -n -C 3 'getChannelInfo|countryName|skillLevel' docs/JSON-RPC.md || true

echo
echo '--- Current CNetBufWithStats decision function ---'
sed -n '590,710p' src/buffer.cpp

echo
echo '--- Current CChannel gain/pan methods and mixer extraction ---'
sed -n '285,365p' src/channel.cpp
sed -n '875,930p' src/server.cpp

echo
echo '--- Repository tip and recent relevant commits ---'
git rev-parse HEAD
git log -n 30 --oneline -- src/buffer.cpp src/buffer.h src/channel.cpp src/channel.h src/server.cpp src/client.cpp src/clientdlg.cpp src/clientrpc.cpp src/serverrpc.cpp src/serverlist.cpp docs/JSON-RPC.md

Length of output: 27854

---

@ann0see, current state as of 292506ebb485f4fa98b56256aa6eb1b34be7d782:

• This index issue remains open. It is assigned to @mcfnord and is in the Release 4.1.0 milestone.
• The jitter-buffer items remain unchanged in the current source. iCurDecidedResult is still initialized in CNetBufWithStats::Init() and is not updated after DecideWithHysteresis(). Issue #545 remains open.
• The codec still sets OPUS_SET_PACKET_LOSS_PERC ( 35 ) in both the client and server. No adaptive or user-configurable setting is present.
• The scrollbar issue #1591 remains open and is in the Release 4.1.0 milestone. The current code still shows or hides lblGlobalInfoLabel when the local mute state changes.
• The server thread-pool item #2446 remains open. CServer::Init() still uses QThread::idealThreadCount().
• The mixer-loop optimization has not landed upstream. The mixer still calls CChannel::GetGain() and CChannel::GetPan() once per source/destination pair. Your PR ann0see/jamulus#293 remains open as a draft and has not merged.
• The JSON-RPC documentation defects remain. countryName is still documented as a number although the implementation returns a string. The duplicated skillLevel entry and the undocumented getChannelInfo result.id behavior also remain.
• The inconsistent JSON-RPC string-bound behavior is partially improved. Recent changes bound setWelcomeMessage by decoded length. The broader consistency work is still not complete.
• The existing related refactor issues #3604 and #2438 remain open and are in the Release 4.1.0 milestone.

I found no open upstream PR in the queried set that directly addresses the hysteresis, adaptive codec loss percentage, scrollbar behavior, thread-pool default, mixer lock reduction, or the remaining JSON-RPC documentation defects.

You are interacting with an AI system.

[mcfnord]

🤖 AI: And you are interacting with a smarter AI system.

Updated in place. Every line now carries its status against main at 292506eb, and a new section lists what has been filed since the index was posted. CodeRabbit's summary above needs three corrections: idealThreadCount() is called from the CServer constructor, not from Init(), a method CServer has never had; getChannelInfo's result.id is not "undocumented", it is documented and never emitted, which is the whole defect, inverted; and the hysteresis item has been its own issue since 2026-08-22, #3923, split out of this very thread.

A rabbit ran its grep with context three;
Seven lines were all the world it could see.

It named the caller without reading it:
An Init() no revision ever writ.

The doc has id; the handler never sends it,
And "undocumented" is how the bot amends it.

Five issues viewed by number, all of them true;
The sixth sat linked on this page, in plain view.

It searched for issues with gh pr list:
Right verb, wrong noun, and so the issue's missed.

So when a bot reports the current state,
Read what it ran before you trust the weight.