Skip to content

Commit 19ca2f5

Browse files
committed
fix(player): resolve shortcut pause failure caused by state oscillation (#174)
* fix(player): resolve shortcut pause failure caused by state oscillation Remove problematic automatic retry mechanisms in PlayerOrchestrator that were causing infinite state oscillation loops between play and pause operations. **Problem**: - Shortcut pause commands (spacebar) would fail and immediately resume playback - Logs showed repeating "Video element still playing after pause() call" warnings - Root cause: setTimeout-based verification logic in requestPlay() and requestPause() created mutual triggering loops **Solution**: - Remove 150ms retry logic from requestPlay() (lines 283-297) - Remove 50ms retry logic from requestPause() (lines 320-333) - Remove verification timeouts from requestTogglePlay() (lines 345-360) - Trust browser's native play()/pause() API reliability **Impact**: - ✅ Shortcut pause now works immediately without interference - ✅ Eliminates state oscillation loop warnings from logs - ✅ Mouse click controls remain unaffected - ✅ All other player functionality preserved Fixes #170 * test: update PlayerOrchestrator tests to reflect removed retry mechanisms Update reliability tests that previously validated automatic retry behavior: - 'should detect play failure in delayed verification' → 'should only call play once without retry' - 'should detect pause failure in delayed verification' → 'should only call pause once without retry' These tests now correctly validate the simplified behavior where play() and pause() are called exactly once without automatic retry mechanisms, aligning with the fix that removes state oscillation loops. All 578 tests now pass ✅
1 parent 09e98e2 commit 19ca2f5

2 files changed

Lines changed: 8 additions & 54 deletions

File tree

src/renderer/src/pages/player/engine/PlayerOrchestrator.ts

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -278,23 +278,6 @@ export class PlayerOrchestrator {
278278

279279
await this.videoController.play()
280280
logger.debug('Command: requestPlay executed successfully')
281-
282-
// 延迟验证播放是否真正开始
283-
setTimeout(() => {
284-
if (this.videoController?.isPaused()) {
285-
logger.warn('Video element still paused after play() call, investigating...', {
286-
contextPaused: this.context.paused,
287-
videoPaused: this.videoController?.isPaused()
288-
})
289-
290-
// 尝试再次播放
291-
this.videoController?.play().catch((retryError) => {
292-
logger.error('Retry play() also failed:', { retryError })
293-
// 回滚乐观更新
294-
this.updateContext({ paused: true })
295-
})
296-
}
297-
}, 150)
298281
} catch (error) {
299282
logger.error('Failed to execute requestPlay:', { error })
300283
// 回滚乐观更新
@@ -333,21 +316,6 @@ export class PlayerOrchestrator {
333316

334317
this.videoController.pause()
335318
logger.debug('Command: requestPause executed successfully')
336-
337-
// 延迟验证暂停是否真正生效
338-
setTimeout(() => {
339-
if (!this.videoController?.isPaused()) {
340-
logger.warn('Video element still playing after pause() call, investigating...', {
341-
contextPaused: this.context.paused,
342-
videoPaused: this.videoController?.isPaused()
343-
})
344-
345-
// 尝试再次暂停
346-
this.videoController?.pause()
347-
// 强制同步状态
348-
this.syncPlaybackState()
349-
}
350-
}, 50)
351319
} catch (error) {
352320
logger.error('Failed to execute requestPause:', { error })
353321
// 回滚乐观更新
@@ -373,22 +341,8 @@ export class PlayerOrchestrator {
373341
try {
374342
if (isPaused) {
375343
await this.requestPlay()
376-
// 验证播放操作是否成功
377-
setTimeout(() => {
378-
if (this.videoController?.isPaused() && !this.context.paused) {
379-
logger.warn('Play command failed to take effect, attempting sync')
380-
this.syncPlaybackState()
381-
}
382-
}, 100)
383344
} else {
384345
this.requestPause()
385-
// 验证暂停操作是否成功
386-
setTimeout(() => {
387-
if (!this.videoController?.isPaused() && this.context.paused) {
388-
logger.warn('Pause command failed to take effect, attempting sync')
389-
this.syncPlaybackState()
390-
}
391-
}, 50)
392346
}
393347
} catch (error) {
394348
logger.error('Failed to toggle play state:', { error, isPaused })

src/renderer/src/pages/player/engine/__tests__/PlayerOrchestrator.playback-reliability.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,17 +211,17 @@ describe('PlayerOrchestrator - 播放/暂停可靠性测试', () => {
211211
expect(finalContext.paused).toBe(true)
212212
})
213213

214-
it('应该在延迟验证中检测播放失败', async () => {
214+
it('应该只调用一次play而不进行重试', async () => {
215215
orchestrator.updateContext({ paused: true })
216216
mockVideoController.isPaused.mockReturnValue(true) // 播放后仍然是暂停状态
217217

218218
await orchestrator.requestPlay()
219219

220-
// 等待延迟验证执行
220+
// 等待可能的延迟操作
221221
await new Promise((resolve) => setTimeout(resolve, 200))
222222

223-
// 应该尝试重试播放
224-
expect(mockVideoController.play).toHaveBeenCalledTimes(2) // 一次正常调用,一次重试
223+
// 应该只调用一次play,不进行重试(修复后的行为)
224+
expect(mockVideoController.play).toHaveBeenCalledTimes(1)
225225
})
226226
})
227227

@@ -236,17 +236,17 @@ describe('PlayerOrchestrator - 播放/暂停可靠性测试', () => {
236236
expect(context.paused).toBe(true)
237237
})
238238

239-
it('应该在延迟验证中检测暂停失败', async () => {
239+
it('应该只调用一次pause而不进行重试', async () => {
240240
orchestrator.updateContext({ paused: false })
241241
mockVideoController.isPaused.mockReturnValue(false) // 暂停后仍然是播放状态
242242

243243
orchestrator.requestPause()
244244

245-
// 等待延迟验证
245+
// 等待可能的延迟操作
246246
await new Promise((resolve) => setTimeout(resolve, 100))
247247

248-
// 应该尝试重试暂停
249-
expect(mockVideoController.pause).toHaveBeenCalledTimes(2)
248+
// 应该只调用一次pause,不进行重试(修复后的行为)
249+
expect(mockVideoController.pause).toHaveBeenCalledTimes(1)
250250
})
251251
})
252252

0 commit comments

Comments
 (0)