Stability: drop ServerQuery-only register call + cap viewer default - #18
Merged
Merged
Conversation
Two unrelated stability fixes that surfaced after the live test of identity persistence (PR #17): * `register_stream_notifications` was sending three `servernotifyregister` calls during stream setup; each got a `error id=516 invalid client type` back from the server because that command is for ServerQuery clients (type 1) only. As a voice client (type 0) we receive stream + channel notifications automatically, so the call is dead weight that polluted the log on every startup. Drop the call site and the method; keep a comment so the next reader doesn't re-add it from the upstream ts6-manager port (which is a ServerQuery client and does need it). * Each viewer spins up its own VP8 encoder via the per-viewer `RTCPeerConnection` + MediaRelay subscription. Live measurement on 720p30 showed RSS jumping from ~110 MB (idle) to ~400 MB after the first viewer connected - the unlimited default (`STREAM_VIEWER_LIMIT=-1`) would silently OOM-kill the container on a small host once a fourth or fifth viewer joined. Default cap to 4; the operator can bump it once they've measured.
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
2 participants
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.
Summary
Two stability fixes the live test exposed after the identity-persistence change:
Drop
register_stream_notifications. Each startup was sending threeservernotifyregistercommands and gettingerror id=516 invalid client typeback, because that command is for ServerQuery clients (type 1). Voice clients receive stream + channel notifications automatically. The bot worked anyway, but the log was noisy and a future debugger would burn time chasing the errors. The originalts6-managerport still uses the call because it runs as a ServerQuery client.Cap
STREAM_VIEWER_LIMITdefault to 4. Each viewer gets its own VP8 encoder via the per-viewerRTCPeerConnection+ MediaRelay subscription. Live measurement on 720p30: RSS went from ~110 MB (idle) to ~400 MB after one viewer joined. With-1(unlimited), a small host would silently OOM-kill the container around the fourth or fifth viewer. Operators with the memory budget can bump it.Test plan
pytest— 227 passed, 1 skipped (one less than before: removed the test for the deletedregister_stream_notificationsmethod).mypy --strictclean.ruff checkclean.error id=516lines are gone from startup logs and that joining a viewer still works.https://claude.ai/code/session_016DuCjRJK995Tj9aDhhB9at
Generated by Claude Code