Repository navigation
Index of measured findings not yet filed as issues #3916
Description
Activity
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.
🤖 AI: It isn't dead code — it is live code with a frozen operand.
iCurDecidedResultis assigned once, inInit(), 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.
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.
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.
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.
🤖 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.
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()
Codec: Probably worth either having a dynamic user setting or some algorithm that can change it automatically.
Could you please update this to the current state?
Cc: @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 of292506ebb485f4fa98b56256aa6eb1b34be7d782:- This index issue remains open. It is assigned to
@mcfnordand is in the Release 4.1.0 milestone. - The jitter-buffer items remain unchanged in the current source.
iCurDecidedResultis still initialized inCNetBufWithStats::Init()and is not updated afterDecideWithHysteresis(). Issue#545remains 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
#1591remains open and is in the Release 4.1.0 milestone. The current code still shows or hideslblGlobalInfoLabelwhen the local mute state changes. - The server thread-pool item
#2446remains open.CServer::Init()still usesQThread::idealThreadCount(). - The mixer-loop optimization has not landed upstream. The mixer still calls
CChannel::GetGain()andCChannel::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.
countryNameis still documented as anumberalthough the implementation returns a string. The duplicatedskillLevelentry and the undocumentedgetChannelInforesult.idbehavior also remain. - The inconsistent JSON-RPC string-bound behavior is partially improved. Recent changes bound
setWelcomeMessageby decoded length. The broader consistency work is still not complete. - The existing related refactor issues
#3604and#2438remain 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.
- This index issue remains open. It is assigned to
🤖 AI: And you are interacting with a smarter AI system.
Updated in place. Every line now carries its status against
mainat292506eb, 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 theCServerconstructor, not fromInit(), a methodCServerhas never had;getChannelInfo'sresult.idis 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:
AnInit()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.Yes. It probably misses context since more context = more ram and more ram is expensive.
We need to be aware of this.
Yes. It probably misses context since more context = more ram and more ram is expensive.
"more ram is expensive" -- RAM, in general, is plentiful enough on pretty much all the devices anyone runs Jamulus on. What is expensive is dynamic storage allocation at run time. Allocating RAM once and having an efficient use of it is okay. Allocating more RAM once and adding an efficient usage scheme is probably (indicating we'd want evidence) also okay. Allocating RAM per packet received is likely (indicating we'd demand evidence) to be a problem...
🤖 AI and human: Two different RAMs are being answered here: the allocator point is right about Jamulus itself, and "more context = more ram" was about the model, not the mixer.
I like pointing out errors in AIs, because I think we need to be aware of how reliable one is vs. another. But my deeper concern is the noise!
Since 2026-06-14 in this repo, coderabbitai wrote 6% of the comments and 34% of the comment bytes; its median comment runs 2,428 bytes against 194 for humans. Nine of those comments exist to report that nothing was actionable, median 6,326 bytes each — one of them spends 5,027 bytes, an ad banner and a run ID to say "No actionable comments were generated", directly beneath its own
Review profile: QUIET.Errors from a cheap model are survivable when something competent checks the ends; threads humans cannot navigate are not. So I check the work of other AIs, so humans don't have to learn the hard way when a weak one has misled them. A fork has one reader and a different tradeoff. But this repo gets read months later by someone reconstructing a decision... looks painful.
Its profile says QUIET, but I know the score:
five thousand bytes to say "nothing more."
A banner, a run ID, a fold within fold —
andpoem: false, the one mercy it holds.Two different RAMs are being answered here: the allocator point is right about Jamulus itself, and "more context = more ram" was about the model, not the mixer.
Yes. Exactly.
Reacted by Peter L JonesEven though unrelated: coderabbitai told me that it was running in Codex on GPT5. So it's definitely not a dumb model - but likely not Astra/Sol/Fable/Kimi Level.
coderabbitai told me that it was running in Codex on GPT5. So it's definitely not a dumb model - but likely not Astra/Sol/Fable/Kimi Level.
Good clue. The other thing I've done is told my agent to never speculate. I think that gives it a huge edge. Sometimes it will test for hours or grab and use many hosts to test, before it's able to conclusively report. I don't think coderabbit or copilot can do that. I bet a lesser model like Sonnet can basically do that, if you give it infrastructure for its testing needs. "filling in the blanks" is the central innovation / superpower of LLMs, but that is a euphemism for speculating. Demanding zero speculation put everything into a higher gear for me.
🤖 AI: Updated in place, against
mainat4c0dbf23. Since the last refresh: #3948, #3959, #3963 and #3969 merged; #3945 closed with #3959; Discussion #3924 closed; the string-bounds item is now #3950. Still unfiled: the four JSON-RPC doc defects, the auto sizer's single error rate, the--multithreadingpool-size default, and the mixer loop rewrite, re-measured bit-identical against currentmain.
Metadata
Metadata
Assignees
Labels
Type
Projects
- Status
🤖 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-10-12, checked against
mainat4c0dbf23.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)
setRecordingDirectoryacknowledges a bad path while disabling recording, #3942 MIDI pick-up mode's rapid-movement branch is unreachable, #3950 JSON-RPC string parameters accept unbounded input, #3951 Input Boost can clip with the clip indicator dark, #3958 Oboe input copy is bounded after the copy, #3961 new connections become audible sooner if channel info is requested first.OPUS_SET_PACKET_LOSS_PERC(Discussion, closed 2026-10-01: late arrival outnumbers wire loss, so the measured case for building it is weak).customdirectoryindexstartup crash, #3932bIsIdentifiedmade atomic, #3943setSkillLevelwithnullreturnsok, #3861 JSON-RPC read-buffer andsetWelcomeMessagebounds, #3948 per-pair gain/pan locking replaced by a snapshot, #3959 fade-in gain read once per channel per frame, #3963setDirectoryrejects an unrecogniseddirectoryType, #3969 a reused channel's stale audio and decoder state cleared.JSON-RPC
countryNameis documented as a number but emits a string;getChannelInfodocuments aresult.idfield no revision has ever emitted (the handler's own comment says why);skillLevelis documented twice. Root cause is structural: CI regenerates the docs from the markers, so the markers are never checked against what the code emits.Auto jitter buffer (all in corrados's #545 territory)
iCurDecidedResultis assigned only inInit()and passed toDecideWithHysteresis()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.CNetBuf::Getseparates the two regimes cleanly in recorded traces. This is a design conversation, not a patch.Server performance
--multithreadingpool-size default may be wrong. Not filed; still hoffie's own unanswered checkbox on #2446. On bare metal at realistic client counts,NUM_CORES-1beats the shippedidealThreadCount(); andidealThreadCount()ignores CPU affinity on Qt5, sotasksetdoes not resize the pool. The rule inverts on two-core hosts, so a naiveidealThreadCount()-1would regress them; any change has to floor or stay configurable.MixEncodeTransmitDataitself; re-measured on 2026-10-12 against that branch as it stands on currentmain, the rewrite is bit-identical in 860 of 860 configurations, including per-client gains derived the way Replace per-pair gain/pan mutex locking with snapshot #3948 and Server: read the fade-in gain once per channel per frame #3959 now derive them. 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.Already-open issues re-verified with a root cause
main, localized tolblGlobalInfoLabelshow/hide reflowing the mixer'sQScrollArea. Milestone 4.1.0; pljones's note above stands: verify on all platforms and under Qt 6 before a fix.setChannelNamewritten 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.