From 2f5013fc35a26fed4f1385d1b9807e13590d3fcb Mon Sep 17 00:00:00 2001 From: Haakon Sporsheim Date: Tue, 6 Feb 2024 15:54:33 +0100 Subject: [PATCH 1/3] peer_connection: Start receiver for all known tracks This fixes RTP receiver from Firefox as it signal SSRCs for all simulcast layers, while Chromium doesn't which ends up in another code path. --- .../peer_connection_internal.rs | 36 +++++------ .../src/rtp_transceiver/rtp_receiver/mod.rs | 12 ---- .../rtp_receiver/rtp_receiver_test.rs | 62 ++++++++++--------- 3 files changed, 49 insertions(+), 61 deletions(-) diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 802f5c11a..24e2b6c0e 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1068,8 +1068,8 @@ impl PeerConnectionInternal { on_track_handler: Arc>>, ) { receiver.start(incoming).await; - for t in receiver.tracks().await { - if t.ssrc() == 0 { + for track in receiver.tracks().await { + if track.ssrc() == 0 { return; } @@ -1077,31 +1077,29 @@ impl PeerConnectionInternal { let transceiver = Arc::clone(&transceiver); let on_track_handler = Arc::clone(&on_track_handler); tokio::spawn(async move { - if let Some(track) = receiver.track().await { - let mut b = vec![0u8; receive_mtu]; - let pkt = match track.peek(&mut b).await { - Ok((pkt, _)) => pkt, - Err(err) => { - log::warn!( - "Could not determine PayloadType for SSRC {} ({})", - track.ssrc(), - err - ); - return; - } - }; - - if let Err(err) = track.check_and_update_track(&pkt).await { + let mut b = vec![0u8; receive_mtu]; + let pkt = match track.peek(&mut b).await { + Ok((pkt, _)) => pkt, + Err(err) => { log::warn!( - "Failed to set codec settings for track SSRC {} ({})", + "Could not determine PayloadType for SSRC {} ({})", track.ssrc(), err ); return; } + }; - RTCPeerConnection::do_track(on_track_handler, track, receiver, transceiver); + if let Err(err) = track.check_and_update_track(&pkt).await { + log::warn!( + "Failed to set codec settings for track SSRC {} ({})", + track.ssrc(), + err + ); + return; } + + RTCPeerConnection::do_track(on_track_handler, track, receiver, transceiver); }); } } diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs index 8a2f40f24..d0fa1f257 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs @@ -487,18 +487,6 @@ impl RTCRtpReceiver { } } - /// track returns the RtpTransceiver TrackRemote - pub async fn track(&self) -> Option> { - let tracks = self.internal.tracks.read().await; - if tracks.len() != 1 { - None - } else { - // Clippy bug (reported at https://github.com/rust-lang/rust-clippy/issues/12560) suggests .first().cloned() - #[allow(clippy::map_clone)] - tracks.first().map(|t| Arc::clone(&t.track)) - } - } - /// tracks returns the RtpTransceiver traclockks /// A RTPReceiver to support Simulcast may now have multiple tracks pub async fn tracks(&self) -> Vec> { diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/rtp_receiver_test.rs b/webrtc/src/rtp_transceiver/rtp_receiver/rtp_receiver_test.rs index 304bff8f7..7520667db 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/rtp_receiver_test.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/rtp_receiver_test.rs @@ -92,36 +92,38 @@ async fn test_set_rtp_parameters() -> Result<()> { Box::pin(async move { receiver.set_rtp_parameters(P.clone()).await; - if let Some(t) = receiver.track().await { - let incoming_track_codecs = t.codec(); - - assert_eq!(P.header_extensions, t.params().header_extensions); - assert_eq!( - P.codecs[0].capability.mime_type, - incoming_track_codecs.capability.mime_type - ); - assert_eq!( - P.codecs[0].capability.clock_rate, - incoming_track_codecs.capability.clock_rate - ); - assert_eq!( - P.codecs[0].capability.channels, - incoming_track_codecs.capability.channels - ); - assert_eq!( - P.codecs[0].capability.sdp_fmtp_line, - incoming_track_codecs.capability.sdp_fmtp_line - ); - assert_eq!( - P.codecs[0].capability.rtcp_feedback, - incoming_track_codecs.capability.rtcp_feedback - ); - assert_eq!(P.codecs[0].payload_type, incoming_track_codecs.payload_type); - - { - let mut done = seen_packet_tx2.lock().await; - done.take(); - } + let tracks = receiver.tracks().await; + assert_eq!(tracks.len(), 1); + let t = tracks.first().unwrap(); + + let incoming_track_codecs = t.codec(); + + assert_eq!(P.header_extensions, t.params().header_extensions); + assert_eq!( + P.codecs[0].capability.mime_type, + incoming_track_codecs.capability.mime_type + ); + assert_eq!( + P.codecs[0].capability.clock_rate, + incoming_track_codecs.capability.clock_rate + ); + assert_eq!( + P.codecs[0].capability.channels, + incoming_track_codecs.capability.channels + ); + assert_eq!( + P.codecs[0].capability.sdp_fmtp_line, + incoming_track_codecs.capability.sdp_fmtp_line + ); + assert_eq!( + P.codecs[0].capability.rtcp_feedback, + incoming_track_codecs.capability.rtcp_feedback + ); + assert_eq!(P.codecs[0].payload_type, incoming_track_codecs.payload_type); + + { + let mut done = seen_packet_tx2.lock().await; + done.take(); } }) })); From 7db18eb2310a815b1cfb99170b99f84367bcb808 Mon Sep 17 00:00:00 2001 From: Haakon Sporsheim Date: Tue, 6 Feb 2024 19:53:15 +0100 Subject: [PATCH 2/3] sdp: extmap: Move SDES_REPAIR_RTP_STREAM_ID_URI definition --- sdp/src/extmap/mod.rs | 3 +++ webrtc/src/lib.rs | 2 -- webrtc/src/peer_connection/peer_connection_internal.rs | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sdp/src/extmap/mod.rs b/sdp/src/extmap/mod.rs index 630990f42..e304e71cd 100644 --- a/sdp/src/extmap/mod.rs +++ b/sdp/src/extmap/mod.rs @@ -20,6 +20,9 @@ pub const TRANSPORT_CC_URI: &str = "http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01"; pub const SDES_MID_URI: &str = "urn:ietf:params:rtp-hdrext:sdes:mid"; pub const SDES_RTP_STREAM_ID_URI: &str = "urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id"; +pub const SDES_REPAIR_RTP_STREAM_ID_URI: &str = + "urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id"; + pub const AUDIO_LEVEL_URI: &str = "urn:ietf:params:rtp-hdrext:ssrc-audio-level"; pub const VIDEO_ORIENTATION_URI: &str = "urn:3gpp:video-orientation"; diff --git a/webrtc/src/lib.rs b/webrtc/src/lib.rs index f1c1bd110..a2decd674 100644 --- a/webrtc/src/lib.rs +++ b/webrtc/src/lib.rs @@ -42,5 +42,3 @@ pub(crate) const RECEIVE_MTU: usize = 1460; pub(crate) const SDP_ATTRIBUTE_RID: &str = "rid"; pub(crate) const SDP_ATTRIBUTE_SIMULCAST: &str = "simulcast"; pub(crate) const GENERATED_CERTIFICATE_ORIGIN: &str = "WebRTC"; -pub(crate) const SDES_REPAIR_RTP_STREAM_ID_URI: &str = - "urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id"; diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 24e2b6c0e..8f728b24f 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -15,7 +15,7 @@ use crate::stats::{ StatsReportType, }; use crate::track::TrackStream; -use crate::{SDES_REPAIR_RTP_STREAM_ID_URI, SDP_ATTRIBUTE_RID}; +use crate::SDP_ATTRIBUTE_RID; pub(crate) struct PeerConnectionInternal { /// a value containing the last known greater mid value @@ -940,7 +940,7 @@ impl PeerConnectionInternal { let (rsid_extension_id, _, _) = self .media_engine .get_header_extension_id(RTCRtpHeaderExtensionCapability { - uri: SDES_REPAIR_RTP_STREAM_ID_URI.to_owned(), + uri: ::sdp::extmap::SDES_REPAIR_RTP_STREAM_ID_URI.to_owned(), }) .await; From e8d776f1af165a8046c9d8ff713b6adcf509234d Mon Sep 17 00:00:00 2001 From: Haakon Sporsheim Date: Wed, 7 Feb 2024 11:13:44 +0100 Subject: [PATCH 3/3] sdp: Don't set track detail SSRCs when parsing rids This ends up tracks being "discovered" via SRTP session accept. --- webrtc/src/peer_connection/sdp/mod.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 21b82753f..4f7542413 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -211,26 +211,20 @@ pub(crate) fn track_details_from_sdp( }; } + // If media line is using RTP Stream Identifier Source Description per RFC8851 + // we will need to override tracks, and remove ssrcs. + // This is in particular important for Firefox, as it uses both 'rid', 'simulcast' + // and 'a=ssrc' lines. let rids = get_rids(media); if !rids.is_empty() && !track_id.is_empty() && !stream_id.is_empty() { - let mut simulcast_track = TrackDetails { + tracks_in_media_section = vec![TrackDetails { mid: SmolStr::from(mid_value), kind: codec_type, stream_id: stream_id.to_owned(), id: track_id.to_owned(), - rids: vec![], + rids: rids.iter().map(|r| SmolStr::from(&r.id)).collect(), ..Default::default() - }; - for rid in &rids { - simulcast_track.rids.push(SmolStr::from(&rid.id)); - } - if simulcast_track.rids.len() == tracks_in_media_section.len() { - for track in &tracks_in_media_section { - simulcast_track.ssrcs.extend(&track.ssrcs) - } - } - - tracks_in_media_section = vec![simulcast_track]; + }]; } incoming_tracks.extend(tracks_in_media_section);