feat: generate cover art thumbnail and preview derivatives in library root (#91)#107
Open
thedavidweng wants to merge 142 commits into
Open
feat: generate cover art thumbnail and preview derivatives in library root (#91)#107thedavidweng wants to merge 142 commits into
thedavidweng wants to merge 142 commits into
Conversation
Contributor
Coverage Report
File Coverage |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #107 +/- ##
=======================================
+ Coverage 77.3% 78.9% +1.6%
=======================================
Files 126 131 +5
Lines 6266 6964 +698
Branches 1984 2140 +156
=======================================
+ Hits 4846 5501 +655
- Misses 1376 1420 +44
+ Partials 44 43 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
Greptile SummaryThis PR adds generated cover-art derivatives for faster thumbnail and backdrop rendering. The main changes are:
Confidence Score: 5/5This PR appears safe to merge. The reviewed paths keep original cover art authoritative, validate derivative files before serving or publishing, guard lazy repair writes with cover-art byte matching, and handle shared derivative deletion by database reference counts. No files require special attention.
 T-Rex LogsWhat T-Rex did
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Import as Import / Extract
participant DB as songs table
participant Art as artwork module
participant FS as <library>/artwork
participant UI as Frontend
participant Remote as Remote sync/publish
Import->>Art: generate derivatives from cover_art bytes
Art->>FS: "write thumb_*.webp and preview_*.webp"
Import->>DB: persist cover_art + derivative paths
UI->>DB: get_cover_art(hash, size)
DB-->>UI: recorded path or original bytes
UI->>Art: validate/read requested derivative
Art-->>UI: derivative bytes or fallback original
Remote->>Art: validate/regenerate local derivatives
Remote->>FS: copy/upload artwork files
Remote->>DB: persist remote derivative paths
%%{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 Import as Import / Extract
participant DB as songs table
participant Art as artwork module
participant FS as <library>/artwork
participant UI as Frontend
participant Remote as Remote sync/publish
Import->>Art: generate derivatives from cover_art bytes
Art->>FS: "write thumb_*.webp and preview_*.webp"
Import->>DB: persist cover_art + derivative paths
UI->>DB: get_cover_art(hash, size)
DB-->>UI: recorded path or original bytes
UI->>Art: validate/read requested derivative
Art-->>UI: derivative bytes or fallback original
Remote->>Art: validate/regenerate local derivatives
Remote->>FS: copy/upload artwork files
Remote->>DB: persist remote derivative paths
Reviews (3): Last reviewed commit: "fix(cover-art): use stable content-key d..." | 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.
![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.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Owner
Author
Maintainer acceptance — merge slot 11, strictly after #106BLOCKED — changes required. Blocking Blob-URL lifecycle leak
Move acquisition/release into a committed effect or make acquisition idempotent per mounted consumer/digest, and add a test that rerenders with equal bytes in a new array (plus StrictMode/aborted-render-safe behavior). After #106 lands, rebase onto its migration/audio result before merging #107. |
5 tasks
Owner
Author
Re-review: blocking Blob-URL ownership/lifetime fix requiredThis PR is stacked after #106. Its current cover-art URL hook performs cache acquisition in Implement exactly this ownership model:
Add deterministic tests (mock
After #106 merges, rebase onto its resulting |
thedavidweng
force-pushed
the
feat/artwork-derivatives-91
branch
from
July 15, 2026 08:57
a91cb11 to
9b7fde0
Compare
3 tasks
thedavidweng
force-pushed
the
feat/artwork-derivatives-91
branch
from
July 16, 2026 03:46
27f7862 to
43a905c
Compare
Owner
Author
|
@greptile review |
1 similar comment
Owner
Author
|
@greptile review |
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
The soft limiter engages above 0.95 amplitude (LIMITER_THRESHOLD), not 0.85 as previously stated in the CHANGELOG entry.
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.
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.
…ndant 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.
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.
Missing eq_gains_db in the Playwright Tauri mock left SettingsEqSection with undefined draft gains and crashed the tree (Preferences not found).
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.
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.
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).
- 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.
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.
…imiter 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.
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.
… 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.
… feat/90-waveform-seekbar
…eat/artwork-derivatives-91 # Conflicts: # CHANGELOG.md # docs/references/generated/db-schema.md # src-tauri/migrations/011_waveforms.sql # src-tauri/src/audio/output.rs # src/components/Settings/SettingsOverlay.state.test.ts # src/components/Settings/SettingsOverlay.state.ts # src/locales/en.json # src/locales/zh-CN.json
Owner
Author
Re-review — no LGTM: scope is not isolated and branch conflictsReviewed head The artwork-specific fixes are directionally complete and CI/Packaging are green, but the current PR is conflicting and shows 142 commits / 106 changed files against After #104/#106 are repaired and merged in order:
The current history/scope is not safe to merge. |
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
songs.cover_artas the authoritative original bytes and generate square lossless WebP derivatives at 80×80 (thumb) and 256×256 (preview) in<library>/artwork/get_cover_artcommand is now async with an optionalsizeparameter and runs disk/decode work inspawn_blocking${songHash}:${size}with byte identity for replacement detectionCoverArtThumbnailfetches the thumb derivative first, falling back to full cover art;PlaybackStageuses the preview for the ambience backdropTest plan
cargo test -q— 398 Rust tests pass (16 new artwork tests)pnpm test— 1312 frontend tests pass (9 new cover-art cache + fetch tests)pnpm lint/pnpm build/cargo fmt/cargo deny check— all cleanpnpm tauri build— full release build succeeds