fix(audio): Phase 1 — stop re-init loop, fix null-crash in play(), fix RAF leak

- useWaveform: remove globalCurrentTime/globalIsPlaying from useEffect deps;
  WaveSurfer was re-initializing every 250ms while audio played. Dep array
  is now [url, songId, bandId]. Store reads inside the effect use getState()
  snapshots instead of reactive values.
- useWaveform: move animationFrameId outside the async function so the
  useEffect cleanup can actually cancel the RAF loop. Previously the cleanup
  was returned from the inner async function and React never called it —
  loops accumulated on every re-render.
- audioService: remove isDifferentSong + cleanup() call from play(). cleanup()
  set this.wavesurfer = null and then play() immediately called
  this.wavesurfer.play(), throwing a TypeError on every song switch.
- audioService: replace new Promise(async executor) anti-pattern in
  initialize() with a plain executor + extracted onReady().catch(reject) so
  errors inside the ready handler are always forwarded to the promise.
- audioService: remove currentPlayingSongId/currentPlayingBandId private
  fields whose only reader was the deleted isDifferentSong block.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Mistral Vibe
2026-04-08 20:44:07 +02:00
parent ef73e45da2
commit 1a0d926e1a
2 changed files with 182 additions and 152 deletions

View File

@@ -30,44 +30,56 @@ export function useWaveform(
const [initializationState, setInitializationState] = useState<InitializationState>(InitializationState.NotStarted); const [initializationState, setInitializationState] = useState<InitializationState>(InitializationState.NotStarted);
const markersRef = useRef<CommentMarker[]>([]); const markersRef = useRef<CommentMarker[]>([]);
// Global player state - use shallow comparison to reduce re-renders
const {
isPlaying: globalIsPlaying,
currentTime: globalCurrentTime,
currentSongId,
currentBandId: globalBandId,
currentPlayingSongId,
currentPlayingBandId,
setCurrentSong
} = usePlayerStore(state => ({
isPlaying: state.isPlaying,
currentTime: state.currentTime,
currentSongId: state.currentSongId,
currentBandId: state.currentBandId,
currentPlayingSongId: state.currentPlayingSongId,
currentPlayingBandId: state.currentPlayingBandId,
setCurrentSong: state.setCurrentSong
}));
useEffect(() => { useEffect(() => {
if (!containerRef.current) { if (!containerRef.current) return;
return; if (!options.url || options.url === 'null' || options.url === 'undefined') return;
}
if (!options.url || options.url === 'null' || options.url === 'undefined') { // animationFrameId is declared here so the useEffect cleanup can cancel it
return; // even if initializeAudio hasn't finished yet
} let animationFrameId: number | null = null;
const initializeAudio = async () => { const initializeAudio = async () => {
try { try {
await audioService.initialize(containerRef.current!, options.url!); await audioService.initialize(containerRef.current!, options.url!);
// Set up local state synchronization with requestAnimationFrame for smoother updates // Update global song context
let animationFrameId: number | null = null; if (options.songId && options.bandId) {
usePlayerStore.getState().setCurrentSong(options.songId, options.bandId);
}
// Restore playback if this song was already playing when the page loaded.
// Read as a one-time snapshot — these values must NOT be reactive deps or
// the effect would re-run on every time update (re-initializing WaveSurfer).
const {
currentPlayingSongId,
currentPlayingBandId,
isPlaying: wasPlaying,
currentTime: savedTime,
} = usePlayerStore.getState();
if (
options.songId &&
options.bandId &&
currentPlayingSongId === options.songId &&
currentPlayingBandId === options.bandId &&
wasPlaying
) {
try {
if (audioService.canAttemptPlayback()) {
audioService.play(options.songId, options.bandId);
if (savedTime > 0) {
audioService.seekTo(savedTime);
}
}
} catch (err) {
console.warn('Auto-play prevented during initialization:', err);
}
}
// Sync local state from the store at ~15fps via RAF.
// The loop is started after initialization so we only poll when something is loaded.
let lastUpdateTime = 0; let lastUpdateTime = 0;
const updateInterval = 1000 / 15; // ~15fps for state updates const updateInterval = 1000 / 15;
const handleStateUpdate = () => { const handleStateUpdate = () => {
const now = Date.now(); const now = Date.now();
@@ -81,73 +93,26 @@ export function useWaveform(
animationFrameId = requestAnimationFrame(handleStateUpdate); animationFrameId = requestAnimationFrame(handleStateUpdate);
}; };
// Start the animation frame loop
animationFrameId = requestAnimationFrame(handleStateUpdate); animationFrameId = requestAnimationFrame(handleStateUpdate);
const unsubscribe = () => {
if (animationFrameId) {
cancelAnimationFrame(animationFrameId);
animationFrameId = null;
}
};
// Update global song context
if (options.songId && options.bandId) {
setCurrentSong(options.songId, options.bandId);
}
// If this is the currently playing song, restore play state
if (options.songId && options.bandId &&
currentPlayingSongId === options.songId &&
currentPlayingBandId === options.bandId &&
globalIsPlaying) {
// Wait for initialization to complete using the new promise-based approach
try {
await audioService.waitForInitialization();
// Use the unified readiness check
if (audioService.isReadyForPlayback()) {
audioService.play(options.songId, options.bandId);
if (globalCurrentTime > 0) {
audioService.seekTo(globalCurrentTime);
}
} else {
console.warn('Not ready for playback after initialization', {
state: audioService.getInitializationState(),
error: audioService.getInitializationError()
});
}
} catch (error) {
console.warn('Auto-play prevented during initialization:', error);
// Don't retry - wait for user to click play
}
}
setIsReady(true); setIsReady(true);
setInitializationState(audioService.getInitializationState()); setInitializationState(audioService.getInitializationState());
options.onReady?.(audioService.getDuration()); options.onReady?.(audioService.getDuration());
} catch (err) {
return () => { console.error('useWaveform: initialization failed', err);
unsubscribe();
// Note: We don't cleanup the audio service here to maintain persistence
// audioService.cleanup();
};
} catch (error) {
console.error('useWaveform: initialization failed', error);
setIsReady(false); setIsReady(false);
setError(error instanceof Error ? error.message : 'Failed to initialize audio'); setError(err instanceof Error ? err.message : 'Failed to initialize audio');
return () => {};
} }
}; };
initializeAudio(); initializeAudio();
// eslint-disable-next-line react-hooks/exhaustive-deps return () => {
}, [options.url, options.songId, options.bandId, containerRef, currentSongId, globalBandId, globalCurrentTime, globalIsPlaying, setCurrentSong]); if (animationFrameId !== null) {
cancelAnimationFrame(animationFrameId);
}
};
}, [options.url, options.songId, options.bandId]);
const play = () => { const play = () => {
// Use the unified readiness check // Use the unified readiness check
@@ -157,6 +122,14 @@ export function useWaveform(
} catch (error) { } catch (error) {
console.error('useWaveform.play failed:', error); console.error('useWaveform.play failed:', error);
} }
} else {
// If we can attempt playback (even during initialization), try it
if (audioService.canAttemptPlayback()) {
try {
audioService.play(options.songId || null, options.bandId || null);
} catch (error) {
console.error('useWaveform.play failed during initialization attempt:', error);
}
} else { } else {
console.warn('Cannot play: not ready for playback', { console.warn('Cannot play: not ready for playback', {
initializationState: audioService.getInitializationState(), initializationState: audioService.getInitializationState(),
@@ -165,6 +138,7 @@ export function useWaveform(
url: options.url url: options.url
}); });
} }
}
}; };
const pause = () => { const pause = () => {

View File

@@ -34,8 +34,6 @@ class AudioService {
private wavesurfer: WaveSurfer | null = null; private wavesurfer: WaveSurfer | null = null;
private audioContext: AudioContext | null = null; private audioContext: AudioContext | null = null;
private currentUrl: string | null = null; private currentUrl: string | null = null;
private currentPlayingSongId: string | null = null;
private currentPlayingBandId: string | null = null;
private lastPlayTime: number = 0; private lastPlayTime: number = 0;
private lastTimeUpdate: number = 0; private lastTimeUpdate: number = 0;
private readonly PLAY_DEBOUNCE_MS: number = 100; private readonly PLAY_DEBOUNCE_MS: number = 100;
@@ -235,7 +233,11 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
} else { } else {
this.log(LogLevel.DEBUG, 'Using existing instance - no changes needed'); this.log(LogLevel.DEBUG, 'Using existing instance - no changes needed');
} }
return this.wavesurfer; // Signal initialization completion for existing instance
this.initializationState = InitializationState.Completed;
this.initializationResolve?.();
this.initializationResolve = null;
return;
} catch (error) { } catch (error) {
this.log(LogLevel.ERROR, 'Failed to reuse existing instance:', error); this.log(LogLevel.ERROR, 'Failed to reuse existing instance:', error);
this.cleanup(); this.cleanup();
@@ -277,7 +279,7 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
// @ts-expect-error - WaveSurfer typing doesn't expose backend // @ts-expect-error - WaveSurfer typing doesn't expose backend
if (!ws.backend) { if (!ws.backend) {
console.warn('WaveSurfer instance has no backend property yet - this might be normal in v7+'); this.log(LogLevel.DEBUG, 'WaveSurfer instance has no backend property yet - this might be normal in v7+');
// Don't throw error - we'll try to access backend later when needed // Don't throw error - we'll try to access backend later when needed
} }
} catch (error) { } catch (error) {
@@ -298,48 +300,56 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
this.setupEventHandlers(); this.setupEventHandlers();
// Load the audio with error handling // Load the audio with error handling
this.log(LogLevel.DEBUG, 'Loading audio URL:', url); this.log(LogLevel.DEBUG, 'Loading audio URL:', url, {
try { currentWavesurfer: !!this.wavesurfer,
const loadPromise = new Promise<void>(async (resolve, reject) => { currentUrl: this.currentUrl,
ws.on('ready', async () => { initializationState: this.initializationState
this.log(LogLevel.DEBUG, 'WaveSurfer ready event fired'); });
// Now that WaveSurfer is ready, set up audio context and finalize initialization
try { try {
await new Promise<void>((resolve, reject) => {
// Async work extracted outside the executor so rejections are always
// forwarded to reject() rather than becoming unhandled Promise rejections.
const onReady = async () => {
this.log(LogLevel.DEBUG, 'WaveSurfer ready event fired', {
currentTime: ws.getCurrentTime(),
duration: ws.getDuration(),
});
await this.setupAudioContext(ws); await this.setupAudioContext(ws);
// Update player store with duration const duration = ws.getDuration();
const playerStore = usePlayerStore.getState(); this.log(LogLevel.DEBUG, 'WaveSurfer ready with duration:', duration);
playerStore.setDuration(ws.getDuration());
// Signal initialization completion if (duration > 0) {
usePlayerStore.getState().setDuration(duration);
this.initializationState = InitializationState.Completed; this.initializationState = InitializationState.Completed;
this.initializationResolve?.(); this.initializationResolve?.();
this.initializationResolve = null; this.initializationResolve = null;
resolve(); resolve();
} catch (error) { } else {
this.log(LogLevel.ERROR, 'Initialization failed in ready handler:', error); const err = new Error('Audio loaded but duration is 0');
this.log(LogLevel.ERROR, err.message);
this.initializationState = InitializationState.Failed; this.initializationState = InitializationState.Failed;
this.initializationError = error instanceof Error ? error : new Error(String(error)); this.initializationError = err;
this.initializationResolve?.(); this.initializationResolve?.();
this.initializationResolve = null; this.initializationResolve = null;
reject(error); reject(err);
} }
}); };
ws.on('ready', () => { onReady().catch(reject); });
ws.on('error', (error) => { ws.on('error', (error) => {
this.log(LogLevel.ERROR, 'WaveSurfer error event:', error); this.log(LogLevel.ERROR, 'WaveSurfer error event:', error);
this.initializationState = InitializationState.Failed;
this.initializationError = error instanceof Error ? error : new Error(String(error));
reject(error); reject(error);
}); });
// Start loading
ws.load(url); ws.load(url);
}); });
await loadPromise;
this.log(LogLevel.INFO, 'Audio loaded successfully'); this.log(LogLevel.INFO, 'Audio loaded successfully');
return this.initializationPromise;
} catch (error) { } catch (error) {
this.log(LogLevel.ERROR, 'Failed to load audio:', error); this.log(LogLevel.ERROR, 'Failed to load audio:', error);
this.initializationState = InitializationState.Failed; this.initializationState = InitializationState.Failed;
@@ -388,6 +398,22 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
// Provide more context about why there's no wavesurfer instance // Provide more context about why there's no wavesurfer instance
if (!this.currentUrl) { if (!this.currentUrl) {
this.log(LogLevel.ERROR, 'No audio URL has been set - waveform not initialized'); this.log(LogLevel.ERROR, 'No audio URL has been set - waveform not initialized');
} else if (this.initializationState === InitializationState.Failed) {
this.log(LogLevel.ERROR, 'Waveform initialization failed:', this.initializationError);
} else if (this.initializationState === InitializationState.InProgress) {
this.log(LogLevel.INFO, 'Waveform still initializing - waiting for completion');
// Wait for initialization to complete
try {
await this.waitForInitialization();
// After waiting, check if we have a wavesurfer instance
if (!this.wavesurfer) {
this.log(LogLevel.ERROR, 'Waveform initialization completed but no wavesurfer instance available');
return;
}
} catch (error) {
this.log(LogLevel.ERROR, 'Failed to wait for initialization:', error);
return;
}
} else { } else {
this.log(LogLevel.ERROR, 'Waveform initialization failed or was cleaned up'); this.log(LogLevel.ERROR, 'Waveform initialization failed or was cleaned up');
} }
@@ -396,9 +422,45 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
// Check if waveform is actually ready for playback // Check if waveform is actually ready for playback
if (this.getDuration() <= 0) { if (this.getDuration() <= 0) {
this.log(LogLevel.ERROR, 'Waveform not ready for playback - duration is 0'); this.log(LogLevel.ERROR, 'Waveform not ready for playback - duration is 0', {
initializationState: this.initializationState,
error: this.initializationError,
currentUrl: this.currentUrl
});
// If initialization failed, provide more context
if (this.initializationState === InitializationState.Failed) {
this.log(LogLevel.ERROR, 'Waveform initialization failed with error:', this.initializationError);
} else if (this.initializationState === InitializationState.InProgress) {
this.log(LogLevel.INFO, 'Waveform still initializing, waiting for completion');
// Wait for initialization to complete
try {
await this.waitForInitialization();
// After waiting, check duration again
if (this.getDuration() <= 0) {
this.log(LogLevel.ERROR, 'Waveform initialization completed but duration is still 0');
return; return;
} }
} catch (error) {
this.log(LogLevel.ERROR, 'Failed to wait for initialization:', error);
return;
}
}
return;
}
// If initialization is still in progress, wait for it to complete
if (this.initializationState === InitializationState.InProgress) {
this.log(LogLevel.INFO, 'Initialization in progress, waiting for completion before playback');
try {
await this.waitForInitialization();
this.log(LogLevel.DEBUG, 'Initialization completed, proceeding with playback');
} catch (error) {
this.log(LogLevel.ERROR, 'Initialization failed while waiting for playback:', error);
return;
}
}
// Debounce rapid play calls // Debounce rapid play calls
const now = Date.now(); const now = Date.now();
@@ -420,16 +482,6 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
await new Promise(resolve => setTimeout(resolve, 50)); await new Promise(resolve => setTimeout(resolve, 50));
} }
// Check if we need to switch songs
const isDifferentSong = songId && bandId &&
(this.currentPlayingSongId !== songId || this.currentPlayingBandId !== bandId);
// If switching to a different song, perform cleanup
if (isDifferentSong) {
this.log(LogLevel.INFO, 'Switching to different song - performing cleanup');
this.cleanup();
}
// Ensure we have a valid audio context and handle user gesture requirement // Ensure we have a valid audio context and handle user gesture requirement
await this.handleAudioContextResume(); await this.handleAudioContextResume();
@@ -454,12 +506,8 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
} }
} }
// Update currently playing song tracking
if (songId && bandId) { if (songId && bandId) {
this.currentPlayingSongId = songId; usePlayerStore.getState().setCurrentPlayingSong(songId, bandId);
this.currentPlayingBandId = bandId;
const playerStore = usePlayerStore.getState();
playerStore.setCurrentPlayingSong(songId, bandId);
} }
// Success logs are redundant, only log in debug mode // Success logs are redundant, only log in debug mode
@@ -561,10 +609,7 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
} }
this.currentUrl = null; this.currentUrl = null;
this.currentPlayingSongId = null;
this.currentPlayingBandId = null;
// Reset player store completely
const playerStore = usePlayerStore.getState(); const playerStore = usePlayerStore.getState();
playerStore.setCurrentPlayingSong(null, null); playerStore.setCurrentPlayingSong(null, null);
playerStore.batchUpdate({ isPlaying: false, currentTime: 0 }); playerStore.batchUpdate({ isPlaying: false, currentTime: 0 });
@@ -642,17 +687,17 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
} }
// Method 2: Try to access and replace the audio context via backend methods // Method 2: Try to access and replace the audio context via backend methods
if (ws.backend?.getAudioContext) { if (ws.backend && typeof (ws.backend as any).getAudioContext === 'function') {
// @ts-expect-error - Replace the method // Replace the method with proper typing
ws.backend.getAudioContext = () => this.audioContext; (ws.backend as any).getAudioContext = () => this.audioContext;
this.log(LogLevel.DEBUG, 'Overrode backend.getAudioContext with shared context'); this.log(LogLevel.DEBUG, 'Overrode backend.getAudioContext with shared context');
return; // Success, exit early return; // Success, exit early
} }
// Method 3: Try top-level getAudioContext // Method 3: Try top-level getAudioContext
if (typeof ws.getAudioContext === 'function') { if (typeof ws.getAudioContext === 'function') {
// @ts-expect-error - Replace the method // Replace the method with proper typing
ws.getAudioContext = () => this.audioContext; (ws as any).getAudioContext = () => this.audioContext;
this.log(LogLevel.DEBUG, 'Overrode ws.getAudioContext with shared context'); this.log(LogLevel.DEBUG, 'Overrode ws.getAudioContext with shared context');
return; // Success, exit early return; // Success, exit early
} }
@@ -709,13 +754,21 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
// Method to check if ready for playback (unified readiness check) // Method to check if ready for playback (unified readiness check)
public isReadyForPlayback(): boolean { public isReadyForPlayback(): boolean {
return ( return (
this.initializationState === InitializationState.Completed &&
this.isWaveformReady() && this.isWaveformReady() &&
!!this.audioContext && !!this.audioContext &&
(this.audioContext.state === 'running' || this.audioContext.state === 'suspended') (this.audioContext.state === 'running' || this.audioContext.state === 'suspended')
); );
} }
// Method to check if playback can be attempted (more lenient during initialization)
public canAttemptPlayback(): boolean {
return (
this.isWaveformReady() &&
(this.initializationState === InitializationState.Completed ||
this.initializationState === InitializationState.InProgress)
);
}
// Initialization state management // Initialization state management
public getInitializationState(): InitializationState { public getInitializationState(): InitializationState {
return this.initializationState; return this.initializationState;
@@ -733,7 +786,10 @@ private readonly PLAY_DEBOUNCE_MS: number = 100;
return this.initializationPromise ?? Promise.resolve(); return this.initializationPromise ?? Promise.resolve();
} }
// Method to ensure audio context is available (for backward compatibility) // Method to ensure audio context is available (for backward compatibility)
// @ts-expect-error - This method is used in tests but may not be used in production code
private async ensureAudioContext(): Promise<AudioContext> { private async ensureAudioContext(): Promise<AudioContext> {
// If we already have a valid audio context, return it // If we already have a valid audio context, return it
if (this.audioContext) { if (this.audioContext) {