Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One single direction per extmap #321

Merged
merged 4 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/examples/simulcast/simulcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions webrtc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions webrtc/src/api/interceptor_registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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());
Expand All @@ -111,15 +111,15 @@ pub fn configure_twcc_sender_only(
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
},
RTPCodecType::Video,
vec![],
None,
)?;

media_engine.register_header_extension(
RTCRtpHeaderExtensionCapability {
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
},
RTPCodecType::Audio,
vec![],
None,
)?;

let sender = Box::new(Sender::builder());
Expand All @@ -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(
Expand All @@ -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());
Expand Down
74 changes: 20 additions & 54 deletions webrtc/src/api/media_engine/media_engine_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ a=rtpmap:111 opus/48000/2
uri: extension.to_owned(),
},
RTPCodecType::Audio,
vec![],
None,
)?;
}

Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand All @@ -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(())
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -748,7 +714,7 @@ a=rtpmap:111 opus/48000/2
uri: extension.to_owned(),
},
RTPCodecType::Video,
vec![],
None,
)?;
}
for extension in [
Expand All @@ -761,7 +727,7 @@ a=rtpmap:111 opus/48000/2
uri: extension.to_owned(),
},
RTPCodecType::Audio,
vec![],
None,
)?;
}

Expand Down Expand Up @@ -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!(&params);

Expand Down
74 changes: 38 additions & 36 deletions webrtc/src/api/media_engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<RTCRtpTransceiverDirection>,
pub(crate) allowed_direction: Option<RTCRtpTransceiverDirection>,
}

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()
algesten marked this conversation as resolved.
Show resolved Hide resolved
|| 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
Expand Down Expand Up @@ -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<RTCRtpTransceiverDirection>,
allowed_direction: Option<RTCRtpTransceiverDirection>,
) -> 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
Expand All @@ -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()
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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![];

Expand All @@ -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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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,
},
);

Expand Down
5 changes: 3 additions & 2 deletions webrtc/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading