Skip to content

Commit 60f4c43

Browse files
algestenalgesten
authored
Merge pull request #321 from webrtc-rs/fix/extmap-direction
One single direction per extmap
2 parents 7a93dc8 + 1fa5f23 commit 60f4c43

File tree

10 files changed

+78
-126
lines changed

10 files changed

+78
-126
lines changed

‎examples/examples/simulcast/simulcast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ async fn main() -> Result<()> {
8585
uri: extension.to_owned(),
8686
},
8787
RTPCodecType::Video,
88-
vec![],
88+
None,
8989
)?;
9090
}
9191
// Create a InterceptorRegistry. This is the user configurable RTP/RTCP Pipeline.

‎webrtc/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
* Stop sequence numbers from increasing in `TrackLocalStaticSample` while the bound `RTCRtpSender` have
99
directions that should not send. [#316](https://github.com/webrtc-rs/webrtc/pull/316)
1010

11+
#### Breaking changes
12+
13+
* Allow one single direction for extmap matching. [#321](https://github.com/webrtc-rs/webrtc/pull/321). API
14+
change for MediaEngine::register_header_extension
15+
1116
## 0.5.1
1217

1318
* 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)

‎webrtc/src/api/interceptor_registry/mod.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ pub fn configure_twcc(mut registry: Registry, media_engine: &mut MediaEngine) ->
7575
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
7676
},
7777
RTPCodecType::Video,
78-
vec![],
78+
None,
7979
)?;
8080

8181
media_engine.register_feedback(
@@ -90,7 +90,7 @@ pub fn configure_twcc(mut registry: Registry, media_engine: &mut MediaEngine) ->
9090
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
9191
},
9292
RTPCodecType::Audio,
93-
vec![],
93+
None,
9494
)?;
9595

9696
let sender = Box::new(Sender::builder());
@@ -111,15 +111,15 @@ pub fn configure_twcc_sender_only(
111111
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
112112
},
113113
RTPCodecType::Video,
114-
vec![],
114+
None,
115115
)?;
116116

117117
media_engine.register_header_extension(
118118
RTCRtpHeaderExtensionCapability {
119119
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
120120
},
121121
RTPCodecType::Audio,
122-
vec![],
122+
None,
123123
)?;
124124

125125
let sender = Box::new(Sender::builder());
@@ -144,7 +144,7 @@ pub fn configure_twcc_receiver_only(
144144
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
145145
},
146146
RTPCodecType::Video,
147-
vec![],
147+
None,
148148
)?;
149149

150150
media_engine.register_feedback(
@@ -159,7 +159,7 @@ pub fn configure_twcc_receiver_only(
159159
uri: sdp::extmap::TRANSPORT_CC_URI.to_owned(),
160160
},
161161
RTPCodecType::Audio,
162-
vec![],
162+
None,
163163
)?;
164164

165165
let receiver = Box::new(Receiver::builder());

‎webrtc/src/api/media_engine/media_engine_test.rs

Lines changed: 20 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ a=rtpmap:111 opus/48000/2
189189
uri: extension.to_owned(),
190190
},
191191
RTPCodecType::Audio,
192-
vec![],
192+
None,
193193
)?;
194194
}
195195

@@ -529,14 +529,11 @@ async fn test_media_engine_header_extension_direction() -> Result<()> {
529529
uri: "webrtc-header-test".to_owned(),
530530
},
531531
RTPCodecType::Audio,
532-
vec![],
532+
None,
533533
)?;
534534

535535
let params = m
536-
.get_rtp_parameters_by_kind(
537-
RTPCodecType::Audio,
538-
&[RTCRtpTransceiverDirection::Recvonly],
539-
)
536+
.get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Recvonly)
540537
.await;
541538

542539
assert_eq!(1, params.header_extensions.len());
@@ -551,14 +548,11 @@ async fn test_media_engine_header_extension_direction() -> Result<()> {
551548
uri: "webrtc-header-test".to_owned(),
552549
},
553550
RTPCodecType::Audio,
554-
vec![RTCRtpTransceiverDirection::Recvonly],
551+
Some(RTCRtpTransceiverDirection::Recvonly),
555552
)?;
556553

557554
let params = m
558-
.get_rtp_parameters_by_kind(
559-
RTPCodecType::Audio,
560-
&[RTCRtpTransceiverDirection::Recvonly],
561-
)
555+
.get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Recvonly)
562556
.await;
563557

564558
assert_eq!(1, params.header_extensions.len());
@@ -573,61 +567,33 @@ async fn test_media_engine_header_extension_direction() -> Result<()> {
573567
uri: "webrtc-header-test".to_owned(),
574568
},
575569
RTPCodecType::Audio,
576-
vec![RTCRtpTransceiverDirection::Sendonly],
570+
Some(RTCRtpTransceiverDirection::Sendonly),
577571
)?;
578572

579573
let params = m
580-
.get_rtp_parameters_by_kind(
581-
RTPCodecType::Audio,
582-
&[RTCRtpTransceiverDirection::Recvonly],
583-
)
574+
.get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Recvonly)
584575
.await;
585576

586577
assert_eq!(0, params.header_extensions.len());
587578
}
588579

589-
//"Invalid Direction"
580+
//"No direction and inactive"
590581
{
591582
let mut m = MediaEngine::default();
592583
register_codec(&mut m)?;
593-
594-
let result = m.register_header_extension(
584+
m.register_header_extension(
595585
RTCRtpHeaderExtensionCapability {
596586
uri: "webrtc-header-test".to_owned(),
597587
},
598588
RTPCodecType::Audio,
599-
vec![RTCRtpTransceiverDirection::Sendrecv],
600-
);
601-
if let Err(err) = result {
602-
assert_eq!(Error::ErrRegisterHeaderExtensionInvalidDirection, err);
603-
} else {
604-
assert!(false);
605-
}
589+
None,
590+
)?;
606591

607-
let result = m.register_header_extension(
608-
RTCRtpHeaderExtensionCapability {
609-
uri: "webrtc-header-test".to_owned(),
610-
},
611-
RTPCodecType::Audio,
612-
vec![RTCRtpTransceiverDirection::Inactive],
613-
);
614-
if let Err(err) = result {
615-
assert_eq!(Error::ErrRegisterHeaderExtensionInvalidDirection, err);
616-
} else {
617-
assert!(false);
618-
}
619-
let result = m.register_header_extension(
620-
RTCRtpHeaderExtensionCapability {
621-
uri: "webrtc-header-test".to_owned(),
622-
},
623-
RTPCodecType::Audio,
624-
vec![RTCRtpTransceiverDirection::Unspecified],
625-
);
626-
if let Err(err) = result {
627-
assert_eq!(Error::ErrRegisterHeaderExtensionInvalidDirection, err);
628-
} else {
629-
assert!(false);
630-
}
592+
let params = m
593+
.get_rtp_parameters_by_kind(RTPCodecType::Audio, RTCRtpTransceiverDirection::Inactive)
594+
.await;
595+
596+
assert_eq!(1, params.header_extensions.len());
631597
}
632598

633599
Ok(())
@@ -713,7 +679,7 @@ async fn test_update_header_extenstion_to_cloned_media_engine() -> Result<()> {
713679
uri: "test-extension".to_owned(),
714680
},
715681
RTPCodecType::Audio,
716-
vec![],
682+
None,
717683
)?;
718684

719685
validate(&m).await?;
@@ -748,7 +714,7 @@ a=rtpmap:111 opus/48000/2
748714
uri: extension.to_owned(),
749715
},
750716
RTPCodecType::Video,
751-
vec![],
717+
None,
752718
)?;
753719
}
754720
for extension in [
@@ -761,7 +727,7 @@ a=rtpmap:111 opus/48000/2
761727
uri: extension.to_owned(),
762728
},
763729
RTPCodecType::Audio,
764-
vec![],
730+
None,
765731
)?;
766732
}
767733

@@ -799,7 +765,7 @@ a=rtpmap:111 opus/48000/2
799765
assert!(!mid_video_enabled);
800766

801767
let params = m
802-
.get_rtp_parameters_by_kind(RTPCodecType::Video, &[RTCRtpTransceiverDirection::Sendonly])
768+
.get_rtp_parameters_by_kind(RTPCodecType::Video, RTCRtpTransceiverDirection::Sendonly)
803769
.await;
804770
dbg!(&params);
805771

‎webrtc/src/api/media_engine/mod.rs

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ use crate::rtp_transceiver::rtp_codec::{
1111
RTCRtpHeaderExtensionCapability, RTCRtpHeaderExtensionParameters, RTCRtpParameters,
1212
RTPCodecType,
1313
};
14-
use crate::rtp_transceiver::rtp_transceiver_direction::{
15-
have_rtp_transceiver_direction_intersection, RTCRtpTransceiverDirection,
16-
};
14+
use crate::rtp_transceiver::rtp_transceiver_direction::RTCRtpTransceiverDirection;
1715
use crate::rtp_transceiver::{PayloadType, RTCPFeedback};
1816
use crate::stats::stats_collector::StatsCollector;
1917
use crate::stats::CodecStats;
@@ -58,8 +56,21 @@ pub(crate) struct MediaEngineHeaderExtension {
5856
pub(crate) uri: String,
5957
pub(crate) is_audio: bool,
6058
pub(crate) is_video: bool,
61-
// If set only Transceivers of this direction are allowed
62-
pub(crate) allowed_directions: Vec<RTCRtpTransceiverDirection>,
59+
pub(crate) allowed_direction: Option<RTCRtpTransceiverDirection>,
60+
}
61+
62+
impl MediaEngineHeaderExtension {
63+
pub fn is_matching_direction(&self, dir: RTCRtpTransceiverDirection) -> bool {
64+
if let Some(allowed_direction) = self.allowed_direction {
65+
use RTCRtpTransceiverDirection::*;
66+
allowed_direction == Inactive && dir == Inactive
67+
|| allowed_direction.has_send() && dir.has_send()
68+
|| allowed_direction.has_recv() && dir.has_recv()
69+
} else {
70+
// None means all directions matches.
71+
true
72+
}
73+
}
6374
}
6475

6576
/// A MediaEngine defines the codecs supported by a PeerConnection, and the
@@ -323,29 +334,18 @@ impl MediaEngine {
323334
}
324335
}
325336

326-
/// register_header_extension adds a header extension to the MediaEngine
327-
/// To determine the negotiated value use [`get_header_extension_id`] after signaling is complete
337+
/// Adds a header extension to the MediaEngine
338+
/// To determine the negotiated value use [`get_header_extension_id`] after signaling is complete.
339+
///
340+
/// The `allowed_direction` controls for which transceiver directions the extension matches. If
341+
/// set to `None` it matches all directions. The `SendRecv` direction would match all transceiver
342+
/// directions apart from `Inactive`. Inactive ony matches inactive.
328343
pub fn register_header_extension(
329344
&mut self,
330345
extension: RTCRtpHeaderExtensionCapability,
331346
typ: RTPCodecType,
332-
mut allowed_directions: Vec<RTCRtpTransceiverDirection>,
347+
allowed_direction: Option<RTCRtpTransceiverDirection>,
333348
) -> Result<()> {
334-
if allowed_directions.is_empty() {
335-
allowed_directions = vec![
336-
RTCRtpTransceiverDirection::Recvonly,
337-
RTCRtpTransceiverDirection::Sendonly,
338-
];
339-
}
340-
341-
for direction in &allowed_directions {
342-
if *direction != RTCRtpTransceiverDirection::Recvonly
343-
&& *direction != RTCRtpTransceiverDirection::Sendonly
344-
{
345-
return Err(Error::ErrRegisterHeaderExtensionInvalidDirection);
346-
}
347-
}
348-
349349
let ext = {
350350
match self
351351
.header_extensions
@@ -358,8 +358,10 @@ impl MediaEngine {
358358
if self.header_extensions.len() > VALID_EXT_IDS.end as usize {
359359
return Err(Error::ErrRegisterHeaderExtensionNoFreeID);
360360
}
361-
self.header_extensions
362-
.push(MediaEngineHeaderExtension::default());
361+
self.header_extensions.push(MediaEngineHeaderExtension {
362+
allowed_direction,
363+
..Default::default()
364+
});
363365

364366
// Unwrap is fine because we just pushed
365367
self.header_extensions.last_mut().unwrap()
@@ -374,8 +376,10 @@ impl MediaEngine {
374376
}
375377

376378
ext.uri = extension.uri;
377-
// TODO: This just overrides the previous allowed directions, which feels wrong
378-
ext.allowed_directions = allowed_directions;
379+
380+
if ext.allowed_direction != allowed_direction {
381+
return Err(Error::ErrRegisterHeaderExtensionInvalidDirection);
382+
}
379383

380384
Ok(())
381385
}
@@ -559,7 +563,7 @@ impl MediaEngine {
559563
uri: extension.to_owned(),
560564
is_audio: local_extension.is_audio && typ == RTPCodecType::Audio,
561565
is_video: local_extension.is_video && typ == RTPCodecType::Video,
562-
allowed_directions: local_extension.allowed_directions.clone(),
566+
allowed_direction: local_extension.allowed_direction,
563567
};
564568
negotiated_header_extensions.insert(id, h);
565569
}
@@ -662,7 +666,7 @@ impl MediaEngine {
662666
pub(crate) async fn get_rtp_parameters_by_kind(
663667
&self,
664668
typ: RTPCodecType,
665-
directions: &[RTCRtpTransceiverDirection],
669+
direction: RTCRtpTransceiverDirection,
666670
) -> RTCRtpParameters {
667671
let mut header_extensions = vec![];
668672

@@ -671,7 +675,7 @@ impl MediaEngine {
671675
{
672676
let negotiated_header_extensions = self.negotiated_header_extensions.lock().await;
673677
for (id, e) in &*negotiated_header_extensions {
674-
if have_rtp_transceiver_direction_intersection(&e.allowed_directions, directions)
678+
if e.is_matching_direction(direction)
675679
&& (e.is_audio && typ == RTPCodecType::Audio
676680
|| e.is_video && typ == RTPCodecType::Video)
677681
{
@@ -686,11 +690,9 @@ impl MediaEngine {
686690
let mut negotiated_header_extensions = self.negotiated_header_extensions.lock().await;
687691

688692
for local_extension in &self.header_extensions {
689-
let relevant = have_rtp_transceiver_direction_intersection(
690-
&local_extension.allowed_directions,
691-
directions,
692-
) && (local_extension.is_audio && typ == RTPCodecType::Audio
693-
|| local_extension.is_video && typ == RTPCodecType::Video);
693+
let relevant = local_extension.is_matching_direction(direction)
694+
&& (local_extension.is_audio && typ == RTPCodecType::Audio
695+
|| local_extension.is_video && typ == RTPCodecType::Video);
694696

695697
if !relevant {
696698
continue;
@@ -739,7 +741,7 @@ impl MediaEngine {
739741
uri: local_extension.uri.clone(),
740742
is_audio: local_extension.is_audio,
741743
is_video: local_extension.is_video,
742-
allowed_directions: local_extension.allowed_directions.clone(),
744+
allowed_direction: local_extension.allowed_direction,
743745
},
744746
);
745747

‎webrtc/src/error.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,9 @@ pub enum Error {
205205
#[error("the requested codec does not have a payloader")]
206206
ErrNoPayloaderForCodec,
207207

208-
/// ErrRegisterHeaderExtensionInvalidDirection indicates that a extension was registered with a direction besides `sendonly` or `recvonly`
209-
#[error("a header extension must be registered as 'recvonly', 'sendonly' or both")]
208+
/// ErrRegisterHeaderExtensionInvalidDirection indicates that a extension was registered with different
209+
/// directions for two different calls.
210+
#[error("a header extension must be registered with the same direction each time")]
210211
ErrRegisterHeaderExtensionInvalidDirection,
211212

212213
/// ErrRegisterHeaderExtensionNoFreeID indicates that there was no extension ID available which

0 commit comments

Comments
 (0)