Skip to content

Fix FFmpeg EOF handling for unknown durations - #116

Draft
Blackspirits wants to merge 1 commit into
upl/review-base-3e4dfrom
audit/ffmpeg-unknown-duration-eof-3e4d
Draft

Blackspirits wants to merge 1 commit into
upl/review-base-3e4dfrom
audit/ffmpeg-unknown-duration-eof-3e4d

Conversation

@Blackspirits

@Blackspirits Blackspirits commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Purpose

Second independent follow-up to merged upstream PR SubtitleEdit#14878 ("FFmpeg player: fix eight teardown, seek and clock defects") on current upstream main 3e4d052adc78464e71bbd5154880d935fd592960.

SubtitleEdit#14878 correctly stopped clamping seeks to zero when a stream reports no duration. The wider audit found that EOF handling still assumes a known duration in several places, so the same raw/transport streams remain incorrect at the other end of playback.

This PR is deliberately separate from #115 (session lifetime/concurrency).

Findings fixed

1. Unknown-duration video jumped back to 0 at EOF

ReachEnd() unconditionally assigned:

_pausedPosition = Duration

and the Position getter returned Duration while _endReached.

For raw/unknown-duration media Duration == 0, so reaching the real end rewound the reported playhead to 0.

Video decode now carries the observed media end into its EOF marker. Known Duration remains authoritative; when duration is unknown, the observed end is retained instead of zero.

2. Video + audio with unknown duration did not wait for trailing audio

The video-EOS path used:

now >= Duration - 0.05

With Duration 0 this is true immediately, so a video stream ending before its audio caused playback to stop without waiting for the remaining audio.

Audio now publishes EOF for the active seek serial together with the media position represented by all PCM actually queued. The presenter waits until that serial's audio EOF exists and the sink clock has reached the queued tail.

3. Audio-only unknown-duration streams never ended

PresentAudioOnlyTick() only called ReachEnd when Duration > 0.

Audio-only streams whose container/stream duration is unavailable therefore remained IsPlaying=true indefinitely after the decoder and device queue were finished.

Audio-only playback now also completes from the serial-scoped audio EOF/drain state.

4. libswresample tail was never flushed

FFmpeg's libswresample API can buffer delayed output during sample-rate conversion. At end of conversion it must be drained with swr_convert(..., NULL, 0).

The old loop freed SwrContext at EOF without draining it, so the final samples could be lost.

This PR drains swr after the decoder reaches AVERROR_EOF, writes all remaining PCM to the sink, and only then publishes audio EOF.

5. Timestamp-less streams had no usable observed end

Both video and audio converted missing timestamps to literal 0. Repeated timestamp-less frames therefore never advanced the observed media end, and seeks into such streams could keep treating later frames as if they started at zero.

Missing video timestamps are now inferred from the prior decoded end (or seek target for the first frame).
Missing audio timestamps advance from an inferred per-serial media position.

Serial / seek safety

Audio EOF is published only when:

  • decoder drain completed;
  • resampler drain completed;
  • final PCM was accepted by the sink;
  • the decoded serial is still both the current and requested seek serial.

Audio-only presentation also requires current == requested serial before accepting EOF, so a pending newer seek cannot be overwritten by an old EOF.

Regression coverage

Pure tests pin:

  • known Duration remains authoritative;
  • unknown Duration falls back to observed end;
  • invalid/no observed end falls back to 0;
  • audio drain requires the current EOF serial;
  • audio drain does not complete until the sink clock reaches the queued media end;
  • empty/no-PCM EOF is allowed to complete immediately.

Existing seek/planar-format/queue/sink tests remain in place.

FFmpeg API contract used

The implementation follows FFmpeg's documented drain model:

  • send NULL to the decoder and receive until AVERROR_EOF;
  • at the end of resampling, call swr_convert with NULL input / zero input count until no delayed output remains.

Branch state

Final CI

Authoritative run: SubtitleEdit#246 on d09bda70cd79ea8a0aedce7a736e925fccd9a789

  • 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
  • retry step skipped

Compiler warnings are confined to pre-existing accessibility/VoiceManager files outside this PR's changed set.

AI assistance: ChatGPT was used for adversarial EOF/timestamp/drain review of the merged FFmpeg-player hardening and to prepare focused regressions.

Track observed video/audio end positions, drain libswresample before audio EOF, and finish unknown-duration video/audio playback without jumping back to zero or staying stuck in playing state.
@Blackspirits
Blackspirits force-pushed the audit/ffmpeg-unknown-duration-eof-3e4d branch from f221094 to d09bda7 Compare September 15, 2026 07:32

@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 the frozen HEAD after CI SubtitleEdit#246. Rechecked the EOF marker semantics, serial/request guards, unknown-duration end-position fallback, audio tail accounting, and libswresample drain path. No blocking defect found in this tranche. CI SubtitleEdit#246 passed build + full suite on this exact HEAD; retry was not used. This remains an audit draft only: no merge/promotion intended.

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