fix: address CodeRabbit review - array padding, storage validation, Q accuracy

- Pad band arrays when import/AutoEQ has fewer filters than MIN_BANDS
- Early return in applyAutoEQBands for zero enabled bands
- Persist interpolated gains before audio rebuild on band count change
- Derive export Q from actual octave spacing between adjacent bands
- Validate inputs in all GEQ storage setters
This commit is contained in:
tryptz 2026-04-07 00:35:20 -04:00 committed by edideaur
parent 55b49cc77a
commit a16d489b9f
3 changed files with 55 additions and 16 deletions

View file

@ -875,6 +875,7 @@ class AudioContextManager {
if (!bands || bands.length === 0) return '';
const enabledBands = bands.filter((b) => b.enabled);
if (enabledBands.length === 0) return '';
const count = Math.max(equalizerSettings.MIN_BANDS, Math.min(equalizerSettings.MAX_BANDS, enabledBands.length));
// Calculate preamp: negative of cumulative peak gain across all bands to prevent clipping
@ -899,13 +900,20 @@ class AudioContextManager {
// Sort bands by frequency so index order is deterministic
const sortedBands = [...enabledBands].sort((a, b) => a.freq - b.freq);
// Build normalized band descriptor arrays
const newFrequencies = sortedBands
.slice(0, count)
.map((b) => Math.round(Math.min(b.freq, (this.audioContext?.sampleRate ?? 48000) / 2 - 1)));
const newTypes = sortedBands.slice(0, count).map((b) => b.type || 'peaking');
const newQs = sortedBands.slice(0, count).map((b) => b.q);
const newGains = sortedBands.slice(0, count).map((b) => this._clampGain(b.gain));
// Build normalized band descriptor arrays, pad if fewer enabled bands than minimum
const maxFreq = (this.audioContext?.sampleRate ?? 48000) / 2 - 1;
const slicedBands = sortedBands.slice(0, count);
const newFrequencies = slicedBands.map((b) => Math.round(Math.min(b.freq, maxFreq)));
const newTypes = slicedBands.map((b) => b.type || 'peaking');
const newQs = slicedBands.map((b) => b.q);
const newGains = slicedBands.map((b) => this._clampGain(b.gain));
while (newFrequencies.length < count) {
const lastFreq = newFrequencies[newFrequencies.length - 1] || 1000;
newFrequencies.push(Math.round(Math.min(lastFreq * 2, maxFreq)));
newTypes.push('peaking');
newQs.push(1.0);
newGains.push(0);
}
// Update band count via class setter to trigger equalizer-band-count-changed event
if (count !== this.bandCount) {
@ -1075,10 +1083,29 @@ class AudioContextManager {
HSC: 'highshelf',
HSF: 'highshelf',
};
this.frequencies = sliced.map((f) => f.freq);
this.currentTypes = sliced.map((f) => typeMap[f.type] || 'peaking');
this.currentQs = sliced.map((f) => f.q);
this.currentGains = sliced.map((f) => this._clampGain(f.gain));
// Pad arrays to bandCount if import has fewer filters than minimum
const padCount = this.bandCount - sliced.length;
const freqs = sliced.map((f) => f.freq);
const types = sliced.map((f) => typeMap[f.type] || 'peaking');
const qs = sliced.map((f) => f.q);
const gains = sliced.map((f) => this._clampGain(f.gain));
if (padCount > 0) {
const lastFreq = freqs[freqs.length - 1] || 1000;
const maxFreq = (this.audioContext?.sampleRate ?? 48000) / 2 - 1;
for (let p = 0; p < padCount; p++) {
const padFreq = Math.min(lastFreq * Math.pow(2, p + 1), maxFreq);
freqs.push(Math.round(padFreq));
types.push('peaking');
qs.push(this._calculateQ(freqs.length - 1));
gains.push(0);
}
}
this.frequencies = freqs;
this.currentTypes = types;
this.currentQs = qs;
this.currentGains = gains;
// Rebuild EQ chain to apply new frequencies, types, and Qs
if (this.isInitialized && this.audioContext) {

View file

@ -1407,6 +1407,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
if (newCount === geqBandCount) return;
geqGains = equalizerSettings._interpolateGains(geqGains, newCount);
geqBandCount = newCount;
equalizerSettings.setGraphicEqGains(geqGains);
audioContextManager.setGraphicEqBandCount(newCount);
rebuildGeq();
geqPresetSelects.forEach((s) => (s.value = ''));
@ -1454,7 +1455,11 @@ export async function initializeSettings(scrobbler, player, api, ui) {
legacyGeqExportBtn.addEventListener('click', () => {
const lines = [`Preamp: ${geqPreamp.toFixed(1)} dB`];
GEQ_FREQUENCIES.forEach((freq, i) => {
const q = (2.5 * Math.sqrt(16 / geqBandCount)).toFixed(2);
// Q from octave spacing between adjacent bands
const prev = GEQ_FREQUENCIES[Math.max(0, i - 1)];
const next = GEQ_FREQUENCIES[Math.min(GEQ_FREQUENCIES.length - 1, i + 1)];
const octaves = Math.log2(next / prev);
const q = (Math.SQRT2 / (2 * Math.sinh((Math.LN2 / 2) * octaves))).toFixed(2);
lines.push(`Filter ${i + 1}: ON PK Fc ${freq} Hz Gain ${geqGains[i].toFixed(1)} dB Q ${q}`);
});
const blob = new Blob([lines.join('\n')], { type: 'text/plain' });

View file

@ -1735,8 +1735,9 @@ export const equalizerSettings = {
},
setGraphicEqBandCount(count) {
const clamped = Math.max(3, Math.min(32, parseInt(count, 10) || 16));
try {
localStorage.setItem(this.GEQ_BAND_COUNT_KEY, String(count));
localStorage.setItem(this.GEQ_BAND_COUNT_KEY, String(clamped));
} catch { /* ignore */ }
},
@ -1754,8 +1755,11 @@ export const equalizerSettings = {
},
setGraphicEqFreqRange(min, max) {
const clampedMin = Math.max(10, Math.min(96000, parseInt(min, 10) || 25));
const clampedMax = Math.max(10, Math.min(96000, parseInt(max, 10) || 20000));
if (clampedMin >= clampedMax) return;
try {
localStorage.setItem(this.GEQ_FREQ_RANGE_KEY, JSON.stringify({ min, max }));
localStorage.setItem(this.GEQ_FREQ_RANGE_KEY, JSON.stringify({ min: clampedMin, max: clampedMax }));
} catch { /* ignore */ }
},
@ -1779,8 +1783,10 @@ export const equalizerSettings = {
},
setGraphicEqGains(gains) {
if (!Array.isArray(gains)) return;
const sanitized = gains.map((v) => (Number.isFinite(v) ? v : 0));
try {
localStorage.setItem(this.GEQ_GAINS_KEY, JSON.stringify(gains));
localStorage.setItem(this.GEQ_GAINS_KEY, JSON.stringify(sanitized));
} catch {
/* ignore */
}
@ -1800,8 +1806,9 @@ export const equalizerSettings = {
},
setGraphicEqPreamp(db) {
const clamped = Math.max(-20, Math.min(20, parseFloat(db) || 0));
try {
localStorage.setItem(this.GEQ_PREAMP_KEY, String(db));
localStorage.setItem(this.GEQ_PREAMP_KEY, String(clamped));
} catch {
/* ignore */
}