Publish the display size the software path settled on - #355
Merged
Conversation
…one's softwareCancellables.removeAll() stood in the middle of loadSoftware's wiring, between two groups of .store(in:) calls, so every sink attached above it was cancelled the moment it ran. What was above it is the SW-PiP cue mirror: after the frame compositor is armed, the CombineLatest over subtitleCues + secondarySubtitleCues never delivered again, and the only cues the compositor ever saw were the snapshot pushed by the pictureInPictureActive didSet. Subtitles in a software-path PiP window therefore froze at whatever was on screen when PiP started. Hoisted above the wiring, where a reset of the outgoing session belongs. The teardown in stopInternal already covers the ordinary path; this one covers a load that reaches here without one. Nothing catches this in unit tests, because both halves work in isolation: the publisher publishes, the sink is correct, and the cancel is invisible from either end. It surfaced while measuring a new sink on the same path with aetherctl play, which is where a session runs long enough to notice that nothing arrives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HX5zV3Fcf7Nzq4DYGdXvQE
A host laying something out over the picture (a libass overlay, the case #311 gave a clock and frame boundaries to) needs the rectangle the frames land in. The only public size was sourceVideoWidth / sourceVideoHeight, which are the CODED dimensions, so anamorphic content was laid out against the wrong rectangle: 720x576 at 64:45 presents as 1024x576, and an overlay sized 5:4 sits inside a 16:9 picture. There is nothing to measure on the layer either, AVSampleBufferDisplayLayer has no videoRect. A host cannot compute it. The ratio is resolved per frame across three sources, first sane wins (#177), and one whose display aspect is impossible is dropped in favour of square pixels (#290), so a host reconstructing it from container metadata disagrees with the screen in exactly the cases that policy exists for. softwareDisplaySize is therefore read off the format description the renderer enqueues, via CMVideoFormatDescriptionGetPresentationDimensions, rather than recomputed from the SAR: that description is the object the layer is handed, and the cache key it lives behind already invalidates on a dimension or PAR change, which is exactly when the size has to be republished. Optional rather than .zero, the argument that shaped SoftwareVideoFrameTime in #311: there is no display size before the first frame is built, and a placeholder in a non-optional field is indistinguishable from a measurement at the call site. Mirrored rather than latched, unlike hasFirstFrameReadyForDisplay: a live source that switches resolution re-shapes the rectangle under a host that already laid out against it. Cleared with the session, so the next source is not laid out against this one's picture. Measured with aetherctl play --sw --frame-times, which now reports disp=WxH per tick and the settled size next to the coded one: 720x576 SAR 64:45 -> disp=1024x576 (coded 720x576) 1280x720 square -> disp=1280x720 (coded 1280x720) Re #353 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HX5zV3Fcf7Nzq4DYGdXvQE
Anamorphic HEVC on the software host reached the layer at its coded dimensions: 720x576 declaring 64:45 presented as 720x576, a 16:9 picture squashed into 5:4. HardwareVideoDecoder attached no pixel aspect ratio, and the renderer builds its format description from the delivered CVPixelBuffer, so a ratio that is not an attachment on that buffer never reaches the layer at all. The libavcodec decoder on the same host has attached it since #177. The two are chosen per codec inside one host, so the defect was one decoder wide and invisible from every source that takes the other one. Resolved once at open() rather than per frame, because VT delivers pixel buffers and not AVFrames: the per-frame source SoftwareVideoDecoder prefers does not exist here, which also makes the #177 latch unnecessary since one resolution cannot oscillate. Both declared sources are read, since only one of them is the container's, and both existing gates run unchanged: the #177 component bound and the #290 display aspect. Square pixels attach nothing, and a stale attachment on a recycled pool buffer is removed, mirroring the libavcodec path. Measured with aetherctl play --sw --frame-times, one synthetic clip encoded four ways, reading the settled size published by #353: 720x576 SAR 64:45 hevc (VT) 720x576 -> 1024x576 fixed 720x576 SAR 64:45 h264 (libavcodec) 1024x576 unchanged 1280x720 square hevc (VT) 1280x720 unchanged 1280x720 square h264 (libavcodec) 1280x720 unchanged Reached in production by the interlaced detour (field order or an SPS that indicates interlacing) and by forward-only sources. Broadcast SD is where interlacing and a non-square SAR occur together. Re #354 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HX5zV3Fcf7Nzq4DYGdXvQE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Raised by @edde746 while adopting #311: with the clock and the frame boundaries in place, an overlay still had no rectangle to draw into.
What it adds
AetherEngine.softwareDisplaySize(CGSize?,@Published): the size the software path's picture presents at, the coded frame under the pixel aspect ratio the decoder attached.The only public size was
sourceVideoWidth/sourceVideoHeight, which are the CODED dimensions, so anamorphic content was laid out against the wrong rectangle (720x576 at 64:45 presents as 1024x576), andAVSampleBufferDisplayLayercarries novideoRectto measure instead. A host cannot compute it either: the ratio is resolved per frame across three sources (#177) and one whose display aspect is impossible is dropped in favour of square pixels (#290), so a reconstruction from container metadata disagrees with the screen in exactly the cases that policy exists for.It is read off the format description the renderer enqueues, via
CMVideoFormatDescriptionGetPresentationDimensions, rather than recomputed from the SAR: that description is the object the layer is handed, and the cache key it lives behind already invalidates on a dimension or PAR change, which is exactly when the value must be republished.Optional rather than
.zero(the argument that shapedSoftwareVideoFrameTimein #311), mirrored rather than latched (a live source can re-shape the picture mid-stream), cleared with the session.Two defects found while measuring it
The software load path cancelled the sinks it had just wired.
softwareCancellables.removeAll()stood between two groups of.store(in:)calls, so everything attached above it died on arrival. What was above it is the SW-PiP cue mirror: subtitles in a software-path PiP window froze at whatever was on screen when PiP started. No unit test can see this, both halves of the wiring work in isolation; it took a session that runs long enough to notice nothing arrives.Anamorphic HEVC rendered at coded dimensions (#354). The VT-backed decoder attached no pixel aspect ratio at all, and the renderer builds its format description from the delivered pixel buffer, so nothing carried the ratio to the layer. The libavcodec decoder on the same host has attached it since #177, so the gap was one decoder wide. Same resolution order, same two gates, applied on the decoder that was missing them.
Measured
aetherctl play --sw --frame-times, which now reportsdisp=WxHper tick and the settled size next to the coded one. One synthetic clip, encoded four ways:Full suite green: 1677 tests, 245 suites.
Re #353
Re #354