Repository navigation
chore: merge upstream AetherEngine 7.40.1 into the Silo fork - #23
Conversation
A DiscDirectoryReader whose listing holds VIDEO_TS/ files is now read as DVD-Video instead of Blu-ray. The title-set code of wrapDVD moves into dvdTitleSet, which takes a listing and a way to read one whole file, so an image and a folder share the VMGI filtering, durations, chapters, languages and the selected title's VTS IFO. wrapDVDFolder concatenates the selected title set's VOBs in part order with MultiFileConcatIOReader; that is the same run of bytes as their sectors on a disc, so the cell spans and the VTS time map apply unchanged. A listing without VIDEO_TS/ is read as Blu-ray, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: OpenAI Codex <noreply@openai.com>
Co-Authored-By: OpenAI Codex <noreply@openai.com>
… routed) Adds the pieces that rebuild a Profile 7 FEL picture, with nothing selecting them yet. - DoviFelComposer (Metal): rebuilds the 12-bit picture from base layer, enhancement layer and RPU: reshaping curves (polynomial and MMR), inverse NLQ, MMR chroma, bilinear EL upsampling, ordered dither down to 10-bit, output in a pooled P010 buffer, composeAsync bounded at maxFramesInFlight. DoviMetalParams is the parameter ABI, DoviShaderSource the kernels. The output pool survives a seek flush (flush() trims idle buffers, releaseOutputPool() drops it on teardown): dropping it made post-seek frames allocate a second full pool beside the buffers the renderer still held, about +260 MB at 4K for ~0.6 s per seek; measured end to end with the decoder in the next PRs, peak 674 -> 500 MB over 12 seeks. - DoviDisplayMetadata: attaches the Profile 8.1 RPU as DolbyVisionRPUData plus DolbyCompatibilityID (a CFString: a CFNumber aborts the render server). Both keys are undocumented; they are defined in this one file and UndocumentedDolbyVisionKeysTests fails if either literal appears in another source file. - DoviRpuConverter.separateDualLayers splits one packet into base layer, enhancement-layer packet, converted RPU and composer parameters; convertPacketToProfile81 is now that walk with the EL dropped. The RPU attached to composed frames is converted with mode 2 (the composer has already applied the reshaping and added the residual, so the display side must not reshape again); DoviComposedFrameRpuTests pins this. Nothing calls the composer, so no behaviour changes for any source. Public API: none. Not in this PR: the dual-layer decoder, the routing flag and host wiring, the aetherctl tooling, and dolby-vision-fel.md (comments here cite its section numbers; the document arrives with the route PR). Tests: GPU curve sweep, edge cases and adversarial inputs, layout probe (the GPU reads each parameter sub-block back at the offset Swift wrote it), shader/ABI sync, dither, enhancement upsampler, FEL math, FEL and CRC-valid-mutation fuzzers, converter and guard tests, async compose, look-ahead pool sizing. The real-clip tests skip without DV_FEL_TESTS_DIR.
…e dual-layer decoder Four self-contained pieces the dual-layer Profile 7 decoder is built from, none of them wired to anything yet: - HEVCPictureOrderCount: the H.265 picture order count (§8.3.1) of one layer, read from the parameter sets and each picture's first slice segment header. No decoding. Anything it cannot follow (unknown PPS/SPS, truncated header, pictures before the first IRAP) answers nil rather than a guess. - DualLayerPictureAlignment: derives each layer's display slot from its POC and gives each enhancement-layer packet the presentation time of the base-layer picture in the same slot, so a stream whose enhancement layer has B-frames over a base layer without them pairs the right pictures. Falls back to access-unit pairing when a layer's POC is unavailable or the slots disagree by more than 16. - FrameSideDataRing: fixed-capacity (32) ring of per-frame side data keyed by PTS, looked up within +-300 ticks. - OrderedDelivery: hands composed frames across the VideoToolbox / Metal queue boundary in decode order. Also exposes A53SEIParser.unescapeRBSP and parseHVCCParameterSets (DoviRpuConverter+Probe) at module scope, which the POC reader and its test use. No behaviour change: nothing calls these types. Not in this PR: the decoder that uses them, the routing, the docs. Tests: DualLayerPictureAlignmentTests, FrameSideDataRingTests, OrderedDeliveryTests.
libdovi is built panic=abort, so a single panic ends the host process. A CRC-valid but hostile Dolby Vision RPU could reach one on the native Profile 7 -> 8.1 conversion. Adds DoviRpuGuard: it checks the RPU CRC and pre-scans the payload for the inputs that make libdovi 3.4 panic or allocate without bound (linear interpolation pieces, polynomial/MMR mixes that break the writer, pivot and block counts the payload cannot hold, undefined display-management block lengths). The existing paths now check each RPU with it before parsing: the Profile 8.1 conversion in DoviRpuConverter (convertPacketToProfile81), the enhancement-layer-type probe, and the DolbyVisionRecordAudit RPU read. The mode 2 convert -> write pair on a parsed RPU is factored into DoviRpuConverter.convertedUnspec62 so the tests drive production's own entry point. A rejected RPU degrades the frame to the HDR10 base exactly as a libdovi failure already did (superuser404notfound#135). This hardens the existing native 8.1 conversion; it changes no routing and adds no Dolby Vision FEL code. The guard knows the paths listed above; it does not prove there are no others. Tests: DoviRpuGuardTests (crafted RPUs, verdicts, conversion round trip) and a DolbyVisionRecordAudit case. The CRC-valid mutation fuzz and the real-file guard test arrive with the composer, whose parameter walk they also exercise.
…or reordered streams Adds the SampleBufferRenderer pieces the dual-layer Dolby Vision path needs for pacing, as inert, opt-in additions: - felVideoLookaheadFrames (9) and mayEnqueueVideoFrame / setFELLookahead: a gate that runs BEFORE decode and refuses a compressed frame more than nine frame intervals ahead of the playback clock, so composed 4K frames (about 25 MB each) cannot pile up behind Apple's readiness gate alone. - displayOrderPTS: estimates a packet's display-order PTS in decode order from the largest amount a later packet fell below the highest PTS seen, so B-frame streams do not swing the cap with the position in the GOP. A gap larger than HEVC's 16-picture reorder ceiling is treated as a timestamp discontinuity and does not poison the learned lead. Nothing calls the gate yet and it is off unless setFELLookahead(enabled:) is called, so non-FEL pacing is unchanged. The decoder and the host wiring come in later PRs; the composer-side pool sizing that follows this cap (DoviFelComposer.outputPoolAllocationThreshold) is pinned in the composer PR. Tests: SampleBufferRendererLookaheadTests, SampleBufferRendererReorderDepthTests.
…tion Matroska TrueHD reaches the software read-ahead with no packet duration and millisecond timestamps that repeat (0.83 ms frames on a 1/1000 time base). SoftwarePacketCoverage.insert rejects a zero duration, so audio coverage stayed empty, the combined A/V frontier was nil, and no seek ever hit the packet cache: every seek, even back to a spot just played, refetched from the source. Audio arrives in presentation order, so a duration-less packet now proves coverage up to the next packet's timestamp, as long as the gap is at most 0.5 s (a larger one is a hole). Packets with a duration are unchanged. Apple TV 4K, 4K Matroska with TrueHD 7.1 over HTTP, 5 seeks per run: cache hits went from 0/10 to 5/5 in every run; AC-3 and video-only already hit 10/10. The new test fails without the change.
The software path's compressed packet cache was a session file on disk. On an Apple TV 4K (A12) appends to it took 100 to 600 ms while a download ran; the producer holds the store across an append, the demux thread's reads waited behind it (and its own reads stalled up to a second), and video it had already parked reached the decoder late. The layer dropped it. On tvOS the store now lives in memory (SoftwarePacketMemoryFIFO behind a SoftwarePacketStore protocol the disk FIFO also conforms to). Records keep the disk layout in chunks of purgeable VM: unread data stays resident, a chunk the reader has finished is made volatile, so the kernel may reclaim it and it stops counting against the footprint; a seek into reclaimed history is a cache miss and refetches. Bounds: 96 MB forward, 128 MB of played history. AVPlayer keeps its progressive-download cache in memory on tvOS as well. macOS, iOS and visionOS keep the disk store. Apple TV 4K (A12), 4K Dolby Vision title over HTTP, 150 s soaks with 5 seeks: disk store 42/39/22 drops, memory store 0 to 3 in 8 of 9 runs (one run 26, not reproduced), no slow reads left. Peak footprint ~655 MB against ~580 MB with the disk store.
DualLayerVideoDecoder: the decoder half of the dual-layer route. It splits each packet into base layer, enhancement layer and RPU (DoviRpuConverter.separateDualLayers), decodes both layers on VideoToolbox through two HardwareVideoDecoder sessions, pairs them by HEVC picture order, composes with DoviFelComposer and delivers the frames in decode order with the Profile 8.1 RPU attached per frame. - Pairing: DualLayerPictureAlignment restamps each enhancement-layer packet with the time of the base picture in the same display slot, so an enhancement layer coded with B-frames over a base layer without them is no longer paired with a neighbouring frame's residual. A layer keeps access-unit pairing until its next sequence-opening IRAP after a seek. - Bounded memory: base-layer skew K = 3 frames, at most 10 waiting enhancement frames, output pool 15 plus a reserve of 3. A late or broken EL falls back to the base layer for that frame. - Seeks: a seek flushes every stage. A stale EL is never paired with a post-seek base layer and a stale BL is never delivered. The composer's output pool is kept across the flush instead of being dropped (it made the frames after a seek allocate a second pool beside the buffers the renderer still held): peak footprint on a 12-seek run of one test clip went from 674 MB to 500 MB. The pool is dropped on close. - HardwareVideoDecoder gains an opt-in no-copy block source and a parameter-set open for the enhancement layer; both are off for every existing caller. FelResidualMeasure (the AETHER_FEL_MEASURE diagnostic, off by default) is here because the decoder calls it. Nothing constructs the decoder yet: no host code selects it until the route PR, so no source plays differently after this one. Public API: none. Not in this PR: the capability gate, the preferDualLayerDolbyVision flag, host wiring, dolby-vision-fel.md. Tests: decoder, GPU slot wait, seek realignment (flush of both sessions and the composer, stale EL not composed, waiting BL dropped at the seek, seek during an in-flight composition), stream classification and packet balance, CPU model, output-pool-across-flush, real-clip tests (gated on DV_FEL_TESTS_DIR, skipped without), and a subtitle compositor test that the Dolby Vision attachments survive compositing.
…erDualLayerDolbyVision Host wiring for the composer and decoder. LoadOptions.preferDualLayerDolbyVision (default false) makes a Profile 7 FEL file play through SoftwarePlaybackHost and DualLayerVideoDecoder when the box can afford it; otherwise the existing Profile 8.1 conversion is untouched. With the flag off no source plays differently. - DualLayerRouteDecision: a pure capability gate. The route is taken only for FEL, with a proven hardware HEVC session, a seekable VOD, dolbyVisionHandling .automatic and a stream inside the real-time budget ((3840x2160 + 1920x1080) x 30 = 311.04 Mpx/s; a 24p UHD FEL with a quarter-area EL needs 80%; an unknown frame rate is budgeted as 24). Every Profile 7 load logs exactly one `DV P7 dual-layer route: taken|not taken (<reason>)` line. It fails closed: when the base-layer size is unknown the budget cannot be evaluated, so the route is refused rather than assumed. The budget is an assumption until the Apple TV run confirms it. - Wiring in AetherEngine, SoftwarePlaybackHost, VideoRoutingPolicy, CodecRoutePolicy and the reload path (preferDualLayerDolbyVision is a tuning field, correctable through reloadAtCurrentPosition(applying:)). An `hev1` source with in-band parameter sets (an empty hvcC) is completed from the head of the stream so it can be classified and take the route, the same rebuild the native path already does for its init.mp4. - Audio trade-off, documented in LoadOptions, api.md and dolby-vision-fel.md and pinned by DualLayerRouteAudioTests: the route runs on the software host, whose audio FFmpeg decodes and the sample-buffer audio renderer plays, and on tvOS only AVPlayer emits Atmos objects. An E-AC-3 (JOC) track therefore plays as its 5.1 / 7.1 channel-bed core, without the objects, where the native route would hand it to AVPlayer unchanged. Not engineered around. - Docs: dolby-vision-fel.md (the design), api.md, formats.md, README, and the DocumentedConstantsTests pins that tie the document's numbers to the code (route budget, pipeline bounds, picture-order pairing bound). The document arrives here, whole; code comments in the earlier PRs cite its section numbers. Mac soak, measured BEFORE the output-pool fix of the decoder PR (peaks are pre-fix; with it 12 seeks peak at 500 MB instead of 674 MB), M2 Pro, debug aetherctl, --window: | Run | Route | Composed / failures | Seeks landed | Drops | Steady | Peak | |---|---|---|---|---|---|---| | 217 s, 12 seeks | taken, budget 80% | 6143 / 0 | 15 | 6 / 6206 | 328-331 MB | 674 MB | | 120 s, 11 seeks | taken | 4248 / 0 | 12 | 7 / 4275 | 282 MB | 600 MB | | pause + seek while paused | taken | 2005 / 0 | 1 | 1 / 1949 | 330 MB | 352 MB | Apple TV results: pending, nothing measured on device yet. Tests: DualLayerRouteDecisionTests (one per exit), VideoRoutingPolicyTests, SoftwareDecoderSelectionTests, InBandParameterSetRebuildTests, DualLayerEdgeCaseTests, DualLayerRouteAudioTests, LoadOptionsTests, DocumentedConstantsTests. Not in this PR: the pulse probe and the aetherctl tooling.
…ming harness, analysis) Probes/DolbyVisionPulse: a reproducible measurement of whether a panel follows Dolby Vision metadata. Clips keep the pixels constant and switch only the RPU (or, for the enhancement-layer clip, only the residual) every 72 frames, so a phone filming the television shows whether the display uses the metadata and the composed pixels. It is a measurement, not a unit test, and it commits no media. - Scripts: gen_pulse.py (pattern, RPU configs, EL residual design), build.sh, serve.py, run_tv.sh, cut.py (one phone recording per arm), analyze.py (verdict per arm), report.py (the change-per-patch table and phase-A levels), check_el.py (checks on the Mac that the EL clip carries the designed residual in phase B and none in A), dump_attachments.swift (what VideoToolbox attaches to a decoded frame), sim_recording.py (a fake recording that validates the analysis). - Device/: a minimal tvOS host that plays the clips through AVPlayer, VideoToolbox and the FEL route (preferDualLayerDolbyVision). It uses only public engine API. - CI: compiles the scripts and builds the host for the tvOS simulator, so a moved public API is found in CI instead of at the television. - The pulse test is the measurement behind the consumption evidence in docs/dolby-vision-fel.md section 4.5, which arrived with the route PR. No library code changes and no behaviour change. Public API: none. Not in this PR: the aetherctl tooling. The host builds in CI; the rest needs a television and a phone. Checked here: Python and shell syntax of the scripts, swiftc -typecheck of dump_attachments.swift, xcodebuild of the host for the tvOS simulator (BUILD SUCCEEDED), swift build --build-tests of the package.
The command-line and probe side of the dual-layer Dolby Vision work. Nothing here is host API: the probes are @_spi(Diagnostics), outside the SemVer contract, and the host surface of the FEL route stays LoadOptions.preferDualLayerDolbyVision alone. - aetherctl play: --cycle-el flips the residual on a timer over a visible surface, --no-fel plays the same file on the native route, --surface presents it, and --dv-pin (DEBUG builds only) arms the consumption pin test. - aetherctl dvsurvey: reports a title's L1 / L2 display-metadata distribution straight from its RPUs (DoviRpuSurvey: no decode, no GPU). - aetherctl feltest: decodes real Profile 7 files through the FEL path and checks the result (AetherEngine+FelProbe: felDecodeProbe, felResidualEnabled). - DoviDisplayMetadataDebug and AetherEngine+DolbyVisionTest: a DEBUG-only hook that substitutes a captured RPU on a duty cycle, the on-device consumption test. It compiles out of Release; DoviDisplayMetadata.attach gains one #if DEBUG call and is otherwise unchanged. - docs/cli.md, the docs/api.md "public but not host API" bullet, and the docs/architecture.md entries for these files. No behaviour change for a host: the SPI is unreachable without @_spi(Diagnostics) and the DEBUG hook is off unless armed. Not in this PR: anything on the route or the decoder. Tests: DoviRpuSurveyResultTests (the survey's pure summaries), DoviPinTestTests (the pin-spec grammar), and the DEBUG pin hook's disarm test in DualLayerVideoDecoderTests.
…readable source When the live HLS ingest's playlist was refused at the join, the probe saw no bytes and the load published customSourceProbeFailed, which a host reads as a stream it cannot play; one that falls back to another route on it moved the channel there for the rest of its session, over a 509 an IPTV panel gives while another device holds the account's last slot. The load now publishes the origin's answer, as an HTTP reader's failed open does: sourceRateLimited (429/503/509) or sourceRefused with the status in underlyingCode, or liveSourceUnavailable when the playlist could not be reached. docs/api.md says so beside customSourceProbeFailed. Tests: PlaybackErrorInfoTests (refused join for 509 and 403, unreachable join).
…nding the session An IPTV panel that limits an account's connections answers a live playlist refresh with 509 while another device holds the last slot, and goes on refusing a session it evicted while a fresh session from the loaded URL is admitted at once. The ingest retried 1, 2 and 4 s apart and went terminal at 12 s (on the software path, the end of the session), and each refusal armed the origin's request pacer, so the host's reload waited up to 10 s for its first request. - A refused or unreachable refresh (429, 5xx, transport) is asked again every second, or after the origin's Retry-After up to 5 s, for up to 60 s; a refused segment likewise, after its two quick retries. - A session URL the loaded URL led to through redirects is asked for afresh through the loaded URL once refused twice in a row, or at once when it answers as gone (a 4xx other than 429 and 408). The tracker follows MEDIA-SEQUENCE, so the segments continue where they left off. - The live ingest's refusals no longer arm OriginRequestBudget's pacer or lower its request budget; a ceiling the host declares or another path learns still binds it. Tests: HLSLiveIngestRefusalTests.
…by swapping its item The wireless-AirPlay route change rebuilt the session to serve it on the device's LAN address. A live session on the HLS ingest stands on a forward-only custom source, whose rebuild is refused (superuser404notfound#526), and the refusal was dropped: the receiver stayed on 127.0.0.1, which it cannot reach, and played nothing while the sender reported external playback. Such a session now swaps the player's item onto the receiver's address, and back onto the loopback when the receiver lets go, as a sequential origin already did. The session, its ingest and its segment cache carry on, so the source is asked for nothing again. The swapped item rejoins its live playlist at the player's join point, as a live reload does. On-demand sessions cross as before. AirPlayPlaylistDecision.routeChangeHop decides which sessions swap, with tests. docs/formats.md (Wireless AirPlay) and docs/architecture.md say how each kind crosses.
…DVD-Video VIDEO_TS folder over HTTP feat(disc): play a DVD-Video VIDEO_TS folder over HTTP
superuser404notfound#756 routed any listing that held a VIDEO_TS/ entry to the DVD path, and a DVD path that found no title VOBs did not fall back to Blu-ray. An image is recognized Blu-ray first with DVD as the fallback; a folder now follows the same order, so a hybrid rip plays its Blu-ray title and a folder whose BDMV tree holds no playable title still plays its DVD. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…ctive-video-decoder fix(playback): report the software host's actual video decoder
…h264-missing-head-cadence fix(demux): repair variable-cadence H264 heads and hierarchical partial offsets Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…peruser404notfound#761) When the whole-file probe hands its held sample to the slot session, nothing is dequeued in between. A non-video packet arriving while the session had no pending picture returned false and went back into the parent's own queue, which is drained ahead of the session's ready queue, so it overtook audio the session had already taken: on the missing-head fixture audio DTS -1024 left after 16384. It now queues behind the session's ready packets; in steady state the demuxer drains before every read, so nothing else changes. The runtime harness now requires every non-video stream to leave in DTS order (fails on the previous code), and the all-stream budget check drains between packets as the demuxer does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…64-frame-cadence # Conflicts: # CHANGELOG.md
…es (superuser404notfound#762) The clamped-CFR verdict and its rewriter derived PTS from picture order and DTS from decode order without checking that one does not precede the other. A stream whose ranks lag their decode index by more than the reorder delay would have been handed out with PTS < DTS. Both now fail closed on that, the verdict back to the previous path and the rewriter to untouched packets. Also covers the path superuser404notfound#762 changed for superuser404notfound#511 files: a coding-order ladder on a whole-tick lattice still classifies as a permutation, not a reconstruction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…pcoming hint The dual-layer decoder delivers frames in decode order. The renderer held four of them to reorder, which left the FEL look-ahead window four frames short and dropped about one frame in five on Apple TV. AVSampleBufferVideoRenderer sorts by presentation time itself if it is told the lowest timestamp still to come, so the FEL path now hands every frame over as it arrives and sets presentationTimeExpectation = .minimumUpcoming(floor). PresentationFloor computes that floor: the first gap in the frame grid, half a frame up, given up once the clock passes a slot that never arrives. After a seek it starts at the skip threshold, since B-frames below the next anchor arrive after it. Apple TV, three rounds: drops 17/296/289 -> 6/5/5, same memory footprint. Without a frame rate the renderer keeps the reorder buffer.
…uperuser404notfound#731) The exact decode and the size, snap, zone, screen and elevation mapping for TrueHD are now built only with the TrueHDReferenceRender trait. A shipped TrueHDObjectAudio build contains the bed split alone, and a decoder that is asked for the exact path without the trait traps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
/ relay fixes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…or 9.1.6 With LibDTSX 3.0 the heights are removed from the 7.1 bed by the standard's own constant fold rather than the gains the stream's metadata states, and presentations with objects are refused, so they keep the channel bridge and play as 7.1. DTS:X is now rendered only into the layouts that carry its bed and four heights one to one (7.1.4, 9.1.6); any other layout keeps the channel bridge with a log line saying so. The decoder accepts the twelve standard channels and nothing else, so no DTS:X channel is ever panned. Docs, changelog and tests follow; the object tests are gone and the alternate sample's verdict is now a refusal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…ion, DTS:X standard presentation only LibTrueHD 3.0.0 (bed companion, interpolation out of the default build) and LibDTSX 3.0.0 (standard presentation, constant height fold). Version sweep, changelog folded, README notes the pbxproj traits form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…lakes fixed The FIFO's backpressure applied only while a segment had bytes left over. When a segment's last chunk was taken just under the ceiling, the next request went out at once with the buffer already over it, which is what made testFullBufferLetsTheCurrentSegmentFinishAndHoldsTheNextRequest flaky on CI (large loopback chunks). The producer now waits for room between segments either way; a new test builds the case deterministically (fails 3/3 before, passes 3/3 after). ItemSwapStillTests' fixture writer counted a starved test task as a stalled AVAssetWriter: both tracks were ready, but the loop had not run for the 20 s on a loaded runner. A stall now also needs 1000 idle polls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…/feat/731-energy-map 7.40.0: TrueHD Atmos from the channel presentation, DTS:X standard presentation only (superuser404notfound#731, superuser404notfound#744)
…r reached the track A TrueHD track that starts after the probe window (John Wick: Chapter 2, TrueHD from 22 s, 45.8 MB in) leaves find_stream_info without a decoded frame, so the stream keeps AV_PROFILE_UNKNOWN and objectAudioRendering played it through the channel bridge. When object rendering is wanted and the profile is unknown, the engine now peeks the track's packets where the first producer starts and reads the major sync the way mlpdec does (four substreams, top bit of substream_info). Measured with the tvOS probe budget (16 MiB / 10 s): from the start 2599 packets, 44.7 MB, 679 ms; on a resume 10 packets, 30 ms; APAC 7.1.4 in both, where the same build without the check served E-AC-3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…hen the probe misses the track Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJcXkGQrMiBPixVEFTSBJS
…/fix/truehd-late-start fix(audio): read TrueHD Atmos from the major sync when the probe never reached the track (7.40.1)
Brings the fork from upstream 7.38.1 (#22) to 7.40.1. Upstream 7.39.0 moved TrueHD Atmos and DTS:X object rendering (the APAC path), Profile 7 FEL composition and Dolby Vision P5 stills behind package traits that are off by default, because those components may need third-party patent licenses. The fork keeps upstream's defaults, so a host that does not set the traits builds without LibTrueHD, LibDTSX or the object renderer, and objectAudioRendering keeps the channel bridge. Conflicts, resolved by keeping both sides: - ci.yml: upstream's 50 min timeout, the fork's split of the authorization suites. - HLSOriginRelay: both sides added the http/https origin check; upstream's form is kept. - PlayerState `.bridged` and the docs/api.md row: upstream's trait wording with the fork's correction that only FLAC keeps the bed lossless. - docs/api.md objectAudioRendering row: upstream's text, plus the `ObjectAudioRendering` reference PublicAPIDocumentationTests needs. - docs/cli.md: upstream's text with the fork's apac.31.02 level for 8 channels; docs/formats.md: upstream's text, unbalanced parenthesis fixed. - DocumentedConstantsTests: upstream's trait gate, plus the fork's 8 and 10 channel APAC CODECS checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
silo-triage
Bot
added
the
size/XXXL
|
Important Review skippedToo many files! This PR contains 267 files, which is 167 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters: 2
📒 Files selected for processing: 267
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
![silo-kody[bot]](https://avatars.githubusercontent.com/u/287058192?s=60&v=4){kind=small}
| rows = [r for r, k in zip(rows, ok) if k] | ||
|
|
||
| # Split into runs where the clip frame index restarts. | ||
| runs, cur = [], [rows[0]] |
There was a problem hiding this comment.
IndexError occurs when the consistency filter removes every decoded row and run splitting indexes rows[0], causing short recordings or recordings with only inconsistent counter reads to bypass the existing actionable no-decodable-frames diagnostic. Check for an empty filtered result before constructing runs.
rows = [r for r, k in zip(rows, ok) if k]
if not rows:
sys.exit(f"no consistent counter frames ({bad} rejected): check framing, exposure, fiducials in view")
# Split into runs where the clip frame index restarts.
runs, cur = [], [rows[0]]Prompt for LLM
File Probes/DolbyVisionPulse/Scripts/analyze.py:
Line 205 to 208:
IndexError occurs when the consistency filter removes every decoded row and run splitting indexes rows[0], causing short recordings or recordings with only inconsistent counter reads to bypass the existing actionable no-decodable-frames diagnostic. Check for an empty filtered result before constructing runs.
Suggested Code:
rows = [r for r, k in zip(rows, ok) if k]
if not rows:
sys.exit(f"no consistent counter frames ({bad} rejected): check framing, exposure, fiducials in view")
# Split into runs where the clip frame index restarts.
runs, cur = [], [rows[0]]
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
Confirmed: if the consistency filter drops every row, rows[0] raises IndexError instead of the no-decodable-frames message. This script comes unchanged from upstream 7.37.0 (73955ad, the Dolby Vision pulse probe), and this PR only merges upstream, so it stays as upstream has it. The probe is measurement tooling outside the AetherEngine library, so no app ships it. Keeping it out of the sync so the fork doesn't drift from upstream.
There was a problem hiding this comment.
Understood—leave analyze.py unchanged in this sync. The bug is real, but it affects upstream-only measurement tooling, not the shipped library, and fixing it here would introduce fork divergence. The original critical severity is not warranted for this scope; any fix can be pursued upstream separately.
| raw = subprocess.check_output(cmd) | ||
| return np.frombuffer(raw, np.uint8).reshape(-1, 540, 960).astype(np.float32) / 255.0 |
There was a problem hiding this comment.
Excessive peak memory usage in clip_luma() results from capturing all 1,584 decoded 960×540 frames and converting the entire video to float32 before simulation: the documented 66-second fixture produces a 3.28 GB float array alongside an 821 MB captured byte buffer and conversion temporaries. Stream decoded frames and retain only the current frame needed for the 30 fps output.
Prompt for LLM
File Probes/DolbyVisionPulse/Scripts/sim_recording.py:
Line 26 to 27:
Excessive peak memory usage in clip_luma() results from capturing all 1,584 decoded 960×540 frames and converting the entire video to float32 before simulation: the documented 66-second fixture produces a 3.28 GB float array alongside an 821 MB captured byte buffer and conversion temporaries. Stream decoded frames and retain only the current frame needed for the 30 fps output.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
Confirmed: decoding the whole clip into one float32 array peaks at several GB. Same as the other note: this is upstream's Dolby Vision pulse probe (7.37.0), unchanged by this merge, and measurement tooling no app builds or ships. Leaving it as upstream has it in this sync.
There was a problem hiding this comment.
Understood. The memory concern is valid, but this is unchanged upstream measurement tooling that no app builds or ships. It does not need to block this sync; leaving it unchanged is reasonable. Streaming the decoded frames can be addressed separately upstream.
| refusalStore.onCredentialRefusal = { [weak self] status in | ||
| Task { @MainActor [weak self] in | ||
| guard let self, self.sessionRequestHeaders === refusalStore else { return } |
There was a problem hiding this comment.
Retain cycle on every load permanently retains each old store and its credential headers because the credential-refusal closure strongly captures refusalStore while RequestHeaderStore strongly retains the same closure in onRefusal. Capture refusalStore weakly and unwrap it before the identity check.
Prompt for LLM
File Sources/AetherEngine/AetherEngine.swift:
Line 4255 to 4257:
Retain cycle on every load permanently retains each old store and its credential headers because the credential-refusal closure strongly captures refusalStore while RequestHeaderStore strongly retains the same closure in onRefusal. Capture refusalStore weakly and unwrap it before the identity check.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
Confirmed: the closure captures refusalStore strongly and the store keeps the closure, so each load's store and its headers stay alive. This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. It does ship in Silo, so I'm taking it up as an upstream fix rather than a fork-only patch.
There was a problem hiding this comment.
Understood—this is a confirmed bug deferred to upstream, not a false positive. Keeping the code unchanged preserves upstream parity for this merge, but the retain cycle still affects Silo until the fix lands.
The upstream fix should weakly capture refusalStore and unwrap it before the identity check. No changes are needed in this PR under that approach.
| renderer!.advance(frameCount: block.frameCount, updates: block.updates) | ||
| renderer!.snapshot(into: &gainSnapshot) | ||
| splitter!.setGains(gainSnapshot) | ||
| pendingFrames += splitter!.process( | ||
| elements: block.planes, bed: bed.planes, frameCount: block.frameCount, | ||
| output: output.planes.map { $0 + pendingFrames }) |
There was a problem hiding this comment.
Metadata timing error applies a TrueHD block's final position or gain state to earlier samples because advancing the renderer through the entire block before taking one gain snapshot discards the updates' frameOffset timing. Process spans ending at metadata updates and refresh the splitter's gains for each span.
Prompt for LLM
File Sources/AetherEngine/Audio/Spatial/SpatialAudioBridge.swift:
Line 427 to 432:
Metadata timing error applies a TrueHD block's final position or gain state to earlier samples because advancing the renderer through the entire block before taking one gain snapshot discards the updates' frameOffset timing. Process spans ending at metadata updates and refresh the splitter's gains for each span.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. It is compiled only with an opt-in package trait that Silo does not enable, so no Silo build contains it. (TrueHDObjectAudio).
There was a problem hiding this comment.
Understood. Since this code is unchanged from upstream and excluded from Silo builds by the disabled TrueHDObjectAudio trait, this should not block the sync PR.
The timing concern may still apply to builds that enable the trait, but it belongs in an upstream follow-up rather than a fork-specific change here. No changes are needed in this PR for this finding.
| let layers = DoviRpuConverter.separateDualLayers( | ||
| packet, | ||
| extractEL: !skipEL, | ||
| useCustomBlockSource: true, | ||
| extractParams: canCompose | ||
| ) |
There was a problem hiding this comment.
NAL framing mismatch silently disables RPU/EL extraction and FEL composition for valid one- or two-byte-length HEVC samples because parsing assumes four-byte lengths instead of the size declared by hvcC. Resolve framing from the stream configuration and pass it to separateDualLayers, pictureAlignment, and the in-band parameter-set walkers.
Prompt for LLM
File Sources/AetherEngine/Decoder/DualLayerVideoDecoder.swift:
Line 551 to 556:
NAL framing mismatch silently disables RPU/EL extraction and FEL composition for valid one- or two-byte-length HEVC samples because parsing assumes four-byte lengths instead of the size declared by hvcC. Resolve framing from the stream configuration and pass it to separateDualLayers, pictureAlignment, and the in-band parameter-set walkers.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. It is compiled only with an opt-in package trait that Silo does not enable, so no Silo build contains it. (DualLayerDolbyVision).
There was a problem hiding this comment.
Understood. Since this code is unchanged from upstream and gated behind DualLayerDolbyVision, which Silo does not enable, the framing concern does not affect Silo’s current builds.
I would treat it as a potential upstream issue for trait-enabled builds, not a required fix for this sync PR. Keeping the upstream code unchanged here is reasonable.
| let closedAfterEnsure = isClosed | ||
| lock.unlock() | ||
| if !closedAfterEnsure { | ||
| el?.decode(packet: elPacket, epoch: epoch) |
There was a problem hiding this comment.
Race condition allows an old packet into post-seek sessions when a flush occurs between the shared-state check and layer submissions because forwarding epoch instead of entryEpoch disables the sub-decoders' stale-feed checks for the DVR reseed caller, which explicitly passes nil. Pass the captured entryEpoch to both layer decoders so their synchronized epoch gates reject stale packets.
el?.decode(packet: elPacket, epoch: entryEpoch)Prompt for LLM
File Sources/AetherEngine/Decoder/DualLayerVideoDecoder.swift:
Line 608:
Race condition allows an old packet into post-seek sessions when a flush occurs between the shared-state check and layer submissions because forwarding epoch instead of entryEpoch disables the sub-decoders' stale-feed checks for the DVR reseed caller, which explicitly passes nil. Pass the captured entryEpoch to both layer decoders so their synchronized epoch gates reject stale packets.
Suggested Code:
el?.decode(packet: elPacket, epoch: entryEpoch)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. It is compiled only with an opt-in package trait that Silo does not enable, so no Silo build contains it. (DualLayerDolbyVision).
There was a problem hiding this comment.
Understood—leaving this unchanged is reasonable for an upstream-sync PR, especially since Silo does not enable DualLayerDolbyVision and therefore does not compile this code.
The race concern still applies to trait-enabled consumers, but it belongs in an upstream report rather than a fork-only patch here. I would treat it as non-blocking for this merge.
| winCond.lock() | ||
| connGeneration &+= 1 | ||
| let generation = connGeneration | ||
| connHeaderRevision = requestHeaderStore?.revision ?? 0 |
There was a problem hiding this comment.
Race condition between line 3368 and request construction can stamp new credentials with an old connection revision because the code captures the revision separately from the headers, making a new token's refusal appear to concern an already-replaced credential. Capture the header dictionary and revision atomically and use that snapshot throughout request and redirect-delegate construction.
Prompt for LLM
File Sources/AetherEngine/Demuxer/AVIOReader.swift:
Line 3368:
Race condition between line 3368 and request construction can stamp new credentials with an old connection revision because the code captures the revision separately from the headers, making a new token's refusal appear to concern an already-replaced credential. Capture the header dictionary and revision atomically and use that snapshot throughout request and redirect-delegate construction.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. Plausible: the revision is read apart from the headers it describes. The effect is a refusal attributed to the wrong header revision. Upstream follow-up rather than a fork-only change in the sync.
There was a problem hiding this comment.
Agreed—leave the upstream code unchanged in this sync PR and treat this as a non-blocking upstream follow-up.
The race remains plausible: reading the revision separately from the headers could attribute a refusal to the wrong credential revision. The upstream fix should capture headers and revision atomically and use that snapshot for both request and redirect-delegate construction.
I can create a tracking issue if you’d like; nothing has been changed.
| if announcedRevision != revision { | ||
| announcedRevision = revision | ||
| callback = onRefusal |
There was a problem hiding this comment.
Stale credential notification occurs when noteCredentialRefusal announces an obsolete credential's 401/403 after replace because it checks only the previously recorded refusal, not revisionValue, causing an in-flight old request to emit sourceAuthorizationRefused for a renewed token. Suppress the callback when the supplied revision differs from the current revision, while retaining historical refusal information if recovery needs it.
Prompt for LLM
File Sources/AetherEngine/Demuxer/RequestHeaderStore.swift:
Line 72 to 74:
Stale credential notification occurs when noteCredentialRefusal announces an obsolete credential's 401/403 after replace because it checks only the previously recorded refusal, not revisionValue, causing an in-flight old request to emit sourceAuthorizationRefused for a renewed token. Suppress the callback when the supplied revision differs from the current revision, while retaining historical refusal information if recovery needs it.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
Confirmed: an in-flight request answered after replace still announces a refusal for the old revision. This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. For Silo the effect is one extra token refresh (noted in Silo-Server/silo-apple#838). Upstream follow-up.
There was a problem hiding this comment.
Understood. The bug is confirmed, but leaving it unchanged in this upstream-sync PR is intentional to preserve parity with 7.40.1. For Silo, the impact is one extra token refresh, noted in Silo-Server/silo-apple#838. The fix belongs in an upstream follow-up, not as a blocker for this merge.
| guard let previous, pts > previous, stream.numerator > 0, stream.denominator > 0, | ||
| Double(pts - previous) * Double(stream.numerator) / Double(stream.denominator) | ||
| <= Self.maxAudioSuccessorGapSeconds else { return } | ||
| audioCoverage.insert(pts: previous, duration: pts - previous) |
There was a problem hiding this comment.
Integer overflow in unchecked pts - previous arithmetic traps before the half-second gap check when two valid, increasing Int64 timestamps differ by more than Int64.max; stored packets copy source PTS values without restricting this range. Use subtractingReportingOverflow and reject overflow before converting or inserting the interval.
guard let previous, pts > previous, stream.numerator > 0, stream.denominator > 0 else { return }
let (gap, overflow) = pts.subtractingReportingOverflow(previous)
guard !overflow,
Double(gap) * Double(stream.numerator) / Double(stream.denominator)
<= Self.maxAudioSuccessorGapSeconds else { return }
audioCoverage.insert(pts: previous, duration: gap)Prompt for LLM
File Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift:
Line 638 to 641:
Integer overflow in unchecked pts - previous arithmetic traps before the half-second gap check when two valid, increasing Int64 timestamps differ by more than Int64.max; stored packets copy source PTS values without restricting this range. Use subtractingReportingOverflow and reject overflow before converting or inserting the interval.
Suggested Code:
guard let previous, pts > previous, stream.numerator > 0, stream.denominator > 0 else { return }
let (gap, overflow) = pts.subtractingReportingOverflow(previous)
guard !overflow,
Double(gap) * Double(stream.numerator) / Double(stream.denominator)
<= Self.maxAudioSuccessorGapSeconds else { return }
audioCoverage.insert(pts: previous, duration: gap)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
Confirmed for timestamps far enough apart to overflow, which only crafted media carries. This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. It ships in Silo, so I'm taking it up as an upstream fix.
There was a problem hiding this comment.
Understood—this is a confirmed overflow bug, deferred to upstream rather than fixed in this synchronization PR. Keeping the merge aligned with upstream doesn’t eliminate the crafted-media crash risk; subtractingReportingOverflow remains the appropriate fix.
I can create a tracking issue linking this finding to PR #23 if you’d like.
| if elType == nil { elType = DoviRpuConverter.enhancementLayerType(packet) } | ||
| // extractEL: false; the survey never touches pixels, so the EL is dead weight. | ||
| let layers = DoviRpuConverter.separateDualLayers(packet, extractEL: false) |
There was a problem hiding this comment.
NAL framing mismatch causes both survey RPU walkers to interpret Annex-B HEVC packets as four-byte-length-prefixed samples, skipping their RPUs and leaving TS/disc surveys empty after scanning the title. Resolve framing from the video stream's extradata and pass it to both calls, as doviProbe already does.
Prompt for LLM
File Sources/AetherEngine/Video/DoviRpuSurvey.swift:
Line 201 to 203:
NAL framing mismatch causes both survey RPU walkers to interpret Annex-B HEVC packets as four-byte-length-prefixed samples, skipping their RPUs and leaving TS/disc surveys empty after scanning the title. Resolve framing from the video stream's extradata and pass it to both calls, as doviProbe already does.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. This is the aetherctl dvsurvey diagnostic; no app calls it.
There was a problem hiding this comment.
Understood—keeping this unchanged is reasonable for an upstream-sync PR, especially since aetherctl dvsurvey is diagnostic-only and no app calls it.
The framing concern still applies to surveys of Annex-B inputs, but it is not an app playback regression or a blocker for this merge. My high-severity rating overstated the impact; any fix belongs upstream rather than as an additional fork patch here.
| func appendArray(nalUnitType: UInt8, nal: [UInt8]) { | ||
| hvcC.append(0x80 | (nalUnitType & 0x3F)) | ||
| hvcC.append(0); hvcC.append(1) | ||
| let nl = UInt16(nal.count) |
There was a problem hiding this comment.
Integer conversion trap in UInt16(nal.count) terminates playback during hvcC rebuilding when a media-controlled VPS, SPS, or PPS exceeds 65,535 bytes; a crafted four-byte-length-prefixed packet containing an oversized type-32/33/34 NAL passes collect. Reject oversized parameter sets and propagate reconstruction failure instead of performing the trapping conversion.
Prompt for LLM
File Sources/AetherEngine/Video/VideoConfigRecord+InBand.swift:
Line 57:
Integer conversion trap in UInt16(nal.count) terminates playback during hvcC rebuilding when a media-controlled VPS, SPS, or PPS exceeds 65,535 bytes; a crafted four-byte-length-prefixed packet containing an oversized type-32/33/34 NAL passes collect. Reject oversized parameter sets and propagate reconstruction failure instead of performing the trapping conversion.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
Confirmed: a VPS, SPS or PPS over 65,535 bytes traps at UInt16(nal.count); only a crafted file carries one. This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. It ships in Silo, so I'm taking it up as an upstream fix.
There was a problem hiding this comment.
Understood—leave the upstream code unchanged in this merge and pursue the fix upstream. The crash remains reachable in Silo with crafted media; keeping the fork aligned defers the fix rather than resolving the finding.
The upstream fix should reject parameter sets larger than UInt16.max and propagate reconstruction failure, with regression tests covering oversized VPS, SPS, and PPS inputs.
If useful, I can create a tracking issue for the upstream follow-up.
| return s | ||
| } | ||
|
|
||
| func denormalize(_ n: Double) -> Int { Int((n * blMax * scale).rounded(.toNearestOrEven)) } |
There was a problem hiding this comment.
Reference-model mismatch occurs because Model.denormalize omits the pre-residual clamp in the production shader's dovi_denormalize, producing different results when residuals bring negative or above-full-scale predictions back into range. Clamp the rounded prediction to 0...Int(p.vdrMax) so modelMatchesGpu, errorAttribution, and the loss census use the actual composition equation.
func denormalize(_ n: Double) -> Int {
let prediction = Int((n * blMax * scale).rounded(.toNearestOrEven))
return min(max(prediction, 0), Int(p.vdrMax))
}Prompt for LLM
File Tests/AetherEngineTests/DoviCpuModelTests.swift:
Line 96:
Reference-model mismatch occurs because Model.denormalize omits the pre-residual clamp in the production shader's dovi_denormalize, producing different results when residuals bring negative or above-full-scale predictions back into range. Clamp the rounded prediction to 0...Int(p.vdrMax) so modelMatchesGpu, errorAttribution, and the loss census use the actual composition equation.
Suggested Code:
func denormalize(_ n: Double) -> Int {
let prediction = Int((n * blMax * scale).rounded(.toNearestOrEven))
return min(max(prediction, 0), Int(p.vdrMax))
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. Test-only, for the DualLayerDolbyVision composer.
There was a problem hiding this comment.
Understood—keeping this unchanged is reasonable for an upstream-sync PR. The concern is limited to the DualLayerDolbyVision test reference model, not a production-code defect demonstrated here. Any correction should be proposed upstream rather than added as a fork-only patch in this merge.
|
|
||
| final class Sink: @unchecked Sendable { | ||
| private let lock = NSLock() | ||
| private(set) var pts: [Double] = [] |
There was a problem hiding this comment.
Data race occurs when sessionRebuildKeepsTheEpoch polls s.sink.pts.count before draining delivery because Sink.pts exposes an unlocked Array getter while take mutates the same array under a lock on OrderedDelivery's asynchronous dispatch queue, risking undefined behavior or Thread Sanitizer failures. Keep storage private and return a snapshot under the same lock.
private var storedPTS: [Double] = []
var pts: [Double] {
lock.lock()
defer { lock.unlock() }
return storedPTS
}
// In take and reset, mutate storedPTS while holding lock.Prompt for LLM
File Tests/AetherEngineTests/DoviFelRealFileTests.swift:
Line 111:
Data race occurs when sessionRebuildKeepsTheEpoch polls s.sink.pts.count before draining delivery because Sink.pts exposes an unlocked Array getter while take mutates the same array under a lock on OrderedDelivery's asynchronous dispatch queue, risking undefined behavior or Thread Sanitizer failures. Keep storage private and return a snapshot under the same lock.
Suggested Code:
private var storedPTS: [Double] = []
var pts: [Double] {
lock.lock()
defer { lock.unlock() }
return storedPTS
}
// In take and reset, mutate storedPTS while holding lock.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. Test-only, for the DualLayerDolbyVision route.
There was a problem hiding this comment.
Understood—keeping this unchanged is reasonable for an upstream-sync PR. The potential race remains relevant when the DualLayerDolbyVision tests run, but it belongs in an upstream follow-up rather than a fork-specific change here. I wouldn’t treat this test-only finding as a blocker for #23.
| defer { reader.close() } | ||
| XCTAssertFalse(server.completed.contains("/seg0.ts"), "the verdict did not wait for the whole segment") | ||
|
|
||
| let stream = Self.drain(reader) |
There was a problem hiding this comment.
Cooperative-executor blocking occurs because Self.drain makes synchronous reader.read calls until the stream ends while the first segment is deliberately still downloading. Run the drain on a GCD worker and bridge completion with a checked continuation to avoid occupying the async test's cooperative-executor thread.
Also found in:
Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:281-281Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:143-143Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:206-206Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:308-308Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:95-95Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:87-87Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:147-147Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:202-202Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:276-276Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:303-303Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:172-172Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:177-177
Kody rule violation: Move blocking decodes off Swift's cooperative executor
let stream: [UInt8] = await withCheckedContinuation { continuation in
DispatchQueue.global(qos: .utility).async {
continuation.resume(returning: Self.drain(reader))
}
}Prompt for LLM
File Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:
Line 117:
Cooperative-executor blocking occurs because Self.drain makes synchronous reader.read calls until the stream ends while the first segment is deliberately still downloading. Run the drain on a GCD worker and bridge completion with a checked continuation to avoid occupying the async test's cooperative-executor thread.
**Also found in:**
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:281-281`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:143-143`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:206-206`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:308-308`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:95-95`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:87-87`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:147-147`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:202-202`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:276-276`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:303-303`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:172-172`
- `Tests/AetherEngineTests/Issue732PersistentHTTP1Tests.swift:177-177`
Suggested Code:
let stream: [UInt8] = await withCheckedContinuation { continuation in
DispatchQueue.global(qos: .utility).async {
continuation.resume(returning: Self.drain(reader))
}
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. Test-only. This suite passed in the fork's runs of both test groups for this branch.
There was a problem hiding this comment.
Understood—keeping this unchanged is reasonable for an upstream-sync PR. I won’t press for a fork-only change here.
The passing test runs don’t eliminate the potential cooperative-executor blocking, but this is a test-only concern better addressed upstream rather than as a requirement for this merge.
| canHardwareDecode: true, | ||
| dvProfile: 7 | ||
| ) | ||
| #expect(decoder is DualLayerVideoDecoder) |
There was a problem hiding this comment.
False test failure occurs when the opted-in decoder test unconditionally requires DualLayerVideoDecoder despite selectVideoDecoder intentionally falling back to HardwareVideoDecoder if DoviFelComposer cannot initialize, including trait-enabled builds without a Metal device or command queue. Gate the composition-specific assertion on composer availability and test the fallback separately.
Prompt for LLM
File Tests/AetherEngineTests/SoftwareDecoderSelectionTests.swift:
Line 38:
False test failure occurs when the opted-in decoder test unconditionally requires DualLayerVideoDecoder despite selectVideoDecoder intentionally falling back to HardwareVideoDecoder if DoviFelComposer cannot initialize, including trait-enabled builds without a Metal device or command queue. Gate the composition-specific assertion on composer availability and test the fallback separately.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This line comes unchanged from upstream 7.40.1; this PR only merges upstream, so it stays as upstream has it to keep the fork in step. Test-only, and it runs only with the DualLayerDolbyVision trait.
There was a problem hiding this comment.
Understood—keeping this unchanged is reasonable for an upstream-sync PR. The potential false failure is limited to DualLayerDolbyVision-enabled tests where the composer cannot initialize; it does not affect default-trait builds or production behavior.
I’d treat this as a non-blocking upstream follow-up, not a required change for this merge.
13 participants
Summary
Brings the Silo fork to upstream AetherEngine 7.40.1, keeping every Silo patch. It builds on #22 (the 7.38.1 merge, which had not been built or tested): this branch contains #22's merge commit, so merging this PR with a merge commit also lands #22.
Upstream 7.39.0 moved TrueHD Atmos and DTS:X object rendering (the APAC path, with LibTrueHD and LibDTSX), Profile 7 FEL composition and Dolby Vision Profile 5 / 10.0 still reshaping behind package traits that are off by default, because those components may need third-party patent licenses. The fork keeps upstream's defaults, so a host that sets no traits builds without them.
What changed
Upstream changes that matter to Silo:
TrueHDObjectAudio/DTSXObjectAudio,objectAudioRenderingkeeps the channel bridge and SwiftPM resolves neither LibTrueHD nor LibDTSX. XcodeGen cannot set traits, so silo-apple ships without them.DolbyVisionStillReshaping. Without it,scrubThumbnailandscrubPreviewFramereturn nil for those sources andsupportsCacheBackedStillsis false. Hosts fall back to their own previews.updateRequestHeaders(_:)replaces a running session's headers, andsourceAuthorizationRefusedpublishes the first refusal of a header set. [bug] Direct play keeps sending an expired access token after the app refreshes it, so playback stalls about an hour into a session silo-apple#833 uses both for direct play. This covers what fix(avio): refresh direct-play credentials on every range request #12 set out to do for direct play, by a different mechanism.Merge conflicts, resolved by keeping both sides:
ci.yml: upstream's 50-minute timeout and the fork's separate authorization-suite run.HLSOriginRelay: both sides added the http/https origin check; upstream's form is kept.PlayerState.bridgedand itsdocs/api.mdrow: upstream's trait wording with the fork's correction that only FLAC keeps the bed lossless.docs/api.mdobjectAudioRenderingrow: upstream's text plus theObjectAudioRenderingreferencePublicAPIDocumentationTestsrequires.docs/cli.md: upstream's text with the fork'sapac.31.02level for 8 channels.docs/formats.md: upstream's text with an unbalanced parenthesis fixed.DocumentedConstantsTests: upstream's trait gate, plus the fork's 8- and 10-channel APACCODECSchecks.Test plan
swift buildpasses with the default traits and with all four shipping traits.Evidence: https://evidence.siloserver.org/r/aetherengine/sync-upstream-7.40.1/
Checklist
CHANGELOG.mdupdated (upstream's entries; the fork's stay under Unreleased)AI disclosure
claude-opus-5-5🤖 Generated with Claude Code
Note
Merge upstream AetherEngine 7.40.1 into the Silo fork
Brings the upstream 7.40.1 changes into the fork in one merge. The largest areas of change:
DualLayerVideoDecoder,DoviFelComposer(Metal composition),DoviRpuConverter.separateDualLayers,DoviRpuGuardinput validation, and picture alignment/restamping — all gated behind the new opt-inDualLayerDolbyVisionpackage trait, with a newpreferDualLayerDolbyVisionload option that defaults to false.updateRequestHeadersAPI andRequestHeaderStore, including HTTP 401/403 credential-refusal detection, retry accounting, and up to three bounded revives after renewal.BedSplitter, DTS:X standard-presentation-only decoding,TrueHDMajorSync.isAtmosdetection), H.264 Matroska CFR timestamp reconstruction, in-band HEVC parameter-set completion, DVD VIDEO_TS folder playback, FLAC block-size repair, resident-range reporting for software VOD, and the Dolby Vision pulse probe tooling.preferDualLayerDolbyVisionrouting gates in VideoRoutingPolicy.swift and SoftwarePlaybackHost.swift, the removed 45-second resource timeout in the persistent transport, credential-refusal reclassification of software failures, and the trait defaults turning all optional components off for existing consumers.Macroscope summarized 898946d.