Add visualizer@v1 with beats, peaks, pitch, and frequency - #259
Merged
Conversation
Negotiates the visualizer@v1 role (downbeat-aware msg 17 frames). BeatTiming flows through BeatHandler, BeatState, and the render_beat_strip pipeline; downbeats render as a filled square while regular beats stay as filled circles. Downbeat wins when both land on the same cell. Schedule appends are deduplicated by timestamp so repeated batches don't paint the same beat twice on the timeline.
The dominant-frequency and pitch arrows shared one cursor row; give each its own line (f_peak above pitch) with the footer below. Each row is gated on the visualizer type the server negotiated in stream/start, so a row is only reserved when its data can actually arrive.
Fold the timeline strips and tonal cursor rows into one keep-priority row budget (peaks, beats, f_peak, pitch, footer). Each strip needs its type in the server-negotiated set, so a strip is hidden when the server won't send it instead of reserving an always-empty row.
Show the pitch arrow and label whenever a confident readout arrives, even if the server didn't list "pitch" in the stream/start types. f_peak and the timeline strips stay type-gated.
Keep the pitch cursor row when "pitch" is negotiated so the arrow appearing and vanishing no longer shifts the spectrum and f_peak rows. Show the arrow whenever a pitch readout exists, regardless of confidence.
Review findings on the visualizer feature: - Reset and detach the peak handler on disconnect and shutdown, and clear the peak strip, matching the beat handler. A reconnect no longer fires a stale asyncio timer or stale peaks against the new stream. - Gate BeatState.is_active on the decaying pulse instead of "a beat ever landed", so the high-rate visualizer refresh idles when paused or silent. - Drop the last-seen loudness/pitch/f_peak on stream/clear and stream/end so a stale readout from one stream can't ride into the next. - Remove the now-unused pitch_confidence plumbing left after the confidence gate was dropped.
The beat strip used to be the only carrier of the "now" cursor, so a stream with peaks but no beats had no time reference. Draw the center playhead on the peak strip too, colored to match the beat strip's.
Contributor
|
Why does it require the server to be released? Shouldn't the server, without support for the role, just ignore it? |
Member
Author
According to the spec, yes. |
Contributor
There was a problem hiding this comment.
Pull request overview
Upgrades the TUI visualizer integration to visualizer@v1, rendering all server-streamed analysis time-aligned to the playhead (spectrum + beats/peaks timelines + tonal cursors) and updating negotiation/support accordingly.
Changes:
- Add beat/peak event scheduling handlers and extend visualizer frame bridging to carry loudness/pitch/f_peak onto spectrum frames.
- Implement beat/peak timeline strips, pitch + dominant-frequency cursors, and updated spectrum rendering behavior in the TUI.
- Expand/refresh tests and update documentation + dependency versions for the new visualizer role.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Bumps locked dependency version for aiosendspin to support visualizer@v1. |
pyproject.toml |
Updates aiosendspin[server] dependency constraint to ~=6.0.1. |
sendspin/visualizer_connector.py |
Adds v1-aware frame handling plus new BeatHandler and PeakHandler schedulers. |
sendspin/tui/visualizer.py |
Adds beat/peak state + strip rendering, tonal helpers/cursors, and spectrum beat-pulse + server f_peak support. |
sendspin/tui/ui.py |
Integrates timelines/cursors into the visualizer layout and adds UI APIs for types/clock/beat/peak updates. |
sendspin/tui/app.py |
Negotiates v1 visualizer types, wires handlers into the app lifecycle, and forwards negotiated types to the UI. |
tests/tui/test_visualizer.py |
Adds extensive coverage for new visualizer rendering/state helpers. |
tests/test_visualizer_connector.py |
Adds coverage for loudness carry-over and stream-clear dropping stale tonal values. |
README.md |
Updates visualizer docs to visualizer@v1 and describes new rendered elements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
223
to
+227
| def _needs_visualizer_refresh(self) -> bool: | ||
| """Check if the visualizer needs periodic refreshes for interpolation.""" | ||
| return self._state.visualizer_enabled and self._state.visualizer_state.is_active | ||
| if not self._state.visualizer_enabled: | ||
| return False | ||
| return self._state.visualizer_state.is_active or self._state.beat_state.is_active |
Comment on lines
+785
to
+789
| """Build the visualizer as raw Text rows, totaling `height`. | ||
|
|
||
| Reserves the top row for the beat timeline strip when there's room; | ||
| the remaining rows are the spectrum. | ||
| """ |
Comment on lines
+454
to
+460
| def place(timestamp_us: int, glyph: str, style: str, *, downbeat: bool) -> None: | ||
| offset_us = timestamp_us - now_us | ||
| if abs(offset_us) > half_us: | ||
| return | ||
| cell = center + int(round((offset_us / half_us) * (width / 2))) | ||
| if not 0 <= cell < width: | ||
| return |
Comment on lines
+475
to
+476
| # Playhead overlays whatever was at center. Grows on beat pulse: | ||
| # idle = thin ┃, mid pulse = heavy ┃, peak pulse = full block █. |
Comment on lines
+534
to
+540
| def place(timestamp_us: int, strength: int, color: str) -> None: | ||
| offset_us = timestamp_us - now_us | ||
| if abs(offset_us) > half_us: | ||
| return | ||
| cell = center + int(round((offset_us / half_us) * (width / 2))) | ||
| if not 0 <= cell < width: | ||
| return |
maximmaxim345
added a commit
that referenced
this pull request
Jun 11, 2026
Follow up to #259. Hides the visualizer panel when the server doesn't activate `visualizer@v1`, instead of showing an empty panel that never receives data. Also keeps the peak strip animating when only peaks are active, and corrects two stale visualizer comments.
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.
Upgrades the TUI visualizer from the old
_draft_r1draft to the newervisualizer@v1role, rendering everything the server streams time-aligned to the playhead: a beats strip with estimated BPM and downbeats, a peaks strip of energy onsets, and pitch and dominant-frequency (f_peak) cursors over the spectrum.Only data that is sent by the server is displayed. For example, Music Assistant 2.9 does not support pitch.