Repository navigation
Conversation
nburns
force-pushed
the
fec-interleaving
branch
2 times, most recently
from
September 22, 2026 18:34
51bdaea to
3a63206
Compare
nburns
marked this pull request as draft
September 22, 2026 23:54
A parity group covered a run of consecutive packets, so any two adjacent losses fell in the same group and one XOR parity could not recover them. Adjacent loss is the normal case on Wi-Fi, where loss arrives in bursts rather than uniformly, which left FEC recovering little of what actually goes wrong. Interleaving assigns group g the packets g, g+groupCount, g+2*groupCount, ... so neighbouring packets belong to different groups. The same single parity and the same overhead then recover any burst up to groupCount packets long: burst tolerance becomes the group count instead of one, for no extra bandwidth. Modelled at a 1% burst-start rate on a 250-packet frame, whole-frame recovery goes from 0.108 to 0.752 for two-packet bursts and 0.081 to 0.649 for three-packet bursts. Doubling parity instead, without interleaving, only reaches 0.135 on the three-packet case for twice the overhead, so the layout is what matters here rather than the parity count. The layout is negotiated through CLIENT_CAPABILITY_FEC_INTERLEAVED and SERVER_FEATURE_FEC_INTERLEAVED rather than assumed from a version. Every client implements the grouping independently, and a receiver using a different layout than the sender XORs a packet out of the wrong group, handing plausible garbage to the decoder instead of failing cleanly. Clients that do not advertise the capability keep the contiguous layout exactly. Interleaved parity is emitted after a frame's data packets, because an interleaved group is not complete until the frame is. tests/TestProtocolFec.cpp covers this at the protocol level rather than inside any one client: both layouts partition the frame exactly once, no group exceeds FEC_GROUP_SIZE (receivers gather groups into fixed-size arrays), interleaving puts every pair of neighbours in different groups, and a burst the contiguous layout loses is recovered up to the group count and no further. The android-vr client opts in. The shared Swift package gains the mirrored layout and a flag, but stays on the contiguous layout: turning it on needs a visionOS build to verify, which this change cannot do.
nburns
force-pushed
the
fec-interleaving
branch
from
September 23, 2026 16:21
3a63206 to
72367f8
Compare
Review fixes for the interleaved FEC layout, plus receiver-level tests. Correctness and robustness: - Attempt recovery whenever the parity held could cover everything missing, instead of only when a single packet is missing. The old gate deferred every burst recovery -- the case interleaving exists for -- to the next frame's first packet, delivering the frame one interval late; that flush path also discarded the triggering packet, seeding the next frame with a self-inflicted loss. Fixed in the Android and Swift receivers alike. - Match parity packets by totalPackets as well as frameIndex. One frame's NALs share a frameIndex and each numbers its packets and groups from zero, so a reordered parity packet from a small NAL (SPS/PPS) could occupy the next NAL's parity slot and XOR-recover garbage. A data packet with a different totalPackets under the same frameIndex likewise starts a new reassembly now. - Keep the inline parity schedule for contiguous connections: a contiguous group is complete mid-frame, and emitting all parity as one tail burst both exposed it to a single burst-loss event and changed behaviour for clients that never opted into interleaving. Only interleaved connections pay the end-of-frame cost. Both paths share one parity-send helper using fixed arrays. - Renumber CLIENT_CAPABILITY_FEC_INTERLEAVED to 0x00001000: 0x00000800 is claimed by CLIENT_CAPABILITY_FOVEATION_CENTER in the gaze foveation PR, and the collision would negotiate each feature on for clients advertising the other. - Latch fecInterleaved_ once at SendNalUnit entry alongside the client snapshot, hoist the negotiation store out of the codec retry loop in both connect handlers, and reset it on disconnect and server start. - Make the Android receiver's flag atomic and route the Swift one through the state lock; both receivers latch the layout per frame so a mid-frame flip can never mix layouts within one recovery. - Guard GroupLayout::GroupOf against zero-packet layouts (division by zero) in C++ and Swift, add the members == 0 skip the Swift recovery already had, and delete the dead contiguous-only fec::GroupRange. - Move the Swift capability/feature bits into ClientCapabilityFlags and ServerFeatureFlags, where clients actually build ClientConnect from. Tests: - Extract the frame reassembly and FEC recovery state machine from NetworkReceiver into a shared, header-only VideoFrameAssembler under common/streaming, so the receiver half of the FEC contract exists and is tested exactly once (in oxrsys_runtime_tests, on macOS) instead of being forked per UDP client. TestVideoFrameAssembler.cpp covers: burst recovery on parity arrival with no next-frame packet needed, the unrecoverable-burst NACK path without losing the next frame's trigger packet, cross-NAL parity rejection, inline contiguous recovery, short-last-packet size restoration, same-frameIndex NAL flushing, and duplicate data/parity handling. - Pin SERVER_FEATURE_FEC_INTERLEAVED and CLIENT_CAPABILITY_FEC_INTERLEAVED values in TestProtocolLayout.cpp and the Swift ProtocolLayoutTests, and mirror the C++ GroupLayout property tests (exactly-once partition, member cap, neighbour separation, empty-layout inertness) in the Swift package so the two implementations cannot drift. - Document the emission schedules and the totalPackets matching rule in docs/protocol.md, and move the FEC section out of the middle of the video-flag bullet list.
nburns
force-pushed
the
fec-interleaving
branch
from
September 23, 2026 18:35
72367f8 to
54091af
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
2 participants
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.
A parity group covers a run of consecutive packets, so two adjacent losses fall
in the same group and one XOR parity cannot recover them. Wi-Fi loss is bursty,
so FEC currently recovers little of what actually goes wrong.
Interleaving assigns group
gthe packetsg, g+groupCount, g+2*groupCount, ...,putting neighbouring packets in different groups. Same parity, same overhead;
burst tolerance becomes the group count instead of one. Modelled whole-frame
recovery at a 1% burst-start rate, 250-packet frame:
Doubling parity without interleaving barely helps against bursts; the layout is
what matters, not the parity count.
The layout is negotiated (
CLIENT_CAPABILITY_FEC_INTERLEAVED+SERVER_FEATURE_FEC_INTERLEAVED) rather than versioned: a receiver using adifferent layout than the sender XORs packets out of the wrong group and hands
plausible garbage to the decoder instead of failing cleanly. Clients that do not
advertise it keep the contiguous layout exactly. Interleaved parity is emitted
after the frame's data packets, since a group is not complete until the frame is.
android-vr opts in. The shared Swift package gains the mirrored layout but stays
contiguous: switching it on needs a visionOS build to verify, so that's a
follow-up and the plumbing is in place.
Tested in the new
tests/TestProtocolFec.cppat the protocol level (each clientreimplements the grouping): both layouts partition every frame size exactly
once, no group exceeds
FEC_GROUP_SIZE(receivers gather into a fixed-sizearray), neighbours always land in different groups, and bursts up to the group
count recover while one beyond it does not.
oxrsys_runtime_tests,swift build, andswift testpass on macOS arm64.