Skip to content

Commit ee18829

Browse files
WandalenWandalen
committed
fix: Handle receiver reuse during renegotiation
- Track renegotiation context in start_rtp_receivers() - Preserve active receivers during renegotiation per RFC 8829 - Add comprehensive test reproducing mesh topology issue - Document fix with root cause and prevention guidelines
1 parent fb3f05a commit ee18829

File tree

2 files changed

+292
-67
lines changed

2 files changed

+292
-67
lines changed

‎webrtc/src/peer_connection/peer_connection_internal.rs‎

Lines changed: 21 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -284,19 +284,6 @@ impl PeerConnectionInternal {
284284
}
285285
}
286286

287-
// VALIDATION LOGGING: H15 (incoming_tracks empty?) and H14 (transceiver list)
288-
// log::info!("🔍 VALIDATION: start_rtp() called, is_renegotiation={}", is_renegotiation);
289-
// log::info!("🔍 VALIDATION: incoming_tracks.len() = {}", track_details.len());
290-
// for (i, track) in track_details.iter().enumerate() {
291-
// log::info!("🔍 VALIDATION: incoming_track[{}]: mid={}, kind={:?}, ssrcs={:?}",
292-
// i, track.mid, track.kind, track.ssrcs);
293-
// }
294-
// log::info!("🔍 VALIDATION: current_transceivers.len() = {}", current_transceivers.len());
295-
// for (i, t) in current_transceivers.iter().enumerate() {
296-
// log::info!("🔍 VALIDATION: transceiver[{}]: mid={:?}, kind={:?}, direction={:?}",
297-
// i, t.mid(), t.kind(), t.direction());
298-
// }
299-
300287
self.start_rtp_receivers(&mut track_details, &current_transceivers, is_renegotiation)
301288
.await?;
302289
if let Some(parsed_remote) = &remote_desc.parsed {
@@ -433,95 +420,67 @@ impl PeerConnectionInternal {
433420
local_transceivers: &[Arc<RTCRtpTransceiver>],
434421
is_renegotiation: bool,
435422
) -> Result<()> {
436-
// Ensure we haven't already started a transceiver for this ssrc
437-
// SKIP filtering during renegotiation - let receiver reset logic handle it
438-
// log::info!("🔍 VALIDATION: start_rtp_receivers() - incoming_tracks.len()={}, is_renegotiation={}", incoming_tracks.len(), is_renegotiation);
423+
// Ensure we haven't already started a transceiver for this ssrc.
424+
// Skip filtering during renegotiation since receiver reuse logic handles it.
439425
let mut filtered_tracks = incoming_tracks.clone();
440426

441427
if !is_renegotiation {
442-
// log::info!("🔍 VALIDATION: Filtering tracks (initial negotiation)");
443428
for incoming_track in incoming_tracks {
444429
// If we already have a TrackRemote for a given SSRC don't handle it again
445430
for t in local_transceivers {
446431
let receiver = t.receiver().await;
447432
let existing_tracks = receiver.tracks().await;
448-
// log::info!("🔍 VALIDATION: Receiver has {} existing tracks", existing_tracks.len());
449433
for track in existing_tracks {
450-
// log::info!("🔍 VALIDATION: Existing track ssrc={}", track.ssrc());
451434
for ssrc in &incoming_track.ssrcs {
452-
// log::info!("🔍 VALIDATION: Comparing with incoming ssrc={}", ssrc);
453435
if *ssrc == track.ssrc() {
454-
// log::warn!("🔍 VALIDATION: ❌ FILTERING OUT track with ssrc={} (already exists)", ssrc);
455436
filter_track_with_ssrc(&mut filtered_tracks, track.ssrc());
456437
}
457438
}
458439
}
459440
}
460441
}
461-
} else {
462-
// log::info!("🔍 VALIDATION: Skipping filtering (renegotiation - receiver reset logic will handle duplicates)");
463442
}
464443

465-
// log::info!("🔍 VALIDATION: After filtering - filtered_tracks.len()={}", filtered_tracks.len());
466-
// for (i, track) in filtered_tracks.iter().enumerate() {
467-
// log::info!("🔍 VALIDATION: filtered_track[{}]: mid={}, ssrcs={:?}", i, track.mid, track.ssrcs);
468-
// }
469-
470444
let mut unhandled_tracks = vec![]; // filtered_tracks[:0]
471445
for incoming_track in filtered_tracks.iter() {
472-
// log::info!("🔍 VALIDATION: Processing incoming_track mid={}", incoming_track.mid);
473446
let mut track_handled = false;
474-
for (_t_idx, t) in local_transceivers.iter().enumerate() {
475-
// log::info!("🔍 VALIDATION: Checking transceiver[{}]", _t_idx);
476-
477-
// Check 1: MID match
478-
let mid_match = t.mid().as_ref() == Some(&incoming_track.mid);
479-
// log::info!("🔍 VALIDATION: MID check: t.mid={:?}, track.mid={}, MATCH={}",
480-
// t.mid(), incoming_track.mid, mid_match);
481-
if !mid_match {
482-
// log::info!("🔍 VALIDATION: ❌ SKIPPED: MID mismatch");
447+
for t in local_transceivers {
448+
if t.mid().as_ref() != Some(&incoming_track.mid) {
483449
continue;
484450
}
485451

486-
// Check 2: Kind match
487-
let kind_match = incoming_track.kind == t.kind();
488-
// log::info!("🔍 VALIDATION: KIND check: track.kind={:?}, t.kind={:?}, MATCH={}",
489-
// incoming_track.kind, t.kind(), kind_match);
490-
491-
// Check 3: Direction check
492-
let direction = t.direction();
493-
let direction_ok = direction == RTCRtpTransceiverDirection::Recvonly
494-
|| direction == RTCRtpTransceiverDirection::Sendrecv;
495-
// log::info!("🔍 VALIDATION: DIRECTION check: t.direction={:?}, OK={}",
496-
// direction, direction_ok);
497-
498-
if !kind_match || !direction_ok {
499-
// log::info!("🔍 VALIDATION: ❌ SKIPPED: kind or direction mismatch");
452+
if (incoming_track.kind != t.kind())
453+
|| (t.direction() != RTCRtpTransceiverDirection::Recvonly
454+
&& t.direction() != RTCRtpTransceiverDirection::Sendrecv)
455+
{
500456
continue;
501457
}
502458

503-
// Check 4: have_received check (handle renegotiation)
504-
// Fix(issue-107): During renegotiation, allow reusing transceivers that are already receiving.
505-
// The same SSRC appears in multiple negotiation rounds in mesh topology. Instead of marking
506-
// these tracks as "NOT HANDLED", we recognize the receiver is already active and mark as handled.
459+
// Fix(issue-749): Handle receiver reuse during renegotiation in mesh topology.
460+
//
461+
// During SDP renegotiation, the same tracks (SSRCs) legitimately appear in
462+
// subsequent negotiation rounds per RFC 8829 Section 3.7. Receivers that are
463+
// already active should be recognized as handling their existing tracks rather
464+
// than being skipped and marked as "NOT HANDLED".
465+
//
466+
// Root cause: The original code didn't distinguish between initial negotiation
467+
// (where skipping active receivers prevents duplicates) and renegotiation
468+
// (where active receivers represent existing media flows to preserve).
507469
let receiver = t.receiver().await;
508470
let already_receiving = receiver.have_received().await;
509-
// log::info!("🔍 VALIDATION: HAVE_RECEIVED check: {}", already_receiving);
510471

511472
if already_receiving {
512473
if !is_renegotiation {
513-
// log::info!("🔍 VALIDATION: ❌ SKIPPED: already receiving (initial negotiation)");
474+
// Initial negotiation: skip if already receiving (safety check)
514475
continue;
515476
} else {
516-
// During renegotiation, receiver already active - just mark as handled
517-
// log::info!("🔍 VALIDATION: ✅ Renegotiation: receiver already active, marking as handled (no restart needed)");
477+
// Renegotiation: receiver already active, mark as handled
518478
track_handled = true;
519479
break;
520480
}
521481
}
522482

523-
// All checks passed - start receiver (only if not already receiving)
524-
// log::info!("🔍 VALIDATION: ✅ ALL CHECKS PASSED - Starting receiver");
483+
// Start receiver for new tracks only
525484
PeerConnectionInternal::start_receiver(
526485
self.setting_engine.get_receive_mtu(),
527486
incoming_track,
@@ -531,15 +490,10 @@ impl PeerConnectionInternal {
531490
)
532491
.await;
533492
track_handled = true;
534-
// log::info!("🔍 VALIDATION: ✅ Receiver started successfully");
535493
}
536494

537495
if !track_handled {
538-
// log::warn!("🔍 VALIDATION: ⚠️ Track NOT HANDLED: mid={}, kind={:?}, ssrcs={:?}",
539-
// incoming_track.mid, incoming_track.kind, incoming_track.ssrcs);
540496
unhandled_tracks.push(incoming_track);
541-
} else {
542-
// log::info!("🔍 VALIDATION: ✅ Track HANDLED: mid={}", incoming_track.mid);
543497
}
544498
}
545499

0 commit comments

Comments
 (0)