feat: add realtime peak envelope visualizer (#87)#102
Conversation
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/eq-equalizer-86 #102 +/- ##
======================================================
+ Coverage 77.7% 78.3% +0.6%
======================================================
Files 127 128 +1
Lines 6399 6562 +163
Branches 2041 2048 +7
======================================================
+ Hits 4976 5143 +167
+ Misses 1379 1375 -4
Partials 44 44
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Greptile SummaryThis PR adds a realtime peak envelope visualizer for fullscreen playback. The main changes are:
Confidence Score: 5/5Safe to merge with minimal risk. The peak publishing path avoids playback mutex reads from IPC, output wiring uses rendered sample counts correctly, and frontend polling handles stale, reordered, slow, and unmounted responses. No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CPAL as CPAL output callback
participant Render as render_output_buffer
participant Acc as PeakAccumulator
participant Ring as PeakRing
participant IPC as get_audio_peaks
participant UI as PeakMeter
CPAL->>Render: render mixed samples
Render->>Render: apply EQ, limiter, fade
Render->>Acc: process(output, rendered, channels)
Acc->>Ring: push(stereo peak) every 512 frames
UI->>IPC: poll at 30 Hz
IPC->>Ring: snapshot()
Ring-->>IPC: writeIndex + peaks
IPC-->>UI: AudioPeakSnapshot
UI->>UI: draw waveform or flat baseline
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CPAL as CPAL output callback
participant Render as render_output_buffer
participant Acc as PeakAccumulator
participant Ring as PeakRing
participant IPC as get_audio_peaks
participant UI as PeakMeter
CPAL->>Render: render mixed samples
Render->>Render: apply EQ, limiter, fade
Render->>Acc: process(output, rendered, channels)
Acc->>Ring: push(stereo peak) every 512 frames
UI->>IPC: poll at 30 Hz
IPC->>Ring: snapshot()
Ring-->>IPC: writeIndex + peaks
IPC-->>UI: AudioPeakSnapshot
UI->>UI: draw waveform or flat baseline
Reviews (3): Last reviewed commit: "fix(peaks): unify snapshot() and test in..." | Re-trigger Greptile |
6e0d280 to
efbc61b
Compare
Maintainer acceptance — merge slot 7, strictly after repaired #114BLOCKED — changes required. Blocking frontend polling race
#87 explicitly requires one request in flight under slow IPC and stale-response suppression. Add a shared in-flight/sequence guard, accept only monotonic newer snapshots, and test delayed out-of-order responses plus visibility lifecycle. GitHub currently reports this PR as |
cbcbf4b to
9195e94
Compare
9195e94 to
3fe11c6
Compare
Re-review: blocking async polling race fix requiredThis PR depends on #114 and must remain unmerged until #114's DSP correction is accepted. Independently, the current Implement exactly the following:
Add deterministic frontend tests with deferred IPC promises:
After #114 merges, rebase this branch onto the resulting |
407f151 to
8a1e762
Compare
222c48f to
5d4f34a
Compare
Re-review — still blocked: PeakMeter still creates overlapping IPC pollsReviewed at This has a user-visible failure mode: when IPC takes longer than ~33 ms, every response is invalidated by a newer timer tick, so the meter can stop rendering entirely while continuously issuing concurrent requests. Implement the required single-flight coordinator inside
Do not alter the Rust ring buffer to compensate for this frontend scheduling defect. |
850cf0c to
a3921ab
Compare
55aaddf to
7d232fc
Compare
Re-review — blocked: the peak snapshot cursor can be newer than the copied dataReviewed at
If the writer advances between those two operations, the frontend accepts Please make the reported cursor describe exactly the copied snapshot: return Add a deterministic regression that forces a write after the retry copy and proves the returned cursor is not ahead of the copied entries; retain the Acquire/Release publication contract and the lossy, lock-free design. Re-run the Rust suite after the fix. |
Re-review — core cursor fix is correct, but the required deterministic regression is still missingReviewed at However, Please add deterministic coverage for that interleaving (a narrowly scoped test-only synchronization hook around the retry copy is acceptable, or an equivalent barrier-based helper). Arrange:
Keep the public API and lock-free/realtime contract unchanged. No LGTM until the race that caused this bug is protected by a regression test. |
PeakRing::snapshot retry path copies entries for second_index and returns (second_index, retry). The original bug returned a third_index loaded after the retry copy, which could be ahead of the copied data if the writer advanced between the retry copy and that final load. Added a test-only snapshot_with_intercepts helper with two interception points: 1. after_first_copy: push to force the retry path 2. after_retry_copy: push to advance write_index past second_index The deterministic regression test forces a write after the retry copy and asserts the returned cursor equals second_index (not the later write_index), and the last peak matches the value at second_index-1.
PeakRing::snapshot retry path copies entries for second_index and returns (second_index, retry). The original bug returned a third_index loaded after the retry copy, which could be ahead of the copied data if the writer advanced between the retry copy and that final load. Added a test-only snapshot_with_intercepts helper with two interception points: 1. after_first_copy: push to force the retry path 2. after_retry_copy: push to advance write_index past second_index The deterministic regression test forces a write after the retry copy and asserts the returned cursor equals second_index (not the later write_index), and the last peak matches the value at second_index-1.
03e4af4 to
f2f6c37
Compare
|
Re-review of head The cursor regression blocker is resolved. Production Do not merge/approve the stacked feature yet: this head predates the latest #114 commit |
6c616cb to
9e29433
Compare
PeakRing::snapshot retry path copies entries for second_index and returns (second_index, retry). The original bug returned a third_index loaded after the retry copy, which could be ahead of the copied data if the writer advanced between the retry copy and that final load. Added a test-only snapshot_with_intercepts helper with two interception points: 1. after_first_copy: push to force the retry path 2. after_retry_copy: push to advance write_index past second_index The deterministic regression test forces a write after the retry copy and asserts the returned cursor equals second_index (not the later write_index), and the last peak matches the value at second_index-1.
b4345c5 to
1173bb3
Compare
Lock-free atomic ring buffer in the CPAL output callback publishes sanitized stereo peak pairs (one per 512 rendered frames, 256-pair capacity) after EQ, limiter, and fade. New read-only get_audio_peaks IPC command copies the ring snapshot without holding the playback mutex. DPR-aware PeakMeter canvas polls at 30 Hz and renders the scrolling waveform in fullscreen player controls.
Mock HTMLCanvasElement.getContext in jsdom so the drawing path is exercised. Adds tests for baseline rendering, writeIndex skip/redraw, and multi-bar rendering to meet the 80% patch coverage target.
- Fix double-counted sample count in peak accumulator (output.rs): 'rendered' is already an interleaved sample count (frames × channels); multiplying by device_channels again caused the peak meter to process channels× too many frames, publishing envelope pairs too frequently. - Fix stale peaks remaining visible when playback stops (PeakMeter.tsx): the redraw-skip optimization froze the canvas on the last waveform when the backend stopped advancing writeIndex. Added a staleness fallback that redraws the flat-line state after a 500 ms grace period. - Add test covering the stale-after-playback-stops case.
Supply eq fields and get_audio_peaks in the e2e mock so Settings opens without crashing and PeakMeter polls cleanly. Cover stale writeIndex flat-baseline path after stop.
Assign a monotonic generation per poll so slower earlier responses cannot overwrite a newer snapshot or paint after teardown.
… dropped Drive two overlapping polls with deferred resolves so a late older snapshot cannot redraw after a newer poll has already applied (requestGeneration).
The snapshot retry path loaded a third write_index after copying from second_index and returned it as the cursor. If the writer advanced between the copy and the final load, the returned cursor pointed past data that was never included in the snapshot, so the frontend could treat unobserved entries as already-seen and skip them on the next poll. Return second_index — the index actually used for the copy — so the cursor always matches the data the reader received.
PeakRing::snapshot retry path copies entries for second_index and returns (second_index, retry). The original bug returned a third_index loaded after the retry copy, which could be ahead of the copied data if the writer advanced between the retry copy and that final load. Added a test-only snapshot_with_intercepts helper with two interception points: 1. after_first_copy: push to force the retry path 2. after_retry_copy: push to advance write_index past second_index The deterministic regression test forces a write after the retry copy and asserts the returned cursor equals second_index (not the later write_index), and the last peak matches the value at second_index-1.
When playback is paused or stopped, the flat-line fallback was being redrawn on every 30 Hz poll cycle even though the canvas content is static. Track a flatLineDrawn flag so the flat baseline is drawn exactly once, then skip redraws until new peaks arrive (writeIndex advances). Also correct the ring duration comment: 256 pairs × 512 frames at 44.1 kHz = ~3 s, not ~5.8 s.
…ty peak When the component mounts and the backend returns non-empty peaks with a writeIndex that doesn't advance from the initial value, lastAdvanceRef was never set, so the flat-line fallback never triggered. Initialize the grace period on the first static non-empty poll so the canvas eventually flat-lines.
The peak ring retains 256 pairs × 512 frames = 131,072 frames, which at 44.1 kHz is ~2.97 s, not ~5.8 s. The PeakMeter.tsx comment already stated ~3 s; the playback contract doc and CHANGELOG entry still carried the incorrect ~5.8 s figure.
6bafe2d to
7fb5f8c
Compare
…peak-envelope-visualizer-87 # Conflicts: # CHANGELOG.md
Fix three issues with fullscreen player lyrics: 1. Lyrics frozen at entry position: readLyricsPlaybackClockMs and readLyricsAdjustedPlaybackMs used AirPlay displayedPositionMs (a stale remote clock) instead of the local playback clock when AirPlay was active. The fullscreen window receives AirPlay state via BroadcastChannel but does not run the AirPlay listener, so the displayed position never updated on local seeks. Lyrics now always use the local playback clock; AirPlay clock is only for CDG sync. Also added transport_generation staleness check in applyPlayerSyncSnapshot to prevent delayed BroadcastChannel messages from regressing the clock. 2. Lyrics not scrolling: getLineScrollMetrics used offsetTop relative to offsetParent — a nested justify-center inner div in audience mode. Lines above the vertical center had negative offsetTop, and getCenteredScrollTop clamped to 0, freezing scroll. Now uses getBoundingClientRect when offsetParent is not the container. Also changed audience lyrics inner container from justify-center to justify-start so all lines are reachable via scrollTop. 3. Audience mode line-click seek now unlocks auto-follow with an idle re-lock timer (4s) instead of immediately resuming tracking. The operator can click a line to jump and browse; after inactivity the view snaps back to the active line — appropriate for an audience-facing second monitor. Standard mode unchanged. Also adds fullscreen player keyboard shortcuts (spacebar play/pause, arrow seek) with tests.
…scrolled containers getLineScrollMetrics computed offsetTop as lineRect.top - (containerRect.top + container.scrollTop), double-subtracting scrollTop because getBoundingClientRect already accounts for scroll position. When the container was scrolled, this produced increasingly negative offsets, getCenteredScrollTop clamped to 0, and the spring target collapsed to the top — causing scrollTop to oscillate between the seek target and 0 after every line change. The formula is now lineRect.top - containerRect.top + container.scrollTop, correctly recovering the content-origin-relative offset regardless of scroll position.
* feat: add 5-band EQ equalizer with auto preamp and soft limiter Add a five-band peaking EQ (60 Hz, 230 Hz, 910 Hz, 3.6 kHz, 14 kHz; ±12 dB per band) with automatic preamp reduction to compensate for positive boosts and a tanh-based soft limiter that engages above 0.85 amplitude. The EqProcessor is wired into the realtime output callback after the source/stem mix and before the play/pause fade, with linear smoothing for gain, preamp, and bypass dry/wet transitions. New set_eq_enabled and set_eq_gains Tauri commands persist to AppConfig and push updates to the active PlaybackController via PlaybackCoordinator. The settings overlay exposes a new SettingsEqSection with per-band sliders, enable toggle, and reset-to-flat. AppSettings now includes eq_enabled and eq_gains_db fields. Closes #86 * docs: fix EQ soft limiter threshold in CHANGELOG The soft limiter engages above 0.95 amplitude (LIMITER_THRESHOLD), not 0.85 as previously stated in the CHANGELOG entry. * fix: update integration tests for render_output_buffer EQ parameter The EQ feature added an `eq_processor` parameter to `render_output_buffer` but the integration tests in `tests/phase2_output.rs` and `tests/phase3_playback_mode.rs` were not updated, causing `cargo test` to fail with E0061 (wrong argument count). Also fixes a clippy `needless_range_loop` warning in the EQ Nyquist guard test. * fix: silence clippy float_literal_f32_fallback in EQ Hertz constructors Rust 1.97 promotes the f32 fallback lint to a hard error under `-D warnings`. Annotate the four `Hertz::from_hz(1.0)` fallback literals in `eq.rs` with `1.0_f32` so CI clippy stays green. * refactor(eq): debounce slider commits, rollback on failure, drop redundant clamp Settings EQ sliders now keep a local draft for immediate visual feedback and commit the full five-value array through a single `setEqGains` IPC call after a 75 ms trailing debounce (or immediately on pointer/key release). This replaces the per-band `setEqBandGain` overlay action, which fired one IPC call per change tick. Both the settings store and the overlay actions now revert to the previous authoritative `eqGainsDb` when `set_eq_gains` fails, so a rejected command no longer leaves the UI showing values the backend did not accept. `set_eq_gains` now reports validation failures as `invalid_playback_state` (with `keep_current_state` fallback) instead of a generic internal error, matching the existing error contract. The post-mix `[-1, 1]` clamp in `render_output_buffer` is removed: the EQ soft limiter already bounds the output, and the clamp masked legitimate headroom during stem mixing. Adds `@testing-library/react` as a dev dependency (already imported by the EQ section tests) and regenerates the Flatpak node sources so the clean-room Flatpak build can resolve it. Adds config-hydration tests covering default, clamped, and non-finite persisted EQ gains. * fix: address review comments on eq-equalizer Add rollback to setEqEnabled and resetEqGains in settings-overlay library-actions: when the backend command fails, revert both the overlay state and the shared settings store to their previous authoritative values instead of leaving stale optimistic updates. Also strengthen the setEqGains rollback test to assert the settings store is reverted, and add a new resetEqGains failure test. * test: cover SettingsEqSection missing lines for 100% patch coverage * test: cover SettingsOverlay EQ action stubs for full patch coverage * fix: surface coordinator EQ apply errors to the caller * fix(e2e): complete AppSettings mock so Settings EQ section opens Missing eq_gains_db in the Playwright Tauri mock left SettingsEqSection with undefined draft gains and crashed the tree (Preferences not found). * fix(eq): store fixed per-frame smooth steps at target change Recomputing remaining/smooth_samples every callback made the 50 ms linear gain ramp buffer-size dependent. Steps are now fixed when targets change and advanced per rendered frame. * fix(eq): update biquad coefficients per-frame for callback-chunk-invariant DSP * fix(eq): make set_eq_enabled/set_eq_gains async to avoid UI freeze set_eq_enabled and set_eq_gains were synchronous Tauri commands that used rx.blocking_recv() to wait for the playback coordinator's reply. Blocking the Tauri command thread freezes the UI while the coordinator processes the command. Both commands are now async fn and use rx.await, matching the pattern already used by the playback commands. Also sync flatpak node sources with the pnpm lockfile to fix CI. * fix(eq): make EQ settings failure-atomic (#114) set_eq_enabled and set_eq_gains previously persisted the new EQ value to config before pushing it to the playback coordinator. If the coordinator rejected the update or the channel was disconnected, the command returned an error after the new value was already on disk, leaving persisted settings divergent from the running audio engine. The commands now apply through the coordinator first and persist only on success. If persistence then fails, the coordinator is reverted to the old value so the engine and config stay consistent. Added focused tests for coordinator-send/reply/apply failure and persistence failure (with coordinator revert). * fix(eq): nyquist-aware preamp and skip filter work while bypassed - Auto preamp now only considers bands that are active at the current sample rate, fixing a volume drop when boosting the 14 kHz band on low-sample-rate audio where that band is skipped by the Nyquist guard. - EqProcessor::process skips per-frame coefficient updates and biquad runs when the dry/wet mix is fully bypassed, eliminating CPU burn while EQ is off. Scalar smoothing still advances so enable transitions resume from the latest target. - Add regression tests for both behaviors. * fix(eq): cancel pending debounce on reset and restore previous toggle on failure * feat: add realtime peak envelope visualizer (#87) Lock-free atomic ring buffer in the CPAL output callback publishes sanitized stereo peak pairs (one per 512 rendered frames, 256-pair capacity) after EQ, limiter, and fade. New read-only get_audio_peaks IPC command copies the ring snapshot without holding the playback mutex. DPR-aware PeakMeter canvas polls at 30 Hz and renders the scrolling waveform in fullscreen player controls. * test: add canvas context mock for PeakMeter drawing coverage Mock HTMLCanvasElement.getContext in jsdom so the drawing path is exercised. Adds tests for baseline rendering, writeIndex skip/redraw, and multi-bar rendering to meet the 80% patch coverage target. * fix: remove unused container variable in PeakMeter test * fix: address review comments on peak-envelope-visualizer - Fix double-counted sample count in peak accumulator (output.rs): 'rendered' is already an interleaved sample count (frames × channels); multiplying by device_channels again caused the peak meter to process channels× too many frames, publishing envelope pairs too frequently. - Fix stale peaks remaining visible when playback stops (PeakMeter.tsx): the redraw-skip optimization froze the canvas on the last waveform when the backend stopped advancing writeIndex. Added a staleness fallback that redraws the flat-line state after a 500 ms grace period. - Add test covering the stale-after-playback-stops case. * test: cover PeakMeter cleanup on unmount for 100% patch coverage * test: replace as any canvas mock cast with typed assertion * docs: renumber EQ commands and sync render-order with peak pipeline * fix(e2e): complete AppSettings mock for peak stack CI Supply eq fields and get_audio_peaks in the e2e mock so Settings opens without crashing and PeakMeter polls cleanly. Cover stale writeIndex flat-baseline path after stop. * fix(peak-meter): ignore stale getAudioPeaks poll responses Assign a monotonic generation per poll so slower earlier responses cannot overwrite a newer snapshot or paint after teardown. * test(peak-meter): assert stale concurrent getAudioPeaks responses are dropped Drive two overlapping polls with deferred resolves so a late older snapshot cannot redraw after a newer poll has already applied (requestGeneration). * fix(peak-meter): add single-flight IPC guard and monotonic write-index check * fix(peak-meter): return cursor matching copied snapshot data The snapshot retry path loaded a third write_index after copying from second_index and returned it as the cursor. If the writer advanced between the copy and the final load, the returned cursor pointed past data that was never included in the snapshot, so the frontend could treat unobserved entries as already-seen and skip them on the next poll. Return second_index — the index actually used for the copy — so the cursor always matches the data the reader received. * chore: sync flatpak node sources and resolve knip.json conflict * fix(peaks): add deterministic race regression for snapshot cursor (#102) PeakRing::snapshot retry path copies entries for second_index and returns (second_index, retry). The original bug returned a third_index loaded after the retry copy, which could be ahead of the copied data if the writer advanced between the retry copy and that final load. Added a test-only snapshot_with_intercepts helper with two interception points: 1. after_first_copy: push to force the retry path 2. after_retry_copy: push to advance write_index past second_index The deterministic regression test forces a write after the retry copy and asserts the returned cursor equals second_index (not the later write_index), and the last peak matches the value at second_index-1. * fix(peaks): unify snapshot() and test interception into one implementation * fix(peak-meter): stop idle 30fps redraws and correct ring duration doc When playback is paused or stopped, the flat-line fallback was being redrawn on every 30 Hz poll cycle even though the canvas content is static. Track a flatLineDrawn flag so the flat baseline is drawn exactly once, then skip redraws until new peaks arrive (writeIndex advances). Also correct the ring duration comment: 256 pairs × 512 frames at 44.1 kHz = ~3 s, not ~5.8 s. * fix(peak-meter): start staleness grace period on first static non-empty peak When the component mounts and the backend returns non-empty peaks with a writeIndex that doesn't advance from the initial value, lastAdvanceRef was never set, so the flat-line fallback never triggered. Initialize the grace period on the first static non-empty poll so the canvas eventually flat-lines. * fix(docs): correct peak ring history duration from ~5.8s to ~3.0s The peak ring retains 256 pairs × 512 frames = 131,072 frames, which at 44.1 kHz is ~2.97 s, not ~5.8 s. The PeakMeter.tsx comment already stated ~3 s; the playback contract doc and CHANGELOG entry still carried the incorrect ~5.8 s figure. * fix(eq): skip Nyquist-guarded bands in preamp calculation and sync coverArtBackdrop across windows * fix(eq): skip coefficient rebuild when smoothing ramps have settled When all gain/preamp/wet smoothing steps are zero (gains stable), the filter coefficients already match the current gains from the last ramp tick. Skip the per-frame Coefficients::from_params rebuild in that case to avoid burning CPU on every audio frame while EQ settings are unchanged. The filter run still executes with the last-valid coefficients. * fix(eq): gate soft limiter on EQ activity to preserve no-EQ fidelity The soft limiter ran on every rendered sample regardless of whether the EQ was enabled, attenuating loud-but-unclipped audio (samples in (0.95, 1.0]) for users who never turned on the EQ. For the default no-EQ single-source path, decoded PCM is in [-1,1] and master/stem gains are <= 1.0, so the mixed value never exceeds 1.0 and the old clamp was a no-op — the limiter only adds unnecessary compression. Add EqProcessor::is_fully_bypassed() and gate the limiter in render_output_buffer so it only runs when the EQ is active or transitioning. This preserves bit-transparent playback for the common no-EQ path while still preventing clipping when EQ boosts or stem summing push levels past 1.0. * feat: add realtime peak envelope visualizer (#87) Lock-free atomic ring buffer in the CPAL output callback publishes sanitized stereo peak pairs (one per 512 rendered frames, 256-pair capacity) after EQ, limiter, and fade. New read-only get_audio_peaks IPC command copies the ring snapshot without holding the playback mutex. DPR-aware PeakMeter canvas polls at 30 Hz and renders the scrolling waveform in fullscreen player controls. * test: add canvas context mock for PeakMeter drawing coverage Mock HTMLCanvasElement.getContext in jsdom so the drawing path is exercised. Adds tests for baseline rendering, writeIndex skip/redraw, and multi-bar rendering to meet the 80% patch coverage target. * fix: remove unused container variable in PeakMeter test * fix: address review comments on peak-envelope-visualizer - Fix double-counted sample count in peak accumulator (output.rs): 'rendered' is already an interleaved sample count (frames × channels); multiplying by device_channels again caused the peak meter to process channels× too many frames, publishing envelope pairs too frequently. - Fix stale peaks remaining visible when playback stops (PeakMeter.tsx): the redraw-skip optimization froze the canvas on the last waveform when the backend stopped advancing writeIndex. Added a staleness fallback that redraws the flat-line state after a 500 ms grace period. - Add test covering the stale-after-playback-stops case. * test: cover PeakMeter cleanup on unmount for 100% patch coverage * test: replace as any canvas mock cast with typed assertion * docs: renumber EQ commands and sync render-order with peak pipeline * fix(e2e): complete AppSettings mock for peak stack CI Supply eq fields and get_audio_peaks in the e2e mock so Settings opens without crashing and PeakMeter polls cleanly. Cover stale writeIndex flat-baseline path after stop. * fix(peak-meter): ignore stale getAudioPeaks poll responses Assign a monotonic generation per poll so slower earlier responses cannot overwrite a newer snapshot or paint after teardown. * test(peak-meter): assert stale concurrent getAudioPeaks responses are dropped Drive two overlapping polls with deferred resolves so a late older snapshot cannot redraw after a newer poll has already applied (requestGeneration). * fix(peak-meter): add single-flight IPC guard and monotonic write-index check * fix(peak-meter): return cursor matching copied snapshot data The snapshot retry path loaded a third write_index after copying from second_index and returned it as the cursor. If the writer advanced between the copy and the final load, the returned cursor pointed past data that was never included in the snapshot, so the frontend could treat unobserved entries as already-seen and skip them on the next poll. Return second_index — the index actually used for the copy — so the cursor always matches the data the reader received. * chore: sync flatpak node sources and resolve knip.json conflict * fix(peaks): add deterministic race regression for snapshot cursor (#102) PeakRing::snapshot retry path copies entries for second_index and returns (second_index, retry). The original bug returned a third_index loaded after the retry copy, which could be ahead of the copied data if the writer advanced between the retry copy and that final load. Added a test-only snapshot_with_intercepts helper with two interception points: 1. after_first_copy: push to force the retry path 2. after_retry_copy: push to advance write_index past second_index The deterministic regression test forces a write after the retry copy and asserts the returned cursor equals second_index (not the later write_index), and the last peak matches the value at second_index-1. * fix(peaks): unify snapshot() and test interception into one implementation * fix(peak-meter): stop idle 30fps redraws and correct ring duration doc When playback is paused or stopped, the flat-line fallback was being redrawn on every 30 Hz poll cycle even though the canvas content is static. Track a flatLineDrawn flag so the flat baseline is drawn exactly once, then skip redraws until new peaks arrive (writeIndex advances). Also correct the ring duration comment: 256 pairs × 512 frames at 44.1 kHz = ~3 s, not ~5.8 s. * fix(peak-meter): start staleness grace period on first static non-empty peak When the component mounts and the backend returns non-empty peaks with a writeIndex that doesn't advance from the initial value, lastAdvanceRef was never set, so the flat-line fallback never triggered. Initialize the grace period on the first static non-empty poll so the canvas eventually flat-lines. * fix(docs): correct peak ring history duration from ~5.8s to ~3.0s The peak ring retains 256 pairs × 512 frames = 131,072 frames, which at 44.1 kHz is ~2.97 s, not ~5.8 s. The PeakMeter.tsx comment already stated ~3 s; the playback contract doc and CHANGELOG entry still carried the incorrect ~5.8 s figure. * fix: address review comments on next-track-gapless - Fix peak meter double-counted sample count (output.rs): rendered is already interleaved samples (frames × channels); multiplying by device_channels again caused channels× too many frames in the peak accumulator, diluting reported levels. - Fix EQ gain smoothing 12× slower than intended (eq.rs): approach() used delta.signum() * step (absolute step) but smoothing_step returns a proportional fraction. Changed to delta * step (first-order exponential) so the EQ_SMOOTH_MS time constant is respected. - Move EQ filter coefficient updates out of the per-frame loop (eq.rs): update_coefficients was called every frame per channel per band even though coefficients are computed once per callback. Moved to a separate pre-loop pass to avoid thousands of redundant writes on the realtime audio thread. - Fill remaining buffer from the next track on gapless swap (output.rs): when EOF lands mid-callback, the swap previously left a zero-filled tail (up to ~10-20ms silence) before the next callback. Now renders the remaining buffer from the new track immediately after the swap, with EQ and peak accumulation applied to the transition tail. - Add test verifying the gapless tail-fill behavior. * test: cover PeakMeter cleanup and player-store gapless actions for 100% patch coverage * fix: silence clippy needless-range-loop and collapsible-if in gapless path Flatten EQ band filter iteration and collapse nested EOF/gapless-swap guards so CI clippy -D warnings passes after rebase onto main. * fix: apply soft limiter to gapless transition tail The gapless transition tail code applied EQ but skipped the soft limiter, unlike the main render path which runs EQ then soft_limit. EQ-boosted audio in the crossover region could clip, causing brief distortion at gapless track transitions for listeners with EQ boost enabled. * fix(gapless): do not auto-advance to next track when paused at EOF The gapless swap in the realtime callback did not check whether the user had paused (or was pausing via a fade-out) before swapping to the prepared next track. A track reaching EOF during a pause fade-out would still auto-advance, defeating the user's intent to stop at the current song. Add a current_track_is_playing helper that returns false when is_playing is false or a FadingOut is in progress, and require it in the gapless swap condition. * fix(gapless): allow gapless swap after pause/resume at EOF (#103) When the user paused near EOF and then resumed, the gapless swap to the preloaded next track was permanently suppressed. Two issues: 1. snapshot() sets is_playing=false when position >= duration. The current_track_is_playing() helper only checked is_playing, so it returned false even after the user pressed resume. 2. The render callback early-return guard returned 0 when !snapshot.is_playing and !FadingOut, skipping the gapless swap check. During FadingIn (user just resumed), is_playing is false (set by snapshot) but the user intent is to play. Fixes: - current_track_is_playing() now treats FadingIn as actively playing, even if is_playing is false (snapshot set it at EOF). - The render callback early-return guard no longer fires during FadingIn, so the gapless swap check is reached. Added regression tests: - current_track_is_playing unit tests (true when playing, false during FadingOut, false when no track, true after resume) - gapless_swap_proceeds_after_resume_from_pause_at_eof (integration test verifying the swap fires after resume) - post_pause_tail_is_silence_not_next_track (verifying the tail buffer is silence, not the next track, during a pause) * fix(gapless): bump transport_generation on swap and add pause/resume transport regression * docs(playback-contract): fix duplicate 11/12 numbering and ring duration The EQ commands (set_eq_enabled, set_eq_gains) were numbered 11 and 12, colliding with the playback-position and track-transitioned event entries. Renumber them to 13 and 14 so each interface item has a unique ordinal. Also correct the peak ring duration from ~5.8 s to ~3 s (256 pairs × 512 frames / 44100 Hz = 2.97 s), matching the PeakMeter component doc. * fix(gapless): serialize preload cancel under lock, skip swap during fade-out, restore stems on auto-advance, hoist EQ coefficient update * fix(gapless): depend on next-candidate ID instead of full queue array The preload effect previously depended on the entire queue array, so any queue mutation (adding/removing unrelated tail entries) cancelled the in-flight or completed preload and re-decoded the often-unchanged next song from scratch. Resolve the next-candidate ID outside the effect and use only that ID as the dependency, so unrelated queue edits no longer trigger a redundant preload cycle. * fix(gapless): serialize preload generation under lock and gate tail limiter on EQ Move the preload_request_generation fetch_add inside the preload_shutdown lock so the generation assignment and CancelPreparedNext send are ordered consistently for concurrent callers. Without this, two overlapping set_preload_candidate invocations could obtain generations in one order but acquire the lock in the opposite order, causing the coordinator to accept an older stale preload candidate while rejecting the newer one. Gate the gapless transition tail soft limiter on EQ activity (!eq_processor.is_fully_bypassed()) to match the main render path. The unconditional soft_limit attenuated loud-but-unclipped audio during the brief song-to-song transition for users who never enable the EQ. Add set_preload_candidate handler to the e2e Tauri mock so the always-on preload effect does not hit the unhandled-command rejection path. * fix(gapless): address review comments on PR #103 * fix(gapless): align test event shapes with current IPC contracts Update use-playback-runtime.test.tsx to use the current TrackTransitionedEvent (requires transition_serial) and PlaybackPositionEvent (requires ms, transport_generation, snapshot) shapes instead of the stale song_id/position_ms fields. Resolves the Frontend quality / build / test TS2741 and TS2353 failures. * chore: trigger CI on merged branch * chore: re-trigger CI * chore: force GitHub mergeable recheck * chore: trigger CI after base change to main
Re-review — no LGTM yet: older version already entered main via #103Reviewed head The current branch contains the corrected peak cursor implementation, shared production/test snapshot algorithm, single-flight polling, stale/idle handling, and smooth decay work; CI is green. However, #103 already carried an older #102 tree into After the #114 recovery/hotfix lands:
The existing green run is not merge evidence for the post-#103 mainline. |
Summary
Peak envelope visualizer (#87)
PeakRing) in the CPAL output callback that publishes sanitized, clamped stereo peak pairs — one per 512 rendered frames, 256-pair capacity (~3.0 s @ 44.1 kHz) — after EQ, limiter, and fadePeakAccumulatorowned by the output closure (besideEqProcessor); device restart starts a fresh partial window while retaining the process-wide ring and write counterget_audio_peaksIPC command that copies the ring snapshot without holding the playback mutexPeakMetercanvas component that polls at 30 Hz and renders a scrolling stereo waveform in the fullscreen player controlsFullscreen player lyrics fixes
readLyricsPlaybackClockMsandreadLyricsAdjustedPlaybackMsused AirPlaydisplayedPositionMs(a stale remote clock) instead of the local playback clock when AirPlay was active. The fullscreen window receives AirPlay state via BroadcastChannel but does not run the AirPlay listener, so the displayed position never updated on local seeks — freezing lyrics at the position when the fullscreen window was opened. Lyrics now always use the local playback clock; the AirPlay clock is only used for CDG sync. Also added atransport_generationstaleness check inapplyPlayerSyncSnapshotto prevent delayed BroadcastChannel messages from regressing the clock.getLineScrollMetricsusedoffsetToprelative tooffsetParent— a nestedjustify-centerinner div in audience mode. Lines above the vertical center had negativeoffsetTop, andgetCenteredScrollTopclamped to 0, freezing scroll at the top. Now usesgetBoundingClientRectwhenoffsetParentis not the container. Also changed the audience lyrics inner container fromjustify-centertojustify-startso all lines are reachable viascrollTop.Fullscreen player keyboard shortcuts
Audience mode seek-unlock
Architecture
src-tauri/src/audio/peaks.rs—PeakRing(single-writer lock-free ring usingAtomicU32bit-packedf32slots +AtomicU64write index withRelease/Acquireordering),PeakAccumulator(512-frame max window),AudioPeakSnapshot(IPC DTO)src-tauri/src/audio/output.rs—render_output_bufferandbuild_output_streamwired to feed the accumulator after the fade stagesrc-tauri/src/state/playback.rs—PlaybackStategainspeak_ring: Arc<PeakRing>src/components/Player/PeakMeter.tsx— 30 Hz polling canvas with writeIndex-skip optimizationsrc/components/Player/FullscreenControls.tsx— keyboard shortcuts for play/pause, seek, and closesrc/lib/lyrics-engine.ts— local-clock-only lyrics position, audience seek-unlock with idle re-locksrc/components/Lyrics/lyrics-scroll.ts—getBoundingClientRectfallback for nestedoffsetParentsrc/stores/player-store.ts—transport_generationstaleness guard inapplyPlayerSyncSnapshotTest plan
get_audio_peakscommandcargo test— 447 passedcargo clippy --all-targets -- -D warnings— cleancargo deny check— cleanpnpm test— 1376 passedpnpm lint— cleanpnpm check:i18n— cleannpx knip— clean