Re-instate changes from webrtc-rs#217

Author: robashton

webrtc/src/peer_connection/mod.rs

@@ -466,12 +466,12 @@ impl RTCPeerConnection {
                     if let Some(m) = m {
                         // Step 5.3.1
                         if t.direction().has_send() {
-                            let _dmsid = match m.attribute(ATTR_KEY_MSID).and_then(|o| o) {
+                            let dmsid = match m.attribute(ATTR_KEY_MSID).and_then(|o| o) {
                                 Some(m) => m,
                                 None => return true, // doesn't contain a single a=msid line
                             };
 
-                            let _sender = match t.sender().await {
+                            let sender = match t.sender().await {
                                 Some(s) => s,
                                 None => {
                                     log::warn!(
@@ -490,17 +490,15 @@ impl RTCPeerConnection {
                             // local description so we can compare all of them. For no we only
                             // consider the first one.
 
-                            // TODO: Go and see what Pion does these days here
-                            //                            let stream_ids = sender.associated_media_stream_ids();
-                            //                            // Different number of lines, 1 vs 0
-                            //                            if stream_ids.is_empty() {
-                            return true;
-                            //                            }
-
+                            let stream_ids = sender.associated_media_stream_ids();
+                            // Different number of lines, 1 vs 0
+                            if stream_ids.is_empty() {
+                                return true;
+                            }
                             // different stream id
-                            //                            if dmsid.split_whitespace().next() != Some(&stream_ids[0]) {
-                            //                                return true;
-                            //                            }
+                            if dmsid.split_whitespace().next() != Some(&stream_ids[0]) {
+                                return true;
+                            }
                         }
                         match local_desc.sdp_type {
                             RTCSdpType::Offer => {

webrtc/src/peer_connection/sdp/mod.rs

@@ -545,33 +545,17 @@ pub(crate) async fn add_transceiver_sdp(
         if let Some(sender) = mt.sender().await {
             let send_parameters = sender.get_parameters().await;
             if let Some(track) = sender.track().await {
+                // Get the different encodings expressed first
                 for encoding in send_parameters.encodings.iter() {
                     media = media.with_media_source(
                         encoding.ssrc,
                         track.stream_id().to_owned(), /* cname */
                         track.stream_id().to_owned(), /* streamLabel */
                         track.id().to_owned(),
                     );
-
-                    // Send msid based on the configured track if we haven't already
-                    // sent on this sender. If we have sent we must keep the msid line consistent, this
-                    // is handled below.
-                    if !is_plan_b {
-                        // We need this?
-                        // related streams don't exist any more in pion (and nor should they?)
-
-                        // I this should become obsolete
-                        if sender.initial_track_id().is_none() {
-                            media = media.with_property_attribute(format!(
-                                "msid:{} {}",
-                                track.stream_id(),
-                                track.id()
-                            ));
-                            sender.set_initial_track_id(track.id().to_string())?;
-                        }
-                    }
                 }
 
+                // Then tell the world about simulcast
                 if send_parameters.encodings.len() > 1 {
                     let mut send_rids: Vec<String> = vec![];
 
@@ -586,10 +570,43 @@ pub(crate) async fn add_transceiver_sdp(
                     s.push_str(send_rids.join(";").as_ref());
                     media = media.with_value_attribute("simulcast".into(), s);
                 }
+                // Send msid based on the configured track if we haven't already
+                // sent on this sender. If we have sent we must keep the msid line consistent, this
+                // is handled below.
+                // And now when we 'break', the above will have been printed properly still?
+                if !is_plan_b && sender.initial_track_id().is_none() {
+                    for stream_id in sender.associated_media_stream_ids() {
+                        media = media.with_property_attribute(format!(
+                            "msid:{} {}",
+                            stream_id,
+                            track.id()
+                        ));
+                    }
+                    sender.set_initial_track_id(track.id().to_string())?;
+                    break;
+                }
+            }
+            if !is_plan_b {
+                if let Some(track_id) = sender.initial_track_id() {
+                    // After we have include an msid attribute in an offer it must stay the same for
+                    // all subsequent offer even if the track or transceiver direction changes.
+                    //
+                    // [RFC 8829 Section 5.2.2](https://datatracker.ietf.org/doc/html/rfc8829#section-5.2.2)
+                    //
+                    // For RtpTransceivers that are not stopped, the "a=msid" line or
+                    // lines MUST stay the same if they are present in the current
+                    // description, regardless of changes to the transceiver's direction
+                    // or track.  If no "a=msid" line is present in the current
+                    // description, "a=msid" line(s) MUST be generated according to the
+                    // same rules as for an initial offer.
+                    for stream_id in sender.associated_media_stream_ids() {
+                        media = media
+                            .with_property_attribute(format!("msid:{} {}", stream_id, track_id));
+                    }
+
+                    break;
+                }
             }
-        }
-        if !is_plan_b {
-            break;
         }
     }
 

webrtc/src/rtp_transceiver/rtp_sender/mod.rs

@@ -108,6 +108,9 @@ pub struct RTCRtpSender {
     /// track id should be use when negotiating.
     pub(crate) initial_track_id: std::sync::Mutex<Option<String>>,
 
+    /// AssociatedMediaStreamIds from the WebRTC specifcations
+    pub(crate) associated_media_stream_ids: std::sync::Mutex<Vec<String>>,
+
     rtp_transceiver: Mutex<Option<Weak<RTCRtpTransceiver>>>,
 
     send_called_tx: Mutex<Option<mpsc::Sender<()>>>,
@@ -144,6 +147,7 @@ impl RTCRtpSender {
         let stop_called_tx = Arc::new(Notify::new());
         let stop_called_rx = stop_called_tx.clone();
         let stop_called_signal = Arc::new(AtomicBool::new(false));
+        let stream_ids = vec![track.stream_id().to_string()];
 
         let internal = Arc::new(RTPSenderInternal {
             send_called_rx: Mutex::new(send_called_rx),
@@ -165,6 +169,7 @@ impl RTCRtpSender {
 
             id,
             initial_track_id: std::sync::Mutex::new(None),
+            associated_media_stream_ids: std::sync::Mutex::new(stream_ids),
             kind: track.kind(),
 
             rtp_transceiver: Mutex::new(None),
@@ -627,6 +632,12 @@ impl RTCRtpSender {
         lock.clone()
     }
 
+    pub(crate) fn associated_media_stream_ids(&self) -> Vec<String> {
+        let lock = self.associated_media_stream_ids.lock().unwrap();
+
+        lock.clone()
+    }
+
     pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> {
         let mut lock = self.initial_track_id.lock().unwrap();