Skip to content

Commit e868607

Browse files
claudeclaude
committed
Refactor subtitle initialization to pure event-driven approach
Removed all setTimeout calls with arbitrary delays (300ms, 600ms, 1000ms) in favor of a reliable event-driven implementation. ## Problems with Previous Approach 1. **Magic Numbers**: Used arbitrary timeouts that might be too short/long 2. **Unreliable**: Depended on guessing when VTT files would load 3. **Complex**: Multiple retry attempts with nested setTimeout calls 4. **Wasteful**: Called activation logic multiple times unnecessarily ## New Pure Event-Driven Approach ### How It Works ```typescript // Check current state if (trackElement.readyState === 2 && track.cues?.length > 0) { // Already loaded - activate immediately track.mode = 'showing'; } else { // Not loaded yet - wait for browser events trackElement.addEventListener('load', activateTrack); track.addEventListener('cuechange', activateTrack); } ``` ### Key Benefits 1. **No Magic Numbers**: Zero setTimeout calls 2. **Reliable**: Activates exactly when VTT is ready (event-driven) 3. **Simple**: Single activation point, clean logic flow 4. **Efficient**: Only processes once (guard at function start) ### How It Handles Different Scenarios **Fast Connection (VTT loads immediately):** - readyState === 2 on first check - Activates immediately, no waiting **Slow Connection (VTT loads later):** - readyState !== 2 on first check - Event listeners fire when ready - Activates when browser completes loading **Already Playing (late initialization):** - Cues already available from browser - Activates immediately ### Changes Made - Removed all setTimeout/retry logic - Simplified to single activation function - Added readyState check for immediate activation - Used load + cuechange events for delayed activation - Removed unnecessary handlePlayWithSubtitles wrapper - Added guard to prevent duplicate processing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 2c911be commit e868607

1 file changed

Lines changed: 62 additions & 141 deletions

File tree

  • packages/ui/src/watch/videos/video-player

‎packages/ui/src/watch/videos/video-player/index.tsx‎

Lines changed: 62 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -117,156 +117,82 @@ const VideoPlayer = (props: VideoPlayerProps) => {
117117
}, []);
118118

119119
// Force subtitle tracks to load and set the default track to 'showing' mode
120+
// This is a pure event-driven approach with no arbitrary timeouts
120121
const initializeSubtitleTracks = useCallback(() => {
121-
if (!playerRef.current) return;
122+
if (!playerRef.current || subtitlesInitialized.current) return;
122123

123124
try {
124-
// Get the internal video element from ReactPlayer
125125
const internalPlayer = playerRef.current.getInternalPlayer();
126126
if (!internalPlayer || !internalPlayer.textTracks) return;
127127

128128
const textTracks = internalPlayer.textTracks;
129129

130-
const activateDefaultTrack = () => {
131-
console.log('Activating tracks, count:', textTracks.length, 'initialized:', subtitlesInitialized.current);
132-
133-
if (textTracks.length === 0) return false;
134-
135-
let activated = false;
136-
for (let i = 0; i < textTracks.length; i++) {
137-
const track = textTracks[i];
138-
139-
console.log('Track status:', {
140-
language: track.language,
141-
mode: track.mode,
142-
cueCount: track.cues?.length || 0
143-
});
144-
145-
// Check if already showing with cues - skip if working
146-
if (track.mode === 'showing' && track.cues && track.cues.length > 0) {
147-
console.log('✓ Track already working, skipping');
148-
subtitlesInitialized.current = true;
149-
return true;
150-
}
151-
152-
// Log the track source for debugging
153-
const trackElement = internalPlayer.querySelector(`track[srclang="${track.language}"]`) as HTMLTrackElement;
154-
if (trackElement) {
155-
console.log('Track element readyState:', trackElement.readyState);
156-
157-
// Listen for track errors
158-
trackElement.addEventListener('error', (e) => {
159-
console.error('❌ Subtitle track error:', trackElement.error);
160-
}, { once: true });
161-
162-
// If track not loaded yet, wait for it
163-
if (trackElement.readyState !== 2) { // 2 = LOADED
164-
console.log('Track not loaded yet, waiting for load event...');
165-
trackElement.addEventListener('load', () => {
166-
console.log('Track loaded, forcing mode to showing');
167-
track.mode = 'showing';
168-
if (track.cues && track.cues.length > 0) {
169-
subtitlesInitialized.current = true;
170-
console.log('✓ Subtitles ready after load event');
171-
}
172-
}, { once: true });
173-
174-
// Also listen for cuechange as backup
175-
track.addEventListener('cuechange', () => {
176-
console.log('Cuechange event, cues:', track.cues?.length);
177-
if (track.cues && track.cues.length > 0 && !subtitlesInitialized.current) {
178-
track.mode = 'showing';
179-
subtitlesInitialized.current = true;
180-
console.log('✓ Subtitles ready after cuechange');
181-
}
182-
}, { once: true });
130+
// Process each text track
131+
for (let i = 0; i < textTracks.length; i++) {
132+
const track = textTracks[i];
133+
const trackElement = internalPlayer.querySelector(
134+
`track[srclang="${track.language}"]`
135+
) as HTMLTrackElement;
136+
137+
if (!trackElement) continue;
138+
139+
// Find matching subtitle configuration
140+
const matchingSubtitle = subtitles?.find(
141+
(sub) => sub.lang === track.language
142+
);
143+
144+
if (!matchingSubtitle) continue;
145+
146+
// Listen for track errors
147+
trackElement.addEventListener(
148+
'error',
149+
() => {
150+
console.error('❌ Subtitle track error:', trackElement.error);
151+
},
152+
{ once: true }
153+
);
154+
155+
if (matchingSubtitle.isDefault) {
156+
// This is the default subtitle track we want to activate
157+
const activateTrack = () => {
158+
if (track.cues && track.cues.length > 0) {
159+
track.mode = 'showing';
160+
subtitlesInitialized.current = true;
161+
console.log('✓ Subtitles activated:', track.language, track.cues.length, 'cues');
183162
}
184-
}
185-
186-
// Find the default subtitle track and force it to 'showing' mode
187-
if (subtitles && subtitles.length > 0) {
188-
const matchingSubtitle = subtitles.find(
189-
sub => sub.lang === track.language
163+
};
164+
165+
// Check if already loaded
166+
if (trackElement.readyState === 2 && track.cues && track.cues.length > 0) {
167+
// Already loaded and has cues - activate immediately
168+
activateTrack();
169+
} else {
170+
// Not loaded yet - set up event listeners
171+
track.mode = 'showing'; // Set to showing so browser will load it
172+
173+
// Listen for track load event
174+
trackElement.addEventListener('load', activateTrack, { once: true });
175+
176+
// Backup: listen for cuechange event
177+
track.addEventListener(
178+
'cuechange',
179+
() => {
180+
if (!subtitlesInitialized.current) {
181+
activateTrack();
182+
}
183+
},
184+
{ once: true }
190185
);
191-
192-
if (matchingSubtitle?.isDefault) {
193-
// Only do mode cycling if track is already loaded
194-
const trackElem = internalPlayer.querySelector(`track[srclang="${track.language}"]`) as HTMLTrackElement;
195-
if (trackElem && trackElem.readyState === 2) { // LOADED
196-
console.log('→ Track is loaded, starting mode cycle for:', track.language);
197-
// Force track mode cycle to trigger VTT parsing
198-
track.mode = 'disabled';
199-
200-
// Wait a bit then set to showing
201-
setTimeout(() => {
202-
track.mode = 'hidden';
203-
setTimeout(() => {
204-
track.mode = 'showing';
205-
console.log('→ Set to showing');
206-
207-
// Verify cues loaded after activation
208-
setTimeout(() => {
209-
const cueCount = track.cues?.length || 0;
210-
console.log('→ Verification check, cues:', cueCount);
211-
if (track.cues && track.cues.length > 0) {
212-
subtitlesInitialized.current = true;
213-
console.log('✓ Subtitles initialized successfully');
214-
} else {
215-
console.warn('⚠️ Track showing but no cues loaded');
216-
}
217-
}, 200);
218-
}, 50);
219-
}, 50);
220-
} else {
221-
console.log('→ Track not ready yet, mode cycling skipped (will activate via event listener)');
222-
// Just ensure it's in showing mode - the event listener will handle it when loaded
223-
track.mode = 'showing';
224-
}
225-
226-
activated = true;
227-
} else if (matchingSubtitle) {
228-
track.mode = 'disabled';
229-
}
230186
}
187+
} else {
188+
// Not the default track - disable it
189+
track.mode = 'disabled';
231190
}
232-
233-
return activated;
234-
};
235-
236-
// Wait for video metadata to load before activating tracks
237-
const handleLoadedMetadata = () => {
238-
activateDefaultTrack();
239-
};
240-
241-
if (internalPlayer.readyState >= 1) {
242-
// Metadata already loaded, activate immediately
243-
console.log('Metadata ready, activating now');
244-
activateDefaultTrack();
245-
} else {
246-
// Wait for metadata to load
247-
console.log('Waiting for metadata...');
248-
internalPlayer.addEventListener('loadedmetadata', handleLoadedMetadata, { once: true });
249191
}
250-
251-
// Multiple retry attempts to ensure consistency
252-
const retryDelays = [300, 600, 1000];
253-
retryDelays.forEach((delay) => {
254-
setTimeout(() => {
255-
if (!subtitlesInitialized.current) {
256-
console.log(`Retry attempt after ${delay}ms`);
257-
activateDefaultTrack();
258-
}
259-
}, delay);
260-
});
261-
262-
// Cleanup event listener
263-
setTimeout(() => {
264-
internalPlayer.removeEventListener('loadedmetadata', handleLoadedMetadata);
265-
}, 2000);
266192
} catch (error) {
267193
console.error('Failed to initialize subtitle tracks:', error);
268194
}
269-
}, [subtitles, testVttFile]);
195+
}, [subtitles]);
270196

271197
// Handle keyboard shortcuts on the wrapper element
272198
useEffect(() => {
@@ -475,12 +401,7 @@ const VideoPlayer = (props: VideoPlayerProps) => {
475401
[onError],
476402
);
477403

478-
// Wrap handlePlay to also initialize subtitles as a fallback
479-
const handlePlayWithSubtitles = useCallback(() => {
480-
handlePlay();
481-
// Try to initialize subtitles again when play starts (fallback)
482-
initializeSubtitleTracks();
483-
}, [handlePlay, initializeSubtitleTracks]);
404+
// No need for fallback - event listeners handle everything
484405

485406
// Reset subtitle initialization when video changes
486407
useEffect(() => {
@@ -529,7 +450,7 @@ const VideoPlayer = (props: VideoPlayerProps) => {
529450
onError={handleError}
530451
onProgress={handleProgress}
531452
onPause={handlePause}
532-
onPlay={handlePlayWithSubtitles}
453+
onPlay={handlePlay}
533454
onSeek={handleSeek}
534455
onEnded={() => {
535456
handleEnded();

0 commit comments

Comments
 (0)