Skip to content

Commit 3be403f

Browse files
rogurotusrogurotus
authored
Make RTCRtpTransceiver.mid spec-compliant (#375)
Change RTCRtpTransceiver::mid to be spec compliant. Also replace the use of `tokio::sync::Mutex` with the more appropriate `tokio::sync::OnceCell`
1 parent e60607f commit 3be403f

File tree

6 files changed

+75
-80
lines changed

6 files changed

+75
-80
lines changed

‎webrtc/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
* Change `RTCPeerConnection::on_track` callback signature to `|track: Arc<TrackRemote>, receiver: Arc<RTCRtpReceiver>, transceiver: Arc<RTCRtpTransceiver>|` [#355](https://github.com/webrtc-rs/webrtc/pull/355).
1111

12+
* Change `RTCPeerConnection::mid` return signature to `Option<String>` [#375](https://github.com/webrtc-rs/webrtc/pull/375).
13+
1214
## v0.6.0
1315

1416
* Added more stats to `RemoteInboundRTPStats` and `RemoteOutboundRTPStats` [#282](https://github.com/webrtc-rs/webrtc/pull/282) by [@k0nserv](https://github.com/k0nserv).

‎webrtc/src/peer_connection/mod.rs

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -454,13 +454,14 @@ impl RTCPeerConnection {
454454
// if t.stopping && !t.stopped {
455455
// return true
456456
// }
457-
let mid = t.mid().await;
458-
let m = get_by_mid(&mid, local_desc);
457+
let mid = t.mid();
458+
let m = mid.as_ref().and_then(|mid| get_by_mid(mid, local_desc));
459459
// Step 5.2
460-
if !t.stopped.load(Ordering::SeqCst) && m.is_none() {
461-
return true;
462-
}
463460
if !t.stopped.load(Ordering::SeqCst) {
461+
if m.is_none() {
462+
return true;
463+
}
464+
464465
if let Some(m) = m {
465466
// Step 5.3.1
466467
if t.direction().has_send() {
@@ -473,7 +474,7 @@ impl RTCPeerConnection {
473474
Some(s) => s.clone(),
474475
None => {
475476
log::warn!(
476-
"RtpSender missing for transeceiver with sending direction {} for mid {}",
477+
"RtpSender missing for transeceiver with sending direction {} for mid {:?}",
477478
t.direction(),
478479
mid
479480
);
@@ -504,7 +505,7 @@ impl RTCPeerConnection {
504505
// Step 5.3.2
505506
if let Some(remote_desc) = &current_remote_description {
506507
if let Some(rm) =
507-
get_by_mid(t.mid().await.as_str(), remote_desc)
508+
t.mid().and_then(|mid| get_by_mid(&mid, remote_desc))
508509
{
509510
if get_peer_direction(m) != t.direction()
510511
&& get_peer_direction(rm) != t.direction().reverse()
@@ -522,7 +523,7 @@ impl RTCPeerConnection {
522523
None => return true,
523524
};
524525
let offered_direction =
525-
match get_by_mid(t.mid().await.as_str(), remote_desc) {
526+
match t.mid().and_then(|mid| get_by_mid(&mid, remote_desc)) {
526527
Some(d) => {
527528
let dir = get_peer_direction(d);
528529
if dir == RTCRtpTransceiverDirection::Unspecified {
@@ -547,14 +548,15 @@ impl RTCPeerConnection {
547548
}
548549
}
549550
// Step 5.4
550-
if t.stopped.load(Ordering::SeqCst) && !t.mid().await.is_empty() {
551-
let current_remote_description = params.current_remote_description.lock().await;
552-
if let Some(remote_desc) = &*current_remote_description {
553-
if get_by_mid(t.mid().await.as_str(), local_desc).is_some()
554-
|| get_by_mid(t.mid().await.as_str(), remote_desc).is_some()
555-
{
556-
return true;
557-
}
551+
if t.stopped.load(Ordering::SeqCst) {
552+
let search_mid = match t.mid() {
553+
Some(mid) => mid,
554+
None => return false,
555+
};
556+
557+
if let Some(remote_desc) = &*params.current_remote_description.lock().await {
558+
return get_by_mid(&search_mid, local_desc).is_some()
559+
|| get_by_mid(&search_mid, remote_desc).is_some();
558560
}
559561
}
560562
}
@@ -787,7 +789,7 @@ impl RTCPeerConnection {
787789
}
788790
}
789791
for t in &current_transceivers {
790-
if !t.mid().await.is_empty() {
792+
if t.mid().is_some() {
791793
continue;
792794
}
793795

@@ -804,10 +806,10 @@ impl RTCPeerConnection {
804806
}
805807
}
806808

807-
t.set_mid(mid).await?;
809+
t.set_mid(mid)?;
808810
} else {
809811
let greater_mid = self.internal.greater_mid.fetch_add(1, Ordering::SeqCst);
810-
t.set_mid(format!("{}", greater_mid + 1)).await?;
812+
t.set_mid(format!("{}", greater_mid + 1))?;
811813
}
812814
}
813815

@@ -1366,8 +1368,8 @@ impl RTCPeerConnection {
13661368
};
13671369

13681370
if let Some(t) = t {
1369-
if t.mid().await.is_empty() {
1370-
t.set_mid(mid_value.to_owned()).await?;
1371+
if t.mid().is_none() {
1372+
t.set_mid(mid_value.to_owned())?;
13711373
}
13721374
} else {
13731375
let receiver = Arc::new(RTCRtpReceiver::new(
@@ -1384,7 +1386,6 @@ impl RTCPeerConnection {
13841386
} else {
13851387
RTCRtpTransceiverDirection::Recvonly
13861388
};
1387-
13881389
let t = RTCRtpTransceiver::new(
13891390
Some(receiver),
13901391
None,
@@ -1398,8 +1399,8 @@ impl RTCPeerConnection {
13981399

13991400
self.internal.add_rtp_transceiver(Arc::clone(&t)).await;
14001401

1401-
if t.mid().await.is_empty() {
1402-
t.set_mid(mid_value.to_owned()).await?;
1402+
if t.mid().is_none() {
1403+
t.set_mid(mid_value.to_owned())?;
14031404
}
14041405
}
14051406
}

‎webrtc/src/peer_connection/peer_connection_internal.rs

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ impl PeerConnectionInternal {
346346
for incoming_track in filtered_tracks.iter() {
347347
let mut track_handled = false;
348348
for t in local_transceivers {
349-
if t.mid().await != incoming_track.mid {
349+
if t.mid().as_ref() != Some(&incoming_track.mid) {
350350
continue;
351351
}
352352

@@ -684,7 +684,7 @@ impl PeerConnectionInternal {
684684
sender.set_negotiated();
685685
}
686686
media_sections.push(MediaSection {
687-
id: t.mid().await,
687+
id: t.mid().unwrap(),
688688
transceivers: vec![Arc::clone(t)],
689689
..Default::default()
690690
});
@@ -802,7 +802,7 @@ impl PeerConnectionInternal {
802802
sender.set_negotiated();
803803
}
804804
media_sections.push(MediaSection {
805-
id: t.mid().await,
805+
id: t.mid().unwrap(),
806806
transceivers: vec![Arc::clone(t)],
807807
..Default::default()
808808
});
@@ -1011,7 +1011,7 @@ impl PeerConnectionInternal {
10111011

10121012
let transceivers = self.rtp_transceivers.lock().await;
10131013
for t in &*transceivers {
1014-
if t.mid().await != mid || t.receiver().await.is_none() {
1014+
if t.mid().as_ref() != Some(&mid) || t.receiver().await.is_none() {
10151015
continue;
10161016
}
10171017

@@ -1182,11 +1182,12 @@ impl PeerConnectionInternal {
11821182
pub(super) async fn has_local_description_changed(&self, desc: &RTCSessionDescription) -> bool {
11831183
let rtp_transceivers = self.rtp_transceivers.lock().await;
11841184
for t in &*rtp_transceivers {
1185-
if let Some(m) = get_by_mid(t.mid().await.as_str(), desc) {
1186-
if get_peer_direction(m) != t.direction() {
1187-
return true;
1188-
}
1189-
} else {
1185+
let m = match t.mid().and_then(|mid| get_by_mid(&mid, desc)) {
1186+
Some(m) => m,
1187+
None => return true,
1188+
};
1189+
1190+
if get_peer_direction(m) != t.direction() {
11901191
return true;
11911192
}
11921193
}
@@ -1228,27 +1229,25 @@ impl PeerConnectionInternal {
12281229
Some(r) => r,
12291230
None => continue,
12301231
};
1231-
let mid = match transeiver.mid().await {
1232-
m if !m.is_empty() => m,
1233-
_ => continue,
1234-
};
12351232

1236-
let tracks = receiver.tracks().await;
1233+
if let Some(mid) = transeiver.mid() {
1234+
let tracks = receiver.tracks().await;
12371235

1238-
for track in tracks {
1239-
let track_id = track.id().await;
1240-
let kind = match track.kind() {
1241-
RTPCodecType::Unspecified => continue,
1242-
RTPCodecType::Audio => "audio",
1243-
RTPCodecType::Video => "video",
1244-
};
1236+
for track in tracks {
1237+
let track_id = track.id().await;
1238+
let kind = match track.kind() {
1239+
RTPCodecType::Unspecified => continue,
1240+
RTPCodecType::Audio => "audio",
1241+
RTPCodecType::Video => "video",
1242+
};
12451243

1246-
track_infos.push(TrackInfo {
1247-
ssrc: track.ssrc(),
1248-
mid: mid.clone(),
1249-
track_id,
1250-
kind,
1251-
});
1244+
track_infos.push(TrackInfo {
1245+
ssrc: track.ssrc(),
1246+
mid: mid.clone(),
1247+
track_id,
1248+
kind,
1249+
});
1250+
}
12521251
}
12531252
}
12541253

@@ -1353,18 +1352,19 @@ impl PeerConnectionInternal {
13531352
kind: &'static str,
13541353
}
13551354
let mut track_infos = vec![];
1356-
for transeiver in transceivers {
1357-
let sender = match transeiver.sender().await {
1355+
for transceiver in transceivers {
1356+
let sender = match transceiver.sender().await {
13581357
Some(r) => r,
13591358
None => continue,
13601359
};
1361-
let mid = match transeiver.mid().await {
1362-
m if !m.is_empty() => m,
1363-
_ => continue,
1360+
1361+
let mid = match transceiver.mid() {
1362+
Some(mid) => mid,
1363+
None => continue,
13641364
};
13651365

13661366
let track = match sender.track().await {
1367-
Some(t) => t,
1367+
Some(track) => track,
13681368
None => continue,
13691369
};
13701370

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,7 @@ pub(crate) fn get_by_mid<'a, 'b>(
855855
) -> Option<&'b MediaDescription> {
856856
if let Some(parsed) = &desc.parsed {
857857
for m in &parsed.media_descriptions {
858-
if let Some(mid) = m.attribute(ATTR_KEY_MID).and_then(|o| o) {
858+
if let Some(mid) = m.attribute(ATTR_KEY_MID).flatten() {
859859
if mid == search_mid {
860860
return Some(m);
861861
}

‎webrtc/src/rtp_transceiver/mod.rs

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use std::future::Future;
2121
use std::pin::Pin;
2222
use std::sync::atomic::{AtomicBool, AtomicU8, Ordering};
2323
use std::sync::Arc;
24-
use tokio::sync::Mutex;
24+
use tokio::sync::{Mutex, OnceCell};
2525
use util::Unmarshal;
2626

2727
pub(crate) mod fmtp;
@@ -172,7 +172,7 @@ pub type TriggerNegotiationNeededFnOption =
172172

173173
/// RTPTransceiver represents a combination of an RTPSender and an RTPReceiver that share a common mid.
174174
pub struct RTCRtpTransceiver {
175-
mid: Mutex<String>, //atomic.Value
175+
mid: OnceCell<String>, //atomic.Value
176176
sender: Mutex<Option<Arc<RTCRtpSender>>>, //atomic.Value
177177
receiver: Mutex<Option<Arc<RTCRtpReceiver>>>, //atomic.Value
178178

@@ -200,7 +200,7 @@ impl RTCRtpTransceiver {
200200
trigger_negotiation_needed: TriggerNegotiationNeededFnOption,
201201
) -> Arc<Self> {
202202
let t = Arc::new(RTCRtpTransceiver {
203-
mid: Mutex::new(String::new()),
203+
mid: OnceCell::new(),
204204
sender: Mutex::new(None),
205205
receiver: Mutex::new(None),
206206

@@ -299,20 +299,15 @@ impl RTCRtpTransceiver {
299299
}
300300

301301
/// set_mid sets the RTPTransceiver's mid. If it was already set, will return an error.
302-
pub(crate) async fn set_mid(&self, mid: String) -> Result<()> {
303-
let mut m = self.mid.lock().await;
304-
if !m.is_empty() {
305-
return Err(Error::ErrRTPTransceiverCannotChangeMid);
306-
}
307-
*m = mid;
308-
309-
Ok(())
302+
pub(crate) fn set_mid(&self, mid: String) -> Result<()> {
303+
self.mid
304+
.set(mid)
305+
.map_err(|_| Error::ErrRTPTransceiverCannotChangeMid)
310306
}
311307

312308
/// mid gets the Transceiver's mid value. When not already set, this value will be set in CreateOffer or create_answer.
313-
pub async fn mid(&self) -> String {
314-
let mid = self.mid.lock().await;
315-
mid.clone()
309+
pub fn mid(&self) -> Option<String> {
310+
self.mid.get().map(Clone::clone)
316311
}
317312

318313
/// kind returns RTPTransceiver's kind.
@@ -394,9 +389,9 @@ impl RTCRtpTransceiver {
394389

395390
let current_direction = self.current_direction();
396391
if previous_direction != current_direction {
397-
let mid = self.mid().await;
392+
let mid = self.mid();
398393
trace!(
399-
"Processing transceiver({}) direction change from {} to {}",
394+
"Processing transceiver({:?}) direction change from {} to {}",
400395
mid,
401396
previous_direction,
402397
current_direction
@@ -498,7 +493,7 @@ pub(crate) async fn find_by_mid(
498493
local_transceivers: &mut Vec<Arc<RTCRtpTransceiver>>,
499494
) -> Option<Arc<RTCRtpTransceiver>> {
500495
for (i, t) in local_transceivers.iter().enumerate() {
501-
if t.mid().await == mid {
496+
if t.mid().as_deref() == Some(mid) {
502497
return Some(local_transceivers.remove(i));
503498
}
504499
}
@@ -531,10 +526,7 @@ pub(crate) async fn satisfy_type_and_direction(
531526

532527
for possible_direction in get_preferred_directions() {
533528
for (i, t) in local_transceivers.iter().enumerate() {
534-
if t.mid().await.is_empty()
535-
&& t.kind == remote_kind
536-
&& possible_direction == t.direction()
537-
{
529+
if t.mid().is_none() && t.kind == remote_kind && possible_direction == t.direction() {
538530
return Some(local_transceivers.remove(i));
539531
}
540532
}

‎webrtc/src/rtp_transceiver/rtp_transceiver_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ async fn test_rtp_transceiver_stopping() -> Result<()> {
322322
offer_pc.set_remote_description(answer).await?;
323323

324324
assert!(
325-
!offer_transceiver.mid().await.is_empty(),
325+
offer_transceiver.mid().is_some(),
326326
"A mid should have been associated with the transceiver when applying the answer"
327327
);
328328
// Stop the transceiver

0 commit comments

Comments
 (0)