Skip to content

Commit 0bfb264

Browse files
algestenalgesten
committed
One single direction per extmap
SDP has the syntax: a=extmap:<id>/<direction> <uri> The /<direction> part is optional (and rarely seen it in the wild). The current MediaEngine API makes it seem like it's possible to provide many directions for each map (it takes a Vec). This commit changes that sov only single optional direction can be provided. This simplifies internal matching logic and fixes a bug where setting the transceiver direction `inactive`, would (incorrectly) result in no extmaps matching for the m-line.
1 parent 7a93dc8 commit 0bfb264

File tree

9 files changed

+67
-124
lines changed

9 files changed

+67
-124
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/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: 32 additions & 34 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
@@ -329,23 +340,8 @@ impl MediaEngine {
329340
&mut self,
330341
extension: RTCRtpHeaderExtensionCapability,
331342
typ: RTPCodecType,
332-
mut allowed_directions: Vec<RTCRtpTransceiverDirection>,
343+
allowed_direction: Option<RTCRtpTransceiverDirection>,
333344
) -> 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-
349345
let ext = {
350346
match self
351347
.header_extensions
@@ -358,8 +354,10 @@ impl MediaEngine {
358354
if self.header_extensions.len() > VALID_EXT_IDS.end as usize {
359355
return Err(Error::ErrRegisterHeaderExtensionNoFreeID);
360356
}
361-
self.header_extensions
362-
.push(MediaEngineHeaderExtension::default());
357+
self.header_extensions.push(MediaEngineHeaderExtension {
358+
allowed_direction,
359+
..Default::default()
360+
});
363361

364362
// Unwrap is fine because we just pushed
365363
self.header_extensions.last_mut().unwrap()
@@ -374,8 +372,10 @@ impl MediaEngine {
374372
}
375373

376374
ext.uri = extension.uri;
377-
// TODO: This just overrides the previous allowed directions, which feels wrong
378-
ext.allowed_directions = allowed_directions;
375+
376+
if ext.allowed_direction != allowed_direction {
377+
return Err(Error::ErrRegisterHeaderExtensionInvalidDirection);
378+
}
379379

380380
Ok(())
381381
}
@@ -559,7 +559,7 @@ impl MediaEngine {
559559
uri: extension.to_owned(),
560560
is_audio: local_extension.is_audio && typ == RTPCodecType::Audio,
561561
is_video: local_extension.is_video && typ == RTPCodecType::Video,
562-
allowed_directions: local_extension.allowed_directions.clone(),
562+
allowed_direction: local_extension.allowed_direction.clone(),
563563
};
564564
negotiated_header_extensions.insert(id, h);
565565
}
@@ -662,7 +662,7 @@ impl MediaEngine {
662662
pub(crate) async fn get_rtp_parameters_by_kind(
663663
&self,
664664
typ: RTPCodecType,
665-
directions: &[RTCRtpTransceiverDirection],
665+
direction: RTCRtpTransceiverDirection,
666666
) -> RTCRtpParameters {
667667
let mut header_extensions = vec![];
668668

@@ -671,7 +671,7 @@ impl MediaEngine {
671671
{
672672
let negotiated_header_extensions = self.negotiated_header_extensions.lock().await;
673673
for (id, e) in &*negotiated_header_extensions {
674-
if have_rtp_transceiver_direction_intersection(&e.allowed_directions, directions)
674+
if e.is_matching_direction(direction)
675675
&& (e.is_audio && typ == RTPCodecType::Audio
676676
|| e.is_video && typ == RTPCodecType::Video)
677677
{
@@ -686,11 +686,9 @@ impl MediaEngine {
686686
let mut negotiated_header_extensions = self.negotiated_header_extensions.lock().await;
687687

688688
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);
689+
let relevant = local_extension.is_matching_direction(direction)
690+
&& (local_extension.is_audio && typ == RTPCodecType::Audio
691+
|| local_extension.is_video && typ == RTPCodecType::Video);
694692

695693
if !relevant {
696694
continue;
@@ -739,7 +737,7 @@ impl MediaEngine {
739737
uri: local_extension.uri.clone(),
740738
is_audio: local_extension.is_audio,
741739
is_video: local_extension.is_video,
742-
allowed_directions: local_extension.allowed_directions.clone(),
740+
allowed_direction: local_extension.allowed_direction.clone(),
743741
},
744742
);
745743

‎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

‎webrtc/src/peer_connection/sdp/mod.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -505,16 +505,8 @@ pub(crate) async fn add_transceiver_sdp(
505505
return Ok((d, false));
506506
}
507507

508-
let mut directions = vec![];
509-
if t.sender().await.is_some() {
510-
directions.push(RTCRtpTransceiverDirection::Sendonly);
511-
}
512-
if t.receiver().await.is_some() {
513-
directions.push(RTCRtpTransceiverDirection::Recvonly);
514-
}
515-
516508
let parameters = media_engine
517-
.get_rtp_parameters_by_kind(t.kind, &directions)
509+
.get_rtp_parameters_by_kind(t.kind, t.direction())
518510
.await;
519511
for rtp_extension in &parameters.header_extensions {
520512
let ext_url = Url::parse(rtp_extension.uri.as_str())?;

‎webrtc/src/rtp_transceiver/rtp_receiver/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ impl RTPReceiverInternal {
312312
async fn get_parameters(&self) -> RTCRtpParameters {
313313
let mut parameters = self
314314
.media_engine
315-
.get_rtp_parameters_by_kind(self.kind, &[RTCRtpTransceiverDirection::Recvonly])
315+
.get_rtp_parameters_by_kind(self.kind, RTCRtpTransceiverDirection::Recvonly)
316316
.await;
317317

318318
let transceiver_codecs = self.transceiver_codecs.lock().await;

0 commit comments

Comments
 (0)