feat(audio): add FFT signal-path diagnostics (slice 3)#56
Merged
Conversation
Slice 3 instrumentation for the braille-visualisers-don't-react bug. Adds a per-~60-frame `trace!` in the TUI render loop, gated behind `RUST_LOG=clitunes::audio=trace`, that reports frames read from the SPMC consumer, peak windowed sample, and peak FFT magnitude. This isolates whether the signal is lost upstream (empty ring on the Spotify path) or downstream (visualiser clamp math). Also adds a `snapshot_from` regression test that proves a known- amplitude sinusoid survives the Hann-windowed FFT tap: windowed peak tracks input amplitude within 5%, and the magnitude peaks at the input frequency bin. If this test ever fails, the break is inside the tap and the diagnostic traces can be skipped. Refs clitunes-of7 / CLI-88 (left open pending live-Spotify trace).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Summary
Slice 3 diagnostics for the braille-visualisers-don't-react-to-music bug (CLI-88 / clitunes-of7). Instrumentation only — no root-cause fix yet.
tracing::trace!in the TUI render loop, once per ~60 frames, gated behindRUST_LOG=clitunes::audio=trace. Reportsframes_readfrom the SPMC consumer, peak windowed sample, peak FFT magnitude, and negotiated sample rate.FftTap::snapshot_fromproving a known sinusoid preserves amplitude (within 5% of the Hann-windowed peak) and that the FFT magnitude peaks at the input frequency bin.Why
The user reports braille visualisers don't visibly react to Spotify playback. Two candidate failure modes:
Walking the code path the plumbing looks sound —
SpotifySource::runpulls frames off the librespot sink and callsTeeWriter::write, which fans out to both the cpal output ring AND the SPMC producer the TUI opens viaPcmTap. Radio uses the sameTeeWriter, and the user has confirmed radio visualisers work — so a silent SPMC path on Spotify would be a surprising regression.Highest-confidence hypothesis (pending live-Spotify trace): signal is present on the SPMC ring but the per-viz clamps are tuned hot enough that only loud transients register at reasonable playback volumes. The
snapshot_fromtest now pins the invariant that the FFT tap itself is not dropping signal, so if live traces show non-zeropeak_samplewe know the fix belongs in the visualiser layer (slice 2 territory, handled in parallel).Leaving CLI-88 / clitunes-of7 open until the live trace lands.
Test plan
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace --all-features(all green, including the newsnapshot_from_preserves_sine_amplitude_and_peaks_at_frequency)RUST_LOG=clitunes::audio=trace cargo run -p clitunesagainst Spotify playback, confirmpeak_sampleandpeak_magare non-zero once a track starts, and file the follow-up fix based on what the numbers say.