From 79313e7a0a8372ae79f44a8560cc22cd75951abc Mon Sep 17 00:00:00 2001 From: tryptz Date: Tue, 7 Apr 2026 09:35:38 -0400 Subject: [PATCH] fix: address PR #523 review comments for EQ and binaural DSP - Remove stale IIR coefficient JSDoc comment - Reset M/S channel state on EQ import to prevent stale assignments - Enforce strictly increasing GEQ frequencies to prevent rounding duplicates - Guard Q calculation against zero octave spacing (Infinity/NaN) - Export EQ from stored metadata instead of live BiquadFilterNode state - Accept .csv in legacy GEQ import file input - Expose public reconnect() on BinauralDSP instead of calling _connectInternal - Dispatch binaural-mode-changed on channel count change, not just mode change - Remove no-op channelCount/channelCountMode on MediaElementAudioSourceNode - Add void to floating promises (toggleBinaural, notifyBinauralChannelCount, _loadBinauralSettings) - Wrap binauralDspSettings._setAll in try/catch for QuotaExceededError - Make generateHRTF synchronous (no awaits, was misleadingly async) --- index.html | 2 +- js/audio-context.js | 22 +++++++--------------- js/binaural-dsp.js | 12 ++++++++++-- js/equalizer.js | 5 ++--- js/hrtf-generator.js | 2 +- js/player.js | 6 +++--- js/settings.js | 11 +++++++++-- js/storage.js | 6 +++++- 8 files changed, 38 insertions(+), 28 deletions(-) diff --git a/index.html b/index.html index c59aec1..927947a 100644 --- a/index.html +++ b/index.html @@ -4595,7 +4595,7 @@ diff --git a/js/audio-context.js b/js/audio-context.js index d734057..10a1588 100644 --- a/js/audio-context.js +++ b/js/audio-context.js @@ -6,11 +6,6 @@ import { isIos } from './platform-detection.js'; import { equalizerSettings, monoAudioSettings, binauralDspSettings } from './storage.js'; import { BinauralDSP } from './binaural-dsp.js'; -/** - * Compute RBJ cookbook IIR coefficients for shelf filters with Q support. - * Web Audio API's BiquadFilterNode ignores Q for lowshelf/highshelf, - * so we use IIRFilterNode with these coefficients instead. - */ // Generate frequency array for given number of bands using logarithmic spacing function generateFrequencies(bandCount, minFreq = 20, maxFreq = 20000) { const frequencies = []; @@ -493,14 +488,6 @@ class AudioContextManager { if (!this.sources.has(audioElement)) { const src = this.audioContext.createMediaElementSource(audioElement); - // Allow multichannel passthrough for Atmos/spatial audio - try { - src.channelCount = 6; - src.channelCountMode = 'max'; - src.channelInterpretation = 'discrete'; - } catch { - // Some browsers may not support this - } this.sources.set(audioElement, src); } this.source = this.sources.get(audioElement); @@ -520,7 +507,7 @@ class AudioContextManager { // Create binaural DSP processor this.binauralDsp = new BinauralDSP(this.audioContext); - this._loadBinauralSettings(); + void this._loadBinauralSettings(); this._createEQ(); this._createGraphicEQ(); @@ -689,7 +676,7 @@ class AudioContextManager { if (this.isBinauralEnabled && this.binauralDsp) { const { input, output } = this.binauralDsp.getNodes(); lastNode.connect(input); - this.binauralDsp._connectInternal(); + this.binauralDsp.reconnect(); lastNode = output; } @@ -1432,6 +1419,10 @@ class AudioContextManager { this.currentQs = qs; this.currentGains = gains; + // Reset M/S channel assignments — imported config has no channel info + this.currentChannels = new Array(this.bandCount).fill('stereo'); + this.msEnabled = false; + // Rebuild EQ chain to apply new frequencies, types, and Qs if (this.isInitialized && this.audioContext) { this._destroyMSFilters(); @@ -1446,6 +1437,7 @@ class AudioContextManager { equalizerSettings.setGains(this.currentGains); equalizerSettings.setBandTypes(this.currentTypes); equalizerSettings.setBandQs(this.currentQs); + equalizerSettings.setBandChannels(this.currentChannels); return true; } catch (e) { diff --git a/js/binaural-dsp.js b/js/binaural-dsp.js index e9417a9..402e9ae 100644 --- a/js/binaural-dsp.js +++ b/js/binaural-dsp.js @@ -88,6 +88,13 @@ export class BinauralDSP { return { input: this.inputNode, output: this.outputNode }; } + /** + * Reconnect internal graph (public API for external callers). + */ + reconnect() { + this._connectInternal(); + } + /** * Connect internal graph based on current state. */ @@ -595,6 +602,7 @@ export class BinauralDSP { */ async detectAndConfigure(channelCount) { const prevMode = this.mode; + const prevChannels = this.channelCount; this.channelCount = channelCount; if (channelCount > 2) { @@ -603,13 +611,13 @@ export class BinauralDSP { this.mode = 'stereo'; } - if (this.enabled && this.mode !== prevMode) { + if (this.enabled && (this.mode !== prevMode || channelCount !== prevChannels)) { await this._ensureNodesCreated(); this._connectInternal(); window.dispatchEvent( new CustomEvent('binaural-mode-changed', { - detail: { mode: this.mode, channels: channelCount }, + detail: { mode: this.mode, channels: this.channelCount }, }) ); } diff --git a/js/equalizer.js b/js/equalizer.js index 7bd626b..ad7ca14 100644 --- a/js/equalizer.js +++ b/js/equalizer.js @@ -621,11 +621,10 @@ export class Equalizer { this.frequencies.forEach((freq, index) => { const gain = this.currentGains[index] || 0; - const filter = this.filters[index]; - const type = filter ? filter.type : 'peaking'; + const type = this.currentTypes[index] || 'peaking'; const typeMap = { peaking: 'PK', lowshelf: 'LSC', highshelf: 'HSC' }; const typeStr = typeMap[type] || 'PK'; - const q = filter ? filter.Q.value : this._calculateQ(index); + const q = this.currentQs[index] || this._calculateQ(index); const filterNum = index + 1; lines.push(`Filter ${filterNum}: ON ${typeStr} Fc ${freq} Hz Gain ${gain.toFixed(1)} dB Q ${q.toFixed(2)}`); }); diff --git a/js/hrtf-generator.js b/js/hrtf-generator.js index ebd577a..30b0bf3 100644 --- a/js/hrtf-generator.js +++ b/js/hrtf-generator.js @@ -48,7 +48,7 @@ function calculateHeadShadow(frequency, azimuthRad) { * @param {number} [elevationDeg=0] - Elevation in degrees (currently simplified) * @returns {Promise} Stereo AudioBuffer with HRTF IR */ -export async function generateHRTF(audioContext, azimuthDeg, elevationDeg = 0) { +export function generateHRTF(audioContext, azimuthDeg, elevationDeg = 0) { const sampleRate = audioContext.sampleRate; const buffer = audioContext.createBuffer(2, IR_LENGTH, sampleRate); diff --git a/js/player.js b/js/player.js index d484fe6..664d47c 100644 --- a/js/player.js +++ b/js/player.js @@ -1885,7 +1885,7 @@ export class Player { if (isAtmosPlaying) { // Auto-enable binaural DSP for spatial content if (binauralDspSettings.getAutoEnableForSpatial() && !binauralDspSettings.isEnabled()) { - audioContextManager.toggleBinaural(true); + void audioContextManager.toggleBinaural(true); // Update toggle in settings UI if visible const toggle = document.getElementById('binaural-dsp-toggle'); if (toggle) toggle.checked = true; @@ -1893,7 +1893,7 @@ export class Player { if (container) container.style.display = 'block'; } // Notify binaural DSP of multichannel content (Atmos is typically 5.1+) - audioContextManager.notifyBinauralChannelCount(6); + void audioContextManager.notifyBinauralChannelCount(6); const binauralActive = audioContextManager.isBinauralActive(); badgeEl.className = 'quality-badge quality-atmos shaka-quality-badge'; @@ -1901,7 +1901,7 @@ export class Player { SVG_ATMOS(20) + (binauralActive ? ' Binaural' : ''); } else { // Notify binaural DSP that we're in stereo mode - audioContextManager.notifyBinauralChannelCount(2); + void audioContextManager.notifyBinauralChannelCount(2); badgeEl.className = 'quality-badge quality-hires shaka-quality-badge'; badgeEl.textContent = text; } diff --git a/js/settings.js b/js/settings.js index 23a8df6..a39b666 100644 --- a/js/settings.js +++ b/js/settings.js @@ -1353,7 +1353,12 @@ export async function initializeSettings(scrobbler, player, api, ui) { const freqs = []; for (let i = 0; i < count; i++) { const t = i / (count - 1); - freqs.push(Math.round(min * Math.pow(max / min, t))); + let freq = Math.round(min * Math.pow(max / min, t)); + // Ensure strictly increasing — rounding can produce duplicates at high band counts + if (freqs.length > 0 && freq <= freqs[freqs.length - 1]) { + freq = freqs[freqs.length - 1] + 1; + } + freqs.push(freq); } return freqs; }; @@ -1561,7 +1566,9 @@ export async function initializeSettings(scrobbler, player, api, ui) { 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); + const q = octaves > 0 + ? (Math.SQRT2 / (2 * Math.sinh((Math.LN2 / 2) * octaves))).toFixed(2) + : Math.SQRT2.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' }); diff --git a/js/storage.js b/js/storage.js index 55ce212..27a598d 100644 --- a/js/storage.js +++ b/js/storage.js @@ -1878,7 +1878,11 @@ export const binauralDspSettings = { }, _setAll(obj) { - localStorage.setItem(this.STORAGE_KEY, JSON.stringify(obj)); + try { + localStorage.setItem(this.STORAGE_KEY, JSON.stringify(obj)); + } catch { + // QuotaExceededError — storage full + } }, isEnabled() {