Skip to content

Commit ca31573

Browse files
fix(subtitles): a landing line has to stand on ground somebody read (#416)
A PGS display set has no end of its own, so the reconstruction pass at a seek landing decides which set is still on screen by the store holding nothing between it and the playhead. Over a stretch nobody read, that silence proves nothing. A run re-aimed just after it harvested a set leaves that set's own clear on the far side of the skipped ground: the set decodes at the landing looking unclosed, becomes the landing's active line, and takes its end from the next stored packet, which is the far side of the authored silence rather than its own successor. The reporter's asset shows a two-second sound-effect caption standing over a scene ten seconds later, on an Apple TV 4K and on a Mac. #362 round 2 established that the packets alone cannot show this and named the ledger as the precise signal: a reader restarted BEHIND leaves a descending harvest sequence at the boundary, but one re-anchored FORWARD hangs its packets in ascending order behind the stretch it skipped, so the pair is indistinguishable from an authored silence. It was left unbuilt for want of a reproducible defect. This is that defect. So the readers state what they read. `SubtitleHarvestCoverage` keeps one span per run in the packet store, anchored where a reader positioned and extended as it goes: the forward prefetcher reports its own read position and every re-anchor, the pump's run begins where the producer opens or restarts (both paths) and reaches at least the playhead, since playback is rendering there. A set whose ground up to the playhead is not covered cannot be the landing's active line, and the same rule closes the #100 hold's door onto the same claim. A store nobody reports to answers every span with yes, so a harvest path without notes behaves exactly as before. The cost is the landing line in the case where a set really is still up and the proof is missing, which takes an authored dwell long enough to span the whole unread stretch; the alternative was paying it for every normally authored set that ends inside one. A refusal says so as `landingWithheld=` on the delivery line, beside `endsWithheld=`. Measured with `aetherctl play` on a 500 s H.264 + PGS fixture whose sets are scheduled (set at 184, authored clear at 190, then silence to 240), over a loopback origin, seeking from 100 to 197 so the initial run's park edge falls between the set and its clear and the restarted pump opens at 192, above it: before, 5 of 5 runs published `CUE 184.00-240.00` at the landing; after, 0 of 5, with `landingWithheld=1` on the runs that met the condition and the authored set at 240 published in every run. Controls unchanged in both arms: a far seek into fresh ground landing inside an authored dwell still publishes its line (292.00-295.50), and a plain playthrough delivers the same four cues. 2053 tests green, tvOS + iOS Simulator builds green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RbWZLwBVLGM1xUVXa9NeJ1
1 parent 079f6c6 commit ca31573

11 files changed

Lines changed: 487 additions & 8 deletions

Sources/AetherEngine/AetherEngine+Subtitles.swift

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,12 @@ extension AetherEngine {
270270
guard !subtitleDrainTargets.isEmpty, let store = activeSubtitlePacketStore else { return }
271271
store.setProtectedStreams(Set(subtitleDrainTargets.values)) // #166: re-assert protection
272272
let playhead = sourceTime
273+
// #416: the pump is rendering the frame at the playhead, so it has necessarily read from
274+
// wherever it opened up to here. That is what the pump can state without a hook in its read
275+
// loop, and it is exactly the stretch a landing claim rests on: the ground between a set
276+
// decoded behind the playhead and the playhead itself. Its forward lookahead is left
277+
// unclaimed, which costs a landing line at worst and can never invent coverage.
278+
store.noteHarvestProgress(.pump, through: playhead)
273279
// #271: wall time since the previous tick, so a tick that itself ran long cannot be read as
274280
// a seek by the next one. See SubtitleOverlayDrainer.drainPlan.
275281
let tickUptime = Double(DispatchTime.now().uptimeNanoseconds) / 1_000_000_000
@@ -425,6 +431,7 @@ extension AetherEngine {
425431
let applied = applySubtitleEvent(event, to: &cues, channel: channel)
426432
tally.admitted += applied.admitted
427433
tally.published += applied.published
434+
tally.landingWithheld += applied.landingWithheld // #416
428435
if applied.changed { didMutate = true }
429436
}
430437
}
@@ -788,6 +795,10 @@ extension AetherEngine {
788795
demuxer: demuxer, to: startAt, anchorStreamIndex: seekAnchor,
789796
fallbackDuration: engineDisplayDuration,
790797
timeout: Self.sideReaderSeekBudgetSeconds)
798+
// #416: this session reads forwards from here, and nothing below it is this reader's to
799+
// claim. Stated even when the positioning fell back or failed: what the loop then reads is
800+
// still forwards from wherever it sits, and the anchor is the earliest it can be.
801+
store.noteHarvestAnchor(.prefetch, at: startAt)
791802
if landed != .seek {
792803
EngineLog.emit(
793804
"[AetherEngine] #151 forward prefetch seek to \(String(format: "%.2f", startAt))s timed out "
@@ -963,9 +974,21 @@ extension AetherEngine {
963974
// #112/#143: during a reconstruction pass any decoded composition at/behind the playhead becomes the
964975
// held active-line candidate, emitted once when the decode reaches the playhead (see
965976
// PGSStaleArrivalGate.admitDuringReconstruction).
977+
// #416: a bitmap set decoded behind the playhead claims to be the line still on screen
978+
// there, and that claim rests on the store being empty between the two. Ask the harvest
979+
// whether it ever read that stretch before reading its silence as an answer.
980+
// Asked for bitmap events only: a text cue carries its own duration, so it claims nothing
981+
// about the ground behind it, and a dense text track would pay the lookup per packet.
982+
let groundIsRead = event.isPGS
983+
? subtitleLandingGroundIsRead(cues: event.cues, playhead: sourceTime)
984+
: true
985+
// A false answer here means an actual refusal: the helper returns true when the event
986+
// carries no cue behind the playhead, so there is nothing that could have been refused.
987+
if !groundIsRead { applied.landingWithheld += 1 }
966988
let admitted = pgsStaleArrivalGates[channel, default: PGSStaleArrivalGate()]
967989
.admit(cues: event.cues, isPGS: event.isPGS,
968-
isSelfContained: event.isSelfContainedPGS, playhead: sourceTime)
990+
isSelfContained: event.isSelfContainedPGS, playhead: sourceTime,
991+
groundIsRead: groundIsRead)
969992
applied.admitted += admitted.count
970993
for cue in admitted {
971994
if insertSorted(cue, into: &cues) {
@@ -983,6 +1006,22 @@ extension AetherEngine {
9831006
Self.insertCueSorted(cue, into: &cues, nextID: &nextRetainedSubtitleCueID)
9841007
}
9851008

1009+
/// #416: was the source between the newest of these cues that lies behind `playhead` and the
1010+
/// playhead itself actually read by some harvest run?
1011+
///
1012+
/// Only that one cue matters: it is the one the gate would make the landing's active line, and
1013+
/// the only stretch its claim depends on is the one between it and the playhead. Cues at or
1014+
/// after the playhead need no ground, and an event carrying none behind it asks nothing here.
1015+
///
1016+
/// Coverage is a property of the SOURCE, not of a stream: a reader demuxes every stream it
1017+
/// passes over, so a span one of them read is read for all of them.
1018+
private func subtitleLandingGroundIsRead(cues: [SubtitleCue], playhead: Double) -> Bool {
1019+
guard let store = activeSubtitlePacketStore else { return true }
1020+
guard let newestBehind = cues.lazy.map(\.startTime).filter({ $0 <= playhead }).max()
1021+
else { return true }
1022+
return store.hasReadSpan(from: newestBehind, through: playhead)
1023+
}
1024+
9861025
/// #107: close every non-image cue (text or rich text) whose window covers `trimAt` (teletext
9871026
/// page-state semantics: each page transmission or erase replaces what came before it). Image
9881027
/// cues are untouched; they have their own PGS trim. Static and pure for unit tests.

Sources/AetherEngine/AetherEngine.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3863,6 +3863,7 @@ public final class AetherEngine: ObservableObject {
38633863
// Live SW: drive the host's ring-backed DVR reseed directly; no AVPlayer-clock translation applies.
38643864
if softwareHost != nil, nativeHost == nil {
38653865
EngineLog.emit("[AetherEngine] SW live seek target=\(target)", category: .engine)
3866+
softwareSubtitlePacketStore?.noteHarvestAnchor(.pump, at: target) // #416
38663867
await softwareHost?.seek(to: target)
38673868
guard loadGeneration == loadGen, seekGeneration == seekGen else { return }
38683869
clock.currentTime = target
@@ -3924,6 +3925,11 @@ public final class AetherEngine: ObservableObject {
39243925
} else if let host = audioHost {
39253926
hostReposition = await host.seek(to: clockTarget)
39263927
} else if let host = softwareHost {
3928+
// #416: the software pump is this path's subtitle harvest, and it reads forwards from
3929+
// wherever this reposition puts it. Everything between where it had got to and here is
3930+
// ground nobody read; the drain must not read an empty store there as an authored
3931+
// silence. Stated before the seek: the demuxer lands at or before the target.
3932+
softwareSubtitlePacketStore?.noteHarvestAnchor(.pump, at: clockTarget)
39273933
hostReposition = await host.seek(to: clockTarget)
39283934
} else {
39293935
// #93 retest: remember the target as recovery intent BEFORE awaiting; a wedged seek

Sources/AetherEngine/Diagnostics/SubtitleDeliveryStatement.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ enum SubtitleDeliveryStatement {
9999
/// store on a different horizon. A window carrying a wrong end with `gapAt` absent said
100100
/// nothing at all until this count existed (the reporter's observation, exactly).
101101
var endsWithheld = 0
102+
/// #416: bitmap events this tick refused to let claim the landing, because the source
103+
/// between the set and the playhead was never read and an empty store there is not
104+
/// evidence of anything. Its own field for the same reason `endsWithheld` has one: a
105+
/// refusal that only shows up as a missing cue is indistinguishable from a delivery that
106+
/// never arrived, and those two have opposite next steps.
107+
var landingWithheld = 0
102108

103109
/// Cues an event carried that the gate did not pass. The subtraction is safe: `admitted`
104110
/// can exceed the tick's own `cues` when a finalized candidate seeded by an earlier tick is
@@ -131,6 +137,9 @@ enum SubtitleDeliveryStatement {
131137
var admitted = 0
132138
/// Cues the insert took. `admitted - published` is the re-decode the store deduped.
133139
var published = 0
140+
/// #416: events whose behind-playhead cues were refused the landing for want of read
141+
/// ground. Counted here so the tick can total it into the delivery line.
142+
var landingWithheld = 0
134143
}
135144

136145
struct Statement: Equatable, Sendable {
@@ -178,6 +187,9 @@ enum SubtitleDeliveryStatement {
178187
if tally.endsWithheld > 0 {
179188
trailing.append("endsWithheld=\(tally.endsWithheld)")
180189
}
190+
if tally.landingWithheld > 0 {
191+
trailing.append("landingWithheld=\(tally.landingWithheld)")
192+
}
181193
return (fields + trailing).joined(separator: " ")
182194
}
183195

Sources/AetherEngine/Subtitles/Issue100PGSStaleArrival.swift

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,26 @@ struct PGSStaleArrivalGate {
7979
/// delegated to `admitDuringReconstruction`, which holds the lead-in's compositions and emits only the line
8080
/// group active at the playhead. Outside reconstruction the #100 stale hold governs: a catch-up backlog of
8181
/// arrivals behind the playhead is held for successor resolution and cannot flash through the overlay.
82-
mutating func admit(cues: [SubtitleCue], isPGS: Bool, isSelfContained: Bool = false, playhead: Double) -> [SubtitleCue] {
82+
/// #416: `groundIsRead` says whether the source between these cues and the playhead was
83+
/// actually read by some harvest run. Only a cue BEHIND the playhead needs it, and it needs it
84+
/// because that is the whole of its claim: a PGS set has no end of its own, so "still on screen
85+
/// at the playhead" means "nothing on this stream happened in between", and an empty store over
86+
/// unread ground is not evidence of that. False makes such a cue ineligible, here and in the
87+
/// reconstruction pass alike; a cue at or after the playhead is unaffected, it claims nothing
88+
/// about ground behind it. Defaults to true so every caller that cannot answer keeps the
89+
/// behaviour it had, which is also what an unreporting harvest path gets from the store.
90+
mutating func admit(cues: [SubtitleCue], isPGS: Bool, isSelfContained: Bool = false,
91+
playhead: Double, groundIsRead: Bool = true) -> [SubtitleCue] {
8392
guard isPGS, !cues.isEmpty else { return cues }
8493
if reconstructing {
85-
return admitDuringReconstruction(cues: cues, isSelfContained: isSelfContained, playhead: playhead)
94+
return admitDuringReconstruction(cues: cues, isSelfContained: isSelfContained,
95+
playhead: playhead, groundIsRead: groundIsRead)
8696
}
8797
let stale = cues.allSatisfy { $0.startTime < playhead - staleEpsilonSeconds }
8898
guard stale else { return cues }
99+
// #416: a stale arrival publishes only if its window turns out to cover the playhead, which
100+
// is the same claim over the same unread ground. Nothing to hold it for.
101+
guard groundIsRead else { return [] }
89102
heldCues = cues
90103
return []
91104
}
@@ -104,11 +117,21 @@ struct PGSStaleArrivalGate {
104117
/// references are missing fails decode and never gets here), and the steady-state path outside reconstruction
105118
/// already publishes Normal compositions unconditionally. `isSelfContained` stays on the signature: callers
106119
/// keep reporting the PCS classification, which the epoch-start-aware backscan direction would need.
107-
private mutating func admitDuringReconstruction(cues: [SubtitleCue], isSelfContained: Bool, playhead: Double) -> [SubtitleCue] {
120+
private mutating func admitDuringReconstruction(cues: [SubtitleCue], isSelfContained: Bool,
121+
playhead: Double,
122+
groundIsRead: Bool = true) -> [SubtitleCue] {
108123
// #146: all objects of one display set arrive in a single decode event sharing a start, so
109124
// the candidate is the newest same-start GROUP behind the playhead; a same-start re-seed is
110125
// a re-decode of the same set and replaces the group wholesale.
111-
let behind = cues.filter { $0.startTime <= playhead }
126+
// #416: only a set whose ground up to the playhead was read can be that candidate. The
127+
// reporter's case is a set the harvest reached just before it was re-aimed somewhere else,
128+
// whose own clear sits in the stretch that was then skipped: it decodes at the landing, no
129+
// successor is stored between it and the playhead because nobody looked, and the pass emits
130+
// half a minute of history over the new scene. Refusing costs the landing line in the case
131+
// where the set really is still up and the proof is missing, which needs a dwell long
132+
// enough to span the whole unread stretch; publishing costs it in every case where it is
133+
// not, which is every normally authored set in that stretch.
134+
let behind = groundIsRead ? cues.filter { $0.startTime <= playhead } : []
112135
if let newestStart = behind.map(\.startTime).max(),
113136
reconstructionCandidates.first.map({ newestStart >= $0.startTime }) ?? true {
114137
reconstructionCandidates = behind.filter { $0.startTime == newestStart }

Sources/AetherEngine/Subtitles/SubtitleForwardPrefetcher.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ enum SubtitleForwardPrefetcher {
2121
/// not already behind the read head when the first packet arrives (#112 round 10).
2222
static let anchorBackscanSeconds: Double = 2.0
2323

24+
/// #416: step between two coverage notes, in seconds of source. Small against every authored
25+
/// dwell (so a hole of subtitle size is always visible as one), large against a packet (so a
26+
/// dense track does not take the store's lock per packet for it).
27+
static let coverageNoteStepSeconds: Double = 0.5
28+
2429
/// Where a positioning attempt landed. The byte estimate is a fallback, not a failure: the
2530
/// reader keeps working from wherever it put the cursor, it just cannot trust timestamps.
2631
enum Positioning: Equatable {
@@ -228,6 +233,10 @@ enum SubtitleForwardPrefetcher {
228233
var timeBaseCache: [Int32: AVRational] = [:]
229234
var timeBaseFailures = 0
230235
var exit = Exit.cancelled
236+
/// #416: the last read position reported to the store's coverage ledger. Reported in steps
237+
/// rather than per packet: the ledger only has to be able to tell a hole of authored size
238+
/// from continuous reading, and a step keeps this off the store's lock on a dense track.
239+
var lastCoverageNoted = -Double.infinity
231240
/// #240: the valve's grant window. Set when a yield hit the cap, checked before the next
232241
/// arbitration so the reader keeps the link for a while rather than for one packet.
233242
var valveGrantedUntil: DispatchTime? = nil
@@ -287,6 +296,12 @@ enum SubtitleForwardPrefetcher {
287296
// Stamped here rather than at request time so the window between the two reads
288297
// as unresolved instead of validating a position the seek has not reached yet.
289298
SubtitlePrefetchTelemetry.recordReanchor(seekGeneration: target.seekGeneration)
299+
// #416: the run that was reading up to here is over, and everything between where
300+
// it got to and this anchor is ground nobody read. Stated as the anchor the reader
301+
// ASKED for, not where the seek landed: a subtitle-axis seek lands at or before the
302+
// request, so this under-claims by up to one authored gap and never over-claims.
303+
store.noteHarvestAnchor(.prefetch, at: target.seconds)
304+
lastCoverageNoted = -Double.infinity
290305
anchorGraceUntil = DispatchTime.now()
291306
+ (link?.anchorGraceSeconds ?? SideReaderLinkPolicy.anchorGraceSeconds)
292307
if let fresh = await playhead() { playheadSnapshot = fresh }
@@ -354,6 +369,12 @@ enum SubtitleForwardPrefetcher {
354369
// `prefetchLead` tracks the reader rather than the last cue, so on a sparse track it
355370
// now moves between cues instead of standing still.
356371
SubtitlePrefetchTelemetry.recordPacket(seconds: position, harvested: harvested)
372+
// #416: this reader has now read to here. The pacing packets are what make the claim
373+
// continuous over an authored silence, which is the stretch the ledger exists for.
374+
if position >= lastCoverageNoted + coverageNoteStepSeconds {
375+
store.noteHarvestProgress(.prefetch, through: position)
376+
lastCoverageNoted = position
377+
}
357378
var didPark = false
358379
while !Task.isCancelled, position > playheadSnapshot + leadSeconds {
359380
if !didPark {

0 commit comments

Comments
 (0)