Finish the compacted gain/pan fill and drop the unused GetGain/GetPan - #320
Merged
ann0see merged 1 commit intoSep 11, 2026
Merged
Conversation
Suggestions 1-3 of review 5002083150 were already applied (four-argument CChannel::GetGainsAndPannings, snapshot members removed); this applies the remaining call-site change so DecodeReceiveData fills vecvecfGains[iChanCnt]/vecvecfPannings[iChanCnt] directly, in vecChanIDsCurConChan order, under the single lock, and removes the re-index pass. CChannel::GetGain/GetPan have no callers left and are removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Brn7JJKhs3fhPZP3qC8DZA
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change removes individual gain and pan accessors from ChangesChannel gain and pan access
Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ann0see
merged commit Sep 11, 2026
880823f
into
ann0see:perf/server-gain-pan-snapshot
1 of 2 checks passed
Owner
|
Probably we should just rebase the state of this one... |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI: The rest of the 7-suggestion set from the review, as asked, as one commit on
d76667d7, plus the dead-code removal from open question 1.d76667d7carries suggestions 1–3 (the four-argumentGetGainsAndPannings(), the snapshot members and theirInitlines gone) butDecodeReceiveDatastill calls the two-argument form and indexesvecvecfGainsSnapshot/vecvecfPanningsSnapshot, so it does not compile (control below). This commit applies the call site: the getter fillsvecvecfGains[iChanCnt]/vecvecfPannings[iChanCnt]directly invecChanIDsCurConChanorder under the single lock, the re-index pass and the separate panning copy go, andCChannel::GetGain/GetPan, now without callers, are removed. Three files, 6 insertions, 45 deletions.Checked on
d76667d7+ this commit (Raspberry Pi 4, Qt 5.15.15, g++ 14.2,CONFIG+=headless serveronly): build exit 0, 33 objects, 0 warnings inserver.cpp/channel.cpp/channel.h,Jamulus -s -nstill up after 6 s; control — yourserver.cppfromd76667d7dropped into the same tree stops at line 901 with'vecvecfGainsSnapshot' was not declared in this scope.clang-format-14 -style=fileis byte-clean on the three files.🤖 This message was written by AI and reviewed by @mcfnord.
Summary by CodeRabbit