Repository navigation
Conversation
12 tasks done
barrulus
marked this pull request as ready for review
October 14, 2026 20:01
There was a problem hiding this comment.
Talia preliminary review
| Check | Result |
|---|---|
| Merge score | 🟢 4/5 — No issues found; likely ready after CI and human review |
| Findings | None found |
| Review depth | 3/5 — how thoroughly the changed code could be checked by reading it |
| Coverage | 24 of 24 changed files reviewed |
What Talia checked: Inspected every changed hunk and callers: IPC v2 bands parse and 4096 limit, registry/output 65-float latching, scene audio uniform names/reflection, GLSL index math, docs (64-entry example) and updated tests; no reachable introduced defect found.
Run details
- Commit reviewed: 7bbb1e0
- Source lookups: 39 file reads, 32 searches
- Prompt version: 859b7490ca3d
Preliminary automated review: Talia read the changed code and related source; nothing was built, run or tested, and AI can make mistakes. It is not a scope acceptance or a promise to merge; a human reviewer makes the final call. See our ethos.
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.
Summary
Expose externally supplied 64-band audio snapshots to effects through
umbriel_audio_spectrum(position). Noctalia can supply its existing spectrum through the companion provider, allowing effects to react to different parts of the spectrum while capture and analysis remain outside the compositor.The
effect-audiov2 contract accepts exactly 64 finite values in [0, 1]. Sampling clamps normalized positions and interpolates between adjacent bands. A single-owner connection, acknowledgements, 4096-byte message bound and 250 ms expiry bound the feed. Documentation, bundled shaders and configuration guidance use the spectrum API.Breaking change: replaces the v1 scalar payload and removes
umbriel_audio_level(). External producers must send v2bands; custom shaders must sample or aggregate the spectrum. The band order is low-to-high; no frequency-to-Hz mapping is promised.Motivation
A scalar level makes every reactive effect follow the same signal. Sharing Noctalia's spectrum gives shaders independent band responses without introducing another audio capture process in Umbriel. This stays within the external-input mechanism for compositor effects.
Type of Change
Testing
just lintandjust formatpassed; configuration example validation andgit diff --checkpassed.effect/audio,effect/audio_kindsandeffect/audio_spectrumharness checks passed. Coverage includes ownership, ACKs, message limits, invalid input, expiry, lock/unlock, renderer recovery, frame-rate limits, output isolation, all 64 bands, interpolation and clamping. Spectrum stress passed 4/4 runs.effects-audio-spectrumandeffects-output-effectsGPU suites passed on both Intel and NVIDIA render nodes.Bounded performance comparison
One paired Tracy capture compared baseline
693b6899with implementationa9a06a8f, using the same profiling-only instrumentation and Tracy 0.13.1 toolchain. The desktop was exited for the comparison: Intel ARL/i915, Mesa 26.2.4, two hardware-headless 1280x720 outputs at 60 Hz, one mapped window and one full-screen audio shader on output A, no audio consumer on B. Both revisions received the same wave; the candidate supplied it in all 64 bands. Pixel checks passed.The draw zone includes uniform/state/draw work; it is not a pure shader-ALU measurement. Nested zones are not added together. The feed was ACK-limited and headless presents do not establish physical scanout pacing. With profiling disconnected, neither revision produced extra effect frames on unused output B or during fresh silence/no feed. Process CPU was 7.33% → 8.17% of one core active, 1.25% → 1.75% during fresh silence and 1.75% → 1.75% without a feed; short sampling windows limit precision.
The comparison measured additional GPU cost. No compositor budget has been agreed, and native border/smoke performance was not measured, so these results do not establish performance-budget compliance.
Manual Coverage
Checklist
SCOPE.md, or its scope was agreed in an issue or on Discord first.CONTRIBUTING.md.just format, or this PR has no C++ changes.docs/andexamples/config.toml, or this PR does not change user-facing configuration or behavior.Additional Notes
Companion: noctalia-dev/noctalia#4860