Repository navigation
Playback: audio callback load and dropout meter (System Usage, ui.audio_load) - #1067
sanderdewijs wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Callback metering and Player access crates/playback/src/load.rs, crates/playback/src/stream.rs, crates/playback/src/lib.rs |
The callback records render time and buffer duration. The load meter publishes average and peak load over 0.5 seconds and tracks worst load, late callbacks, callback count, and buffer duration. Player provides snapshot and reset methods. Silent-clock players initialize an unmeasured meter. Tests cover publication, reset, and invalid timing values. |
System Usage and command reporting crates/ui-egui/src/panels/windows.rs, crates/ui-egui/src/menus.rs, crates/ui-egui/tests/it/*, docs/gaps.md, docs/hardware-parity.md |
The System Usage window displays load metrics and provides a reset control. The ui.audio_load command returns measurements and can request a reset. Integration tests check the command without an audio engine. Documentation records audio-load reporting and platform status. Plugin-instance totals now include track instruments. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant AudioCallback
participant LoadMeter
participant Player
participant UICommand
participant SystemUsage
AudioCallback->>LoadMeter: record render time and buffer duration
SystemUsage->>Player: request audio_load snapshot
Player->>LoadMeter: read snapshot
LoadMeter-->>Player: return published measurements
UICommand->>Player: retrieve or reset audio_load
Player->>LoadMeter: read or request reset
Player-->>UICommand: return serialized measurements
Suggested reviewers: dylanmtaylor, echelon
Merge Risk: 
Minimal · up to 58c54
-
No actionable merge-blocking risk was identified; the change is ready for normal validation.
Security architecture risk: 
Low · up to 58c54
-
The new command reports aggregate callback statistics and can request a statistics reset; it does not change playback or device settings. The inspected ownership and recovery paths keep this control separate from audio processing. Remaining uncertainty concerns end-to-end command access and platform behavior, rather than an identified security defect.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — For a caller already able to invoke the UI command interface, the new operation is bounded to reading the active Player's aggregate metrics and requesting that meter's counter reset. Its parameters provide no selector for another session, device, file, or service.
Security Findings and Attack Paths
- inferred — The inspected new parameter-to-state path terminates at a snapshot read and an atomic reset request. It does not introduce a path from command parameters to device reconfiguration, plugin execution, or transport mutation. No security defect was established on this path.
Trust Boundaries and Controls
- observed — The new command is registered in the existing UI command dispatcher, which SoundApp::run invokes before engine commands. The dispatcher infrastructure is unchanged relative to the PR base; this establishes reuse of that path, not independent verification of external transport authentication.
Resilience and Maintainability Implications
- inferred — Failure containment remains separate from telemetry: guarded rendering handles panic and silencing, while the meter records after the rendering block. Reset does not clear fault flags or resume playback, and replacement prevents an old stream from publishing into the new Player's meter.
-
Pre-merge checks: 
4
-
Full details: Title check
-
Explanation
The title names the two main parts of the change: an audio callback load and dropout meter in Playback, and its exposure in System Usage and through the ui.audio_load command. The changeset adds LoadMeter and AudioLoad in load.rs, wires the meter into the output stream callback, adds the System Usage display and reset button, and adds the audio_load_command with its integration test, so the title matches the main change.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
fbd8877 to
691dd3b
Compare
…io_load) Each device callback is timed against how long its buffer lasts. Window > System Usage shows the average and peak load over the last half second, the worst load and the callbacks that ran past their buffer (dropouts); ui.audio_load returns the same for agents. The audio thread is the only writer (atomics, no locks or allocation). System Usage also counts instruments as plugin instances. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
691dd3b to
58c54c0
Compare
|
Merge-order note: this PR and #1076 each pass on their own, but they interact. #1076 makes
|
eliets2
left a comment
There was a problem hiding this comment.
First-pass review notes (automated; combined from specialist angles: tests, UI code, user experience, docs and text, API and interfaces). Please treat as suggestions and verify before acting.
Summary: This adds an audio callback load and dropout meter: the playback thread times each callback against its buffer duration and publishes average, peak, worst, late-callback count, and buffer size through atomics, shown in Window ▸ System Usage with a Reset button and exposed to agents via ui.audio_load {reset?}; it also changes System Usage to count instruments as plugin instances.
Logical units: Load measurement in crates/playback/src/load.rs; System Usage UI in crates/ui-egui/src/panels/windows.rs; ui.audio_load command; plugin-instance counting fix; unit/integration tests and docs updates.
Findings:
- [tests]
crates/playback/src/load.rs,LoadMeter::record: the guard acceptsaudio = INFINITYand a finitebusy / audiothat overflows to infinity, sobuffer_ms,worst,peak, andaveragecan be published as non-finite, andhostile_timings_are_ignoreddoes not cover these cases. Fix: requireaudio.is_finite()and a finite computed load, and add those cases to the hostile-timings test. - [API and interfaces]
crates/playback/src/load.rs,LoadMeter::reset/record: reset clears onlyworst/late/callbackson the next callback, notaverage,peak,buffer_ms, or the in-progressLoadWindow, so a snapshot afterreset:truemixes pre-reset average/peak with post-reset counters, and a reset with no following callback leaves the old snapshot visible. Fix: clear the published counters inreset()itself (or show a pending state), and pin the intended semantics with tests for average/peak and the partial window across a reset and for reset-then-snapshot before any callback. - [UI code]
crates/ui-egui/src/panels/windows.rs,system_usage: the load bar usesload.peak.clamp(0.0, 1.0)with no text, so any overload above 100% — the dropout case this meter exists to show — renders identically to exactly 100%, and the bar showspeakwhile the label leads withaverage. Fix: label the bar as peak, add the value as the bar's text, and add an overload indication (e.g. red fill/text) whenpeak > 1.0orlate > 0. - [UI code]
crates/ui-egui/src/panels/windows.rs,system_usage:ctx.request_repaint_after(...)is only called in theload.measuredbranch, so if the window is opened while unmeasured, "Audio load: not measured" will not refresh on its own when measurement starts. Fix: request the repaint in both branches. - [user experience]
crates/ui-egui/src/panels/windows.rs,system_usage: the "Late callbacks (dropouts): … · worst …%" label and the Reset button are in oneui.horizontalinside a fixed 320px window, so the long label can push the button out or clip it at large counts. Fix: put the late/worst text on its own line above the button, or shorten the label. - [user experience]
crates/ui-egui/src/panels/windows.rs,system_usage: a non-zero dropout count is rendered as plain text with no visual distinction from the normal case. Fix: highlightlate > 0with a warning color/icon so dropouts are scannable. - [user experience]
crates/ui-egui/src/panels/windows.rs,system_usage:{:.0}%for average/peak/worst rounds low loads (e.g. 0.4%) to0%, hiding a healthy-but-nonzero reading. Fix: use one decimal place, or adaptive precision below 10%. - [user experience]
crates/ui-egui/src/panels/windows.rs,system_usage: the unmeasured branch only says "Audio load: not measured", with no reason. Fix: state why measurement is unavailable. - [tests]
crates/ui-egui/tests/it/audio_load.rs,the_audio_load_is_unmeasured_without_an_audio_engine: only the no-player path is tested, with no test thatreset: truereturns the pre-reset values and then clears them. Fix: add that reset test.
Overlaps: None listed.
Verdict: Needs changes. The 3 most important points are: (1) non-finite load values can be published from infinite/overflowing timings; (2) reset leaves stale/mixed average, peak, and window data, including when playback is stopped; (3) the peak bar is clamped to 100% with no overload indication, hiding the dropouts the meter is meant to show.
What
A load and dropout meter for the audio callback.
ui.audio_load {reset?}returns the same for agents:measured,average,peak,worst,late,callbacks,buffer_ms. The MCP reaches it throughexecutewith--connect.How
crates/playback/src/load.rs: the audio thread is the only writer. It keeps a local half-second window and publishes through atomics, so there are no locks and no allocation in the callback. The UI asks for a reset with a flag, and the audio thread clears the counters at its next callback. cpal doesn't pass on the device's own overload notices, so late callbacks are counted here instead. On the web (wasm32),Instantisn't available and the meter reportsmeasured: false.Why
Glitches during realtime playback were hard to tell apart: plug-in behaviour, scheduling jitter, or plain CPU shortage. Without a number, every fix was a guess. On a real 19-track session the meter immediately separated two causes:
It also gives #886, #1006 and #804 a before/after measure on the same session.
Tests
load.rsunit tests: per-window averaging and peaks, late counting, worst, reset, and hostile timings (NaN, infinite, zero and negative durations are ignored).ui-eguiintegration test:ui.audio_loadwithout an audio engine reportsmeasured: falseand every field, and the command is listed for agents.ui.audio_loadshow the live values.cargo xtask cipasses locally (macOS). Docs:gaps.md(G-01),hardware-parity.md(new row: not yet verified on Windows, Linux or FreeBSD; not measured on the web).🤖 Generated with Claude Code
Summary by CodeRabbit
ui.audio_loadcommand provides audio-load metrics and supports resetting them.