feat: add crossfade playback with equal-power envelope (#89)#104
feat: add crossfade playback with equal-power envelope (#89)#104thedavidweng wants to merge 31 commits into
Conversation
Coverage Report
File Coverage |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/next-track-gapless-88 #104 +/- ##
============================================================
+ Coverage 77.8% 78.2% +0.3%
============================================================
Files 130 131 +1
Lines 6688 6804 +116
Branches 2043 2112 +69
============================================================
+ Hits 5209 5325 +116
Misses 1435 1435
Partials 44 44
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Greptile SummaryThis PR adds optional equal-power crossfade playback on top of prepared next-track preloading. The main changes are:
Confidence Score: 5/5This PR appears safe to merge based on the reviewed diff. The crossfade render path handles the active, paused, stale-state, resampling, and fallback cases covered during review. Settings responses are guarded against stale writes, and IPC, docs, types, and tests are aligned across backend and frontend. No accepted issues remain. No files require special attention.
 T-Rex LogsWhat T-Rex did
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as Settings UI
participant Store as settings-store
participant IPC as Tauri settings command
participant Coord as PlaybackCoordinator
participant Audio as CPAL output callback
participant Queue as PlaybackSession/Queue
UI->>Store: toggle crossfade / adjust duration
Store->>IPC: set_crossfade_enabled / set_crossfade_duration_ms
IPC->>Coord: SetCrossfadeEnabled / SetCrossfadeDuration
Coord->>Audio: update PlaybackController.crossfade_config
IPC-->>Store: AppSettings snapshot
Audio->>Audio: when prepared track enters overlap window
Audio->>Audio: equal-power mix outgoing tail + incoming head
Audio->>Audio: promote incoming track on overlap completion
Audio-->>Queue: track-transitioned(state, serial, fromSongId, toSongId)
Queue->>Queue: apply authoritative state and reconcile queue/history
Queue->>Coord: setPreloadCandidate(new queue head)
%%{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 UI as Settings UI
participant Store as settings-store
participant IPC as Tauri settings command
participant Coord as PlaybackCoordinator
participant Audio as CPAL output callback
participant Queue as PlaybackSession/Queue
UI->>Store: toggle crossfade / adjust duration
Store->>IPC: set_crossfade_enabled / set_crossfade_duration_ms
IPC->>Coord: SetCrossfadeEnabled / SetCrossfadeDuration
Coord->>Audio: update PlaybackController.crossfade_config
IPC-->>Store: AppSettings snapshot
Audio->>Audio: when prepared track enters overlap window
Audio->>Audio: equal-power mix outgoing tail + incoming head
Audio->>Audio: promote incoming track on overlap completion
Audio-->>Queue: track-transitioned(state, serial, fromSongId, toSongId)
Queue->>Queue: apply authoritative state and reconcile queue/history
Queue->>Coord: setPreloadCandidate(new queue head)
Reviews (7): Last reviewed commit: "fix(crossfade): guard stale enable respo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Maintainer acceptance — merge slot 9, strictly after repaired #103BLOCKED — changes required. Blocking crossfade correctness issues
Normalize both duration/remaining calculations into one frame domain and give each stream independent resampler state. Add a non-native-rate outgoing+incoming test that verifies overlap timing, cursor advancement, and no cross-stream state contamination. Also rebase onto final #103 and rerun CI; this current head is not mergeable. |
4784cf8 to
d587555
Compare
01c66fa to
5f60c9e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
23f8de8 to
88891a0
Compare
Re-review: two blocking crossfade audio-correctness fixes requiredKeep this PR stacked after the final #103. The current crossfade path has two correctness defects that must be fixed before any UI acceptance:
Add focused DSP regressions:
Then rebase onto the final #103/ |
bdabe45 to
b100cab
Compare
88891a0 to
c32a92f
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
3251ba7 to
30fe2b9
Compare
1f6007d to
d1d5abf
Compare
30fe2b9 to
849a83b
Compare
d1d5abf to
cf0947a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
849a83b to
9973512
Compare
dbe353c to
049d783
Compare
Re-review — still blocked: crossfade mixes source and device frame domains, and loses the incoming resampler stateReviewed at 1. Overlap timing still mixes frame domains
Convert every duration/remaining limit to output frames before calling the helper. Keep 2. Promotion uses the wrong incoming cursor and drops its resampler historyDuring overlap, Implement this exact handoff:
Do not merge based only on same-rate coverage; these defects are in the mismatched-rate path required by the issue. |
9973512 to
50f35f9
Compare
049d783 to
a96c2f7
Compare
…tubs and history cap
…n paths The crossfade render path and both gapless-swap fallback paths applied EQ but skipped the soft limiter before the fade envelope and peak accumulation, so EQ-boosted samples could exceed 1.0 and clip the DAC during transitions. Mirrors the normal render path (EQ → soft limiter → fade → peaks) in all three EQ process sites.
Validate song ownership before canceling an active crossfade on stem attach, and complete eq/crossfade/peaks fields in the Playwright mock so Settings opens without crashing.
Wrong-song attach must reject without canceling active crossfade or prepared track; same-song attach still cancels and installs stems.
Advance the incoming stem with a dedicated source-frame cursor and give it its own ResamplerCache so device-frame progress and shared rubato state cannot corrupt rate conversion during overlap.
render_output_buffer gained a dedicated incoming ResamplerCache; update phase2/phase3 integration call sites so the full crate test build compiles.
… promotion Convert source-frame quantities to device (output) frames before calling effective_overlap_frames so the overlap timing clamp operates in a single frame domain. Use incoming_source_frame (not rendered_frames) for promote_crossfade_track so the promoted track render_frame matches the incoming source cursor. Swap the incoming resampler cache into the primary lane on promotion to preserve sinc delay history, and clear both lanes on cancellation/seek/device-recreation.
…n tail The gapless swap path in the realtime callback applied EQ to the post-swap remaining buffer but skipped the soft_limit pass that the normal render path and the crossfade fallback gapless path both apply. A positive EQ band gain on the transition tail could exceed full-scale and clip. Add the limiter pass after EQ, matching the other two paths, with a regression test that feeds above-threshold samples through the gapless swap and asserts the tail is bounded.
Verify the PeakRing captures post-limiter values (after soft_limit), not pre-limiter values clamped by sanitize_peak. A track with samples at 1.0 (above the 0.95 threshold) is rendered; the output and peak ring must both show the compressed value (< 1.0, > threshold), proving the peak accumulator runs after the soft limiter in the render pipeline.
… crossfade promotion
promote_crossfade_track replaced the current track but did not bump transport_generation, unlike perform_gapless_swap. Without the bump, a delayed playback-position event from the old song (same generation) could arrive after the new-song snapshot and be accepted by the frontend generation filter, reverting the clock and queue reconciliation back to the old song. Add the bump and a regression test mirroring the gapless swap test. Update the playback contract doc to list crossfade promotion alongside gapless swap as a transport_generation-bumping event, and correct the race-protection note that previously claimed gapless swap does not bump the generation.
Finding 1 (EQ action shadow): Remove duplicate setEqEnabled/setEqGains/ resetEqGains from settings-overlay.library-actions.ts that shadowed the state.ts versions. Re-add the Math.max(-12, Math.min(12, g)) clamp and current.every() no-op guard to setEqGains in SettingsOverlay.state.ts, plus the optimistic settingsStore.patchAppSettings update with rollback on failure for all three EQ actions. Move EQ tests from library-actions.test.ts to state.test.ts with assertions for the new clamping, no-op guard, and patchAppSettings behavior. Finding 2 (duplicate test): Remove the duplicate 'cleans up timer and cancels polling on unmount' test block from PeakMeter.test.tsx so the test appears only once. Findings 3 and 4 (output.rs frame/sample mismatch and soft limiter regression) were already fixed in prior commits on this branch. Update CHANGELOG.md with the fixes.
The crossfade branch added a duplicate "eq" section in both en.json and zh-CN.json (at line 360) while the same key already existed later in each file (at line 430) with more complete content including band labels. JSON parsers keep only the last occurrence, so the earlier duplicate was silently ignored. Removing the duplicate ensures the complete EQ locale section is the only one present.
cancel_crossfade_and_prepared now also clears pending_transition_out so that a stale gapless/crossfade transition (A->B) is never emitted to the frontend after the user manually loads an unrelated song C. Previously, start_track_loading and clear_track cancelled the active crossfade and prepared track but left the pending transition in place, allowing the position emitter to drain it with a snapshot of the newly-loaded song at the new transport_generation — bypassing the frontend staleness guard and corrupting the queue/history reconciliation. Also corrects the inaccurate rationale comment in session.ts that claimed the gapless swap does not bump transport_generation.
The soft limiter is now gated on EQ activity (is_fully_bypassed). The peak_ring_captures_post_limiter_values test used a default-disabled EqProcessor, so the limiter was skipped and the 1.0 samples were not clipped. Enable the EQ in the test so the limiter is active.
5046ec5 to
ce496d5
Compare
… feat/crossfade-playback-89 # Conflicts: # src-tauri/src/audio/output.rs
Merge brought in the gapless branch's test (snake_case fields, two-arg onTrackTransitioned) but the crossfade branch's implementation uses camelCase (via serde rename_all) and passes the whole event object. Update the test to match the crossfade event shape.
The test used `instrumental` which doesn't exist on the StemVolumes interface (vocals/drums/bass/other). This caused a tsc build failure in CI.
The soft limiter is gated on EQ activity (is_fully_bypassed). The gapless_swap_tail_applies_soft_limiter test used a default-disabled EqProcessor, so the limiter was skipped and the 1.0 tail samples were not compressed. Enable the EQ in the test so the limiter is active, matching the fix applied to peak_ring_captures_post_limiter_values in ce496d5.
Status review — closed without merge; no LGTM possibleThis PR was closed unmerged at head If crossfade remains desired, reopen only after rebuilding the work on the repaired current The replacement must retain the reviewed crossfade fixes, present a crossfade-only diff, run CI/Packaging, and complete manual audible transition/pause/seek/device-change acceptance before LGTM. |
1 participant
Summary
Test plan