Skip to content

Fall back to wall clock after FFmpeg audio sink failure - #144

Draft
Blackspirits wants to merge 1 commit into
audit/ffmpeg-audioqueue-start-failclosed-34d7from
audit/ffmpeg-audio-clock-failover-8dab
Draft

Blackspirits wants to merge 1 commit into
audit/ffmpeg-audioqueue-start-failclosed-34d7from
audit/ffmpeg-audio-clock-failover-8dab

Conversation

@Blackspirits

@Blackspirits Blackspirits commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Purpose

Dependent FFmpeg audio-runtime follow-up to #130 ("Fail closed when Core Audio queue cannot start").

#125/#128/#130 make sink writes/reset/start fail closed and make sink rejection observable to the audio decoder thread. This tranche handles the player-level consequence that remained: a real runtime sink failure could leave the player permanently anchored to an audio master clock that had stopped advancing.

This PR intentionally stays on #130's stack. It does not replace #116's unknown-duration EOF/drain work; it only changes runtime clock fallback and packet draining after an output failure.

Current upstream SubtitleEdit/subtitleedit main was rechecked at 6c6b83dd043d8d38fb7a2b1800fdb9465526833b. The new upstream delta is Burn-in/encoder-settings work and does not touch the FFmpeg player or audio sinks. No competing upstream PR was found.

Finding fixed

After an audio sink accepted at least one chunk, the player normally uses audioAnchorPts + PlayedSeconds * audioSpeed as its master clock.

For a real output failure on the still-current serial, no more PCM can be accepted while PlayedSeconds stops advancing. Because _hasAudio remains true and the old anchor remains valid, the player could keep selecting a frozen audio master clock. Video presentation/end detection can then stall indefinitely.

Simply terminating the audio worker is unsafe too: the audio packet queue can fill the shared demux budget and block video.

Fix

A rejected write is treated as a real device/output failure only when:

  • the write was rejected;
  • the Session is not closing;
  • decoded serial == committed serial;
  • decoded serial == latest requested serial.

Seek/reset/close rejections remain normal interruptions.

For a real current-serial failure:

  • preserve the media position represented by the audio clock already heard;
  • clear the audio anchor;
  • mark the audio sink failed;
  • continue from that exact media position on the wall clock;
  • update paused position instead when paused;
  • wake the presenter;
  • keep popping/freeing audio packets without decoding/writing them, preventing demux backpressure.

A later seek/reset clears the failed state and gives the sink one recovery attempt. If output still fails, the new serial deterministically falls back again.

Regression coverage

Pure tests pin:

  • rejected current serial without close/seek => device failure;
  • accepted write => not a failure;
  • close => not a failure;
  • stale committed serial => not a failure;
  • newer requested serial => not a failure;
  • anchored failover preserves last played media position;
  • unanchored failover preserves current wall-clock position.

Existing #125/#128/#130 serial/reset/start regressions remain unchanged.

Scope / branch state

  • dependency/base PR: Fail closed when Core Audio queue cannot start #130
  • base HEAD: 8dab1f404370543ff639c23faa46f0f2c37e53de
  • HEAD: 2c444f33caeecc886804dabb6a25f60f7376c89d
  • 1 commit
  • 2 files
  • +147 / -2
  • current upstream main checked: 6c6b83dd043d8d38fb7a2b1800fdb9465526833b
  • no relevant upstream competing PR found
  • audit draft only; no merge/promotion intended

Scope note

Unknown-duration audio/video EOF and libswresample tail draining remain #116's responsibility. This tranche neither claims nor changes those semantics.

AI assistance: ChatGPT was used for adversarial sink-failure classification, master-clock failover modelling, packet-backpressure analysis, seek/close race review and focused regression design.

Final CI

Authoritative run: SubtitleEdit#283 on 2c444f33caeecc886804dabb6a25f60f7376c89d

  • SeConv: 488 passed / 2 skipped / 0 failed
  • LibUiLogic: 905 passed / 0 skipped / 0 failed
  • LibSE: 2017 passed / 0 skipped / 0 failed
  • UI: 5240 passed / 9 skipped / 0 failed
  • build: 0 errors / 7 pre-existing warnings
  • retry step skipped
  • failure-artifact upload skipped

Final adversarial review was performed on this exact HEAD after CI; no blocker found.

@Blackspirits Blackspirits left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final adversarial review on 2c444f33caeecc886804dabb6a25f60f7376c89d after CI SubtitleEdit#283: no blocker found. Rechecked write-rejection classification against concurrent seek/close, playing/paused clock rebasing, drain-only packet consumption after sink failure, and seek-driven recovery. The wall clock resumes from the same media position previously reported by the audio clock; stale/normal reset rejections do not trigger failover. Unknown-duration EOF remains explicitly outside this tranche. No merge/promotion performed.

@Blackspirits Blackspirits left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final adversarial review on 2c444f33caeecc886804dabb6a25f60f7376c89d after CI SubtitleEdit#283: no blocker found. Sink rejection is classified as device failure only for the still-current/latest serial and outside close; failover preserves the media position already reported by the audio clock; paused state is preserved; drain-only audio prevents demux backpressure; seek/reset gets a deterministic recovery attempt. Unknown-duration EOF remains outside this PR. No merge/promotion performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant