From fbac4415c3d5477bd4a6bd7b8b6e9f35cc688bd1 Mon Sep 17 00:00:00 2001 From: Vincent Herbst Date: Mon, 21 Sep 2026 06:43:15 +0200 Subject: [PATCH] fix(probe): evidence in hand outranks the budget that bounded fetching it Follow-up to PR #583, whose validator and whole-probe controls are right in the core. Four places where a bound meant to limit COST was also deciding what the probe may report. The HDR10+ validator discarded a message it had already parsed in full whenever anything later in the same packet failed the structural walk. Verified against the scan directly: a valid ST 2094-40 SEI followed by a NAL with its forbidden bit set, or by an SEI missing rbsp_trailing_bits, came back negative, where the byte scan this replaced said yes. That is ordinary shape on real media, and it reaches the playback path through HLSSegmentProducer, not just the probe. The walks now stop at the damage and report what they validated. Nothing can invent a positive there: only validT35 ever sets one. The HDR10+ and Atmos detail passes retracted a detection they had already made when their soft wall-clock budget expired mid-pass. The caller cannot tell a withheld confirmation apart from a source that carries none, so an overrun on a slow origin silently became "no Atmos", on the existing API with no opt-in. An overrun now retires the pass without taking its answer back. The whole-probe deadline from #583 is untouched: that one throws, so the caller knows. Passing `limits` switched the open to the still extractor's profile, so probe(url:limits:) read 2 MiB / 2 s where probe(url:) reads 50 MiB / 60 s and could report fewer streams for it. The caller's own limits are the only thing that should bind: the profile is built from .playback with probesize clamped to maxInputBytes, and the deadline plus the FFmpeg interrupt callback bound the analysis. The recordless Dolby Vision audit stays out of reach, and docs/api.md now says so in terms of what a host observes: that audit opens the source a second time by URL, outside this probe's budget and cancellation. A controlled HTTP probe took the origin slot with tryAcquire and failed with sourceBusy the instant any other request held it, which is the ordinary shape when a host probes several items off one server. It waits for the slot until its own deadline instead, since a slot wait is the one wait the watchdog cannot interrupt. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015PM3xUJB6ZQyqnmGK1fp6F --- CHANGELOG.md | 15 +++++++- Sources/AetherEngine/AetherEngine+Probe.swift | 17 ++++++++- .../Audio/AtmosDetectionProbe.swift | 8 ++-- Sources/AetherEngine/Demuxer/AVIOReader.swift | 8 +++- Sources/AetherEngine/ProbeControl.swift | 6 +++ .../Video/HDR10PlusDetectionProbe.swift | 11 ++++-- .../Video/HDR10PlusMetadataScan.swift | 38 +++++++++++-------- .../HDR10PlusMetadataScanTests.swift | 35 +++++++++++++---- .../HDR10PlusProbeIntegrationTests.swift | 15 ++++++-- .../AetherEngineTests/ProbeControlTests.swift | 24 ++++++++++++ docs/api.md | 22 +++++++---- 11 files changed, 154 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aad265f9..c218513e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,20 @@ the public-API contract. - HDR10+ confirmation validates codec metadata structures and the registered ST 2094-40 payload instead of matching a marker anywhere in compressed bytes. Playback and probing share the validator; Dolby Vision stays primary. Per-pass byte limits now reject oversized packets before inspection or - decode, and a result arriving after the pass deadline cannot confirm metadata. + decode. +- A message the HDR10+ validator has parsed in full is not withdrawn by damage elsewhere in the same + packet. Malformed framing after a confirmed ST 2094-40 payload ends the walk and reports the + confirmation, where it previously discarded it, which cost a real badge whenever a vendor SEI, a + trailing byte or a second unreadable NAL sat next to the metadata. +- The HDR10+ and Atmos detail passes no longer retract a detection they already made because their + soft wall-clock budget expired. The budgets bound what a pass spends; a withheld confirmation is + indistinguishable to the caller from a source that carries none. +- A controlled probe opens with the playback analysis budget, clamped by the caller's own + `maxInputBytes`, instead of the still extractor's smaller one, so passing `limits` no longer + reports fewer streams than the same call without it. The recordless Dolby Vision audit remains + unavailable to it: that audit opens the source a second time by URL, outside the probe's budget. +- A controlled HTTP probe waits for an origin request slot until its own deadline rather than + failing with `sourceBusy` the moment another request holds the origin. ## [7.9.0] - 2026-09-20 diff --git a/Sources/AetherEngine/AetherEngine+Probe.swift b/Sources/AetherEngine/AetherEngine+Probe.swift index e685503a..e554db87 100644 --- a/Sources/AetherEngine/AetherEngine+Probe.swift +++ b/Sources/AetherEngine/AetherEngine+Probe.swift @@ -252,9 +252,24 @@ extension AetherEngine { return probe } + /// The caller's own limits are the only thing allowed to bind a controlled probe. + /// + /// Built from `.playback` rather than `.stillExtraction`: a host that asks for limits is asking to + /// bound the COST, not to be answered from a shallower read, and a second hidden budget (the still + /// extractor's 2 MiB probe and 2 s analysis) would make `probe(url:limits:)` report fewer streams + /// than `probe(url:)` on exactly the sparse sources where that matters. `maxInputBytes` clamps the + /// probe size, and the deadline plus the FFmpeg interrupt callback bound the analysis instead. + /// The profile's AVIO tuning is not read on this path at all: a controlled probe always opens + /// through a reader, whose own transport settings live in `ProbeHTTPReader`. + /// + /// The recordless Dolby Vision audit stays off, and that is a real gap rather than a tuning choice: + /// it opens the source a SECOND time by URL (`DolbyVisionRecordAudit.rpuProfileOfSource`), traffic + /// this probe's budget and cancellation do not reach, and `open(reader:)` clears `auditSource` + /// regardless. A controlled probe of an untagged 10-bit HEVC source therefore reports no Dolby + /// Vision record where an uncontrolled one does (AE#567). private nonisolated static func probeOpenProfile(limits: ProbeLimits?) -> DemuxerOpenProfile { guard let limits else { return .playback } - var profile = DemuxerOpenProfile.stillExtraction + var profile = DemuxerOpenProfile.playback // FFmpeg has a minimum probe size. The input seam still enforces smaller caller limits. profile.probesize = max(32, min(profile.probesize, limits.maxInputBytes)) profile.auditsRecordlessDolbyVision = false diff --git a/Sources/AetherEngine/Audio/AtmosDetectionProbe.swift b/Sources/AetherEngine/Audio/AtmosDetectionProbe.swift index 4d75fd66..0697cc09 100644 --- a/Sources/AetherEngine/Audio/AtmosDetectionProbe.swift +++ b/Sources/AetherEngine/Audio/AtmosDetectionProbe.swift @@ -283,11 +283,9 @@ extension AetherEngine { av_packet_free_safe(pkt) if confirmed { - let elapsed = Double(DispatchTime.now().uptimeNanoseconds - start.uptimeNanoseconds) / 1_000_000_000 - guard elapsed < options.timeBudget else { - return AtmosDetectionOutcome(stopReason: .timeCap, packetsRead: packetsRead, - bytesRead: bytesRead, decodedProfile: nil) - } + // A decoded JOC frame is evidence the pass already paid for. The wall-clock budget bounds + // what this pass SPENDS, so an overrun retires the pass, it does not retract its answer: + // a caller cannot tell a withheld confirmation apart from a source that carries no Atmos. return AtmosDetectionOutcome( stopReason: .frameDecoded, packetsRead: packetsRead, bytesRead: bytesRead, decodedProfile: lastProfile diff --git a/Sources/AetherEngine/Demuxer/AVIOReader.swift b/Sources/AetherEngine/Demuxer/AVIOReader.swift index 1da43f3d..d215430c 100644 --- a/Sources/AetherEngine/Demuxer/AVIOReader.swift +++ b/Sources/AetherEngine/Demuxer/AVIOReader.swift @@ -3989,7 +3989,13 @@ final class AVIOReader: AVIOProvider, @unchecked Sendable { } guard !isClosed else { throw CancellationError() } try probeControl.check() - guard let ticket = OriginRequestBudget.shared.tryAcquire(for: url, label: label) else { + // Wait for the slot, but never past the probe's own deadline. A slot wait is the one wait the + // watchdog cannot interrupt (it fires at reads), so the deadline has to bound it here instead. + // Not waiting at all would fail a probe that merely arrived while one other request held the + // origin, which is the ordinary shape when a host probes several items off one server. + let slotWait = probeControl.remainingTime.map { min(timeout, $0) } ?? timeout + guard let ticket = OriginRequestBudget.shared.acquire( + for: url, label: label, timeout: slotWait) else { probeControl.stop(ProbeError.sourceBusy) throw ProbeError.sourceBusy } diff --git a/Sources/AetherEngine/ProbeControl.swift b/Sources/AetherEngine/ProbeControl.swift index afc8bec8..a7c41e3c 100644 --- a/Sources/AetherEngine/ProbeControl.swift +++ b/Sources/AetherEngine/ProbeControl.swift @@ -191,6 +191,12 @@ final class ProbeControl: @unchecked Sendable { catch { return true } } + /// Seconds left on the deadline, or nil when no numeric limits were installed. For the one wait the + /// watchdog cannot interrupt (an origin request slot), so it can be bounded by the deadline instead. + var remainingTime: TimeInterval? { + deadline.map { max(0, $0 - now()) } + } + func inputAllowance(_ requested: Int32) throws -> Int32 { try check() guard let limits else { return requested } diff --git a/Sources/AetherEngine/Video/HDR10PlusDetectionProbe.swift b/Sources/AetherEngine/Video/HDR10PlusDetectionProbe.swift index 6cd685fd..1845ddb4 100644 --- a/Sources/AetherEngine/Video/HDR10PlusDetectionProbe.swift +++ b/Sources/AetherEngine/Video/HDR10PlusDetectionProbe.swift @@ -203,14 +203,17 @@ extension AetherEngine { bytesRead += packetBytes found = HDR10PlusMetadataScan.packetCarriesHDR10Plus(pkt, codecParameters: codecpar) } - guard elapsed() < options.timeBudget else { + // Evidence in hand outranks the soft budget. The caps exist to bound what this pass SPENDS, + // and a confirmation is already paid for; dropping it would turn an overrun into a false + // negative on a slow origin, which the caller cannot tell apart from "this source has none". + if found { return HDR10PlusDetectionOutcome( - stopReason: .timeCap, packetsRead: packetsRead, bytesRead: bytesRead) + stopReason: .found, packetsRead: packetsRead, bytesRead: bytesRead) } - if found { + guard elapsed() < options.timeBudget else { return HDR10PlusDetectionOutcome( - stopReason: .found, packetsRead: packetsRead, bytesRead: bytesRead) + stopReason: .timeCap, packetsRead: packetsRead, bytesRead: bytesRead) } // AVDISCARD_ALL is advisory, so a container that keeps handing back foreign packets still ends. diff --git a/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift b/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift index 390ff8dc..47755ce2 100644 --- a/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift +++ b/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift @@ -59,6 +59,10 @@ enum HDR10PlusMetadataScan { // MARK: - H.264 / HEVC + /// A walk that runs into malformed framing stops and reports what it had already VALIDATED, rather + /// than discarding it. Only `validT35` ever sets the answer, so an aborted walk cannot invent one; + /// abandoning a confirmed payload because a later NAL in the same packet is malformed would be a + /// false negative on the one carriage this scan exists to find. private static func scanNALs( _ bytes: UnsafeBufferPointer, codecID: AVCodecID, framing: VideoNALFraming ) -> Bool { @@ -86,13 +90,13 @@ enum HDR10PlusMetadataScan { case .lengthPrefixed(let width): guard (1...4).contains(width) else { return false } var offset = 0 - while offset < bytes.count { - guard bytes.count - offset >= width else { return false } + while offset < bytes.count, !found { + guard bytes.count - offset >= width else { return found } var count = 0 for index in offset..<(offset + width) { count = (count << 8) | Int(bytes[index]) } offset += width guard count > 0, count <= bytes.count - offset, - visit(offset, offset + count) else { return false } + visit(offset, offset + count) else { return found } offset += count } case .annexB: @@ -103,15 +107,16 @@ enum HDR10PlusMetadataScan { if byte == 1, zeros >= 2 { let end = index - zeros if let start = nalStart { - guard visit(start, end) else { return false } + guard visit(start, end) else { return found } + if found { return true } } else if end != 0 { - return false + return found } nalStart = index + 1 } zeros = byte == 0 ? zeros + 1 : 0 } - guard let start = nalStart, visit(start, bytes.count - zeros) else { return false } + guard let start = nalStart, visit(start, bytes.count - zeros) else { return found } } return found } @@ -138,7 +143,9 @@ enum HDR10PlusMetadataScan { return rbsp } - /// Nil denotes malformed SEI framing; false is a well-formed SEI without HDR10+. + /// Nil denotes malformed SEI framing; false is a well-formed SEI without HDR10+. A message this + /// walk has already VALIDATED outranks framing it cannot finish reading, so nil is only ever the + /// answer when nothing was confirmed. private static func scanSEI(_ bytes: [UInt8]) -> Bool? { var offset = 0 var found = false @@ -157,7 +164,7 @@ enum HDR10PlusMetadataScan { while offset < bytes.count { if offset == bytes.count - 1, bytes[offset] == 0x80 { return found } guard let type = extendedValue(), let size = extendedValue(), - size <= bytes.count - offset else { return nil } + size <= bytes.count - offset else { return found ? true : nil } if type == 4 { let valid = bytes.withUnsafeBufferPointer { validT35($0.baseAddress! + offset, size: size) @@ -166,40 +173,41 @@ enum HDR10PlusMetadataScan { } offset += size } - return nil // rbsp_trailing_bits is mandatory. + return found ? true : nil // rbsp_trailing_bits is mandatory. } // MARK: - AV1 low-overhead OBU stream (demuxed packet framing) + /// Same contract as `scanNALs`: an aborted walk reports what it validated, never less. private static func scanOBUs(_ bytes: UnsafeBufferPointer) -> Bool { var offset = 0 var found = false - while offset < bytes.count { + while offset < bytes.count, !found { let header = bytes[offset] offset += 1 - guard header & 0x81 == 0 else { return false } + guard header & 0x81 == 0 else { return found } let type = (header >> 3) & 15 if header & 4 != 0 { - guard offset < bytes.count, bytes[offset] & 7 == 0 else { return false } + guard offset < bytes.count, bytes[offset] & 7 == 0 else { return found } offset += 1 } let size: Int if header & 2 != 0 { guard let declared = leb128(bytes, offset: &offset, end: bytes.count), - declared <= bytes.count - offset else { return false } + declared <= bytes.count - offset else { return found } size = declared } else { size = bytes.count - offset } let end = offset + size if type == 5 { - guard let metadataType = leb128(bytes, offset: &offset, end: end) else { return false } + guard let metadataType = leb128(bytes, offset: &offset, end: end) else { return found } if metadataType == 4 { // AV1 permits arbitrarily many trailing zero bytes after the stop bit. // They belong to the OBU, not to the byte-aligned T.35 payload. var trailing = end while trailing > offset, bytes[trailing - 1] == 0 { trailing -= 1 } - guard trailing > offset, bytes[trailing - 1] == 0x80 else { return false } + guard trailing > offset, bytes[trailing - 1] == 0x80 else { return found } found = validT35(bytes.baseAddress! + offset, size: trailing - offset - 1) || found } } diff --git a/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift b/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift index ecab32b2..2e4bad2b 100644 --- a/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift +++ b/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift @@ -216,9 +216,6 @@ struct HDR10PlusMetadataScanTests { for cut in 0..(10) + let control = try ProbeControl( + limits: .init(timeBudget: 5), cancellation: nil, + now: { clock.value }, scheduleDeadline: false) + defer { control.finish() } + #expect(control.remainingTime == 5) + clock.update { $0 = 13 } + #expect(control.remainingTime == 2) + clock.update { $0 = 99 } + #expect(control.remainingTime == 0) + + let uncapped = try ProbeControl( + limits: nil, cancellation: ProbeCancellation(), + now: { clock.value }, scheduleDeadline: false) + defer { uncapped.finish() } + #expect(uncapped.remainingTime == nil) + } + @Test("Completion checks time even without another read or watchdog tick") func completionChecksDeadline() throws { let clock = ProbeTestBox(10) diff --git a/docs/api.md b/docs/api.md index 550623b2..44f88101 100644 --- a/docs/api.md +++ b/docs/api.md @@ -426,10 +426,11 @@ Existing calls retain their open policy: `limits: nil, cancellation: nil`. Passi starts a monotonic deadline before opening the source, shares the input budget across disc recognition, container open, `avformat_find_stream_info`, the HDR scan, the Atmos rewind and decode, and disables speculative HTTP prefetch. Passing only `cancellation` enables interruption without installing numeric -limits. The bounded open uses a smaller FFmpeg analysis profile; sparse sources may therefore report less -metadata or need larger limits. Controlled URL probes support files, HTTP and HTTPS; other transports can -use an independent `.custom` reader. They do not open a second URL reader for the optional recordless -Dolby Vision audit. Container-declared Dolby Vision remains primary even when HDR10+ is also confirmed. +limits. The open keeps the ordinary playback analysis budget, with `probesize` clamped to +`maxInputBytes`, so a controlled probe does not answer from a shallower read than the same call +without limits. Controlled URL probes support files, HTTP and HTTPS; other transports can +use an independent `.custom` reader. Container-declared Dolby Vision remains primary even when HDR10+ +is also confirmed. `maxInputBytes` counts cumulative bytes the underlying reader delivers to the probe, including bytes read again after a seek. Reads are clipped to the remaining allowance **before** calling the reader. @@ -447,9 +448,16 @@ FFmpeg also gets an interrupt callback. This is cooperative interruption, **not guarantee**: native computation and a noncooperating reader cannot be forcibly terminated. The call waits for native work to return before freeing its state, drains interruption callbacks before returning the reader, and never publishes a positive obtained after a stop. Cancelled HTTP requests finish their task -callbacks before the probe releases their origin slots or returns. A controlled HTTP probe declines with -`sourceBusy` rather than queueing behind playback's origin request budget. Normal playback's redirect, -cookie, authentication and response policies are unchanged. +callbacks before the probe releases their origin slots or returns. A controlled HTTP probe waits for an +origin request slot until its own deadline and then gives up with `sourceBusy`; a slot wait is the one +wait the deadline watchdog cannot interrupt, so the deadline bounds it directly. Normal playback's +redirect, cookie, authentication and response policies are unchanged. + +**One detail a controlled probe cannot reach.** The recordless Dolby Vision audit (AE#567) opens the +source a SECOND time by URL to read its first RPU, traffic this probe's budget and cancellation do not +police, so a controlled probe does not run it. An untagged 10-bit HEVC source whose container carries no +Dolby Vision record therefore comes back without one from `probe(url:limits:)` while `probe(url:)` +synthesizes it. Probe that class of source without limits, or treat the absence as unconfirmed. ```swift let cancellation = ProbeCancellation()