Repository navigation
fix(engine): portrait and rotated videos export upright - #215
Merged
Merged
Conversation
A camera stores a portrait recording as landscape pixels plus a preferredTransform, and the front camera may add a mirror. A layer instruction's transform replaces that tag, and kadr never applied it, so every portrait iPhone clip exported and previewed sideways. The orientation of every source is now read from the composition's own segments once the clips are placed, and each clip is turned upright before it is scaled to fill. That covers plain chains, transitions (slide included), Tracks and free-floating clips, clip transforms and keyframes, and the preview player. Clips with different orientations on one track each keep their own. ReverseProcessor now copies the source's transform to the reversed file instead of dropping it. Transforms on a layer are collected and emitted once in time order, so the base layout and per-clip keyframes can't overwrite each other by call order. That also fixes a plain clip inheriting the previous clip's transform on the keyframe path.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Problem
Every portrait iPhone clip exported and previewed sideways, stretched to fill the frame.
A camera stores a portrait recording as landscape pixels plus a
preferredTransform(the front camera may add a mirror). A layer instruction's transform replaces that tag, and kadr never read it:grep preferredTransform Sources/had zero hits. This is the most-reported AVFoundation bug, and a first-time user hits it within minutes.Fix
OrientationSchedule(new). After the clips are placed, it reads each source'spreferredTransformandnaturalSizefrom the composition's own segments. Every build path gets it the same way, and lookups are by time, because a portrait clip and a landscape clip can share one composition track.PlaybackComposer(preview and thumbnails)nilvideoComposition when every source is upright, so nothing changes there. A rotated source gets an oriented one, so an export that can't apply it now refuses instead of writing a sideways copy (same rule as Silent passthrough fallback drops entire videoComposition for no-audio compositions (empty audio track fails HEVC compatibility check) #201).ReverseProcessorcopies the source transform onto the reversed file.LayerTransformTimeline. Base layout and per-clip keyframes are collected and emitted once, in time order, so neither overwrites the other by call order.Tests
VideoOrientationTestshas 13 tests. Fixtures are 128×64 four-colour clips tagged with quarter, three-quarter, half and mirrored transforms (iPhone-style, and about the origin). The ground truth isAVAssetImageGeneratorwithappliesPreferredTrackTransform.Cases covered:
Mutation check: run against the old engine, every rotated case fails, and the upright control and the new transform-leak test behave as expected.
Full suite: 695 swift-testing + 49 XCTest pass; the generic iOS build succeeds.
Not in this PR (filed)
MultiInputCompositorstill receives raw, unrotated source frames (it gets unscaled frames by design today).VideoClip.metadata.resolutionreports the stored size, not the display size.