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

sdp: Use vector instead of HashMap for SimulcastRid #532

Merged
merged 1 commit into from
Feb 10, 2024
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
16 changes: 8 additions & 8 deletions webrtc/src/peer_connection/sdp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ pub(crate) fn track_details_from_sdp(
rids: vec![],
..Default::default()
};
for rid in rids.keys() {
simulcast_track.rids.push(SmolStr::from(rid));
for rid in &rids {
simulcast_track.rids.push(SmolStr::from(&rid.id));
}
if simulcast_track.rids.len() == tracks_in_media_section.len() {
for track in &tracks_in_media_section {
Expand All @@ -239,8 +239,8 @@ pub(crate) fn track_details_from_sdp(
incoming_tracks
}

pub(crate) fn get_rids(media: &MediaDescription) -> HashMap<String, SimulcastRid> {
let mut rids = HashMap::new();
pub(crate) fn get_rids(media: &MediaDescription) -> Vec<SimulcastRid> {
let mut rids = vec![];
let mut simulcast_attr: Option<String> = None;
for attr in &media.attributes {
if attr.key.as_str() == SDP_ATTRIBUTE_RID {
Expand All @@ -249,7 +249,7 @@ pub(crate) fn get_rids(media: &MediaDescription) -> HashMap<String, SimulcastRid
.as_ref()
.ok_or(SimulcastRidParseError::SyntaxIdDirSplit)
.and_then(SimulcastRid::try_from)
.map(|rid| rids.insert(rid.id.to_owned(), rid))
.map(|rid| rids.push(rid))
{
log::warn!("Failed to parse RID: {}", err);
}
Expand All @@ -272,7 +272,7 @@ pub(crate) fn get_rids(media: &MediaDescription) -> HashMap<String, SimulcastRid
(sc_id, false)
};

if let Some(rid) = rids.get_mut(sc_id) {
if let Some(rid) = rids.iter_mut().find(|f| f.id == sc_id) {
rid.paused = paused;
}
}
Expand Down Expand Up @@ -551,7 +551,7 @@ pub(crate) async fn add_transceiver_sdp(
let mut recv_sc_list: Vec<String> = vec![];
let mut send_sc_list: Vec<String> = vec![];

for rid in media_section.rid_map.values() {
for rid in &media_section.rid_map {
let rid_syntax = match rid.direction {
SimulcastDirection::Send => {
// If Send rid, then reply with a recv rid
Expand Down Expand Up @@ -749,7 +749,7 @@ pub(crate) struct MediaSection {
pub(crate) id: String,
pub(crate) transceivers: Vec<Arc<RTCRtpTransceiver>>,
pub(crate) data: bool,
pub(crate) rid_map: HashMap<String, SimulcastRid>,
pub(crate) rid_map: Vec<SimulcastRid>,
pub(crate) offered_direction: Option<RTCRtpTransceiverDirection>,
}

Expand Down
28 changes: 11 additions & 17 deletions webrtc/src/peer_connection/sdp/sdp_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,27 +702,20 @@ async fn test_populate_sdp() -> Result<()> {
)
.await;

let mut rid_map = HashMap::new();
let rid_id = "ridkey".to_owned();
rid_map.insert(
rid_id.to_owned(),
let rid_map = vec![
SimulcastRid {
id: rid_id,
id: "ridkey".to_owned(),
direction: SimulcastDirection::Recv,
params: "some".to_owned(),
paused: false,
},
);
let rid_id = "ridpaused".to_owned();
rid_map.insert(
rid_id.to_owned(),
SimulcastRid {
id: rid_id,
id: "ridpaused".to_owned(),
direction: SimulcastDirection::Recv,
params: "some2".to_owned(),
paused: true,
},
);
];
let media_sections = vec![MediaSection {
id: "video".to_owned(),
transceivers: vec![tr],
Expand Down Expand Up @@ -758,7 +751,7 @@ async fn test_populate_sdp() -> Result<()> {
}

let rid_map = get_rids(desc);
if let Some(rid) = rid_map.get("ridkey") {
if let Some(rid) = rid_map.iter().find(|rid| rid.id == "ridkey") {
assert!(!rid.paused, "Rid should be active");
assert_eq!(
rid.direction,
Expand All @@ -767,7 +760,7 @@ async fn test_populate_sdp() -> Result<()> {
);
found += 1;
}
if let Some(rid) = rid_map.get("ridpaused") {
if let Some(rid) = rid_map.iter().find(|rid| rid.id == "ridpaused") {
assert!(rid.paused, "Rid should be paused");
assert_eq!(
rid.direction,
Expand Down Expand Up @@ -831,7 +824,7 @@ async fn test_populate_sdp() -> Result<()> {
id: "video".to_owned(),
transceivers: vec![tr],
data: false,
rid_map: HashMap::new(),
rid_map: vec![],
..Default::default()
}];

Expand Down Expand Up @@ -949,14 +942,14 @@ async fn test_populate_sdp_reject() -> Result<()> {
id: "video".to_owned(),
transceivers: vec![trv],
data: false,
rid_map: HashMap::new(),
rid_map: vec![],
..Default::default()
},
MediaSection {
id: "audio".to_owned(),
transceivers: vec![tra],
data: false,
rid_map: HashMap::new(),
rid_map: vec![],
..Default::default()
},
];
Expand Down Expand Up @@ -1035,7 +1028,8 @@ fn test_get_rids() {

assert!(!rids.is_empty(), "Rid mapping should be present");

assert!(rids.get("f").is_some(), "rid values should contain 'f'");
let f = rids.iter().find(|rid| rid.id == "f");
assert!(f.is_some(), "rid values should contain 'f'");
}

#[test]
Expand Down
Loading