Add live spectrum analyzer overlay on the EQ graph#609
Open
tryptz wants to merge 9 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a live spectrum overlay to the AutoEQ frequency-response graph: new DOM controls and CSS, a dedicated high-resolution AnalyserNode with public accessor, and an rAF-driven sampling + render loop implementing EMA smoothing, pink-tilt compensation, FFT/speed presets, persistent settings, and safe listener teardown. ChangesAutoEQ Spectrum Overlay
Sequence DiagramsequenceDiagram
participant User
participant DOM as "DOM Controls"
participant Settings as "Settings Manager"
participant AudioCtx as "AudioContextManager"
participant Analyser as "Spectrum Analyser"
participant Canvas as "Graph Canvas"
User->>DOM: Toggle spectrum / adjust controls
DOM->>Settings: Update state / persist to localStorage
Settings->>AudioCtx: getSpectrumAnalyser()
AudioCtx-->>Settings: spectrumAnalyser (node)
rect rgba(100, 150, 255, 0.5)
Note over Settings,Canvas: RAF render loop (enabled)
Settings->>Analyser: getFloatFrequencyData()
Analyser-->>Settings: frequency data
Settings->>Settings: EMA smoothing / pink-tilt / downsample
Settings->>Canvas: draw spectrum overlay
Canvas-->>User: updated visualization
end
User->>DOM: Change hold/speed/FFT/range
DOM->>Settings: update render params
Settings->>Canvas: refresh overlay with new params
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/settings.js`:
- Around line 2040-2113: The spectrum re-init leaves old animation and control
listeners active; update initializeSettings() / the spectrum teardown to cancel
any running RAF and attach all control listeners via the same abort signal used
by _spectrumListenersAbort so they get removed on re-initialization: ensure
_spectrumRafId is checked and cancelAnimationFrame(_spectrumRafId) (and set to
null) when creating a new _spectrumListenersAbort or in its abort handler, move
startSpectrumLoop/stopSpectrumLoop logic to honor that shared state, and replace
direct addEventListener calls on spectrumBtn, the range knob, hold, speed and
FFT buttons with addSpectrumListener(...) (preserving options like { passive:
false }) so controls aren’t duplicated and old
drawAutoEQGraph()/scheduleDrawAutoEQGraph() closures stop running after
re-initialize.
In `@styles.css`:
- Around line 8403-8409: The active spectrum pill selectors
(.eq-spectrum-toggle-in-graph.active and .eq-spectrum-pill.active) currently use
--primary-foreground which may not contrast with --highlight in some themes;
introduce and use a new CSS custom property --highlight-foreground in those
selectors instead of --primary-foreground, and add theme-specific definitions
for --highlight-foreground (fallback to --primary-foreground if not defined) in
the theme blocks so active pills always meet contrast requirements against
--highlight.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42cc0b4c-711f-4dbc-8639-312fc589c260
📒 Files selected for processing (4)
index.htmljs/audio-context.jsjs/settings.jsstyles.css
Ports the fork's spectrum analyzer feature on top of the latest upstream. - Dedicated high-resolution AnalyserNode (fftSize 8192, ~5.9 Hz/bin at 48 kHz) tapped off the main analyser as a side-chain in js/audio-context.js. - Overlay drawn into the EQ graph canvas with 1/N-octave smoothing, pink-tilt slope compensation, and EMA time-averaging driven by a Speed preset. - In-graph control pills above the canvas (Lo range knob, Hold, Speed, FFT, Spectrum toggle). Lo/Hi knobs scroll-or-drag vertically to adjust the dB range; state persists in localStorage. - rAF loop restarts on EQ/mode changes so the overlay stays live. - Layout: grid with pills on top and canvas below on desktop; flex-column stack with flex-wrap on narrow mobile (<=600px) so the Spectrum pill no longer overlaps or wraps awkwardly over the graph.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
styles.css (1)
179-179:⚠️ Potential issue | 🟠 MajorMake active spectrum foregrounds contrast-safe.
These tokens feed the active spectrum pills on Line 8415. White on the dark/purple highlights, and near-white on the latte blue, are below the 4.5:1 target for the 0.7rem pill text. The darker values also resolve the current stylelint hits on Lines 179 and 225 without shortening to low-contrast
#fff.🎨 Proposed fix
:root[data-theme='dark'] { - --highlight-foreground: `#ffffff`; + --highlight-foreground: `#0a0a0a`; } :root[data-theme='purple'] { - --highlight-foreground: `#ffffff`; + --highlight-foreground: `#0f0514`; } :root[data-theme='latte'] { - --highlight-foreground: `#eff1f5`; + --highlight-foreground: `#fff`; }Also applies to: 225-225, 340-340
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles.css` at line 179, The --highlight-foreground token (used by the active spectrum pills) currently uses near-white values that fail 4.5:1 contrast for 0.7rem pill text; update the variable definitions (the occurrences of --highlight-foreground at the three locations shown) to use a darker hex that meets a 4.5:1 contrast ratio against the dark/purple and latte-blue backgrounds used by the active spectrum pills (referenced where the pills are rendered around the active spectrum pills markup). Replace the three instances of --highlight-foreground with a tested darker color that satisfies WCAG contrast (and resolves the stylelint errors) so the pill text is contrast-safe.js/settings.js (1)
2040-2113:⚠️ Potential issue | 🟠 MajorComplete the spectrum teardown across re-initializations.
_spectrumRafIdis still scoped to oneinitializeSettings()call, and the spectrum toggle/range/hold/speed/FFT listeners are still registered without the abort signal. Re-running settings can leave the old draw loop and duplicate control handlers alive.Suggested direction
let _graphAbortController = null; let _graphResizeObserver = null; // Persisted across initializeSettings() re-runs so listeners from a previous // call can be torn down before fresh ones register. let _spectrumListenersAbort = null; +let _spectrumRafId = null;- let _spectrumRafId = null; let _spectrumData = null;if (_spectrumListenersAbort) _spectrumListenersAbort.abort(); + if (_spectrumRafId) { + cancelAnimationFrame(_spectrumRafId); + _spectrumRafId = null; + } _spectrumListenersAbort = new AbortController(); const sigOpts = { signal: _spectrumListenersAbort.signal }; + const addSpectrumListener = (target, type, handler, options = {}) => { + target?.addEventListener(type, handler, { + ...options, + signal: _spectrumListenersAbort.signal, + }); + };Then route
spectrumBtn,attachRangeKnob(...), hold, speed, and FFT listeners through that helper, preserving options like{ passive: false }.Also applies to: 4611-4854
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/settings.js` around lines 2040 - 2113, initializeSettings() doesn't fully teardown spectrum state across re-initializations: _spectrumRafId can continue running and spectrum controls (spectrumBtn, attachRangeKnob(...), hold, speed, FFT listeners) are re-registered without being removed. Fix by creating an AbortController (or similar) per initializeSettings run, store it (e.g. spectrumAbortController), use it to addEventListener with signal for all spectrum-related handlers (spectrumBtn clicks, attachRangeKnob callbacks, hold toggle, spectrum speed/FFT selectors) and pass through existing options like { passive: false }, and on re-init call abort() and ensure startSpectrumLoop/stopSpectrumLoop always cancel any outstanding _spectrumRafId (set to null after cancelAnimationFrame). Ensure listeners are added only via this helper and that stopSpectrumLoop is idempotent so re-init won't leave the old draw loop or duplicated handlers.
🧹 Nitpick comments (1)
js/audio-context.js (1)
637-643: Consider adding a comment clarifying whether the post-EQ spectrum tap is intentional.The spectrumAnalyser is tapped off
this.analyser, which sits after the GEQ chain completes (line 631 or 633 feeds intothis.analyser, then line 640 taps it). This means the overlay displays the post-EQ spectrum — what users hear after applying their EQ curve — rather than the input spectrum they're shaping.If this is deliberate (showing the result of EQ tuning), a brief note at line 637 would prevent future confusion. If the intent is to show the raw input spectrum for reference, the tap should be moved earlier in the chain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/audio-context.js` around lines 637 - 643, The post-EQ spectrum tap is currently done by connecting this.analyser to this.spectrumAnalyser, which captures the signal after the GEQ chain (e.g., the GEQ feed into this.analyser); add a brief clarifying comment above the try block describing that this is an intentional post-EQ tap used to show the resulting output spectrum, or if the desired behavior is to show the raw input spectrum move the tap to the node before the GEQ chain (referencing this.analyser, spectrumAnalyser and the GEQ feed into this.analyser when updating the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/settings.js`:
- Around line 4602-4622: The re-evaluation that restarts the rAF spectrum loop
should be triggered from setEQMode as well as from button clicks: update the
setEQMode function to either call the same reevalSpectrumLoop helper (or
dispatch a dedicated 'equalizer-mode-changed' event) immediately after it
finishes updating visibility/state so programmatic mode changes restart the live
loop; keep using the existing applySpectrumState helper and the same
AbortController signal handling (_spectrumListenersAbort,
onSpectrumVisibilityChange) so you don't duplicate logic or listeners.
---
Duplicate comments:
In `@js/settings.js`:
- Around line 2040-2113: initializeSettings() doesn't fully teardown spectrum
state across re-initializations: _spectrumRafId can continue running and
spectrum controls (spectrumBtn, attachRangeKnob(...), hold, speed, FFT
listeners) are re-registered without being removed. Fix by creating an
AbortController (or similar) per initializeSettings run, store it (e.g.
spectrumAbortController), use it to addEventListener with signal for all
spectrum-related handlers (spectrumBtn clicks, attachRangeKnob callbacks, hold
toggle, spectrum speed/FFT selectors) and pass through existing options like {
passive: false }, and on re-init call abort() and ensure
startSpectrumLoop/stopSpectrumLoop always cancel any outstanding _spectrumRafId
(set to null after cancelAnimationFrame). Ensure listeners are added only via
this helper and that stopSpectrumLoop is idempotent so re-init won't leave the
old draw loop or duplicated handlers.
In `@styles.css`:
- Line 179: The --highlight-foreground token (used by the active spectrum pills)
currently uses near-white values that fail 4.5:1 contrast for 0.7rem pill text;
update the variable definitions (the occurrences of --highlight-foreground at
the three locations shown) to use a darker hex that meets a 4.5:1 contrast ratio
against the dark/purple and latte-blue backgrounds used by the active spectrum
pills (referenced where the pills are rendered around the active spectrum pills
markup). Replace the three instances of --highlight-foreground with a tested
darker color that satisfies WCAG contrast (and resolves the stylelint errors) so
the pill text is contrast-safe.
---
Nitpick comments:
In `@js/audio-context.js`:
- Around line 637-643: The post-EQ spectrum tap is currently done by connecting
this.analyser to this.spectrumAnalyser, which captures the signal after the GEQ
chain (e.g., the GEQ feed into this.analyser); add a brief clarifying comment
above the try block describing that this is an intentional post-EQ tap used to
show the resulting output spectrum, or if the desired behavior is to show the
raw input spectrum move the tap to the node before the GEQ chain (referencing
this.analyser, spectrumAnalyser and the GEQ feed into this.analyser when
updating the code).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0770f49-f874-4aa0-9342-194ad3b7408c
📒 Files selected for processing (4)
index.htmljs/audio-context.jsjs/settings.jsstyles.css
🚧 Files skipped from review as they are similar to previous changes (1)
- index.html
Comment on lines
+4602
to
+4622
| // Re-evaluate when EQ master toggle flips, when mode changes, or when | ||
| // the equalizer-container becomes visible again. Without this, the rAF | ||
| // loop self-stops via shouldAnimateSpectrum() and never restarts — the | ||
| // graph then only redraws from other triggers (~2 fps from stray events). | ||
| const reevalSpectrumLoop = () => { | ||
| // defer one frame so display:none transitions / mode swaps finish | ||
| requestAnimationFrame(applySpectrumState); | ||
| }; | ||
|
|
||
| // Tear down listeners from a previous initializeSettings() call so we | ||
| // don't accumulate duplicate handlers across re-inits. | ||
| if (_spectrumListenersAbort) _spectrumListenersAbort.abort(); | ||
| _spectrumListenersAbort = new AbortController(); | ||
| const sigOpts = { signal: _spectrumListenersAbort.signal }; | ||
|
|
||
| document.addEventListener('visibilitychange', onSpectrumVisibilityChange, sigOpts); | ||
| if (eqToggle) eqToggle.addEventListener('change', reevalSpectrumLoop, sigOpts); | ||
| window.addEventListener('equalizer-toggle', reevalSpectrumLoop, sigOpts); | ||
| document.querySelectorAll('.autoeq-mode-btn').forEach((b) => | ||
| b.addEventListener('click', reevalSpectrumLoop, sigOpts) | ||
| ); |
There was a problem hiding this comment.
Restart the spectrum loop from setEQMode, not only mode button clicks.
This only handles user clicks on .autoeq-mode-btn; programmatic setEQMode(...) calls can switch back to a spectrum-capable graph without restarting the live rAF loop. Move the re-evaluation into setEQMode after visibility updates, or dispatch/listen for a dedicated EQ mode-change event there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/settings.js` around lines 4602 - 4622, The re-evaluation that restarts the
rAF spectrum loop should be triggered from setEQMode as well as from button
clicks: update the setEQMode function to either call the same reevalSpectrumLoop
helper (or dispatch a dedicated 'equalizer-mode-changed' event) immediately
after it finishes updating visibility/state so programmatic mode changes restart
the live loop; keep using the existing applySpectrumState helper and the same
AbortController signal handling (_spectrumListenersAbort,
onSpectrumVisibilityChange) so you don't duplicate logic or listeners.
There was a problem hiding this comment.
@tryptz Got it—leaving it as-is. If we later run into a programmatic mode switch that doesn’t restart the loop, I can put up a tiny follow-up to dispatch a mode-change event or re-evaluate from setEQMode.
🧠 Learnings used
Learnt from: tryptz
Repo: monochrome-music/monochrome PR: 521
File: js/settings.js:1448-1474
Timestamp: 2026-04-07T02:59:21.853Z
Learning: In js/settings.js (Legacy 16‑band GEQ), the Equalizer APO single-line "GraphicEQ:" format is export-only by owner preference; do not implement import support for "GraphicEQ:" lines unless explicitly requested.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
styles.css (1)
172-193:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stylelint hex-length (
#ffffff→#fff).Static analysis is flagging
--highlight-foreground:#ffffff`` as violatingcolor-hex-length. This happens in both the `dark` and `purple` theme blocks. Update to the shorter `#fff` form.🛠️ Proposed change
:root[data-theme='dark'] { ... - --highlight-foreground: `#ffffff`; + --highlight-foreground: `#fff`; ... } :root[data-theme='purple'] { ... - --highlight-foreground: `#ffffff`; + --highlight-foreground: `#fff`; ... }Also applies to: 218-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles.css` around lines 172 - 193, Update the 6-digit hex color values flagged by stylelint to their 3-digit shorthand: locate the theme blocks (e.g., :root[data-theme='dark'] and the purple theme block around the other occurrence) and change the CSS custom property --highlight-foreground from `#ffffff` to `#fff` (and any other similar `#ffffff` instances in those theme blocks) so they comply with the color-hex-length rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/settings.js`:
- Around line 2107-2120: drawSpectrumLayer currently returns early while
getSpectrumAnalyser() is null so a previously saved FFT selection
(spectrumFftKey) never gets reapplied; after you obtain a non-null analyser in
drawSpectrumLayer (the block using audioContextManager?.getSpectrumAnalyser?.()
and after the if (!analyser) return), call applyFft() (or the same routine that
applies spectrumFftKey) to reapply the saved FFT selection so the saved
2K/4K/16K choice takes effect once the dedicated analyser exists; reference
drawSpectrumLayer, audioContextManager.getSpectrumAnalyser, spectrumFftKey and
applyFft when adding this one-time reapply.
- Around line 4640-4749: The knob handlers update state and persist but never
trigger the spectrum redraw; after each change call the project's spectrum
redraw function (e.g. requestRedraw or redrawSpectrum) immediately after
render() and persist(). Update the wheel, dblclick, keydown cases and the
pointer handlers' onMove/onUp to invoke that redraw function right after
render() (and persist() where present) so the overlay updates when
set()/get()/render() change the value.
---
Outside diff comments:
In `@styles.css`:
- Around line 172-193: Update the 6-digit hex color values flagged by stylelint
to their 3-digit shorthand: locate the theme blocks (e.g.,
:root[data-theme='dark'] and the purple theme block around the other occurrence)
and change the CSS custom property --highlight-foreground from `#ffffff` to `#fff`
(and any other similar `#ffffff` instances in those theme blocks) so they comply
with the color-hex-length rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b69a241-67cc-4879-a039-733a491eb25d
📒 Files selected for processing (3)
index.htmljs/settings.jsstyles.css
🚧 Files skipped from review as they are similar to previous changes (1)
- index.html
Comment on lines
+2107
to
+2120
| const drawSpectrumLayer = (ctx, padLeft, padTop, w, h, sampleRate) => { | ||
| // Only use the dedicated spectrum analyser. If unavailable, bail so we | ||
| // never mutate fftSize on the shared visualizer node. | ||
| let analyser = null; | ||
| try { | ||
| analyser = audioContextManager?.getSpectrumAnalyser?.() || null; | ||
| } catch { | ||
| return; | ||
| } | ||
| if (!analyser) return; | ||
| if (!_spectrumData || _spectrumData.length !== analyser.frequencyBinCount) { | ||
| _spectrumData = new Float32Array(analyser.frequencyBinCount); | ||
| _spectrumEma = null; | ||
| } |
There was a problem hiding this comment.
Re-apply the saved FFT once the analyser actually exists.
applyFft() silently no-ops when getSpectrumAnalyser() is still null during initialization, and nothing later reapplies spectrumFftKey. After a reload, a saved 2K/4K/16K choice can fall back to the analyser default until the user clicks the FFT pill again.
Suggested fix
} catch {
return;
}
if (!analyser) return;
+ const desiredFftSize = SPECTRUM_FFT_PRESETS[spectrumFftKey];
+ if (analyser.fftSize !== desiredFftSize) {
+ analyser.fftSize = desiredFftSize;
+ }
if (!_spectrumData || _spectrumData.length !== analyser.frequencyBinCount) {
_spectrumData = new Float32Array(analyser.frequencyBinCount);
_spectrumEma = null;
}Also applies to: 4820-4833
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/settings.js` around lines 2107 - 2120, drawSpectrumLayer currently returns
early while getSpectrumAnalyser() is null so a previously saved FFT selection
(spectrumFftKey) never gets reapplied; after you obtain a non-null analyser in
drawSpectrumLayer (the block using audioContextManager?.getSpectrumAnalyser?.()
and after the if (!analyser) return), call applyFft() (or the same routine that
applies spectrumFftKey) to reapply the saved FFT selection so the saved
2K/4K/16K choice takes effect once the dedicated analyser exists; reference
drawSpectrumLayer, audioContextManager.getSpectrumAnalyser, spectrumFftKey and
applyFft when adding this one-time reapply.
Comment on lines
+4640
to
+4749
| const render = () => { | ||
| const v = Math.round(get()); | ||
| valueEl.textContent = String(v); | ||
| btn.setAttribute('aria-valuenow', String(v)); | ||
| btn.setAttribute('aria-valuetext', `${v} dBFS`); | ||
| }; | ||
| const persist = () => { | ||
| try { | ||
| localStorage.setItem(storageKey, String(get())); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| }; | ||
| render(); | ||
|
|
||
| btn.addEventListener( | ||
| 'wheel', | ||
| (e) => { | ||
| e.preventDefault(); | ||
| const delta = e.deltaY > 0 ? -1 : 1; | ||
| set(clamp(get() + delta)); | ||
| render(); | ||
| persist(); | ||
| }, | ||
| { passive: false } | ||
| ); | ||
|
|
||
| btn.addEventListener('dblclick', (e) => { | ||
| e.preventDefault(); | ||
| set(defaultValue); | ||
| render(); | ||
| persist(); | ||
| }); | ||
|
|
||
| // Keyboard: arrows adjust, Home/End jump to bounds, Shift for coarse | ||
| btn.addEventListener('keydown', (e) => { | ||
| const coarse = e.shiftKey ? 6 : 1; | ||
| let handled = true; | ||
| switch (e.key) { | ||
| case 'ArrowUp': | ||
| case 'ArrowRight': | ||
| set(clamp(get() + coarse)); | ||
| break; | ||
| case 'ArrowDown': | ||
| case 'ArrowLeft': | ||
| set(clamp(get() - coarse)); | ||
| break; | ||
| case 'PageUp': | ||
| set(clamp(get() + 10)); | ||
| break; | ||
| case 'PageDown': | ||
| set(clamp(get() - 10)); | ||
| break; | ||
| case 'Home': | ||
| set(min); | ||
| break; | ||
| case 'End': | ||
| set(max); | ||
| break; | ||
| case 'Enter': | ||
| case ' ': | ||
| set(defaultValue); | ||
| break; | ||
| default: | ||
| handled = false; | ||
| } | ||
| if (handled) { | ||
| e.preventDefault(); | ||
| render(); | ||
| persist(); | ||
| } | ||
| }); | ||
|
|
||
| btn.addEventListener('pointerdown', (e) => { | ||
| if (e.button !== 0) return; | ||
| e.preventDefault(); | ||
| try { | ||
| btn.setPointerCapture(e.pointerId); | ||
| } catch { | ||
| /* capture may fail on synthetic events */ | ||
| } | ||
| btn.classList.add('dragging'); | ||
| const startY = e.clientY; | ||
| const startVal = get(); | ||
| const onMove = (ev) => { | ||
| const dy = startY - ev.clientY; | ||
| set(clamp(startVal + dy * 0.4)); | ||
| render(); | ||
| }; | ||
| const onUp = (ev) => { | ||
| // Guard releasePointerCapture: capture may already be lost | ||
| // (e.g. pointercancel fired before pointerup) | ||
| try { | ||
| if (btn.hasPointerCapture?.(e.pointerId)) { | ||
| btn.releasePointerCapture(e.pointerId); | ||
| } | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| btn.classList.remove('dragging'); | ||
| btn.removeEventListener('pointermove', onMove); | ||
| btn.removeEventListener('pointerup', onUp); | ||
| btn.removeEventListener('pointercancel', onUp); | ||
| persist(); | ||
| ev.preventDefault(); | ||
| }; | ||
| btn.addEventListener('pointermove', onMove); | ||
| btn.addEventListener('pointerup', onUp); | ||
| btn.addEventListener('pointercancel', onUp); | ||
| }); |
There was a problem hiding this comment.
Redraw the graph when the Lo range knob changes.
These handlers update state and localStorage, but they never request a redraw. When the spectrum is held or the loop is otherwise stopped, the pill changes while the overlay stays stale until some unrelated repaint.
Suggested fix
const render = () => {
const v = Math.round(get());
valueEl.textContent = String(v);
btn.setAttribute('aria-valuenow', String(v));
btn.setAttribute('aria-valuetext', `${v} dBFS`);
+ scheduleDrawAutoEQGraph();
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/settings.js` around lines 4640 - 4749, The knob handlers update state and
persist but never trigger the spectrum redraw; after each change call the
project's spectrum redraw function (e.g. requestRedraw or redrawSpectrum)
immediately after render() and persist(). Update the wheel, dblclick, keydown
cases and the pointer handlers' onMove/onUp to invoke that redraw function right
after render() (and persist() where present) so the overlay updates when
set()/get()/render() change the value.
Brings in 31 commits from upstream while preserving the AutoEQ fork's custom commits (live spectrum analyzer overlay 4b8ec7b plus housekeeping). Conflicts resolved: - extension/README.md: upstream removed the extension folder (post-Q-DL migration); accept the deletion. - js/storage.js: take upstream's expanded Qobuz default list which adds qdl-api.monochrome.tf alongside qobuz.kennyy.com.br.
am-lyrics bumped to ^1.5.2 in upstream; refresh lockfile so bun install --frozen-lockfile in CI lint workflow passes.
Resolve git conflicts
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.
Ports the fork's spectrum analyzer feature on top of the latest upstream.
Dedicated high-resolution AnalyserNode (fftSize 8192, ~5.9 Hz/bin at 48 kHz) tapped off the main analyser as a side-chain in js/audio-context.js.
Overlay drawn into the EQ graph canvas with 1/N-octave smoothing, pink-tilt slope compensation, and EMA time-averaging driven by a Speed preset.
In-graph control pills above the canvas (Lo range knob, Hold, Speed, FFT, Spectrum toggle). Lo/Hi knobs scroll-or-drag vertically to adjust the dB range; state persists in localStorage.
rAF loop restarts on EQ/mode changes so the overlay stays live.
Layout: grid with pills on top and canvas below on desktop; flex-column stack with flex-wrap on narrow mobile (<=600px) so the Spectrum pill no longer overlaps or wraps awkwardly over the graph.
Description
Type of Change
Checklist
By submitting this PR, I agree to follow the guidelines. I understand that the final decision to merge rests with the maintainers and that not all contributions can be accepted.
Summary by CodeRabbit
New Features
Style
Documentation