From cd622eeb61785c927675d02ca3157a62ffb09259 Mon Sep 17 00:00:00 2001 From: Charles Drani Date: Fri, 2 Jan 2026 13:15:23 -0700 Subject: [PATCH 1/3] fix: prevent EQ and reverb double-application race condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a bug where EQ and reverb effects could be applied twice, causing excessive audio processing. The issue was most likely to occur on Windows due to slower AudioContext operations, but could happen on any platform during rapid track transitions or effect changes. Root causes fixed: 1. Race condition in effect reapplication - Multiple async paths could trigger updateAudioEffect() simultaneously without synchronization 2. Incomplete equalizer cleanup - The filter chain's input/output nodes weren't being disconnected from the external audio chain, allowing duplicate connections to accumulate 3. Redundant effect dispatch - processMediaPlayInit() was calling setEffect() when REQUEST_EFFECT_REAPPLY already handles this Changes: - media-override.ts: Added synchronization lock (_effectUpdateInProgress) to ensure only one effect update runs at a time - audio-manager.ts: Fixed cleanupEffectChain() to properly disconnect equalizer input/output nodes from external chain while preserving internal filter connections - track.ts: Removed redundant setEffect() call from processMediaPlayInit() since REQUEST_EFFECT_REAPPLY message handles effect reapplication - SkipButton.svelte: Added chorus:track-blocked event dispatcher to sync with blocked tracks dialog The fix ensures thread-safe effect application and prevents duplicate audio processing without changing the public API. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- src/lib/audio-effects/audio-manager.ts | 16 +++++-- src/lib/components/SkipButton.svelte | 13 +++++- src/lib/media/media-override.ts | 64 ++++++++++++++++---------- src/lib/observers/track.ts | 4 +- 4 files changed, 65 insertions(+), 32 deletions(-) diff --git a/src/lib/audio-effects/audio-manager.ts b/src/lib/audio-effects/audio-manager.ts index 417e17b8..c662c81f 100644 --- a/src/lib/audio-effects/audio-manager.ts +++ b/src/lib/audio-effects/audio-manager.ts @@ -329,16 +329,22 @@ export default class AudioManager { this._soundTouchNode.disconnect() // Disconnect active effects - // NOTE: For equalizer with separate input/output nodes, we DON'T call disconnect() - // on them because they're part of an internal filter chain. Calling disconnect() - // would break the internal connections between filters. We only need to disconnect - // single-node effects or effects that manage their own internal state. + // NOTE: For equalizer with separate input/output nodes, we need to disconnect + // both the input and output nodes from the external chain, but NOT the internal + // connections between filters (the equalizer class handles those). if (this._activeEffects.equalizer) { if ( typeof this._activeEffects.equalizer === 'object' && 'input' in this._activeEffects.equalizer ) { - // Don't disconnect - the equalizer manages its own internal chain + // Disconnect input and output from the external chain + // This prevents duplicate connections when rebuilding + try { + this._activeEffects.equalizer.input.disconnect() + this._activeEffects.equalizer.output.disconnect() + } catch (e) { + // Ignore disconnect errors during cleanup + } } else { this._activeEffects.equalizer.disconnect() } diff --git a/src/lib/components/SkipButton.svelte b/src/lib/components/SkipButton.svelte index 6e9292d2..03a7b094 100644 --- a/src/lib/components/SkipButton.svelte +++ b/src/lib/components/SkipButton.svelte @@ -34,11 +34,20 @@ async function handleBlock() { if ($nowPlaying.track_id) { nowPlaying.set({ ...$nowPlaying, blocked: true }) + const { cover, track_id } = $nowPlaying + await dataStore.updateTrack({ - track_id: $nowPlaying.track_id, - value: { blocked: true } + track_id, + value: { blocked: true, cover } }) + + document.dispatchEvent( + new CustomEvent('chorus:track-blocked', { + detail: { track_id: track_id } + }) + ) highlightInTrackList() + } trackObserver?.skipTrack() diff --git a/src/lib/media/media-override.ts b/src/lib/media/media-override.ts index 8c10e762..7f2e8703 100644 --- a/src/lib/media/media-override.ts +++ b/src/lib/media/media-override.ts @@ -22,6 +22,7 @@ export default class MediaOverride { private _sources: any[] = [] private _chorusRate: number = 1 private _chorusPreservesPitch: boolean = true + private _effectUpdateInProgress: Promise | null = null constructor(options: MediaOverrideOptions) { this.source = options.source @@ -151,37 +152,52 @@ export default class MediaOverride { return } - try { - await this.audioManager.ensureAudioChainReady() + // Wait for any in-progress effect update to complete before starting a new one + // This prevents race conditions where multiple rapid effect changes could cause double-application + if (this._effectUpdateInProgress) { + await this._effectUpdateInProgress + } + + // Create a new promise for this update operation + this._effectUpdateInProgress = (async () => { + try { + await this.audioManager.ensureAudioChainReady() - // If clear is requested, disconnect all effects - if (effect.clear) { + // If clear is requested, disconnect all effects + if (effect.clear) { + this.audioManager.disconnect() + return + } + + // Disconnect all effects first this.audioManager.disconnect() - return - } - // Disconnect all effects first - this.audioManager.disconnect() + // Apply effects in the order they'll be chained: equalizer → MS processor → reverb + // Each effect can be applied independently + if (effect?.equalizer && effect.equalizer !== 'none') { + this.equalizer.setEQEffect(effect.equalizer) + } - // Apply effects in the order they'll be chained: equalizer → MS processor → reverb - // Each effect can be applied independently - if (effect?.equalizer && effect.equalizer !== 'none') { - this.equalizer.setEQEffect(effect.equalizer) - } + // Apply MS processor (can be combined with any other effect) + if (effect?.msProcessor && effect.msProcessor !== 'none') { + await this.msProcessor.setMSEffect(effect.msProcessor) + } - // Apply MS processor (can be combined with any other effect) - if (effect?.msProcessor && effect.msProcessor !== 'none') { - await this.msProcessor.setMSEffect(effect.msProcessor) + // Apply reverb (can be combined with any other effect) + if (effect?.reverb && effect.reverb !== 'none') { + await this.reverb.setReverbEffect(effect.reverb) + } + } catch (error) { + console.error('Error updating audio effects:', error) + this.audioManager.disconnect() + } finally { + // Clear the lock when done + this._effectUpdateInProgress = null } + })() - // Apply reverb (can be combined with any other effect) - if (effect?.reverb && effect.reverb !== 'none') { - await this.reverb.setReverbEffect(effect.reverb) - } - } catch (error) { - console.error('Error updating audio effects:', error) - this.audioManager.disconnect() - } + // Wait for this update to complete + await this._effectUpdateInProgress } async updateMSParams(params: MSParams): Promise { diff --git a/src/lib/observers/track.ts b/src/lib/observers/track.ts index 11e7f827..c0f834c3 100644 --- a/src/lib/observers/track.ts +++ b/src/lib/observers/track.ts @@ -48,7 +48,9 @@ export class TrackObserver { private async processMediaPlayInit() { await this.trackStateManager.updateTrackType() await this.trackStateManager.setPlayback(this.audioPreset) - this.setEffect() + // Note: setEffect() will be called via REQUEST_EFFECT_REAPPLY message + // dispatched from media-element.ts, so we don't need to call it here + // to avoid redundant effect applications } // Simplified getters for commonly accessed stores From df1cd5916eef1134c815d5a4ba0d0c35687ed942 Mon Sep 17 00:00:00 2001 From: Charles Drani Date: Fri, 2 Jan 2026 15:45:19 -0700 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=90=9B=20fix:=20sync=20blocked=20trac?= =?UTF-8?q?k=20state=20between=20SkipButton=20and=20TrackListSkipButton?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Listen for both chorus:track-blocked and chorus:track-unblocked events in TrackListSkipButton to properly reflect block state changes made from SkipButton or BlockedTracksDialog. Query dataStore.blocked as source of truth when handling events instead of relying on static prop values. --- src/lib/components/SkipButton.svelte | 32 ++----------------- src/lib/components/TrackListSkipButton.svelte | 25 ++++++++------- 2 files changed, 17 insertions(+), 40 deletions(-) diff --git a/src/lib/components/SkipButton.svelte b/src/lib/components/SkipButton.svelte index 03a7b094..f91b314c 100644 --- a/src/lib/components/SkipButton.svelte +++ b/src/lib/components/SkipButton.svelte @@ -7,34 +7,10 @@ import * as Tooltip from '$lib/components/ui/tooltip' import { buttonVariants } from '$lib/components/ui/button' - function highlightInTrackList() { - const rowsQuery = document.querySelectorAll('[data-testid="tracklist-row"]') - if (!rowsQuery?.length) return - - const trackRows = Array.from(rowsQuery) - - const context = trackRows.find( - (row) => - row.querySelector('a[data-testid="internal-track-link"] div')?.textContent === - $nowPlaying.title - ) - - if (!context) return - - const blockIcon = context.querySelector('button[role="block"]') - if (!blockIcon) return - - const svg = blockIcon.querySelector('svg') - if (!svg) return - - blockIcon.setAttribute('aria-label', 'Block Track') - svg.style.stroke = '#1ed760' - } - - async function handleBlock() { + async function handleBlock(): Promise { if ($nowPlaying.track_id) { - nowPlaying.set({ ...$nowPlaying, blocked: true }) const { cover, track_id } = $nowPlaying + console.log({ cover, track_id }) await dataStore.updateTrack({ track_id, @@ -43,11 +19,9 @@ document.dispatchEvent( new CustomEvent('chorus:track-blocked', { - detail: { track_id: track_id } + detail: { track_id: track_id, cover } }) ) - highlightInTrackList() - } trackObserver?.skipTrack() diff --git a/src/lib/components/TrackListSkipButton.svelte b/src/lib/components/TrackListSkipButton.svelte index 78df992d..74aae7b2 100644 --- a/src/lib/components/TrackListSkipButton.svelte +++ b/src/lib/components/TrackListSkipButton.svelte @@ -9,21 +9,25 @@ import type { SimpleTrack } from '$lib/stores/data/cache' let { track }: { track: SimpleTrack } = $props() - let isBlocked = $state(track?.blocked ?? false) + let isBlocked = $state(track?.blocked || false) - // Listen for unblock events from BlockedTracksDialog (event-driven, no polling) $effect(() => { - if (!isBlocked) return - - const handleUnblock = (event: Event) => { + const handleBlockUnBlock = (event: Event) => { const customEvent = event as CustomEvent<{ track_id: string }> if (customEvent.detail.track_id === track.track_id) { - isBlocked = false + const found = dataStore.blocked.find( + (blockedTrack) => blockedTrack.track_id == track.track_id + ) + isBlocked = !!found } } - document.addEventListener('chorus:track-unblocked', handleUnblock) - return () => document.removeEventListener('chorus:track-unblocked', handleUnblock) + document.addEventListener('chorus:track-blocked', handleBlockUnBlock) + document.addEventListener('chorus:track-unblocked', handleBlockUnBlock) + return () => { + document.removeEventListener('chorus:track-blocked', handleBlockUnBlock) + document.removeEventListener('chorus:track-unblocked', handleBlockUnBlock) + } }) function getCoverArt() { @@ -41,14 +45,13 @@ const cover = track?.cover || getCoverArt() await dataStore.updateTrack({ track_id: track.track_id, - value: { blocked: isBlocked, cover } + value: { blocked: isBlocked || null, cover } }) - // Emit event for reactive updates if (isBlocked) { document.dispatchEvent( new CustomEvent('chorus:track-blocked', { - detail: { track_id: track.track_id } + detail: { track_id: track.track_id, cover } }) ) From a520dfdb5d0b19c68acd11cec06e0ad4db6ca3c9 Mon Sep 17 00:00:00 2001 From: Charles Drani Date: Fri, 2 Jan 2026 21:14:32 -0700 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=90=9B=20fix:=20correct=20loop=20coun?= =?UTF-8?q?t=20behavior=20and=20enable=20looping=20without=20snip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix loop count logic: iteration now decrements to 0 before stopping - Enable looping for full tracks (not just snipped tracks) - Handle track advancement after loop count exhausted - Properly separate infinite vs count-based loop handling --- src/lib/observers/track.ts | 20 +++++++++---- src/lib/services/playback-controller.ts | 39 ++++++++++++++++++------- src/lib/stores/loop.ts | 2 +- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/lib/observers/track.ts b/src/lib/observers/track.ts index c0f834c3..85a0b67d 100644 --- a/src/lib/observers/track.ts +++ b/src/lib/observers/track.ts @@ -89,7 +89,7 @@ export class TrackObserver { playbackObserver.updateChorusUI() } - private isAtSnipEnd(currentTimeMS: number): boolean { + private isAtTrackOrSnipEnd(currentTimeMS: number): boolean { return this.trackStateManager.isTrackAtSnipEnd(currentTimeMS, this.currentSong) } @@ -151,10 +151,13 @@ export class TrackObserver { return this.updateCurrentTime(this.snip.start_time) } - // Handle looping if at snip end - if (this.loop.looping && currentSong.snip && this.isAtSnipEnd(currentTimeMS)) { + // Handle looping if at track/snip end + if (this.loop.looping && this.isAtTrackOrSnipEnd(currentTimeMS)) { const loopHandled = await this.playbackController.handleLooping(currentTimeMS) if (loopHandled) return + // Loop count exhausted, skip to next track + this.skipTrack() + return } // Handle shared snip URL cleanup @@ -164,17 +167,24 @@ export class TrackObserver { // Check if at or past snip end for auto-advance const atSnipEnd = currentSong.snip && currentTimeMS >= currentSong.snip.end_time * 1000 + const atTrackEnd = currentTimeMS >= currentSong.duration * 1000 - 100 // Handle track end or snip end - auto-advance if at or past end const shouldSkip = - (currentSong.snip || currentSong.blocked) && - (currentTimeMS >= currentSong.duration * 1000 || atSnipEnd) + (currentSong.snip || currentSong.blocked) && (atTrackEnd || atSnipEnd) if (shouldSkip) { this.skipTrack() return } + // Handle end of track after loop count exhausted (for non-snip tracks) + // When looping ends, iteration is 0 and looping is false + if (this.loop.type === 'amount' && this.loop.iteration === 0 && atTrackEnd) { + this.skipTrack() + return + } + // Early return if we have a snip but not yet at the end (still playing within snip) if (currentSong.snip && !atSnipEnd) return }, 50) diff --git a/src/lib/services/playback-controller.ts b/src/lib/services/playback-controller.ts index 385bb25a..02821716 100644 --- a/src/lib/services/playback-controller.ts +++ b/src/lib/services/playback-controller.ts @@ -53,24 +53,41 @@ export class PlaybackController { } shouldSkipTrack(songInfo: NowPlaying): boolean { - return songInfo?.blocked || - configStore.checkIfTrackShouldBeSkipped({ - title: songInfo?.title ?? '', - artist: songInfo?.artist ?? '' - }) + return ( + songInfo?.blocked || + configStore.checkIfTrackShouldBeSkipped({ + title: songInfo?.title ?? '', + artist: songInfo?.artist ?? '' + }) + ) } async handleLooping(currentTimeMS: number): Promise { const loop = get(loopStore) const currentSong = get(nowPlaying) - + if (!loop.looping) return false + if (loop.type === 'infinite') { + // Infinite loop: always seek back to start + this.updateCurrentTime(currentSong.snip?.start_time ?? 0) + return true + } + + // Count-based loop: check if we have iterations left if (loop.type === 'amount') { - await loopStore.decrement() + if (loop.iteration >= 1) { + // Iterations remaining, decrement and loop back + await loopStore.decrement() + this.updateCurrentTime(currentSong.snip?.start_time ?? 0) + return true + } else { + // No iterations left (iteration === 0), stop looping and advance + await loopStore.resetIteration() + return false + } } - - this.updateCurrentTime(currentSong.snip?.start_time ?? 0) - return true + + return false } -} \ No newline at end of file +} diff --git a/src/lib/stores/loop.ts b/src/lib/stores/loop.ts index 0c03c4d7..cc5c09fa 100644 --- a/src/lib/stores/loop.ts +++ b/src/lib/stores/loop.ts @@ -75,7 +75,7 @@ function createLoopStore() { const newIteration = state.iteration - 1 return { ...state, - iteration: newIteration == 0 ? state.amount : newIteration, + iteration: Math.max(0, newIteration), looping: newIteration > 0 } })