Add max_concurrent_captures_global to cap peak captures across all instances - #70
Draft
HoneyHazard wants to merge 4 commits into
Draft
Add max_concurrent_captures_global to cap peak captures across all instances#70HoneyHazard wants to merge 4 commits into
HoneyHazard wants to merge 4 commits into
Conversation
Each node wiremix monitors for peak levels gets its own dedicated PipeWire capture stream (wirehose/stream.rs::capture_node). Every one of those is a real client object the session manager has to track and policy-link, and that cost scales with how many exist at once - not with anything CPU-throttleable, since it's driven by stream count, not per-quantum processing. lazy_capture already limits this to on-screen nodes, but on views where many nodes are visible simultaneously (a busy Output Devices tab, a tall terminal), that alone doesn't bound the concurrent stream count. Adds max_concurrent_captures: Option<usize> (unset = current unbounded behavior). When set and more nodes are eligible for capture than the cap allows, which ones are actually captured rotates on a fixed 3s interval (deliberately much slower than render cadence - rotating every frame would create more stream churn than not capping at all) so every eligible node eventually gets sampled rather than whichever ones happened to become eligible first holding their slot indefinitely. Meters for nodes outside the active window keep showing their last captured value until their next turn, rather than resetting to zero. The cap is enforced as a hard invariant directly in start_capture() (not just in the rotation logic), so it can never be transiently exceeded even if several nodes become eligible at once before the next rotation tick runs. Verified live against the real PipeWire graph (not just unit tests): with --max-concurrent-captures 2, `pw-dump` showed exactly 2 wiremix-capture streams at any moment, and the actual target node IDs fully changed after the 3s interval elapsed - confirming both the cap and the rotation are real, not just passing in isolation. Tested: cargo test (148/148, including 4 new tests covering the cap being enforced by start_capture, rotation respecting the cap, the active window actually advancing between rotations, and the no-op case where fewer nodes are eligible than the cap), cargo fmt --check / cargo clippy -- -D warnings / cargo doc (matching wiremix's CI) all clean.
StreamRegistry::add_stream() evicted an existing stream from its map without calling disconnect() on it first, unlike remove(), which already did. Dropping the evicted StreamRc alone destroys the client-side stream object (pw_stream_destroy) without ever calling pw_stream_disconnect, so the corresponding PipeWire node can be left registered in the graph until something else tears it down. This path is hit every time a capture is renewed after CaptureEligibility::NeedsRestart, since that calls start_capture() again for an object_id that already has an active stream.
…stances Extends max_concurrent_captures with a second, optional cap that counts "wiremix-capture" nodes across the whole PipeWire graph rather than just this instance's own captures - a best-effort budget shared by every running wiremix instance that also sets this option. The naive approach of gating solely on the live graph reading undercounts this instance's own just-issued captures: node_capture_start() is fire-and-forget over an async channel to a separate PipeWire thread, and handle_events() drains a whole burst of eligibility events synchronously before any of that burst's own captures can round-trip back into local state. Combining the graph reading with capturing_objects.len() (always exact and lag-free, since it updates the instant this instance decides to capture something) closes that gap without changing the inherent, documented cross-instance best-effort behavior. Depends on the capture-stream disconnect fix (separate PR) - without it, repeated CaptureEligibility::NeedsRestart churn can leave orphaned capture nodes in the graph that inflate what every instance perceives as the current global count.
The previous design jumped the entire max-sized capture window forward every fixed 3-second wall-clock interval: every currently-captured object went stale for the full interval, then all of them changed at once. That felt frozen, then jumpy, rather than real-time - and the 3-second constant had no relationship to fps or eligible-object count, so coverage latency could be far worse than the interval itself suggested (a fixed max per rotation, however many objects are waiting). Replaced with two changes: - Slide the window by exactly one object per tick instead of jumping it by the full window size, so at most one capture starts and one stops per tick and everything else is left alone - a continuous trickle instead of a batch swap. - Tie the tick itself to the render loop's own frame cadence (every Nth actually-rendered frame) instead of an independent wall-clock Duration, so rotation speed - and the PipeWire stream churn it costs - scales automatically with whatever fps a user has configured, with no second unrelated timer to reason about. rotate_capturing() is now only ever called in step with a rendered frame (moved its call site accordingly), so counting frames is exact. Verified live against the real PipeWire graph: sampled which specific objects a running instance was capturing every ~300ms and confirmed the pair slides by one each time rather than jumping to an unrelated pair.
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
1 participant
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.
That being said, I hope these can be helpful and useful additions that users could appreciate.
Depends on #66 (
max_concurrent_captures) and #69 (capture-stream disconnect fix) - this branch is stacked on both, so the diff shown here includes their changes too until they merge. Please review/merge those first if possible; happy to rebase this once they land.#66 added
max_concurrent_capturesto cap how many nodes one instance actively monitors at once. This adds a second, optional setting,max_concurrent_captures_global, that extends the same idea across every running wiremix instance:"wiremix-capture"-named nodes across the whole PipeWire graph (not just this instance's owncapturing_objects) and refuses to start a new capture once that count reaches the configured max."wiremix-capture"node name) back off - only instances that also set this option consider each other.max_concurrent_captures- both are checked independently, so whichever is more restrictive applies.What made this hard to get right, and what #69 and the self-lag fix here actually address:
Simply counting the live graph state and comparing it to the configured max isn't enough on its own, for two separate reasons I found through live testing against a real, busy PipeWire graph (not just the mock-based unit tests):
Orphaned capture nodes on restart (fixed in Disconnect evicted capture streams before dropping them #69, a prerequisite for this to behave well over time): every time a capture gets renewed after
CaptureEligibility::NeedsRestart, the old capture stream needs to be properly disconnected before being replaced. It wasn't -StreamRegistry::add_stream()'s eviction path skipped thedisconnect()call thatStreamRegistry::remove()already did, so repeated restarts (flapping nodes, or normal churn from streams reconnecting) could leave a growing number of orphaned nodes in the graph, inflating what every instance perceives as the current global count.This instance's own captures being invisible to its own count during a startup burst (fixed here):
node_capture_start()is fire-and-forget over an async channel to a separate PipeWire thread, andhandle_events()drains a whole burst ofCaptureEligibility::Eligibleevents synchronously (itswhile let Ok(event) = self.rx.try_recv()loop) before any of that burst's own just-issued captures can round-trip back into local state. So a flood of eligible nodes at once (e.g. at startup) sees the same pre-burst graph count for the entire burst, letting a single instance blow well past its own configured budget - confirmed live: one instance withmax_concurrent_captures_global = 20and no other instances contending ended up with 28 of its own active captures.capturing_objects.len()has no such lag (it updates the instant this instance decides to capture something, before any round trip), so folding it in as a floor closes the gap.With both fixes in place, live-tested repeatedly: a test instance configured with
max_concurrent_captures_global = 20landed at exactly 20 of its own captures at startup, stayed exactly at 20 under sustained artificial restart-churn (repeatedpw-catconnects/disconnects to forceCaptureEligibility::NeedsRestart), and cleanly returned to baseline with nothing orphaned on shutdown - verified viapw-dump, cross-referencing each"wiremix-capture"node's owning client/PID against the running instance.Tested:
cargo test --release: 150/150 passingcargo fmt --check/cargo clippy -- -D warnings/cargo doc(matching this repo's CI): all cleanmax_concurrent_captures_globalunset reproduces stock behavior exactly