diff --git a/examples/examples/simulcast/simulcast.rs b/examples/examples/simulcast/simulcast.rs index 0e9bbc1db..bd804a2b9 100644 --- a/examples/examples/simulcast/simulcast.rs +++ b/examples/examples/simulcast/simulcast.rs @@ -85,7 +85,7 @@ async fn main() -> Result<()> { uri: extension.to_owned(), }, RTPCodecType::Video, - vec![], + None, )?; } // Create a InterceptorRegistry. This is the user configurable RTP/RTCP Pipeline. diff --git a/webrtc/CHANGELOG.md b/webrtc/CHANGELOG.md index 60a9d46f5..3e59eb173 100644 --- a/webrtc/CHANGELOG.md +++ b/webrtc/CHANGELOG.md @@ -8,6 +8,11 @@ * Stop sequence numbers from increasing in `TrackLocalStaticSample` while the bound `RTCRtpSender` have directions that should not send. [#316](https://github.com/webrtc-rs/webrtc/pull/316) +#### Breaking changes + +* Allow one single direction for extmap matching. [#321](https://github.com/webrtc-rs/webrtc/pull/321). API +change for MediaEngine::register_header_extension + ## 0.5.1 * Promote agent lock in ice_gather.rs create_agent() to top level of the function to avoid a race condition. [#290 Promote create_agent lock to top of function, to avoid race condition](https://github.com/webrtc-rs/webrtc/pull/290) contributed by [efer-ms](https://github.com/efer-ms) diff --git a/webrtc/src/api/interceptor_registry/mod.rs b/webrtc/src/api/interceptor_registry/mod.rs index e19e84c2d..d82c6aec6 100644 --- a/webrtc/src/api/interceptor_registry/mod.rs +++ b/webrtc/src/api/interceptor_registry/mod.rs @@ -75,7 +75,7 @@ pub fn configure_twcc(mut registry: Registry, media_engine: &mut MediaEngine) -> uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(), }, RTPCodecType::Video, - vec![], + None, )?; media_engine.register_feedback( @@ -90,7 +90,7 @@ pub fn configure_twcc(mut registry: Registry, media_engine: &mut MediaEngine) -> uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(), }, RTPCodecType::Audio, - vec![], + None, )?; let sender = Box::new(Sender::builder()); @@ -111,7 +111,7 @@ pub fn configure_twcc_sender_only( uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(), }, RTPCodecType::Video, - vec![], + None, )?; media_engine.register_header_extension( @@ -119,7 +119,7 @@ pub fn configure_twcc_sender_only( uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(), }, RTPCodecType::Audio, - vec![], + None, )?; let sender = Box::new(Sender::builder()); @@ -144,7 +144,7 @@ pub fn configure_twcc_receiver_only( uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(), }, RTPCodecType::Video, - vec![], + None, )?; media_engine.register_feedback( @@ -159,7 +159,7 @@ pub fn configure_twcc_receiver_only( uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(), }, RTPCodecType::Audio, - vec![], + None, )?; let receiver = Box::new(Receiver::builder()); diff --git a/webrtc/src/api/media_engine/media_engine_test.rs b/webrtc/src/api/media_engine/media_engine_test.rs index 605790331..20a2184f1 100644 --- a/webrtc/src/api/media_engine/media_engine_test.rs +++ b/webrtc/src/api/media_engine/media_engine_test.rs @@ -189,7 +189,7 @@ a=rtpmap:111 opus/48000/2 uri: extension.to_owned(), }, RTPCodecType::Audio, - vec![], + None, )?; } @@ -529,14 +529,11 @@ async fn test_media_engine_header_extension_direction() -> Result<()> { uri: "webrtc-header-test".to_owned(), }, RTPCodecType::Audio, - vec![], + None, )?; let params = m - .get_rtp_parameters_by_kind( - RTPCodecType::Audio, - &[RTCRtpTransceiverDirection::Recvonly], - ) + .get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Recvonly) .await; assert_eq!(1, params.header_extensions.len()); @@ -551,14 +548,11 @@ async fn test_media_engine_header_extension_direction() -> Result<()> { uri: "webrtc-header-test".to_owned(), }, RTPCodecType::Audio, - vec![RTCRtpTransceiverDirection::Recvonly], + Some(RTCRtpTransceiverDirection::Recvonly), )?; let params = m - .get_rtp_parameters_by_kind( - RTPCodecType::Audio, - &[RTCRtpTransceiverDirection::Recvonly], - ) + .get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Recvonly) .await; assert_eq!(1, params.header_extensions.len()); @@ -573,61 +567,33 @@ async fn test_media_engine_header_extension_direction() -> Result<()> { uri: "webrtc-header-test".to_owned(), }, RTPCodecType::Audio, - vec![RTCRtpTransceiverDirection::Sendonly], + Some(RTCRtpTransceiverDirection::Sendonly), )?; let params = m - .get_rtp_parameters_by_kind( - RTPCodecType::Audio, - &[RTCRtpTransceiverDirection::Recvonly], - ) + .get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Recvonly) .await; assert_eq!(0, params.header_extensions.len()); } - //"Invalid Direction" + //"No direction and inactive" { let mut m = MediaEngine::default(); register_codec(&mut m)?; - - let result = m.register_header_extension( + m.register_header_extension( RTCRtpHeaderExtensionCapability { uri: "webrtc-header-test".to_owned(), }, RTPCodecType::Audio, - vec![RTCRtpTransceiverDirection::Sendrecv], - ); - if let Err(err) = result { - assert_eq!(Error::ErrRegisterHeaderExtensionInvalidDirection, err); - } else { - assert!(false); - } + None, + )?; - let result = m.register_header_extension( - RTCRtpHeaderExtensionCapability { - uri: "webrtc-header-test".to_owned(), - }, - RTPCodecType::Audio, - vec![RTCRtpTransceiverDirection::Inactive], - ); - if let Err(err) = result { - assert_eq!(Error::ErrRegisterHeaderExtensionInvalidDirection, err); - } else { - assert!(false); - } - let result = m.register_header_extension( - RTCRtpHeaderExtensionCapability { - uri: "webrtc-header-test".to_owned(), - }, - RTPCodecType::Audio, - vec![RTCRtpTransceiverDirection::Unspecified], - ); - if let Err(err) = result { - assert_eq!(Error::ErrRegisterHeaderExtensionInvalidDirection, err); - } else { - assert!(false); - } + let params = m + .get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Inactive) + .await; + + assert_eq!(1, params.header_extensions.len()); } Ok(()) @@ -713,7 +679,7 @@ async fn test_update_header_extenstion_to_cloned_media_engine() -> Result<()> { uri: "test-extension".to_owned(), }, RTPCodecType::Audio, - vec![], + None, )?; validate(&m).await?; @@ -748,7 +714,7 @@ a=rtpmap:111 opus/48000/2 uri: extension.to_owned(), }, RTPCodecType::Video, - vec![], + None, )?; } for extension in [ @@ -761,7 +727,7 @@ a=rtpmap:111 opus/48000/2 uri: extension.to_owned(), }, RTPCodecType::Audio, - vec![], + None, )?; } @@ -799,7 +765,7 @@ a=rtpmap:111 opus/48000/2 assert!(!mid_video_enabled); let params = m - .get_rtp_parameters_by_kind(RTPCodecType::Video, &[RTCRtpTransceiverDirection::Sendonly]) + .get_rtp_parameters_by_kind(RTPCodecType::Video, RTCRtpTransceiverDirection::Sendonly) .await; dbg!(¶ms); diff --git a/webrtc/src/api/media_engine/mod.rs b/webrtc/src/api/media_engine/mod.rs index 57a66becf..ea41ba6a1 100644 --- a/webrtc/src/api/media_engine/mod.rs +++ b/webrtc/src/api/media_engine/mod.rs @@ -11,9 +11,7 @@ use crate::rtp_transceiver::rtp_codec::{ RTCRtpHeaderExtensionCapability, RTCRtpHeaderExtensionParameters, RTCRtpParameters, RTPCodecType, }; -use crate::rtp_transceiver::rtp_transceiver_direction::{ - have_rtp_transceiver_direction_intersection, RTCRtpTransceiverDirection, -}; +use crate::rtp_transceiver::rtp_transceiver_direction::RTCRtpTransceiverDirection; use crate::rtp_transceiver::{PayloadType, RTCPFeedback}; use crate::stats::stats_collector::StatsCollector; use crate::stats::CodecStats; @@ -58,8 +56,21 @@ pub(crate) struct MediaEngineHeaderExtension { pub(crate) uri: String, pub(crate) is_audio: bool, pub(crate) is_video: bool, - // If set only Transceivers of this direction are allowed - pub(crate) allowed_directions: Vec, + pub(crate) allowed_direction: Option, +} + +impl MediaEngineHeaderExtension { + pub fn is_matching_direction(&self, dir: RTCRtpTransceiverDirection) -> bool { + if let Some(allowed_direction) = self.allowed_direction { + use RTCRtpTransceiverDirection::*; + allowed_direction == Inactive && dir == Inactive + || allowed_direction.has_send() && dir.has_send() + || allowed_direction.has_recv() && dir.has_recv() + } else { + // None means all directions matches. + true + } + } } /// A MediaEngine defines the codecs supported by a PeerConnection, and the @@ -323,29 +334,18 @@ impl MediaEngine { } } - /// register_header_extension adds a header extension to the MediaEngine - /// To determine the negotiated value use [`get_header_extension_id`] after signaling is complete + /// Adds a header extension to the MediaEngine + /// To determine the negotiated value use [`get_header_extension_id`] after signaling is complete. + /// + /// The `allowed_direction` controls for which transceiver directions the extension matches. If + /// set to `None` it matches all directions. The `SendRecv` direction would match all transceiver + /// directions apart from `Inactive`. Inactive ony matches inactive. pub fn register_header_extension( &mut self, extension: RTCRtpHeaderExtensionCapability, typ: RTPCodecType, - mut allowed_directions: Vec, + allowed_direction: Option, ) -> Result<()> { - if allowed_directions.is_empty() { - allowed_directions = vec![ - RTCRtpTransceiverDirection::Recvonly, - RTCRtpTransceiverDirection::Sendonly, - ]; - } - - for direction in &allowed_directions { - if *direction != RTCRtpTransceiverDirection::Recvonly - && *direction != RTCRtpTransceiverDirection::Sendonly - { - return Err(Error::ErrRegisterHeaderExtensionInvalidDirection); - } - } - let ext = { match self .header_extensions @@ -358,8 +358,10 @@ impl MediaEngine { if self.header_extensions.len() > VALID_EXT_IDS.end as usize { return Err(Error::ErrRegisterHeaderExtensionNoFreeID); } - self.header_extensions - .push(MediaEngineHeaderExtension::default()); + self.header_extensions.push(MediaEngineHeaderExtension { + allowed_direction, + ..Default::default() + }); // Unwrap is fine because we just pushed self.header_extensions.last_mut().unwrap() @@ -374,8 +376,10 @@ impl MediaEngine { } ext.uri = extension.uri; - // TODO: This just overrides the previous allowed directions, which feels wrong - ext.allowed_directions = allowed_directions; + + if ext.allowed_direction != allowed_direction { + return Err(Error::ErrRegisterHeaderExtensionInvalidDirection); + } Ok(()) } @@ -559,7 +563,7 @@ impl MediaEngine { uri: extension.to_owned(), is_audio: local_extension.is_audio && typ == RTPCodecType::Audio, is_video: local_extension.is_video && typ == RTPCodecType::Video, - allowed_directions: local_extension.allowed_directions.clone(), + allowed_direction: local_extension.allowed_direction, }; negotiated_header_extensions.insert(id, h); } @@ -662,7 +666,7 @@ impl MediaEngine { pub(crate) async fn get_rtp_parameters_by_kind( &self, typ: RTPCodecType, - directions: &[RTCRtpTransceiverDirection], + direction: RTCRtpTransceiverDirection, ) -> RTCRtpParameters { let mut header_extensions = vec![]; @@ -671,7 +675,7 @@ impl MediaEngine { { let negotiated_header_extensions = self.negotiated_header_extensions.lock().await; for (id, e) in &*negotiated_header_extensions { - if have_rtp_transceiver_direction_intersection(&e.allowed_directions, directions) + if e.is_matching_direction(direction) && (e.is_audio && typ == RTPCodecType::Audio || e.is_video && typ == RTPCodecType::Video) { @@ -686,11 +690,9 @@ impl MediaEngine { let mut negotiated_header_extensions = self.negotiated_header_extensions.lock().await; for local_extension in &self.header_extensions { - let relevant = have_rtp_transceiver_direction_intersection( - &local_extension.allowed_directions, - directions, - ) && (local_extension.is_audio && typ == RTPCodecType::Audio - || local_extension.is_video && typ == RTPCodecType::Video); + let relevant = local_extension.is_matching_direction(direction) + && (local_extension.is_audio && typ == RTPCodecType::Audio + || local_extension.is_video && typ == RTPCodecType::Video); if !relevant { continue; @@ -739,7 +741,7 @@ impl MediaEngine { uri: local_extension.uri.clone(), is_audio: local_extension.is_audio, is_video: local_extension.is_video, - allowed_directions: local_extension.allowed_directions.clone(), + allowed_direction: local_extension.allowed_direction, }, ); diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index de3fcf0c1..088a695cb 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -205,8 +205,9 @@ pub enum Error { #[error("the requested codec does not have a payloader")] ErrNoPayloaderForCodec, - /// ErrRegisterHeaderExtensionInvalidDirection indicates that a extension was registered with a direction besides `sendonly` or `recvonly` - #[error("a header extension must be registered as 'recvonly', 'sendonly' or both")] + /// ErrRegisterHeaderExtensionInvalidDirection indicates that a extension was registered with different + /// directions for two different calls. + #[error("a header extension must be registered with the same direction each time")] ErrRegisterHeaderExtensionInvalidDirection, /// ErrRegisterHeaderExtensionNoFreeID indicates that there was no extension ID available which diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 385ffd799..ae9576045 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -505,16 +505,8 @@ pub(crate) async fn add_transceiver_sdp( return Ok((d, false)); } - let mut directions = vec![]; - if t.sender().await.is_some() { - directions.push(RTCRtpTransceiverDirection::Sendonly); - } - if t.receiver().await.is_some() { - directions.push(RTCRtpTransceiverDirection::Recvonly); - } - let parameters = media_engine - .get_rtp_parameters_by_kind(t.kind, &directions) + .get_rtp_parameters_by_kind(t.kind, t.direction()) .await; for rtp_extension in ¶meters.header_extensions { let ext_url = Url::parse(rtp_extension.uri.as_str())?; diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs index 74f3bec3a..e4e0a398c 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs @@ -312,7 +312,7 @@ impl RTPReceiverInternal { async fn get_parameters(&self) -> RTCRtpParameters { let mut parameters = self .media_engine - .get_rtp_parameters_by_kind(self.kind, &[RTCRtpTransceiverDirection::Recvonly]) + .get_rtp_parameters_by_kind(self.kind, RTCRtpTransceiverDirection::Recvonly) .await; let transceiver_codecs = self.transceiver_codecs.lock().await; diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index af70a108a..eca3a4d7e 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -246,7 +246,7 @@ impl RTCRtpSender { RTCRtpSendParameters { rtp_parameters: self .media_engine - .get_rtp_parameters_by_kind(kind, &[RTCRtpTransceiverDirection::Sendonly]) + .get_rtp_parameters_by_kind(kind, RTCRtpTransceiverDirection::Sendonly) .await, encodings: vec![RTCRtpEncodingParameters { ssrc: self.ssrc, @@ -328,7 +328,7 @@ impl RTCRtpSender { id: context.id.clone(), params: self .media_engine - .get_rtp_parameters_by_kind(t.kind(), &[RTCRtpTransceiverDirection::Sendonly]) + .get_rtp_parameters_by_kind(t.kind(), RTCRtpTransceiverDirection::Sendonly) .await, ssrc: context.ssrc, write_stream: context.write_stream.clone(), @@ -386,7 +386,7 @@ impl RTCRtpSender { } else { RTPCodecType::default() }, - &[RTCRtpTransceiverDirection::Sendonly], + RTCRtpTransceiverDirection::Sendonly, ) .await, ssrc: parameters.encodings[0].ssrc, diff --git a/webrtc/src/rtp_transceiver/rtp_transceiver_direction.rs b/webrtc/src/rtp_transceiver/rtp_transceiver_direction.rs index 03652ce72..3991945aa 100644 --- a/webrtc/src/rtp_transceiver/rtp_transceiver_direction.rs +++ b/webrtc/src/rtp_transceiver/rtp_transceiver_direction.rs @@ -106,20 +106,6 @@ impl RTCRtpTransceiverDirection { } } -pub(crate) fn have_rtp_transceiver_direction_intersection( - haystack: &[RTCRtpTransceiverDirection], - needle: &[RTCRtpTransceiverDirection], -) -> bool { - for n in needle { - for h in haystack { - if n == h { - return true; - } - } - } - false -} - #[cfg(test)] mod test { use super::*;