From 1629272adb7a72cd9036b04adfef890bfc54c164 Mon Sep 17 00:00:00 2001 From: Mistral Vibe Date: Wed, 8 Apr 2026 20:07:20 +0200 Subject: [PATCH] Cleanup --- web/src/hooks/useWaveform.ts | 60 +++-- web/src/services/audioService.ts | 243 +++++++++++++++---- web/tests/audioContextInitialization.test.ts | 97 ++++++++ 3 files changed, 334 insertions(+), 66 deletions(-) create mode 100644 web/tests/audioContextInitialization.test.ts diff --git a/web/src/hooks/useWaveform.ts b/web/src/hooks/useWaveform.ts index 23abccc..7599f1b 100755 --- a/web/src/hooks/useWaveform.ts +++ b/web/src/hooks/useWaveform.ts @@ -1,5 +1,5 @@ import { useEffect, useRef, useState } from "react"; -import { audioService } from "../services/audioService"; +import { audioService, InitializationState } from "../services/audioService"; import { usePlayerStore } from "../stores/playerStore"; export interface UseWaveformOptions { @@ -27,6 +27,7 @@ export function useWaveform( const [currentTime, setCurrentTime] = useState(0); const [duration, setDuration] = useState(0); const [error, setError] = useState(null); + const [initializationState, setInitializationState] = useState(InitializationState.NotStarted); const markersRef = useRef([]); // Global player state - use shallow comparison to reduce re-renders @@ -103,26 +104,30 @@ export function useWaveform( - // Wait for the waveform to be ready and audio context to be available - const checkReady = setInterval(() => { - if (audioService.getDuration() > 0) { - clearInterval(checkReady); - // Only attempt to play if we have a valid audio context - // This prevents autoplay policy violations - try { - audioService.play(options.songId, options.bandId); - if (globalCurrentTime > 0) { - audioService.seekTo(globalCurrentTime); - } - } catch (error) { - console.warn('Auto-play prevented by browser policy, waiting for user gesture:', error); - // Don't retry - wait for user to click play + // 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() + }); } - }, 50); + } catch (error) { + console.warn('Auto-play prevented during initialization:', error); + // Don't retry - wait for user to click play + } } setIsReady(true); + setInitializationState(audioService.getInitializationState()); options.onReady?.(audioService.getDuration()); return () => { @@ -145,16 +150,17 @@ export function useWaveform( }, [options.url, options.songId, options.bandId, containerRef, currentSongId, globalBandId, globalCurrentTime, globalIsPlaying, setCurrentSong]); const play = () => { - // Only attempt to play if waveform is ready - if (audioService.isWaveformReady()) { + // Use the unified readiness check + if (audioService.isReadyForPlayback()) { try { audioService.play(options.songId || null, options.bandId || null); } catch (error) { console.error('useWaveform.play failed:', error); } } else { - console.warn('Cannot play: waveform not ready', { - hasWavesurfer: !!audioService.isPlaying(), + console.warn('Cannot play: not ready for playback', { + initializationState: audioService.getInitializationState(), + error: audioService.getInitializationError(), duration: audioService.getDuration(), url: options.url }); @@ -235,7 +241,19 @@ export function useWaveform( markersRef.current = []; }; - return { isPlaying, isReady, currentTime, duration, play, pause, seekTo, addMarker, clearMarkers, error }; + return { + isPlaying, + isReady, + currentTime, + duration, + play, + pause, + seekTo, + addMarker, + clearMarkers, + error, + initializationState + }; } function formatTime(seconds: number): string { diff --git a/web/src/services/audioService.ts b/web/src/services/audioService.ts index c585639..f54af49 100755 --- a/web/src/services/audioService.ts +++ b/web/src/services/audioService.ts @@ -9,6 +9,14 @@ enum LogLevel { ERROR = 3 } +// Initialization state enum +enum InitializationState { + NotStarted = 'not_started', + InProgress = 'in_progress', + Completed = 'completed', + Failed = 'failed' +} + // Type extension for WaveSurfer backend access interface WaveSurferWithBackend extends WaveSurfer { backend?: { @@ -39,6 +47,12 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; private lastLogTime: number = 0; private readonly LOG_THROTTLE_MS: number = 100; + // Initialization tracking + private initializationState: InitializationState = InitializationState.NotStarted; + private initializationError: Error | null = null; + private initializationPromise: Promise | null = null; + private initializationResolve: (() => void) | null = null; + private constructor() { // Set appropriate log level based on environment this.setLogLevel(this.detectLogLevel()); @@ -132,6 +146,13 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; this.log(LogLevel.DEBUG, 'Audio context state changed:', this.audioContext?.state); }; } + + // Resume suspended audio context + if (this.audioContext && this.audioContext.state === 'suspended') { + await this.audioContext.resume(); + this.log(LogLevel.INFO, 'Audio context resumed successfully'); + } + return this.audioContext; } catch (error) { this.log(LogLevel.ERROR, 'Failed to initialize audio context:', error); @@ -170,17 +191,32 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; this.instance = undefined as any; } - public async initialize(container: HTMLElement, url: string) { + public async initialize(container: HTMLElement, url: string): Promise { this.log(LogLevel.DEBUG, 'AudioService.initialize called', { url, containerExists: !!container }); + // Reset initialization state + this.initializationState = InitializationState.InProgress; + this.initializationError = null; + this.initializationPromise = new Promise((resolve) => { + this.initializationResolve = resolve; + }); + // Validate inputs if (!container) { this.log(LogLevel.ERROR, 'AudioService: container element is null'); + this.initializationState = InitializationState.Failed; + this.initializationError = new Error('Container element is required'); + this.initializationResolve?.(); + this.initializationResolve = null; throw new Error('Container element is required'); } if (!url || url === 'null' || url === 'undefined') { this.log(LogLevel.ERROR, 'AudioService: invalid URL', { url }); + this.initializationState = InitializationState.Failed; + this.initializationError = new Error('Valid audio URL is required'); + this.initializationResolve?.(); + this.initializationResolve = null; throw new Error('Valid audio URL is required'); } @@ -256,7 +292,7 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; // Get audio context from wavesurfer // Note: In WaveSurfer v7+, backend might not be available immediately // We'll try to access it now, but also set up a handler to get it when ready - this.setupAudioContext(ws); + await this.setupAudioContext(ws); // Set up event handlers before loading this.setupEventHandlers(); @@ -264,17 +300,31 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; // Load the audio with error handling this.log(LogLevel.DEBUG, 'Loading audio URL:', url); try { - const loadPromise = new Promise((resolve, reject) => { - ws.on('ready', () => { + const loadPromise = new Promise(async (resolve, reject) => { + ws.on('ready', async () => { this.log(LogLevel.DEBUG, 'WaveSurfer ready event fired'); // Now that WaveSurfer is ready, set up audio context and finalize initialization - this.setupAudioContext(ws); - - // Update player store with duration - const playerStore = usePlayerStore.getState(); - playerStore.setDuration(ws.getDuration()); - - resolve(); + try { + await this.setupAudioContext(ws); + + // Update player store with duration + const playerStore = usePlayerStore.getState(); + playerStore.setDuration(ws.getDuration()); + + // Signal initialization completion + this.initializationState = InitializationState.Completed; + this.initializationResolve?.(); + this.initializationResolve = null; + + resolve(); + } catch (error) { + this.log(LogLevel.ERROR, 'Initialization failed in ready handler:', error); + this.initializationState = InitializationState.Failed; + this.initializationError = error instanceof Error ? error : new Error(String(error)); + this.initializationResolve?.(); + this.initializationResolve = null; + reject(error); + } }); ws.on('error', (error) => { @@ -289,13 +339,16 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; await loadPromise; this.log(LogLevel.INFO, 'Audio loaded successfully'); + return this.initializationPromise; } catch (error) { this.log(LogLevel.ERROR, 'Failed to load audio:', error); + this.initializationState = InitializationState.Failed; + this.initializationError = error instanceof Error ? error : new Error(String(error)); + this.initializationResolve?.(); + this.initializationResolve = null; this.cleanup(); throw error; } - - return ws; } private setupEventHandlers() { @@ -341,6 +394,12 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; return; } + // Check if waveform is actually ready for playback + if (this.getDuration() <= 0) { + this.log(LogLevel.ERROR, 'Waveform not ready for playback - duration is 0'); + return; + } + // Debounce rapid play calls const now = Date.now(); if (now - this.lastPlayTime < this.PLAY_DEBOUNCE_MS) { @@ -514,36 +573,15 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; } - private setupAudioContext(ws: WaveSurferWithBackend) { - // Simplified audio context setup - we now manage audio context centrally + private async setupAudioContext(ws: WaveSurferWithBackend) { + // Simplified and more robust audio context setup try { // If we already have an audio context, ensure WaveSurfer uses it if (this.audioContext) { - // Try multiple ways to share the audio context with WaveSurfer - try { - // Method 1: Try to set via backend if available - if (ws.backend) { - ws.backend.audioContext = this.audioContext; - this.log(LogLevel.DEBUG, 'Shared audio context with WaveSurfer backend'); - } - - // Method 2: Try to access and replace the audio context - if (ws.backend?.getAudioContext) { - // @ts-expect-error - Replace the method - ws.backend.getAudioContext = () => this.audioContext; - this.log(LogLevel.DEBUG, 'Overrode backend.getAudioContext with shared context'); - } - - // Method 3: Try top-level getAudioContext - if (typeof ws.getAudioContext === 'function') { - // @ts-expect-error - Replace the method - ws.getAudioContext = () => this.audioContext; - this.log(LogLevel.DEBUG, 'Overrode ws.getAudioContext with shared context'); - } - - } catch (error) { - this.log(LogLevel.WARN, 'Could not share audio context with WaveSurfer, but continuing:', error); - } + this.log(LogLevel.DEBUG, 'Using existing audio context'); + + // Centralized method to share audio context with WaveSurfer + this.shareAudioContextWithWaveSurfer(ws); return; } @@ -562,23 +600,69 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; sampleRate: this.audioContext.sampleRate }); - // Note: We don't automatically resume suspended audio contexts here - // because that requires a user gesture. The resume will be handled - // in handleAudioContextResume() when the user clicks play. + // Resume suspended audio context automatically if (this.audioContext.state === 'suspended') { - this.log(LogLevel.DEBUG, 'Audio context is suspended, will resume on user gesture'); + try { + await this.audioContext.resume(); + this.log(LogLevel.INFO, 'Audio context resumed successfully'); + } catch (error) { + this.log(LogLevel.WARN, 'Failed to resume audio context:', error); + } } // Set up state change monitoring this.audioContext.onstatechange = () => { this.log(LogLevel.DEBUG, 'Audio context state changed:', this.audioContext?.state); }; + return; } + + // Don't create new audio context if WaveSurfer doesn't provide methods + // This maintains backward compatibility and allows graceful degradation + this.log(LogLevel.DEBUG, 'No audio context available from WaveSurfer, continuing without it'); + } catch (error) { this.log(LogLevel.ERROR, 'Error setting up audio context:', error); // Don't throw - we can continue with our existing audio context } } + + private shareAudioContextWithWaveSurfer(ws: WaveSurferWithBackend) { + if (!this.audioContext) { + this.log(LogLevel.WARN, 'No audio context available to share'); + return; + } + + try { + // Method 1: Try to set via backend if available + if (ws.backend) { + ws.backend.audioContext = this.audioContext; + this.log(LogLevel.DEBUG, 'Shared audio context with WaveSurfer backend'); + return; // Success, exit early + } + + // Method 2: Try to access and replace the audio context via backend methods + if (ws.backend?.getAudioContext) { + // @ts-expect-error - Replace the method + ws.backend.getAudioContext = () => this.audioContext; + this.log(LogLevel.DEBUG, 'Overrode backend.getAudioContext with shared context'); + return; // Success, exit early + } + + // Method 3: Try top-level getAudioContext + if (typeof ws.getAudioContext === 'function') { + // @ts-expect-error - Replace the method + ws.getAudioContext = () => this.audioContext; + this.log(LogLevel.DEBUG, 'Overrode ws.getAudioContext with shared context'); + return; // Success, exit early + } + + this.log(LogLevel.WARN, 'Could not share audio context with WaveSurfer - no compatible method found'); + + } catch (error) { + this.log(LogLevel.WARN, 'Could not share audio context with WaveSurfer:', error); + } + } @@ -617,6 +701,75 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; return !!this.wavesurfer && this.getDuration() > 0; } + // Method to check if audio service is properly initialized + public isInitialized(): boolean { + return !!this.wavesurfer && this.getDuration() > 0 && !!this.audioContext; + } + + // Method to check if ready for playback (unified readiness check) + public isReadyForPlayback(): boolean { + return ( + this.initializationState === InitializationState.Completed && + this.isWaveformReady() && + !!this.audioContext && + (this.audioContext.state === 'running' || this.audioContext.state === 'suspended') + ); + } + + // Initialization state management + public getInitializationState(): InitializationState { + return this.initializationState; + } + + public getInitializationError(): Error | null { + return this.initializationError; + } + + public isInitializationComplete(): boolean { + return this.initializationState === InitializationState.Completed; + } + + public async waitForInitialization(): Promise { + return this.initializationPromise ?? Promise.resolve(); + } + + // Method to ensure audio context is available (for backward compatibility) + private async ensureAudioContext(): Promise { + // If we already have a valid audio context, return it + if (this.audioContext) { + // Resume if suspended (common in mobile browsers) + if (this.audioContext.state === 'suspended') { + try { + await this.audioContext.resume(); + this.log(LogLevel.INFO, 'Audio context resumed successfully'); + } catch (error) { + this.log(LogLevel.ERROR, 'Failed to resume audio context:', error); + throw error; + } + } + return this.audioContext; + } + + // Create new audio context + try { + this.audioContext = new (window.AudioContext || (window as { webkitAudioContext?: new () => AudioContext }).webkitAudioContext)(); + this.log(LogLevel.INFO, 'New audio context created', { + state: this.audioContext.state, + sampleRate: this.audioContext.sampleRate + }); + + // Set up state change monitoring + this.audioContext.onstatechange = () => { + this.log(LogLevel.DEBUG, 'Audio context state changed:', this.audioContext?.state); + }; + + return this.audioContext; + } catch (error) { + this.log(LogLevel.ERROR, 'Failed to create audio context:', error); + throw new Error(`Audio context creation failed: ${error instanceof Error ? error.message : String(error)}`); + } + } + // Method to get WaveSurfer version for debugging public getWaveSurferVersion(): string | null { if (this.wavesurfer) { @@ -638,4 +791,4 @@ private readonly PLAY_DEBOUNCE_MS: number = 100; } export const audioService = AudioService.getInstance(); -export { AudioService, LogLevel }; // Export class and enum for testing +export { AudioService, LogLevel, InitializationState }; // Export class and enums for testing diff --git a/web/tests/audioContextInitialization.test.ts b/web/tests/audioContextInitialization.test.ts new file mode 100644 index 0000000..2b2f1cf --- /dev/null +++ b/web/tests/audioContextInitialization.test.ts @@ -0,0 +1,97 @@ +import { AudioService } from '../src/services/audioService'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; + +describe('AudioService Audio Context Initialization', () => { + let audioService: AudioService; + + beforeEach(() => { + // Reset the singleton instance before each test + AudioService.resetInstance(); + audioService = AudioService.getInstance(); + + // Mock AudioContext for testing + global.window.AudioContext = vi.fn().mockImplementation(() => ({ + state: 'suspended', + sampleRate: 44100, + resume: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + onstatechange: null, + suspend: vi.fn().mockResolvedValue(undefined) + })) as any; + }); + + afterEach(() => { + // Clean up any audio contexts + if (audioService['audioContext']) { + audioService['audioContext'].close?.().catch(() => {}); + } + + // Clean up mock + delete global.window.AudioContext; + }); + + it('should initialize audio context successfully', async () => { + const context = await audioService.initializeAudioContext(); + expect(context).toBeDefined(); + expect(context.state).toBe('suspended'); // Should start suspended + expect(audioService['audioContext']).toBe(context); + }); + + it('should handle audio context resume', async () => { + // Initialize context first + await audioService.initializeAudioContext(); + + // Mock user gesture by resuming + const resumeSpy = vi.spyOn(audioService['audioContext']!, 'resume'); + + // This should attempt to resume the context + await audioService['handleAudioContextResume'](); + + expect(resumeSpy).toHaveBeenCalled(); + }); + + it('should share audio context with WaveSurfer', async () => { + // Initialize audio context + await audioService.initializeAudioContext(); + + // Create mock WaveSurfer instance + const mockWaveSurfer = { + backend: { + audioContext: null + } + }; + + // Call the sharing method + audioService['shareAudioContextWithWaveSurfer'](mockWaveSurfer); + + // Verify context was shared + expect(mockWaveSurfer.backend.audioContext).toBe(audioService['audioContext']); + }); + + it('should handle WaveSurfer without backend gracefully', async () => { + // Initialize audio context + await audioService.initializeAudioContext(); + + // Create mock WaveSurfer instance without backend + const mockWaveSurfer = { + getAudioContext: vi.fn() + }; + + // This should not throw + expect(() => { + audioService['shareAudioContextWithWaveSurfer'](mockWaveSurfer); + }).not.toThrow(); + }); + + it('should check initialization state correctly', async () => { + // Initially should not be initialized + expect(audioService.isInitialized()).toBe(false); + + // After audio context initialization, still not fully initialized + await audioService.initializeAudioContext(); + expect(audioService.isInitialized()).toBe(false); + + // Note: Full initialization would require WaveSurfer instance + // which we can't easily mock here without DOM + }); +}); \ No newline at end of file