Skip to content

Commit 82ade95

Browse files
fix(player): hold the last frame through a software-path seek (#90)
SoftwarePlaybackHost.seek() flushed the renderer, and SampleBufferRenderer.flush() unconditionally removed the displayed image (modern: flush(removingDisplayedImage: true); legacy: flushAndRemoveImage()), so the visible frame was cleared before the post-seek keyframe decoded, a black flash on slow sources like MPEG-2. The native/AVPlayer path holds the frame through a seek. flush() now takes a removingDisplayedImage flag (default true for stop/teardown); seek() passes false so the previous frame stays on screen until the post-seek frame is enqueued. DisplayFlushOp is the pure decision (modern rendererFlush vs legacy flushAndRemoveImage vs legacy hold) split out so the contract is unit-testable without a live AVSampleBufferDisplayLayer; 4 tests cover the full truth table. Confirmed on-device by the reporter: MPEG-2 episodes no longer flash black on seek. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 55bb619 commit 82ade95

4 files changed

Lines changed: 79 additions & 7 deletions

File tree

Sources/AetherEngine/Native/SoftwarePlaybackHost.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,10 @@ final class SoftwarePlaybackHost {
436436

437437
videoDecoder.flush()
438438
audioDecoder?.flush()
439-
renderer.flush()
439+
// Hold the last frame through the seek (don't blank the display) so the viewer sees the previous
440+
// frame until the post-seek frame decodes, instead of a black flash (issue #90). Stop/teardown and
441+
// background-return still clear via the default.
442+
renderer.flush(removingDisplayedImage: false)
440443
audioOutput?.flush()
441444

442445
// Live source is forward-only; DVR rewind reseeds decoders from the ring without touching the live demuxer's read position.

Sources/AetherEngine/Renderer/SampleBufferRenderer.swift

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,25 @@ import CoreVideo
88
/// Includes a small reorder buffer (4 frames) to handle B-frame decode
99
/// order from VTDecompressionSession. Frames are sorted by PTS before
1010
/// being enqueued to the display layer in strict presentation order.
11+
/// Which display-layer flush operation a flush request maps to. Split out of SampleBufferRenderer.flush()
12+
/// as a pure value so the seek-holds-frame contract (issue #90) is testable without a live
13+
/// AVSampleBufferDisplayLayer.
14+
enum DisplayFlushOp: Equatable {
15+
/// tvOS 18+/iOS 18+/macOS 15+: AVSampleBufferVideoRenderer.flush(removingDisplayedImage:).
16+
case rendererFlush(removingDisplayedImage: Bool)
17+
/// Legacy AVSampleBufferDisplayLayer.flushAndRemoveImage() — clears the visible frame.
18+
case removeImage
19+
/// Legacy AVSampleBufferDisplayLayer.flush() — keeps the last frame on screen (hold through seek).
20+
case holdImage
21+
22+
static func resolve(removingDisplayedImage: Bool, modernRenderer: Bool) -> DisplayFlushOp {
23+
if modernRenderer {
24+
return .rendererFlush(removingDisplayedImage: removingDisplayedImage)
25+
}
26+
return removingDisplayedImage ? .removeImage : .holdImage
27+
}
28+
}
29+
1130
final class SampleBufferRenderer: @unchecked Sendable {
1231

1332
private(set) var displayLayer: AVSampleBufferDisplayLayer
@@ -135,19 +154,28 @@ final class SampleBufferRenderer: @unchecked Sendable {
135154
reorderLock.unlock()
136155
}
137156

138-
/// Discard all buffered and displayed frames (seek/stop). Clears the currently visible frame immediately.
139-
func flush() {
157+
/// Discard all buffered frames. `removingDisplayedImage: true` (stop/teardown) also clears the visible
158+
/// frame; `false` (seek) holds the last frame on screen until the post-seek frame is enqueued, so a seek
159+
/// doesn't flash black between the old and new positions (matches the hardware path's hold-last-frame).
160+
func flush(removingDisplayedImage: Bool = true) {
140161
reorderLock.lock()
141162
reorderBuffer.removeAll()
142163
// Invalidate the format description cache; the next load() may open a stream with different colorimetry at the same resolution.
143164
cachedFormatDesc = nil
144165
cachedFormatKey = nil
145166
reorderLock.unlock()
146167

147-
if #available(tvOS 18.0, iOS 18.0, macOS 15.0, *) {
148-
displayLayer.sampleBufferRenderer.flush(removingDisplayedImage: true) { }
149-
} else {
168+
let modern: Bool
169+
if #available(tvOS 18.0, iOS 18.0, macOS 15.0, *) { modern = true } else { modern = false }
170+
switch DisplayFlushOp.resolve(removingDisplayedImage: removingDisplayedImage, modernRenderer: modern) {
171+
case .rendererFlush(let remove):
172+
if #available(tvOS 18.0, iOS 18.0, macOS 15.0, *) {
173+
displayLayer.sampleBufferRenderer.flush(removingDisplayedImage: remove) { }
174+
}
175+
case .removeImage:
150176
displayLayer.flushAndRemoveImage()
177+
case .holdImage:
178+
displayLayer.flush()
151179
}
152180
}
153181

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import XCTest
2+
@testable import AetherEngine
3+
4+
/// Issue #90: the software-decode path blanked the display on every seek. SoftwarePlaybackHost.seek()
5+
/// flushed the renderer, and SampleBufferRenderer.flush() unconditionally removed the displayed image
6+
/// (modern: flush(removingDisplayedImage: true); legacy: flushAndRemoveImage()), so the visible frame
7+
/// was cleared before the post-seek keyframe decoded, a black flash on slow sources (MPEG-2). The
8+
/// hardware/AVPlayer path holds the last frame through a seek. DisplayFlushOp is the pure decision split
9+
/// out of flush() so the hold-vs-clear contract is unit-testable without a live AVSampleBufferDisplayLayer.
10+
final class DisplayFlushOpTests: XCTestCase {
11+
12+
// MARK: - Seek holds the last frame (the fix)
13+
14+
func testSeekOnModernRendererHoldsDisplayedImage() {
15+
let op = DisplayFlushOp.resolve(removingDisplayedImage: false, modernRenderer: true)
16+
XCTAssertEqual(op, .rendererFlush(removingDisplayedImage: false),
17+
"a seek must forward removingDisplayedImage: false so the last frame stays on screen")
18+
}
19+
20+
func testSeekOnLegacyRendererHoldsLastFrame() {
21+
let op = DisplayFlushOp.resolve(removingDisplayedImage: false, modernRenderer: false)
22+
XCTAssertEqual(op, .holdImage,
23+
"the legacy path is the regression: a seek must call flush() (hold), not flushAndRemoveImage()")
24+
}
25+
26+
// MARK: - Stop / teardown still clears the visible frame (unchanged default)
27+
28+
func testStopOnModernRendererRemovesDisplayedImage() {
29+
let op = DisplayFlushOp.resolve(removingDisplayedImage: true, modernRenderer: true)
30+
XCTAssertEqual(op, .rendererFlush(removingDisplayedImage: true),
31+
"stop/teardown must forward removingDisplayedImage: true")
32+
}
33+
34+
func testStopOnLegacyRendererRemovesImage() {
35+
let op = DisplayFlushOp.resolve(removingDisplayedImage: true, modernRenderer: false)
36+
XCTAssertEqual(op, .removeImage,
37+
"stop/teardown on the legacy path must call flushAndRemoveImage()")
38+
}
39+
}

docs/architecture.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ Source URL ──► Demuxer ──┬─► SoftwareVideoDecoder (dav1d) ──
3838
AVR / speakers
3939
```
4040

41+
A seek holds the last frame on screen rather than blanking it. `SampleBufferRenderer.flush()` takes a `removingDisplayedImage` flag (`DisplayFlushOp` is the pure decision split out for testing): stop/teardown clears the visible frame (the default), but `SoftwarePlaybackHost.seek()` passes `false`, so the previous frame stays up until the post-seek keyframe decodes instead of flashing black on slow sources like MPEG-2. This matches the native/AVPlayer path, which holds the frame through a seek (#90).
42+
4143
`AudioDecoder` stamps each `CMSampleBuffer` from a running sample count anchored to the first frame (`AudioClockAnchor`), not from the container-quantized per-packet PTS. Container timebases are coarse (1 ms in MKV), so when a frame's duration is not an integer number of ticks (a 1536-sample AC-3 frame is 34.83 ms at 44.1 kHz but exactly 32 ms at 48 kHz) the quantized PTS leave a sub-millisecond gap or overlap at every buffer boundary, and `AVSampleBufferAudioRenderer` reconciles a discontinuity at each one (~29 clicks/sec, a continuous crackle). Anchoring to the sample clock makes consecutive buffers abut exactly; a real source discontinuity (> 100 ms off the predicted clock, i.e. a seek or edit) re-anchors so genuine gaps are not papered over, and `flush()` drops the anchor. The clock advances only on a successfully emitted buffer, so a dropped buffer injects no phantom samples.
4244

4345
AV1+DV (Profile 10.0 / 10.1 / 10.4) routes through the native path on hardware-AV1 hosts via the `dav1` / `av01` track type plus the source's `dvvC` box. AV1+Atmos is genuinely rare in the wild (mastering still runs in HEVC overwhelmingly), so the SW pipeline's lack of Atmos passthrough is a theoretical limitation rather than a real one. The dispatch happens once at load time; hosts see a unified `@Published` state surface either way.
@@ -179,7 +181,7 @@ Sources/AetherEngine/
179181
├── Network/
180182
│ └── HLSLocalServer.swift Native path: local HTTP server (127.0.0.1) serving playlist + segments
181183
├── Renderer/
182-
│ └── SampleBufferRenderer.swift SW path: AVSampleBufferDisplayLayer + B-frame reorder, HDR10+ attachments
184+
│ └── SampleBufferRenderer.swift SW path: AVSampleBufferDisplayLayer + B-frame reorder, HDR10+ attachments; `flush(removingDisplayedImage:)` holds the last frame through a seek (`DisplayFlushOp`, #90)
183185
├── Subtitles/
184186
│ ├── ASSScriptBuilder.swift Reassembles raw ASS event cues + TrackInfo.assHeader into a complete script for whole-file renderers
185187
│ ├── MovTextSampleBuilder.swift Stateless tx3g (mov_text) sample builder for the native legible-subtitle injection path (LoadOptions.prepareNativeSubtitles, #55)

0 commit comments

Comments
 (0)