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)
2026-04-07 09:35:38 -04:00 · 2026-04-07 09:35:38 -04:00 · 79313e7a0a
commit 79313e7a0a
parent 88b01570f5
8 changed files with 38 additions and 28 deletions
--- a/index.html
+++ b/index.html
@ -4595,7 +4595,7 @@
                                            <input
                                                type="file"
                                                id="legacy-geq-import-file"
-                                                accept=".txt"
+                                                accept=".txt,.csv"
                                                style="display: none"
                                            />
                                        </div>
--- 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) {
--- 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 },
                })
            );
        }
--- 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)}`);
        });
--- 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<AudioBuffer>} 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);

--- 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 ? ' <span class="binaural-badge">Binaural</span>' : '');
                    } 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;
                    }
--- 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' });
--- 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() {