fix(endpoint): route RTX/FEC repair packets to primary stream receiver by nightness · Pull Request #75 · webrtc-rs/rtc

nightness · 2026-04-01T12:50:36Z

Summary

Three bugs caused all RTX/FEC repair packets to be silently dropped:

Bug 1 — find_track_id_by_ssrc: outer SSRC search misses repair SSRCs
RTX/FEC SSRCs live in coding.rtx.ssrc / coding.fec.ssrc, not coding.ssrc. The outer transceiver search only checked coding.ssrc, so any packet with an RTX or FEC SSRC fell through to None and was dropped. Fix: extend the .any() predicate to also check rtx/fec SSRCs. When a match is found via repair SSRC, the packet is routed to the primary stream's receiver immediately without firing OnOpen events.

Bug 2 — find_track_id_by_rid (rrid branch): always returned None
The rrid header extension identifies a repair packet for a base stream identified by rid. The code correctly updated coding.rtx.ssrc = ssrc, but then fell through to None instead of returning the primary stream's track_id. Every RTX packet carrying rrid was dropped. Fix: add return Some(receiver.track().track_id().clone()) after the SSRC update.

Bug 3 — handle_undeclared_ssrc: RTX codec matched as primary
RTX codec entries are included in get_codec_preferences() (added during SDP negotiation). If an RTX packet arrived first in the single-section non-rid path, the codec lookup would find the RTX codec and incorrectly set it up as the primary track's codec. Fix: exclude RTX mime-type codecs from the lookup so only primary codecs are accepted.

Review feedback addressed

Redundant lookups: Refactored find_track_id_by_ssrc to use find_map with a single-pass SSRC classification (primary vs repair), eliminating duplicate receiver() and get_coding_parameters() calls on the hot RTP receive path.
Comment/predicate mismatch: Primary codec lookup now actually excludes repair codecs (RTX + FEC) via is_repair_mime_type helper, matching its comment.
RTX mime matching: Replaced exact MIME_TYPE_RTX (video/rtx) check with a general is_repair_mime_type() helper that covers all RTX variants (video/rtx, audio/rtx) and FEC types (ulpfec, flexfec, flexfec-03) case-insensitively.
Stats gap for rrid packets: Added register_rtx_ssrc() to RTCStatsAccumulator so late-discovered RTX SSRCs (via rrid header extension) are registered in the reverse lookup map, enabling on_rtx_packet_received_if_rtx() to track retransmission stats.
Regression tests: Added 10 unit tests: 3 for is_repair_mime_type + 7 integration-style tests covering RTX/FEC SSRC routing, no-OnOpen for repair SSRCs, rrid stats registration, and undeclared-SSRC RTX rejection.
Import order: Fixed {MediaEngine, MIME_TYPE_RTX} -> alphabetical order to pass cargo fmt.
Missing import: Added RTCRtpRtxParameters import (was causing compile error).

Test plan

cargo fmt --check — passes
cargo check — passes
cargo clippy — passes
cargo test -p rtc — all tests pass (173 unit + 76 doc tests)
Verify OnOpen is not fired for RTX SSRC sub-streams (unit test)
Integration: send a VP8 stream with RTX enabled; verify retransmitted packets are received by the media pipeline rather than dropped

🤖 Generated with Claude Code

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes routing and identification of RTX/FEC repair RTP packets so they reach the primary stream receiver instead of being dropped.

Changes:

Extend SSRC matching to include RTX/FEC repair SSRCs and early-route them to the primary receiver without track-open side effects.
Fix rrid handling to return the base track immediately after registering the repair SSRC.
Prevent RTX codecs from being selected as the primary codec in the undeclared-SSRC path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-04T14:14:06Z

+                // For primary streams, look up the primary codec only (not RTX/FEC codecs).
+                // RTX/FEC packets are routed via the early-return above.
                let track_codec = if is_track_codec_empty
                    && let Some(rtp_header) = rtp_header
                    && let Some(codec) = receiver
                        .get_codec_preferences()
                        .iter()
                        .find(|codec| codec.payload_type == rtp_header.payload_type)
-                //TODO: what about RTX/FEC stream?
                {
                    Some(codec.rtp_codec.clone())
                } else {


The comment says this lookup is 'primary codec only (not RTX/FEC codecs)', but the predicate currently matches only payload_type and does not exclude RTX (and potentially other repair-related codecs). Either (a) update the predicate to actually exclude repair codecs (consistent with the later change at line ~570), or (b) adjust the comment to reflect the actual behavior/assumptions of this branch.

Addressed: the predicate now includes && !is_repair_mime_type(...) which excludes all repair codecs (video/rtx, audio/rtx, ulpfec, flexfec, flexfec-03). This comment should be marked outdated — the fix is on adjacent lines.

Copilot · 2026-04-04T14:14:06Z

+                    .find(|codec| {
+                        codec.payload_type == rtp_header.payload_type
+                            && !codec
+                                .rtp_codec
+                                .mime_type
+                                .eq_ignore_ascii_case(MIME_TYPE_RTX)
+                    })


The RTX exclusion is an exact match against a single MIME_TYPE_RTX string. In SDP/RTP ecosystems RTX commonly appears as video/rtx and can also appear as audio/rtx; if MIME_TYPE_RTX is only one of these, the other will not be filtered and could still be incorrectly selected as the primary codec. Consider excluding RTX by a more general check (e.g., case-insensitive suffix /rtx or a shared is_rtx_mime_type helper) to ensure all RTX mime-type variants are rejected here.

Addressed: replaced the single MIME_TYPE_RTX check with is_repair_mime_type() which case-insensitively matches all repair MIME types across any media type (video/rtx, audio/rtx, ulpfec, flexfec, flexfec-03). This comment should be marked outdated.

- Fix import order (MIME_TYPE_RTX, MediaEngine) to pass cargo fmt - Add missing RTCRtpRtxParameters import (was causing compilation error) - Eliminate redundant receiver()/get_coding_parameters() lookups by using find_map to classify SSRC as primary vs repair in a single pass - Fix comment/predicate mismatch: primary codec lookup now actually excludes repair codecs (RTX and FEC) via is_repair_mime_type helper - Replace hardcoded MIME_TYPE_RTX check with general is_repair_mime_type that covers all RTX variants (video/rtx, audio/rtx) and FEC types (ulpfec, flexfec, flexfec-03) case-insensitively - Add regression tests for is_repair_mime_type covering RTX variants, FEC variants, and primary codec rejection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T08:07:22Z

+        // Determine whether the SSRC matches a primary or repair (RTX/FEC) sub-stream
+        // in a single pass, avoiding redundant receiver/coding-parameter lookups.
+        //
+        // First, classify the SSRC across all transceivers. We record whether the match
+        // was against a primary SSRC or a repair (RTX/FEC) SSRC so the subsequent code
+        // can take the right path without re-scanning coding parameters.
+        let ssrc_match = self
+            .rtp_transceivers
+            .iter()
+            .enumerate()
+            .find_map(|(id, transceiver)| {
+                let receiver = transceiver.receiver().as_ref()?;
+                let mut is_repair = false;
+                let matched = receiver.get_coding_parameters().iter().any(|coding| {
+                    if coding.ssrc.is_some_and(|coding_ssrc| coding_ssrc == ssrc) {
+                        return true;
                    }
-                })
-        {
+                    // Also match RTX/FEC repair SSRCs so repair packets are routed to
+                    // the primary stream's receiver rather than silently dropped.
+                    if coding.rtx.as_ref().is_some_and(|r| r.ssrc == ssrc)
+                        || coding.fec.as_ref().is_some_and(|f| f.ssrc == ssrc)
+                    {
+                        is_repair = true;
+                        return true;
+                    }
+                    false
+                });
+                if matched {
+                    // Grab the track_id while we have the receiver borrowed immutably,
+                    // so repair packets can be returned without a second lookup.
+                    let track_id = receiver.track().track_id().clone();
+                    Some((id, is_repair, track_id))
+                } else {
+                    None
+                }
+            });
+
+        if let Some((id, is_repair, repair_track_id)) = ssrc_match {
+            // If the SSRC belongs to a repair (RTX/FEC) sub-stream, route it to the primary
+            // stream's receiver without firing track-open events or updating codec state.
+            if is_repair {
+                return Some(repair_track_id);
+            }


New routing behavior for repair SSRCs (RTX/FEC) is introduced here, but the added unit tests only cover is_repair_mime_type. Consider adding regression tests that exercise find_track_id_by_ssrc/find_track_id_by_rid with an RTX/FEC SSRC to ensure the packet is routed to the primary track_id and that no RTCTrackEvent::OnOpen is emitted for the repair SSRC.

Addressed: 7 regression tests added covering find_track_id_by_ssrc with RTX, FEC, primary SSRCs, unknown SSRCs, and rrid stats registration. This comment should be marked outdated — the tests are in the same file.

Copilot · 2026-04-08T08:07:22Z

+                    // rrid identifies the base stream (rid) that this repair/RTX packet belongs to.
+                    // Associate the repair SSRC with the base stream's RTX parameters, then route
+                    // the packet to the primary stream's receiver (not a new track).
+                    if let Some(coding) = receiver.get_coding_parameter_mut_by_rid(rrid.as_str()) {
+                        match coding.rtx.as_mut() {
+                            Some(rtx) => rtx.ssrc = ssrc,
+                            None => coding.rtx = Some(RTCRtpRtxParameters { ssrc }),
+                        }
+                    }
+                    return Some(receiver.track().track_id().clone());


When rrid packets arrive, this updates coding.rtx.ssrc but does not update the stats accumulator’s RTX↔primary reverse lookup (populated in RTCStatsAccumulator::get_or_create_inbound_rtp_streams). If the primary stream’s stats entry was already created before rrid is seen, on_rtx_packet_received_if_rtx() will never recognize the new RTX SSRC, so RTX receive stats won’t be tracked. Consider updating the stats mapping (and, if applicable, the existing inbound stream’s rtx_ssrc field) when registering the RTX SSRC here.

Addressed: added register_rtx_ssrc(rtx_ssrc, primary_ssrc) to RTCStatsAccumulator, called from the rrid branch. Updates both the reverse lookup map and the inbound stream's rtx_ssrc field. This comment should be marked outdated.

Address remaining Copilot review comments on PR webrtc-rs#75: - Fix stats gap for rrid packets: when an RTX SSRC is learned via the rrid header extension after the primary stream's stats entry exists, register it in the accumulator's reverse lookup map so on_rtx_packet_received_if_rtx() can track retransmission stats. - Add register_rtx_ssrc() to RTCStatsAccumulator for late-discovered RTX SSRCs. - Add 7 regression tests covering: - RTX SSRC routed to primary track via find_track_id_by_ssrc - FEC SSRC routed to primary track via find_track_id_by_ssrc - No OnOpen event emitted for repair SSRC routing - Primary SSRC still routes correctly - Unknown SSRC returns None - rrid branch registers RTX SSRC in stats accumulator - handle_undeclared_ssrc rejects RTX codec as primary Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

webrtc-rs#12) Implements rrid (urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id) handling, resolving webrtc-rs#12. rrid is already parsed from RTP header extensions and the URI is registered — the handler body was a TODO stub. When an RTP packet carries rrid, it is a repair/RTX stream for the base stream identified by that rrid value (= the base stream's rid). Map the repair SSRC into the base stream's coding parameters' RTX field so downstream routing and stats can correlate the repair stream with its source stream. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Storing the RTX SSRC in coding parameters alone was insufficient — the interceptor also needs to know about the repair stream so RTX packets are actually demuxed and forwarded. This mirrors the existing RTX handling in interceptor_remote_streams_op. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

If the base stream's coding parameters don't exist yet when a repair/RTX packet arrives, the SSRC association is silently dropped. Add a warn! log so this failure mode is observable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Exercises the rrid code path in endpoint.rs::find_track_id_by_rid(): - Sends base RTP packets with rid extension for 3 simulcast layers - Sends RTX packets with rrid extension (different SSRC, RTX payload type) - Asserts exactly 3 tracks are created (no spurious tracks for RTX SSRCs) This verifies the RTX SSRC→base stream association and interceptor registration work correctly through the full pipeline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Guard interceptor registration behind has_base_coding: skip binding repair stream when no base coding parameters exist for the rrid, preventing invalid/unknown rrid values from creating orphan remote streams - Fix RTX payload type lookup: use codec.payload_type directly since the packet is already an RTX packet (find_rtx_payload_type was searching for apt=<rtx_pt> which never matches) - Fix test poll_read loop: drain all message types to prevent stalling on non-RTP messages (e.g. RTCP) - Remove unused import (RTCIceConnectionState) and find_rtx_payload_type Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add `update_inbound_rtx_ssrc` to RTCStatsAccumulator so rrid-discovered RTX SSRCs are reflected in stats (rtx_ssrc_to_primary map + inbound stream's rtx_ssrc field) - Return `Some(track_id)` from the rrid branch in find_track_id_by_rid so RTX packets are routed to the correct receiver instead of dropped - Add unit tests for update_inbound_rtx_ssrc (with and without existing inbound stream) - Fix clippy warnings in test and stats_tests - Add note in integration test explaining write_rtp limitation for RTX Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Three bugs caused all RTX/FEC repair packets to be silently dropped: 1. find_track_id_by_ssrc: outer SSRC search only matched primary SSRCs (coding.ssrc). RTX/FEC SSRCs live in coding.rtx.ssrc / coding.fec.ssrc and were never found. Fix: extend the .any() predicate to also check rtx/fec SSRCs; when matched via repair SSRC, return the primary stream's track_id immediately without triggering OnOpen events. 2. find_track_id_by_rid (rrid branch): after correctly associating the repair SSRC with the base stream's RTX parameters, the function fell through to None instead of returning Some(track_id). RTX packets with the rrid header extension were always dropped on every packet. 3. handle_undeclared_ssrc: codec lookup could match the RTX codec entry (which IS in codec_preferences) and incorrectly set up a repair stream as a primary track. Fix: explicitly exclude RTX codecs so only primary codecs are accepted here. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Fix import order (MIME_TYPE_RTX, MediaEngine) to pass cargo fmt - Add missing RTCRtpRtxParameters import (was causing compilation error) - Eliminate redundant receiver()/get_coding_parameters() lookups by using find_map to classify SSRC as primary vs repair in a single pass - Fix comment/predicate mismatch: primary codec lookup now actually excludes repair codecs (RTX and FEC) via is_repair_mime_type helper - Replace hardcoded MIME_TYPE_RTX check with general is_repair_mime_type that covers all RTX variants (video/rtx, audio/rtx) and FEC types (ulpfec, flexfec, flexfec-03) case-insensitively - Add regression tests for is_repair_mime_type covering RTX variants, FEC variants, and primary codec rejection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Address remaining Copilot review comments on PR webrtc-rs#75: - Fix stats gap for rrid packets: when an RTX SSRC is learned via the rrid header extension after the primary stream's stats entry exists, register it in the accumulator's reverse lookup map so on_rtx_packet_received_if_rtx() can track retransmission stats. - Add register_rtx_ssrc() to RTCStatsAccumulator for late-discovered RTX SSRCs. - Add 7 regression tests covering: - RTX SSRC routed to primary track via find_track_id_by_ssrc - FEC SSRC routed to primary track via find_track_id_by_ssrc - No OnOpen event emitted for repair SSRC routing - Primary SSRC still routes correctly - Unknown SSRC returns None - rrid branch registers RTX SSRC in stats accumulator - handle_undeclared_ssrc rejects RTX codec as primary Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nightness · 2026-04-10T05:26:12Z

Rebased onto PR #72's branch. Merge #72 first, then this PR applies cleanly. Previous branch structure has been cleaned up to avoid merge conflicts.

The test called register_rtx_ssrc() but the actual method on RTCStatsAccumulator is update_inbound_rtx_ssrc(). Fixed the test to use the correct method name and argument order. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nightness force-pushed the fix/rtx-fec-routing branch from 9b00c53 to 71e8f84 Compare April 1, 2026 18:44

rainliu requested a review from Copilot April 4, 2026 14:07

Copilot AI reviewed Apr 4, 2026

View reviewed changes

Copilot started reviewing on behalf of rainliu April 4, 2026 14:22 View session

nightness requested a review from Copilot April 8, 2026 08:00

Copilot started reviewing on behalf of nightness April 8, 2026 08:00 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

nightness and others added 9 commits April 10, 2026 00:15

nightness force-pushed the fix/rtx-fec-routing branch from 8c6cc18 to b83017f Compare April 10, 2026 05:22

nightness mentioned this pull request Apr 10, 2026

Summary: 19 PRs ready for review #87

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(endpoint): route RTX/FEC repair packets to primary stream receiver#75

fix(endpoint): route RTX/FEC repair packets to primary stream receiver#75
nightness wants to merge 10 commits into
webrtc-rs:masterfrom
Brainwires:fix/rtx-fec-routing

nightness commented Apr 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

nightness Apr 9, 2026

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

nightness Apr 9, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

nightness Apr 9, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

nightness Apr 9, 2026

Uh oh!

nightness commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nightness commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review feedback addressed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

nightness Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

nightness commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nightness commented Apr 1, 2026 •

edited

Loading