Skip to content

perf(dovi): release FEL base frames by display deadline and compose in the base-layer buffer - #913

Open
pkajaba wants to merge 4 commits into
superuser404notfound:mainfrom
pkajaba:fel/bundle-pr
Open

pkajaba wants to merge 4 commits into
superuser404notfound:mainfrom
pkajaba:fel/bundle-pr

Conversation

@pkajaba

@pkajaba pkajaba commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Everything FEL-specific is behind the opt-in DualLayerDolbyVision trait (off by default). With the trait off the build changes only in CHANGELOG.md, docs/dolby-vision-fel.md, a renderer counter and its log field that nothing reaches (see below), an unreachable el_type probe change, and a log helper that only fires on the dual-layer route. Open as a draft: the Mac side is tested, the Apple TV checks are listed at the end.

What changes

  • Deadline release. A base frame whose enhancement layer has not arrived is released as its base layer once its presentation time is within a margin of maxFramesInFlight frame intervals (at least 100 ms), after a short grace (one frame interval, 15 to 50 ms, none past the presentation time) for a layer that is only a moment late. Before, it waited for three newer base frames, so a damaged or missing enhancement-layer stretch held the picture and dropped frames in bursts. The host hands the decoder its render clock; a paused or prerolling clock holds the frames (bounded at 2 s, or the margin for a preroll), and a count of 7 newer frames stays as a backstop below the stall guard's 8. An access unit with no enhancement-layer NAL releases its base frame at once. The sweep re-arms on every arrival. The 1 Hz summary line splits the bypasses into six counters by the rule that released them.
  • Compose in place. When the decoded base-layer buffer is IOSurface-backed, 10-bit biplanar video range, even-sized and the luma curve is not MMR, the chroma pass and then the luma pass write into that buffer, so the output pool is not allocated for those frames. A buffer is never composed twice at once; a command buffer that fails after it started drops the frame rather than showing a mix of layers. Anything else takes the pool path unchanged. AETHER_FEL_INPLACE=0 turns it off for an A/B run.
  • Chroma sampling switch (experimental, off by default). AETHER_FEL_CHROMA picks the chroma sampling of the composed frame: 4:2:0 (the default, and the composition itself), 4:2:2 or 4:4:4. The composition is defined on the 4:2:0 sites; 4:2:2 and 4:4:4 step it up by co-sited means (a copy on a site, the mean of the sites around a sample between them). 4:4:4 has no reconstruction in the spec and is marked experimental. The two experimental formats take the pool path, since in-place composition is 4:2:0 only, and their step-up kernels recompute the 4:2:0 sites (about 4 times and 16 times the chroma work; a later change can compose the sites once). Nothing changes unless the variable is set.
  • Bypassed frames keep their mapping. A frame released by its deadline, bypassed (missing layer or RPU, no layer in the access unit, end of stream, single layer) or left as its base layer by a failed composition used to carry the composed-frame RPU, whose curves are set to no-op because the composer is meant to have reshaped the picture. Nothing had, so the display showed an unreshaped frame between composed ones. Those frames now carry the Profile 8.1 RPU with the source's mapping and display metadata, built next to the composed form (mode 1 then mode 2 for a FEL RPU; mode 4 rewrites the luma curve) and carried beside it through the side-data ring. libdovi's mode 2 sets the curves to no-op only when the RPU's own el_type is FEL, and that comes from the NLQ block, which is absent when disable_residual_flag is set, so the composed form also has its mapping removed after mode 2 (dovi_rpu_remove_mapping, dual-layer route only; the remux route is unchanged). Cost: a second libdovi parse and conversion per RPU on the dual-layer route and one more NAL per side-data entry. When the mapping-keeping form cannot be built, the frame carries the composed form, which on the dual-layer route now always has its mapping removed, so that practically unreachable fallback gives an unreshaped frame (the second parse reads the same bytes through the same guard).
  • Composer fixes. The chroma planes round up (texture binds, dispatch, kernel grid), so the last chroma column and row of an odd luma size is written and a 1-sample-wide enhancement layer no longer divides by a zero chroma size. A vdr_in_max that shifts down to 0 drops the residual instead of clamping it, and the prediction is clamped in float before the integer conversion (an MMR result far outside the range overflowed int). An enhancement slot with no base time pairs by access unit when the base layer has never reordered; a reordering base layer keeps extrapolating.
  • el_type probe. It rewinds with the same 5 s bounded seek as the interlace probe; a stalled rewind leaves the type unknown, so the route is not taken.
  • Arrival-order skips. The arrival-order path counts the frames its seek threshold discards (skip= in the cadence line, apart from lost=) and logs the first once per seek. FrameSideDataRing documents its 90 kHz keys (with a test).
  • Logging. The producer logs once per session that an E-AC-3 track on the dual-layer route is decoded to PCM (the Atmos objects are lost); the dual-layer decoder logs session totals (composed, deadline, held-wall and backstop releases) at info level, at most every 10 s and only when a count moved, so a device run can be judged from the host's log without a verbose trace.
  • Tests. A real-clip test (skipped without the clip directory) checks that every delivered frame carries one of its own packet's converted RPU forms: the composed form for a composed frame, the mapping-preserving form for a bypassed one. The heavy dual-layer suites sit under one serialized parent, so the trait-on run in parallel mode no longer reports time-limit issues caused by suites queueing for the one-lane trait while the real-clip decodes hold it.

Not trait-gated, and why it is safe: the renderer's skip counter and the skip= log field sit in the generic arrival-order code; they are reached only when setArrivalOrderHandover gets a frame rate, which the host passes only for the dual-layer decoder, so without the trait the counter stays 0. The el_type probe change is unreachable without the trait (DualLayerRouteDecision answers .notBuiltIn before the probe). The E-AC-3 log helper returns nil unless the dual-layer route is active.

Credit: the mapping-keeping RPU for a raw base layer follows TheFireKahuna/AetherEngine@d3a1531c ("compose base-only when the enhancement layer is missing; a raw base layer keeps its mapping") and TheFireKahuna/AetherEngine@c8910a0e ("strip the mapping from every composed frame's RPU, not only those libdovi reads as FEL"). From the first only the RPU half is taken (a raw base layer carries the mapping-keeping RPU; the frame is still shown as its base layer, not composed base-only); the second is the same fix as the mapping removal after mode 2 here. Thanks @TheFireKahuna.

Overlap with open PRs: #902 and #867 touch SoftwarePlaybackHost.swift and merge cleanly with this branch; #703 touches SoftwarePlaybackHost.swift and SampleBufferRenderer.swift but already conflicts with main in 34 files. #810 will need a CHANGELOG.md merge only.

Tested on Mac

On the branch tip, in build slots, every build and test line behind the soak guard:

  • swift build --build-tests, trait off and trait on (--traits DualLayerDolbyVision): both build clean.
  • Test-fix round (2026-10-26): DualLayerDeadlineBypass and DualLayerBypassRpu (31 tests in 2 suites) passed three times with the trait on, and the build is clean with the trait on and off; the two suites now carry a two-minute time limit.
  • Trait on, the covering filters (DoviGpuEdgeCase, DoviFelInPlaceCompose, DoviFelMath, DualLayerPictureAlignment, DualLayerDeadlineBypass, DualLayerBypassRpu, DoviComposedFrameRpu, DualLayerVideoDecoder, FrameSideDataRing, PresentationFloor, ArrivalOrderHandover, SampleBufferRendererLookahead, DualLayerEdgeCase, DoviCpuModel, SoftwareAtmosRoute, OrderedDelivery, DocumentedConstants): 246 Swift Testing tests in 18 suites passed, XCTest 25 passed.
  • Trait off, the filters that exist without the trait: 49 Swift Testing tests in 4 suites and 23 XCTest passed.
  • Real clips, trait on, serialized (--no-parallel --filter DoviFelRealFileTests, DV_FEL_TESTS_DIR set): 12 of 12 passed (411 s on a loaded Mac).
  • swift build --enable-all-traits --build-tests: clean. 14 covering suites with all traits: 161 tests passed. The Probes/AllTraits tvOS device xcodebuild: BUILD SUCCEEDED.
  • Flush totals: a unit test pins that a flush() folds the interval in progress into the session totals before the per-second counters are zeroed (it passed with the DualLayerVideoDecoder, deadline-bypass, in-place compose and related suites: 130 tests in 7 suites, trait on).
  • swift test, trait off: Swift Testing 5074 tests in 698 suites passed, XCTest 798 (1 skipped), 0 failures.
  • swift test --traits DualLayerDolbyVision: Swift Testing 5378 tests in 727 suites passed, XCTest 800 (1 skipped), 0 failures; no time-limit issues in parallel mode. The count includes the chroma sampling switch (the DoviChromaFormatTests suite, 10 tests, with its 4:2:0 bit-exactness, 4:2:2 and 4:4:4 mean checks, odd sizes and in-place refusal).
  • Strict pool (LIBDISPATCH_COOPERATIVE_POOL_STRICT=1 swift test --disable-xctest), trait off 5074 passed, trait on 5378 passed. One earlier trait-on strict run had failures in three replay and TLS suites that pass when run alone; the full run passed on the rerun.
  • tvOS Simulator and iOS Simulator build-for-testing (trait off): TEST BUILD SUCCEEDED, both. With the trait off the objects of the composer, decoder, side-data ring and picture-alignment files are empty.
  • Earlier Mac numbers for the in-place path (from the commits this squashes): the decoder-writability probe (writing into a delivered decoder buffer leaves the later decoded frames unchanged) passed on 300 of 300 frames, on the Mac and on the A12; the in-place against pool spot check on the real decoder covered 18 + 1 frames, and the rest rests on the unit tests; 4K, 200 s, 7 seeks, two runs each: our process about 300 MB lower, the decoder service about 170 MB higher, net about 150 MB lower; drops unchanged.

Still needs Apple TV

  • In-place path, memory and drops on the A12, on a quiet Mac host with three runs per arm for medians. The 2026-10-24 drops campaign gave arm C (in place) 2 to 3 drops per run (the same as before), 0 compose failures and a 110 to 240 MB lower peak, and the CPU-write probe passed on the A12; the medians and a picture A/B remain open. The earlier A/B/C soak ran under heavy Mac load (63 / 39 / 157 drops, 700 / 780 / 619 MB peak for base / malloc / in-place), so it is not comparable. If the in-place arm shows more drops than the base arm, the in-place part comes out of this PR.
  • A12 picture check, in place against the pool path. The CPU-write probe passed on the A12, but the GPU write into decoder surfaces was checked only on the Mac. Visual A/B with AETHER_FEL_INPLACE=0 and 1 (colour and HDR mode switch, picture match), plus inPlaceDropped and composeFailures in the summary line.
  • 4:2:2 and 4:4:4 (experimental, AETHER_FEL_CHROMA): the picture on the TV against the 4:2:0 default, and the chroma-location label the output carries (copied from the base layer; dump one 4:2:2 frame's attachments on the Mac first). Not needed for the 4:2:0 default.
  • Deadline release on a title with a damaged or reordered enhancement layer: the picture stays right (EL visible), paused seeks, a pause longer than 2 s, 60 fps and a rate other than 1, and screen lock. The totals: lines of the host log give the numbers (delta of the deadline, held-wall and backstop fields).
  • Peak memory during an enhancement-layer-loss stretch. With a render clock up to 7 base frames can wait (about 175 MB at 4K); the earlier TV run measured drops only.
  • FEL gate arms the maintainer named for fix(dovi): answer the review notes on the FEL stack (probe seek and byte budget, skip count, pairing, composer edge cases, tooling, test traits) #807: the enhancement-layer pulse is visible on the film and the base- and enhancement-layer counters agree; a FEL title playing through a seek shows skip= in the cadence line and no change in drops.
  • Optional: the per-packet CPU cost of the second RPU parse on the A12.
  • Earlier TV evidence for the deadline release, on an earlier build: with the enhancement layer removed for 60 s of a clip, 1 drop against 45 and 71 on main; a 14-seek soak of 359 s had 3 drops and 0 compose failures. Debug build, so not strictly comparable with Release baselines.
  • The odd-size composer path is not exercised by any shipping title (UHD sizes are even); it is covered on the Mac only.

…n the base-layer buffer

Everything FEL-specific is behind the DualLayerDolbyVision trait.

Deadline release. A base frame whose enhancement layer has not arrived is released as its base layer once its presentation time is within a margin of maxFramesInFlight frame intervals (at least 100 ms), after a short grace for a layer that is only a moment late. Before, it waited for three newer base frames, so a damaged or missing enhancement-layer stretch held the picture and dropped frames in bursts. A paused or prerolling clock holds the frames (bounded at 2 s) and a count of 7 newer frames stays as a backstop.

Compose in place. When the decoded base-layer buffer is IOSurface-backed, 10-bit biplanar video range, even-sized and the luma curve is not MMR, the chroma and luma passes write into that buffer, so the output pool is not allocated for those frames. A buffer is never composed twice at once and a failed command buffer drops the frame. AETHER_FEL_INPLACE=0 turns it off.

Bypassed frames keep their mapping. A frame shown as its base layer carries the Profile 8.1 RPU with the source's mapping, built next to the composed form; the composed form has its mapping removed whatever the el_type. The raw-base-layer RPU half follows TheFireKahuna/AetherEngine@d3a1531c and @c8910a0e.

Composer fixes. Chroma planes round up for odd sizes, a zero NLQ ceiling drops the residual, the prediction is clamped in float, a passed enhancement slot pairs by access unit unless the base layer reorders, and the el_type probe rewinds with a bounded seek. The arrival-order path counts and logs the frames its seek threshold discards.

Logging. The E-AC-3 decoded-to-PCM route is logged once per session and the dual-layer decoder logs session totals of its releases at info level; a flush folds the second in progress into those totals first.

The heavy dual-layer test suites run one at a time under a serialized parent.
They are small synthetic suites with an injected clock, so they stay out of the heavy serialized group, but a loaded runner can pass 60 s.
The composed Dolby Vision frame is defined on the 4:2:0 chroma sites. AETHER_FEL_CHROMA=422 or 444
writes it as 4:2:2 or 4:4:4 instead: a site keeps its 4:2:0 composition, and a sample between two
sites is the mean of the clamped 4:2:0 samples around it, rounded once. 4:2:0 stays the default and
the output playback uses; any other value keeps it. 4:4:4 has no reconstruction in the spec and is
marked experimental. Both formats take the pool path, since in-place composition is 4:2:0 only.

The 4:2:0 composition moves into shared helpers with its arithmetic unchanged.
@pkajaba
pkajaba marked this pull request as ready for review October 26, 2026 23:36

@quint-aether quint-aether Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review

This PR releases a FEL base frame by its display deadline instead of waiting for three newer frames, composes into the decoded base-layer buffer, and attaches a Profile 8.1 RPU that keeps the mapping to frames shown as their raw base layer. It also adds an opt-in chroma sampling switch and several composer fixes. The direction is right: everything is fixed in the engine and kept behind the DualLayerDolbyVision trait. The PR gives a reason for each part that is not gated, and the reasons hold up (the renderer skip counter only runs on the arrival-order path, the el_type probe change cannot be reached without the trait, and the E-AC-3 note returns nil off the dual-layer route).

I checked the decoder's locking and epoch handling, the sweep timer's retirement on flush and close, OrderedDelivery.skip, the exclusive in-place claim, how the measurement run is excluded, the removeMapping scope (keepRpuBytes is false on the remux route), and how the render clock is read outside the decoder lock. I found nothing wrong there.

Findings

  • Should fix: docs/dolby-vision-fel.md describes skip= as frames the clock had already passed. The code counts frames below the seek threshold, which is also what the CHANGELOG says. (docs/dolby-vision-fel.md:640)
  • Should fix: the docs now quote several new figures: the 100 ms margin floor, the 15 to 50 ms grace, the 2 s held-clock bound, the backstop of 7, the 2 to 250 ms sweep and the 10 s totals interval. None of them is pinned by DocumentedConstantsTests, and the one existing pin was loosened to drop the "3". (Tests/AetherEngineTests/DocumentedConstantsTests.swift:92)
  • Question: in-place composition binds the same texture as access::read and as access::write in one dispatch. Metal's defined way to read and write one texture in one kernel is access::read_write. This is bit-identical on the Mac, but the GPU write into decoder surfaces has not been checked on the A12. (Sources/AetherEngine/Video/Metal/DoviFelComposer.swift:835)
  • Nit: summaryInPlaceDropped is not cleared in flush() or close(), unlike the other per-second counters. A drop counted just before a seek or a close shows up in the next summary.

Checklist: CHANGELOG yes, docs yes, tests yes, traits both ways yes (CI confirms both configurations)

Needs a device run:

  • In-place composition on the A12: drop medians against the pool path, a picture A/B with AETHER_FEL_INPLACE=0 and 1, and the GPU write into VideoToolbox surfaces.
  • The deadline release on a title with a damaged enhancement layer. Cover a pause longer than 2 s, a seek while paused, 60 fps, a rate other than 1, and peak memory with up to 7 waiting 4K frames.
  • 4:2:2 and 4:4:4 picture output and their chroma-location label.

The author lists all of these already.

Thanks for a large, carefully measured and well-documented change.

Automated review by Quint (Claude) of 101d864. It reads the code, it does not build or run it; a maintainer reviews and merges.

Comment thread docs/dolby-vision-fel.md
count of composed frames and of the base frames released by the deadline, the held-wall bound and the
backstop. It is sent at most every 10 s and only when one of the counts moved since the last one, so a
device run is judged from it. The software host's `[SWDiag]` line carries `skip=`, the number of frames
the renderer skipped on arrival because the clock had already passed them, next to `lost=` and `layerDrop=`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence says skip= counts frames "the renderer skipped on arrival because the clock had already passed them". That does not match the code. _arrivalSkippedFramesDropped is incremented only for frames below the seek threshold: arrivalSkipBelow in handOverOnArrival, and skipUntilPTS when arrivalFloor != nil. The CHANGELOG and the Cadence.arrivalSkipped doc comment both describe it that way. A reader of a device log would take a non-zero skip= during steady playback as late frames, which it cannot be. Suggest: "the number of frames the arrival-order path discarded below a seek's target (the start of the GOP the seek landed in), apart from lost=".


assertDocumented("once K = 3 newer base-layer", fel)
assertDocumented("is the maximum layer skew, 3;", fel)
assertDocumented("(`maxLayerSkewFrames`) is the maximum layer", fel)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pin used to fix K = 3 in the prose ("is the maximum layer skew, 3;"). It now matches only the symbol name, so the number can drift. The PR also adds several numbers to docs/dolby-vision-fel.md §5.5, §5.10 and §5.11 that are not pinned:

  • the 100 ms margin floor (deadlineMarginFloor)
  • the 15 to 50 ms grace (enhancementGraceFloor/Cap)
  • the 2 s held-clock bound (heldClockWallBound)
  • maxDeadlineHoldFrames = 7 and the stall threshold of 8
  • the 2 to 250 ms sweep bounds
  • the 10 s totals interval (totalsLogIntervalSeconds)
  • the 175 MB figure

Per CLAUDE.md, numbers quoted in docs are pinned here. Please add assertDocumented lines plus XCTAssertEqual on the constants, the same way the K = 3 and capacity-32 entries do. The sweep bounds are private static, so they need to become internal or be dropped from the doc.

let outYTex: MTLTexture
let outUvTex: MTLTexture
if inPlace {
outYTex = blYTex

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: in place, one MTLTexture is bound to texture(1) as texture2d<ushort, access::read> and to texture(3) as access::write in the chroma pass. The luma pass does the same with texture(0) and texture(3). Metal's documented way to read and write one texture inside a kernel is a single access::read_write argument. Aliasing two bindings with different access is not covered by that contract, even though each thread reads its own sample before writing it. It is bit-identical on the Mac. On the A12 only the CPU-write probe ran, so the GPU path into decoder surfaces is unverified there.

Two options:

  • Add read_write variants of the two kernels for the in-place dispatch. r16Uint/rg16Uint read-write needs tier 2 texture support, so check device.readWriteTextureSupport in inPlaceDecision.
  • Keep this approach and make the A12 picture A/B with AETHER_FEL_INPLACE=0/1 a hard gate before merge.

If you have a reference that says aliased read and write bindings are defined behaviour, a comment citing it would settle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant