Stop rewriting H264 profile-level-id to 42e032#1006
Draft
pblazej wants to merge 2 commits into
Draft
Conversation
The encoder/decoder/simulcast factories were collapsing every H264 codec entry to a single ConstrainedBaseline Level 5.0 (profile-level-id=42e032). On iOS 26.3 VideoToolbox refuses hardware acceleration when only ConstrainedBaseline is on offer, dropping H264 simulcast publishers onto the software encoder and saturating ~5 cores (see #1002). The workaround was originally added for macOS in #118 and extended to iOS in #147 to fix 1080p simulcast failing on iOS — at the time upstream WebRTC pinned ConstrainedHigh at Level 3.1, too low for 1080p sources. Modern WebRTC consults a per-device profile table (UIDevice+H264Profile.mm) and publishes ConstrainedHigh at the device's actual max level (5.2 on iPhone 13 Pro+), so the override is now actively harmful. Drop the rewrite, keep the factory subclasses as @unchecked Sendable shims, and add a regression test that asserts both ConstrainedHigh (640c…) and ConstrainedBaseline (42e0…) profiles are exposed with a level ≥ 3.1 floor. Fixes #1002. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`set(preferredVideoCodec:)` used `first { name == codec.name }` which was
fine when each codec name had exactly one capability entry. With the H264
factory rewrite removed (previous commit), upstream WebRTC exposes two
H264 entries — ConstrainedHigh and ConstrainedBaseline — and the
single-pick logic dropped the second one, leaving only ConstrainedHigh
in the SDP offer. LiveKit Cloud's H264 negotiation rejected that and
answered with no H264 at all, falling the publisher back to VP8.
Filter all capabilities matching the preferred codec name and keep them
together at the front of `setCodecPreferences`, mirroring the upstream
intent (and Android's sort-based approach).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
18c200e to
88a81f4
Compare
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
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.
Removes the
rewriteCodecsIfNeeded()mutation inRTC.swiftthat forced every H264 entry toprofile-level-id=42e032. The override was added in #147 to work around upstream WebRTC pinning ConstrainedHigh at Level 3.1 (issue #144); modern WebRTC now publishes ConstrainedHigh at the device's actual max viaUIDevice+H264Profile.mm, making the rewrite obsolete. On iOS 26 the rewrite is also actively harmful — combined withEnableLowLatencyRateControl, an explicit Baseline ProfileLevel disables HW acceleration in VideoToolbox (#1002).Companion upstream patch in
livekit/webrtc(branchblaze/h264-accel) gatesEnableLowLatencyRateControlon the High-profile family so the dual-profile advertisement enabled here works end-to-end: ConstrainedHigh negotiations take the LL HW path; ConstrainedBaseline takes the plain HW path.Fixes #1002. Related: #144, #147, #118.