From 27426325224c847b71b4840496c77a9f14b52f44 Mon Sep 17 00:00:00 2001 From: robashton Date: Fri, 7 Oct 2022 15:02:53 +0100 Subject: [PATCH 01/37] Tentative first pass at making simulcast egest possible This is not done "properly", and is an (almost) one for one copy of changes from: https://github.com/pion/webrtc/commit/37e16a3b15a3a474573b88eb7eeb728afaeb3c58 --- interceptor/src/stream_info.rs | 1 + webrtc/src/error.rs | 17 + webrtc/src/peer_connection/mod.rs | 17 +- .../peer_connection_internal.rs | 20 +- webrtc/src/peer_connection/sdp/mod.rs | 80 +-- webrtc/src/rtp_transceiver/mod.rs | 2 + .../src/rtp_transceiver/rtp_receiver/mod.rs | 2 + webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 506 +++++++++++------- .../rtp_sender/rtp_sender_test.rs | 2 +- webrtc/src/track/track_local/mod.rs | 12 +- .../track_local/track_local_static_rtp.rs | 10 + .../track_local/track_local_static_sample.rs | 9 + 12 files changed, 426 insertions(+), 252 deletions(-) diff --git a/interceptor/src/stream_info.rs b/interceptor/src/stream_info.rs index 139cdbc38..c4d0b7cb5 100644 --- a/interceptor/src/stream_info.rs +++ b/interceptor/src/stream_info.rs @@ -11,6 +11,7 @@ pub struct RTPHeaderExtension { #[derive(Default, Debug, Clone)] pub struct StreamInfo { pub id: String, + pub rid: String, pub attributes: Attributes, pub ssrc: u32, pub payload_type: u8, diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index de3fcf0c1..b45cd4ffd 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,6 +197,23 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, + #[error("Sender cannot encoding due to RID collision")] + ErrRTPSenderRIDCollision, + + #[error("Sender cannot add encoding as provided track does not match base track")] + ErrRTPSenderBaseEncodingMismatch, + + /// ErrRTPSenderStopped indicates the sender was already stopped + #[error("Sender has already been stopped")] + ErrRTPSenderStopped, + + /// ErrRTPSenderRidNil indicates that the track RID was empty + #[error("Sender cannot add encoding as rid is empty")] + ErrRTPSenderRidNil, + + #[error("Sender cannot add encoding as there is no base track")] + ErrRTPSenderNoBaseEncoding, + /// ErrUnbindFailed indicates that a TrackLocal was not able to be unbind #[error("failed to unbind TrackLocal from PeerConnection")] ErrUnbindFailed, diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs index cd678cdbe..dcee628ce 100644 --- a/webrtc/src/peer_connection/mod.rs +++ b/webrtc/src/peer_connection/mod.rs @@ -490,16 +490,17 @@ impl RTCPeerConnection { // local description so we can compare all of them. For no we only // consider the first one. - let stream_ids = sender.associated_media_stream_ids(); - // Different number of lines, 1 vs 0 - if stream_ids.is_empty() { - return true; - } + // 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; + // } // 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 => { diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 5c0539ef4..3efb1b729 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1141,6 +1141,7 @@ impl PeerConnectionInternal { let stream_info = create_stream_info( "".to_owned(), ssrc, + "".to_owned(), params.codecs[0].payload_type, params.codecs[0].capability.clone(), ¶ms.header_extensions, @@ -1535,20 +1536,23 @@ impl PeerConnectionInternal { None => continue, }; - let track_id = track.id().to_string(); let kind = match track.kind() { RTPCodecType::Unspecified => continue, RTPCodecType::Audio => "audio", RTPCodecType::Video => "video", }; - track_infos.push(TrackInfo { - track_id, - ssrc: sender.ssrc, - mid: mid.clone(), - rid: None, - kind, - }); + let encodings = sender.track_encodings.lock().await; + for e in encodings.iter() { + let track_id = track.id().to_string(); + track_infos.push(TrackInfo { + track_id, + ssrc: e.ssrc, + mid: mid.clone(), + rid: Some(track.rid().to_owned()), + kind, + }); + } } let stream_stats = self diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 385ffd799..12a7464e2 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -540,55 +540,57 @@ pub(crate) async fn add_transceiver_sdp( ); } + // This is equivalent to addSenderSDP in Pion (now) for mt in transceivers { if let Some(sender) = mt.sender().await { + let send_parameters = sender.get_parameters().await; if let Some(track) = sender.track().await { - media = media.with_media_source( - sender.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 && 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() - )); + 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())?; + } } - - 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)); - } + if send_parameters.encodings.len() > 1 { + let mut send_rids: Vec = vec![]; - break; + for e in send_parameters.encodings.iter() { + let mut s: String = e.rid.clone(); + s.push_str(" send"); + media = media.with_value_attribute("rid".into(), s); + send_rids.push(e.rid.clone()) + } + // Simulcast) + let mut s: String = "send ".to_owned(); + s.push_str(send_rids.join(";").as_ref()); + media = media.with_value_attribute("simulcast".into(), s); } } } + if !is_plan_b { + break; + } } let direction = match params.offered_direction { diff --git a/webrtc/src/rtp_transceiver/mod.rs b/webrtc/src/rtp_transceiver/mod.rs index 8e6058d52..3d2fcaea5 100644 --- a/webrtc/src/rtp_transceiver/mod.rs +++ b/webrtc/src/rtp_transceiver/mod.rs @@ -132,6 +132,7 @@ pub struct RTCRtpTransceiverInit { pub(crate) fn create_stream_info( id: String, ssrc: SSRC, + rid: String, payload_type: PayloadType, codec: RTCRtpCodecCapability, webrtc_header_extensions: &[RTCRtpHeaderExtensionParameters], @@ -154,6 +155,7 @@ pub(crate) fn create_stream_info( StreamInfo { id, + rid, attributes: Attributes::new(), ssrc, payload_type, diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs index 74f3bec3a..9eecc9a8b 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs @@ -529,6 +529,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), encoding.ssrc, + "".to_owned(), 0, codec.clone(), &global_params.header_extensions, @@ -586,6 +587,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), rtx_ssrc, + "".to_owned(), 0, codec.clone(), &global_params.header_extensions, diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 7e63bb62d..0aa267ce6 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -26,21 +26,20 @@ pub(crate) struct RTPSenderInternal { pub(crate) send_called_rx: Mutex>, pub(crate) stop_called_rx: Arc, pub(crate) stop_called_signal: Arc, - pub(crate) rtcp_interceptor: Mutex>>, } impl RTPSenderInternal { /// read reads incoming RTCP for this RTPReceiver - async fn read(&self, b: &mut [u8]) -> Result<(usize, Attributes)> { + async fn read( + &self, + encoding: &Arc, + b: &mut [u8], + ) -> Result<(usize, Attributes)> { let mut send_called_rx = self.send_called_rx.lock().await; tokio::select! { _ = send_called_rx.recv() =>{ - let rtcp_interceptor = { - let rtcp_interceptor = self.rtcp_interceptor.lock().await; - rtcp_interceptor.clone() - }; - if let Some(rtcp_interceptor) = rtcp_interceptor{ + let rtcp_interceptor = encoding.rtcp_interceptor.clone(); let a = Attributes::new(); tokio::select! { _ = self.stop_called_rx.notified() => { @@ -50,9 +49,6 @@ impl RTPSenderInternal { Ok(result?) } } - }else{ - Err(Error::ErrInterceptorNotBind) - } } _ = self.stop_called_rx.notified() =>{ Err(Error::ErrClosedPipe) @@ -63,10 +59,11 @@ impl RTPSenderInternal { /// read_rtcp is a convenience method that wraps Read and unmarshals for you. async fn read_rtcp( &self, + encoding: &Arc, receive_mtu: usize, ) -> Result<(Vec>, Attributes)> { let mut b = vec![0u8; receive_mtu]; - let (n, attributes) = self.read(&mut b).await?; + let (n, attributes) = self.read(encoding, &mut b).await?; let mut buf = &b[..n]; let pkts = rtcp::packet::unmarshal(&mut buf)?; @@ -75,19 +72,26 @@ impl RTPSenderInternal { } } -/// RTPSender allows an application to control how a given Track is encoded and transmitted to a remote peer -pub struct RTCRtpSender { +pub struct TrackEncoding { pub(crate) track: Mutex>>, pub(crate) srtp_stream: Arc, + + pub(crate) rtcp_interceptor: Arc, pub(crate) stream_info: Mutex, pub(crate) context: Mutex, + pub(crate) ssrc: SSRC, +} + +/// RTPSender allows an application to control how a given Track is encoded and transmitted to a remote peer +pub struct RTCRtpSender { + pub(crate) track_encodings: Mutex>>, + pub(crate) transport: Arc, pub(crate) payload_type: PayloadType, - pub(crate) ssrc: SSRC, receive_mtu: usize, /// a transceiver sender since we can just check the @@ -96,14 +100,13 @@ pub struct RTCRtpSender { pub(crate) media_engine: Arc, pub(crate) interceptor: Arc, + pub(crate) kind: RTPCodecType, pub(crate) id: String, /// The id of the initial track, even if we later change to a different /// track id should be use when negotiating. pub(crate) initial_track_id: std::sync::Mutex>, - /// AssociatedMediaStreamIds from the WebRTC specifcations - pub(crate) associated_media_stream_ids: std::sync::Mutex>, rtp_transceiver: Mutex>>, @@ -140,44 +143,19 @@ impl RTCRtpSender { let (send_called_tx, send_called_rx) = mpsc::channel(1); let stop_called_tx = Arc::new(Notify::new()); let stop_called_rx = stop_called_tx.clone(); - let ssrc = rand::random::(); let stop_called_signal = Arc::new(AtomicBool::new(false)); let internal = Arc::new(RTPSenderInternal { send_called_rx: Mutex::new(send_called_rx), stop_called_rx, stop_called_signal: Arc::clone(&stop_called_signal), - rtcp_interceptor: Mutex::new(None), }); - let srtp_stream = Arc::new(SrtpWriterFuture { - closed: AtomicBool::new(false), - ssrc, - rtp_sender: Arc::downgrade(&internal), - rtp_transport: Arc::clone(&transport), - rtcp_read_stream: Mutex::new(None), - rtp_write_session: Mutex::new(None), - }); - - let srtp_rtcp_reader = Arc::clone(&srtp_stream) as Arc; - let rtcp_interceptor = interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; - { - let mut internal_rtcp_interceptor = internal.rtcp_interceptor.lock().await; - *internal_rtcp_interceptor = Some(rtcp_interceptor); - } - - let stream_ids = vec![track.stream_id().to_string()]; - RTCRtpSender { - track: Mutex::new(Some(track)), - - srtp_stream, - stream_info: Mutex::new(StreamInfo::default()), - - context: Mutex::new(TrackLocalContext::default()), + let sender = RTCRtpSender { + track_encodings: Mutex::new(vec![]), transport, payload_type: 0, - ssrc, receive_mtu, negotiated: AtomicBool::new(false), @@ -187,7 +165,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), @@ -198,7 +176,85 @@ impl RTCRtpSender { paused: Arc::new(AtomicBool::new(start_paused)), internal, + }; + + sender.add_encoding_internal(track).await; + + // Add track + sender + } + + pub async fn add_encoding(&self, track: Arc) -> Result<()> { + if track.rid() == "" { + return Err(Error::ErrRTPSenderRidNil); + } + + if self.has_stopped().await { + return Err(Error::ErrRTPSenderStopped); } + + if self.has_sent().await { + return Err(Error::ErrRTPSenderSendAlreadyCalled); + } + + // oops, somebody code-golf this for me + { + let ref_track = if let Some(t) = self.first_encoding().await? { + let t = t.track.lock().await; + if let Some(t) = &*t { + if t.rid() != "" { + t.clone() + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + } + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + } + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + }; + + if ref_track.id() != track.id() + || ref_track.stream_id() != track.stream_id() + || ref_track.kind() != track.kind() + { + return Err(Error::ErrRTPSenderBaseEncodingMismatch); + } + + if self.encoding_for_rid(track.rid()).await.is_some() { + return Err(Error::ErrRTPSenderRIDCollision); + } + } + + self.add_encoding_internal(track).await; + Ok(()) + } + + pub(crate) async fn add_encoding_internal(&self, track: Arc) { + let ssrc = rand::random::(); + let srtp_stream = Arc::new(SrtpWriterFuture { + closed: AtomicBool::new(false), + ssrc, + rtp_sender: Arc::downgrade(&self.internal), + rtp_transport: Arc::clone(&self.transport), + rtcp_read_stream: Mutex::new(None), + rtp_write_session: Mutex::new(None), + }); + + let srtp_rtcp_reader = Arc::clone(&srtp_stream) as Arc; + let rtcp_interceptor = self.interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; + + let track_encoding = TrackEncoding { + track: Mutex::new(Some(track)), + srtp_stream, + ssrc, + rtcp_interceptor, + stream_info: Mutex::new(StreamInfo::default()), + context: Mutex::new(TrackLocalContext::default()), + }; + + let mut encodings = self.track_encodings.lock().await; + encodings.push(Arc::new(track_encoding)); } pub(crate) fn is_negotiated(&self) -> bool { @@ -233,27 +289,31 @@ impl RTCRtpSender { /// get_parameters describes the current configuration for the encoding and /// transmission of media on the sender's track. pub async fn get_parameters(&self) -> RTCRtpSendParameters { - let kind = { - let track = self.track.lock().await; - if let Some(t) = &*track { - t.kind() - } else { - RTPCodecType::default() - } - }; + let mut encodings: Vec = vec![]; - let mut send_parameters = { - RTCRtpSendParameters { - rtp_parameters: self - .media_engine - .get_rtp_parameters_by_kind(kind, &[RTCRtpTransceiverDirection::Sendonly]) - .await, - encodings: vec![RTCRtpEncodingParameters { - ssrc: self.ssrc, + { + let track_encodings = self.track_encodings.lock().await; + for te in track_encodings.iter() { + let track = te.track.lock().await; + let rid = track + .as_ref() + .map_or(String::from(""), |t| String::from(t.rid())); + + encodings.push(RTCRtpEncodingParameters { + ssrc: te.ssrc, payload_type: self.payload_type, + rid, ..Default::default() - }], + }) } + } + + let mut send_parameters = RTCRtpSendParameters { + rtp_parameters: self + .media_engine + .get_rtp_parameters_by_kind(self.kind, &[RTCRtpTransceiverDirection::Sendonly]) + .await, + encodings, }; let codecs = { @@ -262,10 +322,10 @@ impl RTCRtpSender { if let Some(t) = t.upgrade() { t.get_codecs().await } else { - self.media_engine.get_codecs_by_kind(kind).await + self.media_engine.get_codecs_by_kind(self.kind).await } } else { - self.media_engine.get_codecs_by_kind(kind).await + self.media_engine.get_codecs_by_kind(self.kind).await } }; send_parameters.rtp_parameters.codecs = codecs; @@ -275,8 +335,13 @@ impl RTCRtpSender { /// track returns the RTCRtpTransceiver track, or nil pub async fn track(&self) -> Option> { - let track = self.track.lock().await; - track.clone() + let encodings = self.track_encodings.lock().await; + if let Some(t) = encodings.first() { + let track = t.track.lock().await; + track.clone() + } else { + None + } } /// replace_track replaces the track currently being used as the sender's source with a new TrackLocal. @@ -287,6 +352,11 @@ impl RTCRtpSender { track: Option>, ) -> Result<()> { if let Some(t) = &track { + let encodings = self.track_encodings.lock().await; + if encodings.len() > 1 { + // return ErrRTPSenderNewTrackHasIncorrectEnvelope + return Err(Error::ErrRTPSenderNewTrackHasIncorrectKind); + } let tr = self.rtp_transceiver.lock().await; if let Some(r) = &*tr { if let Some(r) = r.upgrade() { @@ -301,68 +371,80 @@ impl RTCRtpSender { } } - if self.has_sent().await { - let t = { - let t = self.track.lock().await; - t.clone() - }; - if let Some(t) = t { - let context = self.context.lock().await; - t.unbind(&context).await?; + let encodings = self.track_encodings.lock().await; + if let Some(re) = encodings.first() { + if self.has_sent().await { + let t = { + let t = re.track.lock().await; + t.clone() + }; + if let Some(t) = t { + let context = re.context.lock().await; + t.unbind(&context).await?; + } } - } - if !self.has_sent().await || track.is_none() { - let mut t = self.track.lock().await; - *t = track; - return Ok(()); - } + if !self.has_sent().await || track.is_none() { + let mut t = re.track.lock().await; + *t = track; + return Ok(()); + } - let context = { - let context = self.context.lock().await; - context.clone() - }; + let context = { + let context = re.context.lock().await; + context.clone() + }; - let result = if let Some(t) = &track { - let new_context = TrackLocalContext { - id: context.id.clone(), - params: self - .media_engine - .get_rtp_parameters_by_kind(t.kind(), &[RTCRtpTransceiverDirection::Sendonly]) - .await, - ssrc: context.ssrc, - write_stream: context.write_stream.clone(), + let result = if let Some(t) = &track { + let new_context = TrackLocalContext { + id: context.id.clone(), + params: self + .media_engine + .get_rtp_parameters_by_kind( + t.kind(), + &[RTCRtpTransceiverDirection::Sendonly], + ) + .await, + ssrc: context.ssrc, + write_stream: context.write_stream.clone(), + rtcp_intercepter: context.rtcp_intercepter.clone(), + }; + + t.bind(&new_context).await + } else { + Err(Error::ErrRTPSenderTrackNil) }; - t.bind(&new_context).await - } else { - Err(Error::ErrRTPSenderTrackNil) - }; + match result { + Err(err) => { + // Re-bind the original track + let track = re.track.lock().await; + if let Some(t) = &*track { + t.bind(&context).await?; + } - match result { - Err(err) => { - // Re-bind the original track - let track = self.track.lock().await; - if let Some(t) = &*track { - t.bind(&context).await?; + Err(err) } + Ok(codec) => { + // Codec has changed + if self.payload_type != codec.payload_type { + let mut context = re.context.lock().await; + context.params.codecs = vec![codec]; + } - Err(err) - } - Ok(codec) => { - // Codec has changed - if self.payload_type != codec.payload_type { - let mut context = self.context.lock().await; - context.params.codecs = vec![codec]; - } + { + let mut t = re.track.lock().await; + *t = track; + } - { - let mut t = self.track.lock().await; - *t = track; + Ok(()) } - - Ok(()) } + } else { + // Is it though? + // How do we end up in a state where we don't have at the very least, the default track + // encoding? + Ok(()) } } @@ -372,66 +454,72 @@ impl RTCRtpSender { return Err(Error::ErrRTPSenderSendAlreadyCalled); } - let write_stream = Arc::new(InterceptorToTrackLocalWriter::new(self.paused.clone())); - let (context, stream_info) = { - let track = self.track.lock().await; - let mut context = TrackLocalContext { - id: self.id.clone(), - params: self - .media_engine - .get_rtp_parameters_by_kind( - if let Some(t) = &*track { - t.kind() - } else { - RTPCodecType::default() - }, - &[RTCRtpTransceiverDirection::Sendonly], - ) - .await, - ssrc: parameters.encodings[0].ssrc, - write_stream: Some( - Arc::clone(&write_stream) as Arc - ), - }; + // This is quite a long lived lock? + let encodings = self.track_encodings.lock().await; + for te in encodings.iter() { + let write_stream = Arc::new(InterceptorToTrackLocalWriter::new(self.paused.clone())); + let (context, stream_info) = { + let track = te.track.lock().await; + let mut context = TrackLocalContext { + id: self.id.clone(), + params: self + .media_engine + .get_rtp_parameters_by_kind( + if let Some(t) = &*track { + t.kind() + } else { + RTPCodecType::default() + }, + &[RTCRtpTransceiverDirection::Sendonly], + ) + .await, + ssrc: te.ssrc, + rtcp_intercepter: Some(te.rtcp_interceptor.clone()), + write_stream: Some( + Arc::clone(&write_stream) as Arc + ), + }; - let codec = if let Some(t) = &*track { - t.bind(&context).await? - } else { - RTCRtpCodecParameters::default() + let (codec, rid) = if let Some(t) = &*track { + let codec = t.bind(&context).await?; + (codec, t.rid()) + } else { + (RTCRtpCodecParameters::default(), "") + }; + let payload_type = codec.payload_type; + let capability = codec.capability.clone(); + context.params.codecs = vec![codec]; + let stream_info = create_stream_info( + self.id.clone(), + te.ssrc, + rid.to_owned(), + payload_type, + capability, + ¶meters.rtp_parameters.header_extensions, + ); + + (context, stream_info) }; - let payload_type = codec.payload_type; - let capability = codec.capability.clone(); - context.params.codecs = vec![codec]; - let stream_info = create_stream_info( - self.id.clone(), - parameters.encodings[0].ssrc, - payload_type, - capability, - ¶meters.rtp_parameters.header_extensions, - ); - - (context, stream_info) - }; - let srtp_rtp_writer = Arc::clone(&self.srtp_stream) as Arc; - let rtp_interceptor = self - .interceptor - .bind_local_stream(&stream_info, srtp_rtp_writer) - .await; - { - let mut interceptor_rtp_writer = write_stream.interceptor_rtp_writer.lock().await; - *interceptor_rtp_writer = Some(rtp_interceptor); - } + let srtp_rtp_writer = Arc::clone(&te.srtp_stream) as Arc; + let rtp_interceptor = self + .interceptor + .bind_local_stream(&stream_info, srtp_rtp_writer) + .await; + { + let mut interceptor_rtp_writer = write_stream.interceptor_rtp_writer.lock().await; + *interceptor_rtp_writer = Some(rtp_interceptor); + } - { - let mut ctx = self.context.lock().await; - *ctx = context; - } - { - let mut si = self.stream_info.lock().await; - *si = stream_info; + { + let mut ctx = te.context.lock().await; + *ctx = context; + } + { + let mut si = te.stream_info.lock().await; + *si = stream_info; + } } - { let mut send_called_tx = self.send_called_tx.lock().await; send_called_tx.take(); @@ -454,24 +542,72 @@ impl RTCRtpSender { self.replace_track(None).await?; - { - let stream_info = self.stream_info.lock().await; + let encodings = self.track_encodings.lock().await; + for te in encodings.iter() { + let stream_info = te.stream_info.lock().await; self.interceptor.unbind_local_stream(&stream_info).await; + te.srtp_stream.close().await? } - - self.srtp_stream.close().await + Ok(()) } /// read reads incoming RTCP for this RTPReceiver pub async fn read(&self, b: &mut [u8]) -> Result<(usize, Attributes)> { - self.internal.read(b).await + if let Some(encoding) = self.first_encoding().await? { + self.internal.read(&encoding, b).await + } else { + Err(Error::ErrInterceptorNotBind) + } + } + + // Having a mutex on that little collection sure does make this whole module fun + // These helpers exist because otherwise I accidentally end up locking on blocking calls + // because I'm incapable of writing threadsafe code + async fn encoding_for_rid(&self, rid: &str) -> Option> { + let encodings = self.track_encodings.lock().await; + for e in encodings.iter() { + if let Some(track) = &*e.track.lock().await { + if track.rid() == rid { + return Some(e.clone()); + } + }; + } + None + } + + async fn first_encoding(&self) -> Result>> { + let encodings = self.track_encodings.lock().await; + return Ok(encodings.first().map(|x| (*x).clone())); + } + + pub async fn read_simulcast(&self, b: &mut [u8], rid: &str) -> Result<(usize, Attributes)> { + if let Some(encoding) = self.encoding_for_rid(rid).await { + return self.internal.read(&encoding, b).await; + } else { + Err(Error::ErrInterceptorNotBind) + } + } + + pub async fn read_simulcast_rtcp( + &self, + rid: &str, + ) -> Result<(Vec>, Attributes)> { + if let Some(encoding) = self.encoding_for_rid(rid).await { + return self.internal.read_rtcp(&encoding, self.receive_mtu).await; + } else { + Err(Error::ErrInterceptorNotBind) + } } /// read_rtcp is a convenience method that wraps Read and unmarshals for you. pub async fn read_rtcp( &self, ) -> Result<(Vec>, Attributes)> { - self.internal.read_rtcp(self.receive_mtu).await + if let Some(encoding) = self.first_encoding().await? { + self.internal.read_rtcp(&encoding, self.receive_mtu).await + } else { + Err(Error::ErrInterceptorNotBind) + } } /// has_sent tells if data has been ever sent for this instance @@ -502,22 +638,4 @@ impl RTCRtpSender { Ok(()) } - - pub(crate) fn associate_media_stream_id(&self, id: String) -> bool { - let mut lock = self.associated_media_stream_ids.lock().unwrap(); - - if lock.contains(&id) { - return false; - } - - lock.push(id); - - true - } - - pub(crate) fn associated_media_stream_ids(&self) -> Vec { - let lock = self.associated_media_stream_ids.lock().unwrap(); - - lock.clone() - } } diff --git a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs index 12bcef3e4..f3486b8e3 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs @@ -145,7 +145,7 @@ async fn test_rtp_sender_get_parameters() -> Result<()> { let parameters = sender.get_parameters().await; assert_ne!(0, parameters.rtp_parameters.codecs.len()); assert_eq!(1, parameters.encodings.len()); - assert_eq!(sender.ssrc, parameters.encodings[0].ssrc); + assert_eq!(sender.track_encodings[0].ssrc, parameters.encodings[0].ssrc); } else { assert!(false); } diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index ffbcd3c96..7b5f574fc 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -9,7 +9,7 @@ use crate::rtp_transceiver::rtp_codec::*; use crate::rtp_transceiver::*; use async_trait::async_trait; -use interceptor::{Attributes, RTPWriter}; +use interceptor::{Attributes, RTCPReader, RTPWriter}; use std::any::Any; use std::fmt; use std::sync::atomic::{AtomicBool, Ordering}; @@ -29,12 +29,13 @@ pub trait TrackLocalWriter: fmt::Debug { /// TrackLocalContext is the Context passed when a TrackLocal has been Binded/Unbinded from a PeerConnection, and used /// in Interceptors. -#[derive(Default, Debug, Clone)] +#[derive(Default, Clone)] pub struct TrackLocalContext { pub(crate) id: String, pub(crate) params: RTCRtpParameters, pub(crate) ssrc: SSRC, pub(crate) write_stream: Option>, + pub(crate) rtcp_intercepter: Option>, } impl TrackLocalContext { @@ -62,6 +63,10 @@ impl TrackLocalContext { self.write_stream.clone() } + pub fn rtcp_reader(&self) -> Option> { + self.rtcp_intercepter.clone() + } + /// id is a unique identifier that is used for both bind/unbind pub fn id(&self) -> String { self.id.clone() @@ -86,6 +91,9 @@ pub trait TrackLocal { /// and stream_id would be 'desktop' or 'webcam' fn id(&self) -> &str; + /// rid is the RTP identifier for this Track + fn rid(&self) -> &str; + /// stream_id is the group this track belongs too. This must be unique fn stream_id(&self) -> &str; diff --git a/webrtc/src/track/track_local/track_local_static_rtp.rs b/webrtc/src/track/track_local/track_local_static_rtp.rs index 4974cd076..fc6561297 100644 --- a/webrtc/src/track/track_local/track_local_static_rtp.rs +++ b/webrtc/src/track/track_local/track_local_static_rtp.rs @@ -10,6 +10,7 @@ pub struct TrackLocalStaticRTP { pub(crate) bindings: Mutex>>, codec: RTCRtpCodecCapability, id: String, + rid: String, stream_id: String, } @@ -20,10 +21,15 @@ impl TrackLocalStaticRTP { codec, bindings: Mutex::new(vec![]), id, + rid: "".to_owned(), stream_id, } } + pub fn set_rid(&mut self, rid: String) { + self.rid = rid; + } + /// codec gets the Codec of the track pub fn codec(&self) -> RTCRtpCodecCapability { self.codec.clone() @@ -90,6 +96,10 @@ impl TrackLocal for TrackLocalStaticRTP { self.stream_id.as_str() } + fn rid(&self) -> &str { + self.rid.as_str() + } + /// kind controls if this TrackLocal is audio or video fn kind(&self) -> RTPCodecType { if self.codec.mime_type.starts_with("audio/") { diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index 9433e7921..e455b67bd 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -41,6 +41,11 @@ impl TrackLocalStaticSample { self.rtp_track.codec() } + // Sets the RID for this track + pub fn set_rid(&mut self, rid: String) { + self.rtp_track.set_rid(rid); + } + /// write_sample writes a Sample to the TrackLocalStaticSample /// If one PeerConnection fails the packets will still be sent to /// all PeerConnections. The error message will contain the ID of the failed @@ -133,6 +138,10 @@ impl TrackLocal for TrackLocalStaticSample { self.rtp_track.id() } + fn rid(&self) -> &str { + self.rtp_track.rid() + } + /// stream_id is the group this track belongs too. This must be unique fn stream_id(&self) -> &str { self.rtp_track.stream_id() From d4160e16051ef39b622f3ba3e7adf73586cb4ce3 Mon Sep 17 00:00:00 2001 From: robashton Date: Fri, 7 Oct 2022 15:45:26 +0100 Subject: [PATCH 02/37] Fix the tests (for now) --- webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs index f3486b8e3..f6ede2006 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs @@ -142,10 +142,11 @@ async fn test_rtp_sender_get_parameters() -> Result<()> { signal_pair(&mut offerer, &mut answerer).await?; if let Some(sender) = rtp_transceiver.sender().await { + let encoding = { sender.track_encodings.lock().await[0].clone() }; let parameters = sender.get_parameters().await; assert_ne!(0, parameters.rtp_parameters.codecs.len()); assert_eq!(1, parameters.encodings.len()); - assert_eq!(sender.track_encodings[0].ssrc, parameters.encodings[0].ssrc); + assert_eq!(encoding.ssrc, parameters.encodings[0].ssrc); } else { assert!(false); } From 9fdf70b78c8f21df29d5e94955aefd769df98ada Mon Sep 17 00:00:00 2001 From: robashton Date: Fri, 7 Oct 2022 15:57:57 +0100 Subject: [PATCH 03/37] clippy warnings (I guess I need to update/downgrade my rust) --- webrtc/src/peer_connection/mod.rs | 6 +++--- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs index dcee628ce..9fe7f75b6 100644 --- a/webrtc/src/peer_connection/mod.rs +++ b/webrtc/src/peer_connection/mod.rs @@ -466,13 +466,13 @@ 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 { - Some(s) => s.clone(), + let _sender = match t.sender().await { + Some(s) => s, None => { log::warn!( "RtpSender missing for transeceiver with sending direction {} for mid {}", diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 0aa267ce6..145e13fa5 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -582,7 +582,7 @@ impl RTCRtpSender { pub async fn read_simulcast(&self, b: &mut [u8], rid: &str) -> Result<(usize, Attributes)> { if let Some(encoding) = self.encoding_for_rid(rid).await { - return self.internal.read(&encoding, b).await; + self.internal.read(&encoding, b).await } else { Err(Error::ErrInterceptorNotBind) } @@ -593,7 +593,7 @@ impl RTCRtpSender { rid: &str, ) -> Result<(Vec>, Attributes)> { if let Some(encoding) = self.encoding_for_rid(rid).await { - return self.internal.read_rtcp(&encoding, self.receive_mtu).await; + self.internal.read_rtcp(&encoding, self.receive_mtu).await } else { Err(Error::ErrInterceptorNotBind) } From ec670b4fdb4d4296916b5f1774bb2603f0ff7389 Mon Sep 17 00:00:00 2001 From: robashton Date: Mon, 10 Oct 2022 13:36:51 +0100 Subject: [PATCH 04/37] Re-instate changes from #217 --- webrtc/src/peer_connection/mod.rs | 22 ++++---- webrtc/src/peer_connection/sdp/mod.rs | 59 +++++++++++++------- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 11 ++++ 3 files changed, 59 insertions(+), 33 deletions(-) diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs index 9fe7f75b6..e16d92e51 100644 --- a/webrtc/src/peer_connection/mod.rs +++ b/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 => { diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 12a7464e2..ee65ea833 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -545,6 +545,7 @@ 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, @@ -552,26 +553,9 @@ pub(crate) async fn add_transceiver_sdp( 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 = 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; } } diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 145e13fa5..237968200 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/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>, + /// AssociatedMediaStreamIds from the WebRTC specifcations + pub(crate) associated_media_stream_ids: std::sync::Mutex>, + rtp_transceiver: Mutex>>, send_called_tx: Mutex>>, @@ -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 { + 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(); From 4974cf4ab684eb3a039c005424b31307a6f218ba Mon Sep 17 00:00:00 2001 From: robashton Date: Mon, 10 Oct 2022 13:49:25 +0100 Subject: [PATCH 05/37] Replace the mutex on track_encodings with a RwLock (most of our operations are reads) --- .../peer_connection_internal.rs | 2 +- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 25 +++++++++---------- .../rtp_sender/rtp_sender_test.rs | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 3efb1b729..5c9a9619f 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1542,7 +1542,7 @@ impl PeerConnectionInternal { RTPCodecType::Video => "video", }; - let encodings = sender.track_encodings.lock().await; + let encodings = sender.track_encodings.read().await; for e in encodings.iter() { let track_id = track.id().to_string(); track_infos.push(TrackInfo { diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 237968200..ed7e773a6 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -20,7 +20,7 @@ use interceptor::stream_info::StreamInfo; use interceptor::{Attributes, Interceptor, RTCPReader, RTPWriter}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Weak}; -use tokio::sync::{mpsc, Mutex, Notify}; +use tokio::sync::{mpsc, Mutex, Notify, RwLock}; pub(crate) struct RTPSenderInternal { pub(crate) send_called_rx: Mutex>, @@ -87,7 +87,7 @@ pub struct TrackEncoding { /// RTPSender allows an application to control how a given Track is encoded and transmitted to a remote peer pub struct RTCRtpSender { - pub(crate) track_encodings: Mutex>>, + pub(crate) track_encodings: RwLock>>, pub(crate) transport: Arc, @@ -156,7 +156,7 @@ impl RTCRtpSender { }); let sender = RTCRtpSender { - track_encodings: Mutex::new(vec![]), + track_encodings: RwLock::new(vec![]), transport, payload_type: 0, @@ -258,7 +258,7 @@ impl RTCRtpSender { context: Mutex::new(TrackLocalContext::default()), }; - let mut encodings = self.track_encodings.lock().await; + let mut encodings = self.track_encodings.write().await; encodings.push(Arc::new(track_encoding)); } @@ -297,7 +297,7 @@ impl RTCRtpSender { let mut encodings: Vec = vec![]; { - let track_encodings = self.track_encodings.lock().await; + let track_encodings = self.track_encodings.read().await; for te in track_encodings.iter() { let track = te.track.lock().await; let rid = track @@ -340,7 +340,7 @@ impl RTCRtpSender { /// track returns the RTCRtpTransceiver track, or nil pub async fn track(&self) -> Option> { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; if let Some(t) = encodings.first() { let track = t.track.lock().await; track.clone() @@ -357,7 +357,7 @@ impl RTCRtpSender { track: Option>, ) -> Result<()> { if let Some(t) = &track { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; if encodings.len() > 1 { // return ErrRTPSenderNewTrackHasIncorrectEnvelope return Err(Error::ErrRTPSenderNewTrackHasIncorrectKind); @@ -376,7 +376,7 @@ impl RTCRtpSender { } } - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; if let Some(re) = encodings.first() { if self.has_sent().await { let t = { @@ -459,8 +459,7 @@ impl RTCRtpSender { return Err(Error::ErrRTPSenderSendAlreadyCalled); } - // This is quite a long lived lock? - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; for te in encodings.iter() { let write_stream = Arc::new(InterceptorToTrackLocalWriter::new(self.paused.clone())); let (context, stream_info) = { @@ -547,7 +546,7 @@ impl RTCRtpSender { self.replace_track(None).await?; - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; for te in encodings.iter() { let stream_info = te.stream_info.lock().await; self.interceptor.unbind_local_stream(&stream_info).await; @@ -569,7 +568,7 @@ impl RTCRtpSender { // These helpers exist because otherwise I accidentally end up locking on blocking calls // because I'm incapable of writing threadsafe code async fn encoding_for_rid(&self, rid: &str) -> Option> { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; for e in encodings.iter() { if let Some(track) = &*e.track.lock().await { if track.rid() == rid { @@ -581,7 +580,7 @@ impl RTCRtpSender { } async fn first_encoding(&self) -> Result>> { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; return Ok(encodings.first().map(|x| (*x).clone())); } diff --git a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs index f6ede2006..7c7e2b23f 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs @@ -142,7 +142,7 @@ async fn test_rtp_sender_get_parameters() -> Result<()> { signal_pair(&mut offerer, &mut answerer).await?; if let Some(sender) = rtp_transceiver.sender().await { - let encoding = { sender.track_encodings.lock().await[0].clone() }; + let encoding = { sender.track_encodings.read().await[0].clone() }; let parameters = sender.get_parameters().await; assert_ne!(0, parameters.rtp_parameters.codecs.len()); assert_eq!(1, parameters.encodings.len()); From 379f4035af9472c1662646b9cd0d41d0127588a6 Mon Sep 17 00:00:00 2001 From: robashton Date: Mon, 10 Oct 2022 15:13:56 +0100 Subject: [PATCH 06/37] Initial feedback from PR --- webrtc/src/error.rs | 5 ++- webrtc/src/peer_connection/sdp/mod.rs | 14 +++++---- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 33 +++++++++++++------- webrtc/src/track/track_local/mod.rs | 4 +-- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index b45cd4ffd..841159a32 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,7 +197,10 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, - #[error("Sender cannot encoding due to RID collision")] + #[error("Sender does not have track for RID")] + ErrRTPSenderNoTrackForRID, + + #[error("Sender cannot add encoding due to RID collision")] ErrRTPSenderRIDCollision, #[error("Sender cannot add encoding as provided track does not match base track")] diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index ee65ea833..cdae588c7 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -546,7 +546,7 @@ pub(crate) async fn add_transceiver_sdp( 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() { + for encoding in &send_parameters.encodings { media = media.with_media_source( encoding.ssrc, track.stream_id().to_owned(), /* cname */ @@ -559,11 +559,13 @@ pub(crate) async fn add_transceiver_sdp( if send_parameters.encodings.len() > 1 { let mut send_rids: Vec = vec![]; - for e in send_parameters.encodings.iter() { - let mut s: String = e.rid.clone(); - s.push_str(" send"); - media = media.with_value_attribute("rid".into(), s); - send_rids.push(e.rid.clone()) + for e in &send_parameters.encodings { + if let Some(rid) = e.rid.clone() { + let mut s: String = rid.clone(); + send_rids.push(rid); + s.push_str(" send"); + media = media.with_value_attribute("rid".into(), s); + } } // Simulcast) let mut s: String = "send ".to_owned(); diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index ed7e773a6..6e8490d4a 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -39,13 +39,13 @@ impl RTPSenderInternal { tokio::select! { _ = send_called_rx.recv() =>{ - let rtcp_interceptor = encoding.rtcp_interceptor.clone(); + let rtcp_reader = encoding.rtcp_reader.clone(); let a = Attributes::new(); tokio::select! { _ = self.stop_called_rx.notified() => { Err(Error::ErrClosedPipe) } - result = rtcp_interceptor.read(b, &a) => { + result = rtcp_reader.read(b, &a) => { Ok(result?) } } @@ -77,7 +77,7 @@ pub struct TrackEncoding { pub(crate) srtp_stream: Arc, - pub(crate) rtcp_interceptor: Arc, + pub(crate) rtcp_reader: Arc, pub(crate) stream_info: Mutex, pub(crate) context: Mutex, @@ -247,13 +247,13 @@ impl RTCRtpSender { }); let srtp_rtcp_reader = Arc::clone(&srtp_stream) as Arc; - let rtcp_interceptor = self.interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; + let rtcp_reader = self.interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; let track_encoding = TrackEncoding { track: Mutex::new(Some(track)), srtp_stream, ssrc, - rtcp_interceptor, + rtcp_reader, stream_info: Mutex::new(StreamInfo::default()), context: Mutex::new(TrackLocalContext::default()), }; @@ -412,7 +412,7 @@ impl RTCRtpSender { .await, ssrc: context.ssrc, write_stream: context.write_stream.clone(), - rtcp_intercepter: context.rtcp_intercepter.clone(), + rtcp_reader: context.rtcp_reader.clone(), }; t.bind(&new_context).await @@ -478,7 +478,7 @@ impl RTCRtpSender { ) .await, ssrc: te.ssrc, - rtcp_intercepter: Some(te.rtcp_interceptor.clone()), + rtcp_reader: Some(te.rtcp_reader.clone()), write_stream: Some( Arc::clone(&write_stream) as Arc ), @@ -564,9 +564,6 @@ impl RTCRtpSender { } } - // Having a mutex on that little collection sure does make this whole module fun - // These helpers exist because otherwise I accidentally end up locking on blocking calls - // because I'm incapable of writing threadsafe code async fn encoding_for_rid(&self, rid: &str) -> Option> { let encodings = self.track_encodings.read().await; for e in encodings.iter() { @@ -588,7 +585,7 @@ impl RTCRtpSender { if let Some(encoding) = self.encoding_for_rid(rid).await { self.internal.read(&encoding, b).await } else { - Err(Error::ErrInterceptorNotBind) + Err(Error::ErrRTPSenderNoTrackForRID) } } @@ -599,7 +596,7 @@ impl RTCRtpSender { if let Some(encoding) = self.encoding_for_rid(rid).await { self.internal.read_rtcp(&encoding, self.receive_mtu).await } else { - Err(Error::ErrInterceptorNotBind) + Err(Error::ErrRTPSenderNoTrackForRID) } } @@ -637,6 +634,18 @@ impl RTCRtpSender { lock.clone() } + pub(crate) fn associate_media_stream_id(&self, id: String) -> bool { + let mut lock = self.associated_media_stream_ids.lock().unwrap(); + + if lock.contains(&id) { + return false; + } + + lock.push(id); + + true + } + pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { let mut lock = self.initial_track_id.lock().unwrap(); diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index 7b5f574fc..5ffcdedaa 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -35,7 +35,7 @@ pub struct TrackLocalContext { pub(crate) params: RTCRtpParameters, pub(crate) ssrc: SSRC, pub(crate) write_stream: Option>, - pub(crate) rtcp_intercepter: Option>, + pub(crate) rtcp_reader: Option>, } impl TrackLocalContext { @@ -64,7 +64,7 @@ impl TrackLocalContext { } pub fn rtcp_reader(&self) -> Option> { - self.rtcp_intercepter.clone() + self.rtcp_reader.clone() } /// id is a unique identifier that is used for both bind/unbind From 21607aced0b2068490ecb1df4827c61c69dcfebc Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:37:34 +0100 Subject: [PATCH 07/37] Make Rid an Option and see how that falls out --- examples/examples/simulcast/simulcast.rs | 6 +- interceptor/src/stream_info.rs | 2 +- webrtc/src/error.rs | 4 ++ .../peer_connection_internal.rs | 10 ++- webrtc/src/rtp_transceiver/mod.rs | 4 +- .../src/rtp_transceiver/rtp_receiver/mod.rs | 12 ++-- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 61 +++++++++---------- webrtc/src/track/track_local/mod.rs | 2 +- .../track_local/track_local_static_rtp.rs | 10 +-- .../track_local/track_local_static_sample.rs | 15 ++--- webrtc/src/track/track_remote/mod.rs | 8 +-- 11 files changed, 68 insertions(+), 66 deletions(-) diff --git a/examples/examples/simulcast/simulcast.rs b/examples/examples/simulcast/simulcast.rs index 0e9bbc1db..181dbd136 100644 --- a/examples/examples/simulcast/simulcast.rs +++ b/examples/examples/simulcast/simulcast.rs @@ -160,7 +160,7 @@ async fn main() -> Result<()> { if let Some(track) = track { println!("Track has started"); - let rid = track.rid().to_owned(); + let rid = track.rid().unwrap_or("".to_owned()); let output_track = if let Some(output_track) = output_tracks.get(&rid) { Arc::clone(output_track) } else { @@ -199,7 +199,7 @@ async fn main() -> Result<()> { tokio::spawn(async move { // Read RTP packets being sent to webrtc-rs - println!("enter track loop {}", track.rid()); + println!("enter track loop {:?}", track.rid()); while let Ok((rtp, _)) = track.read_rtp().await { if let Err(err) = output_track.write_rtp(&rtp).await { if Error::ErrClosedPipe != err { @@ -210,7 +210,7 @@ async fn main() -> Result<()> { } } } - println!("exit track loop {}", track.rid()); + println!("exit track loop {:?}", track.rid()); }); } Box::pin(async {}) diff --git a/interceptor/src/stream_info.rs b/interceptor/src/stream_info.rs index c4d0b7cb5..95967545e 100644 --- a/interceptor/src/stream_info.rs +++ b/interceptor/src/stream_info.rs @@ -11,7 +11,7 @@ pub struct RTPHeaderExtension { #[derive(Default, Debug, Clone)] pub struct StreamInfo { pub id: String, - pub rid: String, + pub rid: Option, pub attributes: Attributes, pub ssrc: u32, pub payload_type: u8, diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index 841159a32..62b8b536c 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,6 +197,10 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, + // TODO: Delete this once we've added the ones we actually need + // errRTPSenderTrackNil = errors.New("Track must not be nil") + // errRTPSenderDTLSTransportNil = errors.New("DTLSTransport must not be nil") + // errRTPSenderTrackRemoved = errors.New("Sender Track has been removed or replaced to nil") #[error("Sender does not have track for RID")] ErrRTPSenderNoTrackForRID, diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 5c9a9619f..918aac221 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -173,10 +173,8 @@ impl PeerConnectionInternal { let mut receiver_needs_stopped = false; for t in tracks { - if !t.rid().is_empty() { - if let Some(details) = - track_details_for_rid(&track_details, t.rid().to_owned()) - { + if let Some(rid) = t.rid().clone() { + if let Some(details) = track_details_for_rid(&track_details, rid) { t.set_id(details.id.clone()).await; t.set_stream_id(details.stream_id.clone()).await; continue; @@ -1141,7 +1139,7 @@ impl PeerConnectionInternal { let stream_info = create_stream_info( "".to_owned(), ssrc, - "".to_owned(), + None, params.codecs[0].payload_type, params.codecs[0].capability.clone(), ¶ms.header_extensions, @@ -1549,7 +1547,7 @@ impl PeerConnectionInternal { track_id, ssrc: e.ssrc, mid: mid.clone(), - rid: Some(track.rid().to_owned()), + rid: track.rid().clone(), kind, }); } diff --git a/webrtc/src/rtp_transceiver/mod.rs b/webrtc/src/rtp_transceiver/mod.rs index 3d2fcaea5..763ae48d2 100644 --- a/webrtc/src/rtp_transceiver/mod.rs +++ b/webrtc/src/rtp_transceiver/mod.rs @@ -93,7 +93,7 @@ pub struct RTCRtpRtxParameters { /// #[derive(Default, Debug, Clone, Serialize, Deserialize)] pub struct RTCRtpCodingParameters { - pub rid: String, + pub rid: Option, pub ssrc: SSRC, pub payload_type: PayloadType, pub rtx: RTCRtpRtxParameters, @@ -132,7 +132,7 @@ pub struct RTCRtpTransceiverInit { pub(crate) fn create_stream_info( id: String, ssrc: SSRC, - rid: String, + rid: Option, payload_type: PayloadType, codec: RTCRtpCodecCapability, webrtc_header_extensions: &[RTCRtpHeaderExtensionParameters], diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs index 9eecc9a8b..d8a656335 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs @@ -198,7 +198,7 @@ impl RTPReceiverInternal { let tracks = self.tracks.read().await; for t in &*tracks { - if t.track.rid() == rid { + if t.track.rid().map_or(false, |r| r == rid) { if let Some(rtcp_interceptor) = &t.stream.rtcp_interceptor { let a = Attributes::new(); @@ -529,7 +529,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), encoding.ssrc, - "".to_owned(), + None, 0, codec.clone(), &global_params.header_extensions, @@ -587,7 +587,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), rtx_ssrc, - "".to_owned(), + None, 0, codec.clone(), &global_params.header_extensions, @@ -656,7 +656,7 @@ impl RTCRtpReceiver { let mut encodings = vec![RTCRtpDecodingParameters::default(); encoding_size]; for (i, encoding) in encodings.iter_mut().enumerate() { if incoming.rids.len() > i { - encoding.rid = incoming.rids[i].clone(); + encoding.rid = Some(incoming.rids[i].clone()); } if incoming.ssrcs.len() > i { encoding.ssrc = incoming.ssrcs[i]; @@ -751,7 +751,7 @@ impl RTCRtpReceiver { ) -> Result> { let mut tracks = self.internal.tracks.write().await; for t in &mut *tracks { - if t.track.rid() == rid { + if t.track.rid().map_or(false, |r| r == rid) { t.track.set_kind(self.kind); if let Some(codec) = params.codecs.first() { t.track.set_codec(codec.clone()).await; @@ -779,7 +779,7 @@ impl RTCRtpReceiver { let mut tracks = self.internal.tracks.write().await; let l = tracks.len(); for t in &mut *tracks { - if (ssrc != 0 && l == 1) || t.track.rid() == rsid { + if (ssrc != 0 && l == 1) || t.track.rid().map_or(false, |r| r == rsid) { t.repair_stream = repair_stream; let receive_mtu = self.receive_mtu; diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 6e8490d4a..612b8453f 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -190,10 +190,6 @@ impl RTCRtpSender { } pub async fn add_encoding(&self, track: Arc) -> Result<()> { - if track.rid() == "" { - return Err(Error::ErrRTPSenderRidNil); - } - if self.has_stopped().await { return Err(Error::ErrRTPSenderStopped); } @@ -202,33 +198,38 @@ impl RTCRtpSender { return Err(Error::ErrRTPSenderSendAlreadyCalled); } - // oops, somebody code-golf this for me - { - let ref_track = if let Some(t) = self.first_encoding().await? { - let t = t.track.lock().await; - if let Some(t) = &*t { - if t.rid() != "" { - t.clone() - } else { - return Err(Error::ErrRTPSenderNoBaseEncoding); - } + if track.rid().is_none() { + return Err(Error::ErrRTPSenderRidNil); + } + + if let Some(rid) = track.rid() { + if self.encoding_for_rid(&rid).await.is_some() { + return Err(Error::ErrRTPSenderRIDCollision); + } + } else { + return Err(Error::ErrRTPSenderRidNil); + } + + let ref_track = if let Some(t) = self.first_encoding().await? { + let t = t.track.lock().await; + if let Some(t) = &*t { + if t.rid().is_some() { + t.clone() } else { return Err(Error::ErrRTPSenderNoBaseEncoding); } } else { return Err(Error::ErrRTPSenderNoBaseEncoding); - }; - - if ref_track.id() != track.id() - || ref_track.stream_id() != track.stream_id() - || ref_track.kind() != track.kind() - { - return Err(Error::ErrRTPSenderBaseEncodingMismatch); } + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + }; - if self.encoding_for_rid(track.rid()).await.is_some() { - return Err(Error::ErrRTPSenderRIDCollision); - } + if ref_track.id() != track.id() + || ref_track.stream_id() != track.stream_id() + || ref_track.kind() != track.kind() + { + return Err(Error::ErrRTPSenderBaseEncodingMismatch); } self.add_encoding_internal(track).await; @@ -300,9 +301,7 @@ impl RTCRtpSender { let track_encodings = self.track_encodings.read().await; for te in track_encodings.iter() { let track = te.track.lock().await; - let rid = track - .as_ref() - .map_or(String::from(""), |t| String::from(t.rid())); + let rid = track.as_ref().map_or(None, |t| t.rid().clone()); encodings.push(RTCRtpEncodingParameters { ssrc: te.ssrc, @@ -486,9 +485,9 @@ impl RTCRtpSender { let (codec, rid) = if let Some(t) = &*track { let codec = t.bind(&context).await?; - (codec, t.rid()) + (codec, t.rid().clone()) } else { - (RTCRtpCodecParameters::default(), "") + (RTCRtpCodecParameters::default(), None) }; let payload_type = codec.payload_type; let capability = codec.capability.clone(); @@ -496,7 +495,7 @@ impl RTCRtpSender { let stream_info = create_stream_info( self.id.clone(), te.ssrc, - rid.to_owned(), + rid.clone(), payload_type, capability, ¶meters.rtp_parameters.header_extensions, @@ -568,7 +567,7 @@ impl RTCRtpSender { let encodings = self.track_encodings.read().await; for e in encodings.iter() { if let Some(track) = &*e.track.lock().await { - if track.rid() == rid { + if track.rid().map_or(false, |r| r == rid) { return Some(e.clone()); } }; diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index 5ffcdedaa..bac1f5eb9 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -92,7 +92,7 @@ pub trait TrackLocal { fn id(&self) -> &str; /// rid is the RTP identifier for this Track - fn rid(&self) -> &str; + fn rid(&self) -> Option; /// stream_id is the group this track belongs too. This must be unique fn stream_id(&self) -> &str; diff --git a/webrtc/src/track/track_local/track_local_static_rtp.rs b/webrtc/src/track/track_local/track_local_static_rtp.rs index fc6561297..c23e1006c 100644 --- a/webrtc/src/track/track_local/track_local_static_rtp.rs +++ b/webrtc/src/track/track_local/track_local_static_rtp.rs @@ -10,7 +10,7 @@ pub struct TrackLocalStaticRTP { pub(crate) bindings: Mutex>>, codec: RTCRtpCodecCapability, id: String, - rid: String, + rid: Option, stream_id: String, } @@ -21,13 +21,13 @@ impl TrackLocalStaticRTP { codec, bindings: Mutex::new(vec![]), id, - rid: "".to_owned(), + rid: None, stream_id, } } pub fn set_rid(&mut self, rid: String) { - self.rid = rid; + self.rid = Some(rid); } /// codec gets the Codec of the track @@ -96,8 +96,8 @@ impl TrackLocal for TrackLocalStaticRTP { self.stream_id.as_str() } - fn rid(&self) -> &str { - self.rid.as_str() + fn rid(&self) -> Option { + self.rid.clone() } /// kind controls if this TrackLocal is audio or video diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index e455b67bd..4c791979d 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -71,12 +71,13 @@ impl TrackLocalStaticSample { if sample.prev_dropped_packets > 0 { packetizer.skip_samples(samples * sample.prev_dropped_packets as u32); } - /*println!( - "clock_rate={}, samples={}, {}", - clock_rate, - samples, - sample.duration.as_secs_f64() - );*/ + // log::info!( + // "clock_rate={}, samples={}, {}", + // clock_rate, + // samples, + // sample.duration.as_secs_f64() + // ); + // packetizer.packetize(&sample.data, samples).await? } else { vec![] @@ -138,7 +139,7 @@ impl TrackLocal for TrackLocalStaticSample { self.rtp_track.id() } - fn rid(&self) -> &str { + fn rid(&self) -> Option { self.rtp_track.rid() } diff --git a/webrtc/src/track/track_remote/mod.rs b/webrtc/src/track/track_remote/mod.rs index 0f6382af3..ae0321b40 100644 --- a/webrtc/src/track/track_remote/mod.rs +++ b/webrtc/src/track/track_remote/mod.rs @@ -48,7 +48,7 @@ pub struct TrackRemote { ssrc: AtomicU32, //SSRC, codec: Mutex, pub(crate) params: Mutex, - rid: String, + rid: Option, media_engine: Arc, interceptor: Arc, @@ -79,7 +79,7 @@ impl TrackRemote { receive_mtu: usize, kind: RTPCodecType, ssrc: SSRC, - rid: String, + rid: Option, receiver: Weak, media_engine: Arc, interceptor: Arc, @@ -135,8 +135,8 @@ impl TrackRemote { /// rid gets the RTP Stream ID of this Track /// With Simulcast you will have multiple tracks with the same ID, but different RID values. /// In many cases a TrackRemote will not have an RID, so it is important to assert it is non-zero - pub fn rid(&self) -> &str { - self.rid.as_str() + pub fn rid(&self) -> Option { + self.rid.clone() } /// payload_type gets the PayloadType of the track From a3c2a20b307a97abab4754f938545ebf0e872ffb Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:47:35 +0100 Subject: [PATCH 08/37] revert change to log (that I did when I was debugging my own stuff\!) --- webrtc/src/track/track_local/track_local_static_sample.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index 4c791979d..d0175a63a 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -71,7 +71,7 @@ impl TrackLocalStaticSample { if sample.prev_dropped_packets > 0 { packetizer.skip_samples(samples * sample.prev_dropped_packets as u32); } - // log::info!( + // println!( // "clock_rate={}, samples={}, {}", // clock_rate, // samples, From d6ff71d59d729721bb48650076558405b3c8cc54 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:53:37 +0100 Subject: [PATCH 09/37] unique track ids in stats --- webrtc/src/peer_connection/peer_connection_internal.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 918aac221..5cc5dd196 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1542,7 +1542,9 @@ impl PeerConnectionInternal { let encodings = sender.track_encodings.read().await; for e in encodings.iter() { - let track_id = track.id().to_string(); + let track_id = track.rid().map_or(track.id().to_string(), |rid| { + track.id().to_string() + "-" + rid.as_str() + }); track_infos.push(TrackInfo { track_id, ssrc: e.ssrc, From d4c27fcb4f34d219a5b4faba5ca7d3802e905472 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:54:01 +0100 Subject: [PATCH 10/37] debug impl for TrackLocalContext --- webrtc/src/track/track_local/mod.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index bac1f5eb9..c2255fe3f 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -38,6 +38,17 @@ pub struct TrackLocalContext { pub(crate) rtcp_reader: Option>, } +impl std::fmt::Debug for TrackLocalContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TrackLocalContext") + .field("id", &self.id) + .field("params", &self.params) + .field("ssrc", &self.ssrc) + .field("write_stream", &self.write_stream) + .finish() + } +} + impl TrackLocalContext { /// codec_parameters returns the negotiated RTPCodecParameters. These are the codecs supported by both /// PeerConnections and the SSRC/PayloadTypes From 19fc8e98346d04fbe02b4174c3adf9c4dc0c8201 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:02:25 +0100 Subject: [PATCH 11/37] ensure we only do simulcast send directives if we're doing a send on this tranceiver --- webrtc/src/peer_connection/sdp/mod.rs | 48 ++++++++++++++------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index cdae588c7..b53dc8b4d 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -543,34 +543,36 @@ pub(crate) async fn add_transceiver_sdp( // This is equivalent to addSenderSDP in Pion (now) for mt in transceivers { 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 { - media = media.with_media_source( - encoding.ssrc, - track.stream_id().to_owned(), /* cname */ - track.stream_id().to_owned(), /* streamLabel */ - track.id().to_owned(), - ); - } + if mt.direction().has_send() { + let send_parameters = sender.get_parameters().await; + // Get the different encodings expressed first + for encoding in &send_parameters.encodings { + media = media.with_media_source( + encoding.ssrc, + track.stream_id().to_owned(), /* cname */ + track.stream_id().to_owned(), /* streamLabel */ + track.id().to_owned(), + ); + } - // Then tell the world about simulcast - if send_parameters.encodings.len() > 1 { - let mut send_rids: Vec = vec![]; + // Then tell the world about simulcast + if send_parameters.encodings.len() > 1 { + let mut send_rids: Vec = vec![]; - for e in &send_parameters.encodings { - if let Some(rid) = e.rid.clone() { - let mut s: String = rid.clone(); - send_rids.push(rid); - s.push_str(" send"); - media = media.with_value_attribute("rid".into(), s); + for e in &send_parameters.encodings { + if let Some(rid) = e.rid.clone() { + let mut s: String = rid.clone(); + send_rids.push(rid); + s.push_str(" send"); + media = media.with_value_attribute("rid".into(), s); + } } + // Simulcast) + let mut s: String = "send ".to_owned(); + s.push_str(send_rids.join(";").as_ref()); + media = media.with_value_attribute("simulcast".into(), s); } - // Simulcast) - let mut s: String = "send ".to_owned(); - 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 From acef0fd9af0323c933d16e637c17ce85a26164af Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:08:50 +0100 Subject: [PATCH 12/37] apparently format! is a thing --- webrtc/src/peer_connection/peer_connection_internal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 5cc5dd196..e6beb7e34 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1543,7 +1543,7 @@ impl PeerConnectionInternal { let encodings = sender.track_encodings.read().await; for e in encodings.iter() { let track_id = track.rid().map_or(track.id().to_string(), |rid| { - track.id().to_string() + "-" + rid.as_str() + format!("{}-{}", track.id(), rid) }); track_infos.push(TrackInfo { track_id, From ac59df18bedda5faf27e764be7d5814bc36382ca Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:24:46 +0100 Subject: [PATCH 13/37] a few things in rtp_sender that we didn't ike --- webrtc/src/error.rs | 7 +- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 118 +++++++++---------- 2 files changed, 59 insertions(+), 66 deletions(-) diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index 62b8b536c..831cf7494 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,10 +197,9 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, - // TODO: Delete this once we've added the ones we actually need - // errRTPSenderTrackNil = errors.New("Track must not be nil") - // errRTPSenderDTLSTransportNil = errors.New("DTLSTransport must not be nil") - // errRTPSenderTrackRemoved = errors.New("Sender Track has been removed or replaced to nil") + #[error("Cannot call replace on a sender with multiple tracks")] + ErrRTPSenderCannotReplaceSimulcast, + #[error("Sender does not have track for RID")] ErrRTPSenderNoTrackForRID, diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 612b8453f..3d7b4b337 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -358,8 +358,7 @@ impl RTCRtpSender { if let Some(t) = &track { let encodings = self.track_encodings.read().await; if encodings.len() > 1 { - // return ErrRTPSenderNewTrackHasIncorrectEnvelope - return Err(Error::ErrRTPSenderNewTrackHasIncorrectKind); + return Err(Error::ErrRTPSenderCannotReplaceSimulcast); } let tr = self.rtp_transceiver.lock().await; if let Some(r) = &*tr { @@ -376,79 +375,74 @@ impl RTCRtpSender { } let encodings = self.track_encodings.read().await; - if let Some(re) = encodings.first() { - if self.has_sent().await { - let t = { - let t = re.track.lock().await; - t.clone() - }; - if let Some(t) = t { - let context = re.context.lock().await; - t.unbind(&context).await?; - } - } + let re = match encodings.first() { + Some(re) => re, + None => return Ok(()), + }; - if !self.has_sent().await || track.is_none() { - let mut t = re.track.lock().await; - *t = track; - return Ok(()); + if self.has_sent().await { + let t = { + let t = re.track.lock().await; + t.clone() + }; + if let Some(t) = t { + let context = re.context.lock().await; + t.unbind(&context).await?; } + } - let context = { - let context = re.context.lock().await; - context.clone() - }; + if !self.has_sent().await || track.is_none() { + let mut t = re.track.lock().await; + *t = track; + return Ok(()); + } - let result = if let Some(t) = &track { - let new_context = TrackLocalContext { - id: context.id.clone(), - params: self - .media_engine - .get_rtp_parameters_by_kind( - t.kind(), - &[RTCRtpTransceiverDirection::Sendonly], - ) - .await, - ssrc: context.ssrc, - write_stream: context.write_stream.clone(), - rtcp_reader: context.rtcp_reader.clone(), - }; + let context = { + let context = re.context.lock().await; + context.clone() + }; - t.bind(&new_context).await - } else { - Err(Error::ErrRTPSenderTrackNil) + let result = if let Some(t) = &track { + let new_context = TrackLocalContext { + id: context.id.clone(), + params: self + .media_engine + .get_rtp_parameters_by_kind(t.kind(), &[RTCRtpTransceiverDirection::Sendonly]) + .await, + ssrc: context.ssrc, + write_stream: context.write_stream.clone(), + rtcp_reader: context.rtcp_reader.clone(), }; - match result { - Err(err) => { - // Re-bind the original track - let track = re.track.lock().await; - if let Some(t) = &*track { - t.bind(&context).await?; - } + t.bind(&new_context).await + } else { + Err(Error::ErrRTPSenderTrackNil) + }; - Err(err) + match result { + Err(err) => { + // Re-bind the original track + let track = re.track.lock().await; + if let Some(t) = &*track { + t.bind(&context).await?; } - Ok(codec) => { - // Codec has changed - if self.payload_type != codec.payload_type { - let mut context = re.context.lock().await; - context.params.codecs = vec![codec]; - } - { - let mut t = re.track.lock().await; - *t = track; - } + Err(err) + } + Ok(codec) => { + // Codec has changed + if self.payload_type != codec.payload_type { + let mut context = re.context.lock().await; + context.params.codecs = vec![codec]; + } - Ok(()) + { + let mut t = re.track.lock().await; + *t = track; } + + Ok(()) } - } else { - // Is it though? - // How do we end up in a state where we don't have at the very least, the default track - // encoding? - Ok(()) } } From 4b041800d6909cdf8c41c6180d541ba370e4b1e9 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:35:10 +0100 Subject: [PATCH 14/37] unnecessary clone now it's an option --- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 3d7b4b337..0bab3fcea 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -6,7 +6,7 @@ use crate::dtls_transport::RTCDtlsTransport; use crate::error::{Error, Result}; use crate::rtp_transceiver::rtp_codec::{RTCRtpCodecParameters, RTPCodecType}; use crate::rtp_transceiver::rtp_transceiver_direction::RTCRtpTransceiverDirection; -use crate::rtp_transceiver::srtp_writer_future::SrtpWriterFuture; +use crate::rtp_transceiver::trtp_writer_future::SrtpWriterFuture; use crate::rtp_transceiver::{ create_stream_info, PayloadType, RTCRtpEncodingParameters, RTCRtpSendParameters, RTCRtpTransceiver, SSRC, @@ -301,7 +301,7 @@ impl RTCRtpSender { let track_encodings = self.track_encodings.read().await; for te in track_encodings.iter() { let track = te.track.lock().await; - let rid = track.as_ref().map_or(None, |t| t.rid().clone()); + let rid = track.as_ref().map_or(None, |t| t.rid()); encodings.push(RTCRtpEncodingParameters { ssrc: te.ssrc, From 246044e19b0ac7c3eacb633d45c3854958629b69 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:52:17 +0100 Subject: [PATCH 15/37] typo (got vimmed) --- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 0bab3fcea..d46ba868f 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -6,7 +6,7 @@ use crate::dtls_transport::RTCDtlsTransport; use crate::error::{Error, Result}; use crate::rtp_transceiver::rtp_codec::{RTCRtpCodecParameters, RTPCodecType}; use crate::rtp_transceiver::rtp_transceiver_direction::RTCRtpTransceiverDirection; -use crate::rtp_transceiver::trtp_writer_future::SrtpWriterFuture; +use crate::rtp_transceiver::srtp_writer_future::SrtpWriterFuture; use crate::rtp_transceiver::{ create_stream_info, PayloadType, RTCRtpEncodingParameters, RTCRtpSendParameters, RTCRtpTransceiver, SSRC, From 850fcb579966f241449726d99baefc34fbd06c02 Mon Sep 17 00:00:00 2001 From: robashton Date: Fri, 7 Oct 2022 15:02:53 +0100 Subject: [PATCH 16/37] Tentative first pass at making simulcast egest possible This is not done "properly", and is an (almost) one for one copy of changes from: https://github.com/pion/webrtc/commit/37e16a3b15a3a474573b88eb7eeb728afaeb3c58 --- interceptor/src/stream_info.rs | 1 + webrtc/src/error.rs | 17 + webrtc/src/peer_connection/mod.rs | 17 +- .../peer_connection_internal.rs | 20 +- webrtc/src/peer_connection/sdp/mod.rs | 80 +-- webrtc/src/rtp_transceiver/mod.rs | 2 + .../src/rtp_transceiver/rtp_receiver/mod.rs | 2 + webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 488 +++++++++++------- .../rtp_sender/rtp_sender_test.rs | 2 +- webrtc/src/track/track_local/mod.rs | 12 +- .../track_local/track_local_static_rtp.rs | 10 + .../track_local/track_local_static_sample.rs | 9 + 12 files changed, 428 insertions(+), 232 deletions(-) diff --git a/interceptor/src/stream_info.rs b/interceptor/src/stream_info.rs index 139cdbc38..c4d0b7cb5 100644 --- a/interceptor/src/stream_info.rs +++ b/interceptor/src/stream_info.rs @@ -11,6 +11,7 @@ pub struct RTPHeaderExtension { #[derive(Default, Debug, Clone)] pub struct StreamInfo { pub id: String, + pub rid: String, pub attributes: Attributes, pub ssrc: u32, pub payload_type: u8, diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index 088a695cb..51a7d0e37 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,6 +197,23 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, + #[error("Sender cannot encoding due to RID collision")] + ErrRTPSenderRIDCollision, + + #[error("Sender cannot add encoding as provided track does not match base track")] + ErrRTPSenderBaseEncodingMismatch, + + /// ErrRTPSenderStopped indicates the sender was already stopped + #[error("Sender has already been stopped")] + ErrRTPSenderStopped, + + /// ErrRTPSenderRidNil indicates that the track RID was empty + #[error("Sender cannot add encoding as rid is empty")] + ErrRTPSenderRidNil, + + #[error("Sender cannot add encoding as there is no base track")] + ErrRTPSenderNoBaseEncoding, + /// ErrUnbindFailed indicates that a TrackLocal was not able to be unbind #[error("failed to unbind TrackLocal from PeerConnection")] ErrUnbindFailed, diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs index cd678cdbe..dcee628ce 100644 --- a/webrtc/src/peer_connection/mod.rs +++ b/webrtc/src/peer_connection/mod.rs @@ -490,16 +490,17 @@ impl RTCPeerConnection { // local description so we can compare all of them. For no we only // consider the first one. - let stream_ids = sender.associated_media_stream_ids(); - // Different number of lines, 1 vs 0 - if stream_ids.is_empty() { - return true; - } + // 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; + // } // 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 => { diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 5c0539ef4..3efb1b729 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1141,6 +1141,7 @@ impl PeerConnectionInternal { let stream_info = create_stream_info( "".to_owned(), ssrc, + "".to_owned(), params.codecs[0].payload_type, params.codecs[0].capability.clone(), ¶ms.header_extensions, @@ -1535,20 +1536,23 @@ impl PeerConnectionInternal { None => continue, }; - let track_id = track.id().to_string(); let kind = match track.kind() { RTPCodecType::Unspecified => continue, RTPCodecType::Audio => "audio", RTPCodecType::Video => "video", }; - track_infos.push(TrackInfo { - track_id, - ssrc: sender.ssrc, - mid: mid.clone(), - rid: None, - kind, - }); + let encodings = sender.track_encodings.lock().await; + for e in encodings.iter() { + let track_id = track.id().to_string(); + track_infos.push(TrackInfo { + track_id, + ssrc: e.ssrc, + mid: mid.clone(), + rid: Some(track.rid().to_owned()), + kind, + }); + } } let stream_stats = self diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index ae9576045..4f685532d 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -532,55 +532,57 @@ pub(crate) async fn add_transceiver_sdp( ); } + // This is equivalent to addSenderSDP in Pion (now) for mt in transceivers { if let Some(sender) = mt.sender().await { + let send_parameters = sender.get_parameters().await; if let Some(track) = sender.track().await { - media = media.with_media_source( - sender.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 && 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() - )); + 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())?; + } } - - 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)); - } + if send_parameters.encodings.len() > 1 { + let mut send_rids: Vec = vec![]; - break; + for e in send_parameters.encodings.iter() { + let mut s: String = e.rid.clone(); + s.push_str(" send"); + media = media.with_value_attribute("rid".into(), s); + send_rids.push(e.rid.clone()) + } + // Simulcast) + let mut s: String = "send ".to_owned(); + s.push_str(send_rids.join(";").as_ref()); + media = media.with_value_attribute("simulcast".into(), s); } } } + if !is_plan_b { + break; + } } let direction = match params.offered_direction { diff --git a/webrtc/src/rtp_transceiver/mod.rs b/webrtc/src/rtp_transceiver/mod.rs index 8e6058d52..3d2fcaea5 100644 --- a/webrtc/src/rtp_transceiver/mod.rs +++ b/webrtc/src/rtp_transceiver/mod.rs @@ -132,6 +132,7 @@ pub struct RTCRtpTransceiverInit { pub(crate) fn create_stream_info( id: String, ssrc: SSRC, + rid: String, payload_type: PayloadType, codec: RTCRtpCodecCapability, webrtc_header_extensions: &[RTCRtpHeaderExtensionParameters], @@ -154,6 +155,7 @@ pub(crate) fn create_stream_info( StreamInfo { id, + rid, attributes: Attributes::new(), ssrc, payload_type, diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs index e4e0a398c..fa1c0c139 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs @@ -529,6 +529,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), encoding.ssrc, + "".to_owned(), 0, codec.clone(), &global_params.header_extensions, @@ -586,6 +587,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), rtx_ssrc, + "".to_owned(), 0, codec.clone(), &global_params.header_extensions, diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index eca3a4d7e..4ddb2e9c8 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -26,21 +26,20 @@ pub(crate) struct RTPSenderInternal { pub(crate) send_called_rx: Mutex>, pub(crate) stop_called_rx: Arc, pub(crate) stop_called_signal: Arc, - pub(crate) rtcp_interceptor: Mutex>>, } impl RTPSenderInternal { /// read reads incoming RTCP for this RTPReceiver - async fn read(&self, b: &mut [u8]) -> Result<(usize, Attributes)> { + async fn read( + &self, + encoding: &Arc, + b: &mut [u8], + ) -> Result<(usize, Attributes)> { let mut send_called_rx = self.send_called_rx.lock().await; tokio::select! { _ = send_called_rx.recv() =>{ - let rtcp_interceptor = { - let rtcp_interceptor = self.rtcp_interceptor.lock().await; - rtcp_interceptor.clone() - }; - if let Some(rtcp_interceptor) = rtcp_interceptor{ + let rtcp_interceptor = encoding.rtcp_interceptor.clone(); let a = Attributes::new(); tokio::select! { _ = self.stop_called_rx.notified() => { @@ -50,9 +49,6 @@ impl RTPSenderInternal { Ok(result?) } } - }else{ - Err(Error::ErrInterceptorNotBind) - } } _ = self.stop_called_rx.notified() =>{ Err(Error::ErrClosedPipe) @@ -63,10 +59,11 @@ impl RTPSenderInternal { /// read_rtcp is a convenience method that wraps Read and unmarshals for you. async fn read_rtcp( &self, + encoding: &Arc, receive_mtu: usize, ) -> Result<(Vec>, Attributes)> { let mut b = vec![0u8; receive_mtu]; - let (n, attributes) = self.read(&mut b).await?; + let (n, attributes) = self.read(encoding, &mut b).await?; let mut buf = &b[..n]; let pkts = rtcp::packet::unmarshal(&mut buf)?; @@ -75,19 +72,26 @@ impl RTPSenderInternal { } } -/// RTPSender allows an application to control how a given Track is encoded and transmitted to a remote peer -pub struct RTCRtpSender { +pub struct TrackEncoding { pub(crate) track: Mutex>>, pub(crate) srtp_stream: Arc, + + pub(crate) rtcp_interceptor: Arc, pub(crate) stream_info: Mutex, pub(crate) context: Mutex, + pub(crate) ssrc: SSRC, +} + +/// RTPSender allows an application to control how a given Track is encoded and transmitted to a remote peer +pub struct RTCRtpSender { + pub(crate) track_encodings: Mutex>>, + pub(crate) transport: Arc, pub(crate) payload_type: PayloadType, - pub(crate) ssrc: SSRC, receive_mtu: usize, /// a transceiver sender since we can just check the @@ -96,14 +100,13 @@ pub struct RTCRtpSender { pub(crate) media_engine: Arc, pub(crate) interceptor: Arc, + pub(crate) kind: RTPCodecType, pub(crate) id: String, /// The id of the initial track, even if we later change to a different /// track id should be use when negotiating. pub(crate) initial_track_id: std::sync::Mutex>, - /// AssociatedMediaStreamIds from the WebRTC specifcations - pub(crate) associated_media_stream_ids: std::sync::Mutex>, rtp_transceiver: Mutex>>, @@ -140,44 +143,19 @@ impl RTCRtpSender { let (send_called_tx, send_called_rx) = mpsc::channel(1); let stop_called_tx = Arc::new(Notify::new()); let stop_called_rx = stop_called_tx.clone(); - let ssrc = rand::random::(); let stop_called_signal = Arc::new(AtomicBool::new(false)); let internal = Arc::new(RTPSenderInternal { send_called_rx: Mutex::new(send_called_rx), stop_called_rx, stop_called_signal: Arc::clone(&stop_called_signal), - rtcp_interceptor: Mutex::new(None), - }); - - let srtp_stream = Arc::new(SrtpWriterFuture { - closed: AtomicBool::new(false), - ssrc, - rtp_sender: Arc::downgrade(&internal), - rtp_transport: Arc::clone(&transport), - rtcp_read_stream: Mutex::new(None), - rtp_write_session: Mutex::new(None), }); - let srtp_rtcp_reader = Arc::clone(&srtp_stream) as Arc; - let rtcp_interceptor = interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; - { - let mut internal_rtcp_interceptor = internal.rtcp_interceptor.lock().await; - *internal_rtcp_interceptor = Some(rtcp_interceptor); - } - - let stream_ids = vec![track.stream_id().to_string()]; - RTCRtpSender { - track: Mutex::new(Some(track)), - - srtp_stream, - stream_info: Mutex::new(StreamInfo::default()), - - context: Mutex::new(TrackLocalContext::default()), + let sender = RTCRtpSender { + track_encodings: Mutex::new(vec![]), transport, payload_type: 0, - ssrc, receive_mtu, negotiated: AtomicBool::new(false), @@ -187,7 +165,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), @@ -198,7 +176,85 @@ impl RTCRtpSender { paused: Arc::new(AtomicBool::new(start_paused)), internal, + }; + + sender.add_encoding_internal(track).await; + + // Add track + sender + } + + pub async fn add_encoding(&self, track: Arc) -> Result<()> { + if track.rid() == "" { + return Err(Error::ErrRTPSenderRidNil); + } + + if self.has_stopped().await { + return Err(Error::ErrRTPSenderStopped); + } + + if self.has_sent().await { + return Err(Error::ErrRTPSenderSendAlreadyCalled); } + + // oops, somebody code-golf this for me + { + let ref_track = if let Some(t) = self.first_encoding().await? { + let t = t.track.lock().await; + if let Some(t) = &*t { + if t.rid() != "" { + t.clone() + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + } + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + } + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + }; + + if ref_track.id() != track.id() + || ref_track.stream_id() != track.stream_id() + || ref_track.kind() != track.kind() + { + return Err(Error::ErrRTPSenderBaseEncodingMismatch); + } + + if self.encoding_for_rid(track.rid()).await.is_some() { + return Err(Error::ErrRTPSenderRIDCollision); + } + } + + self.add_encoding_internal(track).await; + Ok(()) + } + + pub(crate) async fn add_encoding_internal(&self, track: Arc) { + let ssrc = rand::random::(); + let srtp_stream = Arc::new(SrtpWriterFuture { + closed: AtomicBool::new(false), + ssrc, + rtp_sender: Arc::downgrade(&self.internal), + rtp_transport: Arc::clone(&self.transport), + rtcp_read_stream: Mutex::new(None), + rtp_write_session: Mutex::new(None), + }); + + let srtp_rtcp_reader = Arc::clone(&srtp_stream) as Arc; + let rtcp_interceptor = self.interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; + + let track_encoding = TrackEncoding { + track: Mutex::new(Some(track)), + srtp_stream, + ssrc, + rtcp_interceptor, + stream_info: Mutex::new(StreamInfo::default()), + context: Mutex::new(TrackLocalContext::default()), + }; + + let mut encodings = self.track_encodings.lock().await; + encodings.push(Arc::new(track_encoding)); } pub(crate) fn is_negotiated(&self) -> bool { @@ -233,27 +289,31 @@ impl RTCRtpSender { /// get_parameters describes the current configuration for the encoding and /// transmission of media on the sender's track. pub async fn get_parameters(&self) -> RTCRtpSendParameters { - let kind = { - let track = self.track.lock().await; - if let Some(t) = &*track { - t.kind() - } else { - RTPCodecType::default() - } - }; + let mut encodings: Vec = vec![]; - let mut send_parameters = { - RTCRtpSendParameters { - rtp_parameters: self - .media_engine - .get_rtp_parameters_by_kind(kind, RTCRtpTransceiverDirection::Sendonly) - .await, - encodings: vec![RTCRtpEncodingParameters { - ssrc: self.ssrc, + { + let track_encodings = self.track_encodings.lock().await; + for te in track_encodings.iter() { + let track = te.track.lock().await; + let rid = track + .as_ref() + .map_or(String::from(""), |t| String::from(t.rid())); + + encodings.push(RTCRtpEncodingParameters { + ssrc: te.ssrc, payload_type: self.payload_type, + rid, ..Default::default() - }], + }) } + } + + let mut send_parameters = RTCRtpSendParameters { + rtp_parameters: self + .media_engine + .get_rtp_parameters_by_kind(self.kind, RTCRtpTransceiverDirection::Sendonly) + .await, + encodings, }; let codecs = { @@ -262,10 +322,10 @@ impl RTCRtpSender { if let Some(t) = t.upgrade() { t.get_codecs().await } else { - self.media_engine.get_codecs_by_kind(kind).await + self.media_engine.get_codecs_by_kind(self.kind).await } } else { - self.media_engine.get_codecs_by_kind(kind).await + self.media_engine.get_codecs_by_kind(self.kind).await } }; send_parameters.rtp_parameters.codecs = codecs; @@ -275,8 +335,13 @@ impl RTCRtpSender { /// track returns the RTCRtpTransceiver track, or nil pub async fn track(&self) -> Option> { - let track = self.track.lock().await; - track.clone() + let encodings = self.track_encodings.lock().await; + if let Some(t) = encodings.first() { + let track = t.track.lock().await; + track.clone() + } else { + None + } } /// replace_track replaces the track currently being used as the sender's source with a new TrackLocal. @@ -287,6 +352,11 @@ impl RTCRtpSender { track: Option>, ) -> Result<()> { if let Some(t) = &track { + let encodings = self.track_encodings.lock().await; + if encodings.len() > 1 { + // return ErrRTPSenderNewTrackHasIncorrectEnvelope + return Err(Error::ErrRTPSenderNewTrackHasIncorrectKind); + } let tr = self.rtp_transceiver.lock().await; if let Some(r) = &*tr { if let Some(r) = r.upgrade() { @@ -301,69 +371,81 @@ impl RTCRtpSender { } } - if self.has_sent().await { - let t = { - let t = self.track.lock().await; - t.clone() - }; - if let Some(t) = t { - let context = self.context.lock().await; - t.unbind(&context).await?; + let encodings = self.track_encodings.lock().await; + if let Some(re) = encodings.first() { + if self.has_sent().await { + let t = { + let t = re.track.lock().await; + t.clone() + }; + if let Some(t) = t { + let context = re.context.lock().await; + t.unbind(&context).await?; + } } - } - if !self.has_sent().await || track.is_none() { - let mut t = self.track.lock().await; - *t = track; - return Ok(()); - } + if !self.has_sent().await || track.is_none() { + let mut t = re.track.lock().await; + *t = track; + return Ok(()); + } - let context = { - let context = self.context.lock().await; - context.clone() - }; + let context = { + let context = re.context.lock().await; + context.clone() + }; - let result = if let Some(t) = &track { - let new_context = TrackLocalContext { - id: context.id.clone(), - params: self - .media_engine - .get_rtp_parameters_by_kind(t.kind(), RTCRtpTransceiverDirection::Sendonly) - .await, - ssrc: context.ssrc, - write_stream: context.write_stream.clone(), - paused: self.paused.clone(), + let result = if let Some(t) = &track { + let new_context = TrackLocalContext { + id: context.id.clone(), + params: self + .media_engine + .get_rtp_parameters_by_kind( + t.kind(), + RTCRtpTransceiverDirection::Sendonly, + ) + .await, + ssrc: context.ssrc, + write_stream: context.write_stream.clone(), + rtcp_intercepter: context.rtcp_intercepter.clone(), + paused: self.paused.clone(), + }; + + t.bind(&new_context).await + } else { + Err(Error::ErrRTPSenderTrackNil) }; - t.bind(&new_context).await - } else { - Err(Error::ErrRTPSenderTrackNil) - }; + match result { + Err(err) => { + // Re-bind the original track + let track = re.track.lock().await; + if let Some(t) = &*track { + t.bind(&context).await?; + } - match result { - Err(err) => { - // Re-bind the original track - let track = self.track.lock().await; - if let Some(t) = &*track { - t.bind(&context).await?; + Err(err) } + Ok(codec) => { + // Codec has changed + if self.payload_type != codec.payload_type { + let mut context = re.context.lock().await; + context.params.codecs = vec![codec]; + } - Err(err) - } - Ok(codec) => { - // Codec has changed - if self.payload_type != codec.payload_type { - let mut context = self.context.lock().await; - context.params.codecs = vec![codec]; - } + { + let mut t = re.track.lock().await; + *t = track; + } - { - let mut t = self.track.lock().await; - *t = track; + Ok(()) } - - Ok(()) } + } else { + // Is it though? + // How do we end up in a state where we don't have at the very least, the default track + // encoding? + Ok(()) } } @@ -396,44 +478,72 @@ impl RTCRtpSender { paused: self.paused.clone(), }; - let codec = if let Some(t) = &*track { - t.bind(&context).await? - } else { - RTCRtpCodecParameters::default() + let encodings = self.track_encodings.lock().await; + for te in encodings.iter() { + let write_stream = Arc::new(InterceptorToTrackLocalWriter::new(self.paused.clone())); + let (context, stream_info) = { + let track = te.track.lock().await; + let mut context = TrackLocalContext { + id: self.id.clone(), + params: self + .media_engine + .get_rtp_parameters_by_kind( + if let Some(t) = &*track { + t.kind() + } else { + RTPCodecType::default() + }, + RTCRtpTransceiverDirection::Sendonly, + ) + .await, + ssrc: te.ssrc, + rtcp_intercepter: Some(te.rtcp_interceptor.clone()), + write_stream: Some( + Arc::clone(&write_stream) as Arc + ), + paused: self.paused.clone(), + }; + + let (codec, rid) = if let Some(t) = &*track { + let codec = t.bind(&context).await?; + (codec, t.rid()) + } else { + (RTCRtpCodecParameters::default(), "") + }; + let payload_type = codec.payload_type; + let capability = codec.capability.clone(); + context.params.codecs = vec![codec]; + let stream_info = create_stream_info( + self.id.clone(), + te.ssrc, + rid.to_owned(), + payload_type, + capability, + ¶meters.rtp_parameters.header_extensions, + ); + + (context, stream_info) }; - let payload_type = codec.payload_type; - let capability = codec.capability.clone(); - context.params.codecs = vec![codec]; - let stream_info = create_stream_info( - self.id.clone(), - parameters.encodings[0].ssrc, - payload_type, - capability, - ¶meters.rtp_parameters.header_extensions, - ); - - (context, stream_info) - }; - let srtp_rtp_writer = Arc::clone(&self.srtp_stream) as Arc; - let rtp_interceptor = self - .interceptor - .bind_local_stream(&stream_info, srtp_rtp_writer) - .await; - { - let mut interceptor_rtp_writer = write_stream.interceptor_rtp_writer.lock().await; - *interceptor_rtp_writer = Some(rtp_interceptor); - } + let srtp_rtp_writer = Arc::clone(&te.srtp_stream) as Arc; + let rtp_interceptor = self + .interceptor + .bind_local_stream(&stream_info, srtp_rtp_writer) + .await; + { + let mut interceptor_rtp_writer = write_stream.interceptor_rtp_writer.lock().await; + *interceptor_rtp_writer = Some(rtp_interceptor); + } - { - let mut ctx = self.context.lock().await; - *ctx = context; + { + let mut ctx = te.context.lock().await; + *ctx = context; + } + { + let mut si = te.stream_info.lock().await; + *si = stream_info; + } } - { - let mut si = self.stream_info.lock().await; - *si = stream_info; - } - { let mut send_called_tx = self.send_called_tx.lock().await; send_called_tx.take(); @@ -456,24 +566,72 @@ impl RTCRtpSender { self.replace_track(None).await?; - { - let stream_info = self.stream_info.lock().await; + let encodings = self.track_encodings.lock().await; + for te in encodings.iter() { + let stream_info = te.stream_info.lock().await; self.interceptor.unbind_local_stream(&stream_info).await; + te.srtp_stream.close().await? } - - self.srtp_stream.close().await + Ok(()) } /// read reads incoming RTCP for this RTPReceiver pub async fn read(&self, b: &mut [u8]) -> Result<(usize, Attributes)> { - self.internal.read(b).await + if let Some(encoding) = self.first_encoding().await? { + self.internal.read(&encoding, b).await + } else { + Err(Error::ErrInterceptorNotBind) + } + } + + // Having a mutex on that little collection sure does make this whole module fun + // These helpers exist because otherwise I accidentally end up locking on blocking calls + // because I'm incapable of writing threadsafe code + async fn encoding_for_rid(&self, rid: &str) -> Option> { + let encodings = self.track_encodings.lock().await; + for e in encodings.iter() { + if let Some(track) = &*e.track.lock().await { + if track.rid() == rid { + return Some(e.clone()); + } + }; + } + None + } + + async fn first_encoding(&self) -> Result>> { + let encodings = self.track_encodings.lock().await; + return Ok(encodings.first().map(|x| (*x).clone())); + } + + pub async fn read_simulcast(&self, b: &mut [u8], rid: &str) -> Result<(usize, Attributes)> { + if let Some(encoding) = self.encoding_for_rid(rid).await { + return self.internal.read(&encoding, b).await; + } else { + Err(Error::ErrInterceptorNotBind) + } + } + + pub async fn read_simulcast_rtcp( + &self, + rid: &str, + ) -> Result<(Vec>, Attributes)> { + if let Some(encoding) = self.encoding_for_rid(rid).await { + return self.internal.read_rtcp(&encoding, self.receive_mtu).await; + } else { + Err(Error::ErrInterceptorNotBind) + } } /// read_rtcp is a convenience method that wraps Read and unmarshals for you. pub async fn read_rtcp( &self, ) -> Result<(Vec>, Attributes)> { - self.internal.read_rtcp(self.receive_mtu).await + if let Some(encoding) = self.first_encoding().await? { + self.internal.read_rtcp(&encoding, self.receive_mtu).await + } else { + Err(Error::ErrInterceptorNotBind) + } } /// has_sent tells if data has been ever sent for this instance @@ -504,22 +662,4 @@ impl RTCRtpSender { Ok(()) } - - pub(crate) fn associate_media_stream_id(&self, id: String) -> bool { - let mut lock = self.associated_media_stream_ids.lock().unwrap(); - - if lock.contains(&id) { - return false; - } - - lock.push(id); - - true - } - - pub(crate) fn associated_media_stream_ids(&self) -> Vec { - let lock = self.associated_media_stream_ids.lock().unwrap(); - - lock.clone() - } } diff --git a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs index 12bcef3e4..f3486b8e3 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs @@ -145,7 +145,7 @@ async fn test_rtp_sender_get_parameters() -> Result<()> { let parameters = sender.get_parameters().await; assert_ne!(0, parameters.rtp_parameters.codecs.len()); assert_eq!(1, parameters.encodings.len()); - assert_eq!(sender.ssrc, parameters.encodings[0].ssrc); + assert_eq!(sender.track_encodings[0].ssrc, parameters.encodings[0].ssrc); } else { assert!(false); } diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index c64a7077d..2a5fad020 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -9,7 +9,7 @@ use crate::rtp_transceiver::rtp_codec::*; use crate::rtp_transceiver::*; use async_trait::async_trait; -use interceptor::{Attributes, RTPWriter}; +use interceptor::{Attributes, RTCPReader, RTPWriter}; use std::any::Any; use std::fmt; use std::sync::atomic::{AtomicBool, Ordering}; @@ -29,13 +29,14 @@ pub trait TrackLocalWriter: fmt::Debug { /// TrackLocalContext is the Context passed when a TrackLocal has been Binded/Unbinded from a PeerConnection, and used /// in Interceptors. -#[derive(Default, Debug, Clone)] +#[derive(Default, Clone)] pub struct TrackLocalContext { pub(crate) id: String, pub(crate) params: RTCRtpParameters, pub(crate) ssrc: SSRC, pub(crate) write_stream: Option>, pub(crate) paused: Arc, + pub(crate) rtcp_intercepter: Option>, } impl TrackLocalContext { @@ -63,6 +64,10 @@ impl TrackLocalContext { self.write_stream.clone() } + pub fn rtcp_reader(&self) -> Option> { + self.rtcp_intercepter.clone() + } + /// id is a unique identifier that is used for both bind/unbind pub fn id(&self) -> String { self.id.clone() @@ -87,6 +92,9 @@ pub trait TrackLocal { /// and stream_id would be 'desktop' or 'webcam' fn id(&self) -> &str; + /// rid is the RTP identifier for this Track + fn rid(&self) -> &str; + /// stream_id is the group this track belongs too. This must be unique fn stream_id(&self) -> &str; diff --git a/webrtc/src/track/track_local/track_local_static_rtp.rs b/webrtc/src/track/track_local/track_local_static_rtp.rs index 58bd40033..d6a35fc16 100644 --- a/webrtc/src/track/track_local/track_local_static_rtp.rs +++ b/webrtc/src/track/track_local/track_local_static_rtp.rs @@ -10,6 +10,7 @@ pub struct TrackLocalStaticRTP { pub(crate) bindings: Mutex>>, codec: RTCRtpCodecCapability, id: String, + rid: String, stream_id: String, } @@ -20,10 +21,15 @@ impl TrackLocalStaticRTP { codec, bindings: Mutex::new(vec![]), id, + rid: "".to_owned(), stream_id, } } + pub fn set_rid(&mut self, rid: String) { + self.rid = rid; + } + /// codec gets the Codec of the track pub fn codec(&self) -> RTCRtpCodecCapability { self.codec.clone() @@ -105,6 +111,10 @@ impl TrackLocal for TrackLocalStaticRTP { self.stream_id.as_str() } + fn rid(&self) -> &str { + self.rid.as_str() + } + /// kind controls if this TrackLocal is audio or video fn kind(&self) -> RTPCodecType { if self.codec.mime_type.starts_with("audio/") { diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index 7c8237788..0462073a5 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -44,6 +44,11 @@ impl TrackLocalStaticSample { self.rtp_track.codec() } + // Sets the RID for this track + pub fn set_rid(&mut self, rid: String) { + self.rtp_track.set_rid(rid); + } + /// write_sample writes a Sample to the TrackLocalStaticSample /// If one PeerConnection fails the packets will still be sent to /// all PeerConnections. The error message will contain the ID of the failed @@ -172,6 +177,10 @@ impl TrackLocal for TrackLocalStaticSample { self.rtp_track.id() } + fn rid(&self) -> &str { + self.rtp_track.rid() + } + /// stream_id is the group this track belongs too. This must be unique fn stream_id(&self) -> &str { self.rtp_track.stream_id() From e20c15de521c5667e8e28377e00089b66662c2a0 Mon Sep 17 00:00:00 2001 From: robashton Date: Fri, 7 Oct 2022 15:45:26 +0100 Subject: [PATCH 17/37] Fix the tests (for now) --- webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs index f3486b8e3..f6ede2006 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs @@ -142,10 +142,11 @@ async fn test_rtp_sender_get_parameters() -> Result<()> { signal_pair(&mut offerer, &mut answerer).await?; if let Some(sender) = rtp_transceiver.sender().await { + let encoding = { sender.track_encodings.lock().await[0].clone() }; let parameters = sender.get_parameters().await; assert_ne!(0, parameters.rtp_parameters.codecs.len()); assert_eq!(1, parameters.encodings.len()); - assert_eq!(sender.track_encodings[0].ssrc, parameters.encodings[0].ssrc); + assert_eq!(encoding.ssrc, parameters.encodings[0].ssrc); } else { assert!(false); } From 866965efb67b6ce93f905f4522a9a921d75c8dc6 Mon Sep 17 00:00:00 2001 From: robashton Date: Fri, 7 Oct 2022 15:57:57 +0100 Subject: [PATCH 18/37] clippy warnings (I guess I need to update/downgrade my rust) --- webrtc/src/peer_connection/mod.rs | 6 +++--- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs index dcee628ce..9fe7f75b6 100644 --- a/webrtc/src/peer_connection/mod.rs +++ b/webrtc/src/peer_connection/mod.rs @@ -466,13 +466,13 @@ 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 { - Some(s) => s.clone(), + let _sender = match t.sender().await { + Some(s) => s, None => { log::warn!( "RtpSender missing for transeceiver with sending direction {} for mid {}", diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 4ddb2e9c8..23cf5952d 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -606,7 +606,7 @@ impl RTCRtpSender { pub async fn read_simulcast(&self, b: &mut [u8], rid: &str) -> Result<(usize, Attributes)> { if let Some(encoding) = self.encoding_for_rid(rid).await { - return self.internal.read(&encoding, b).await; + self.internal.read(&encoding, b).await } else { Err(Error::ErrInterceptorNotBind) } @@ -617,7 +617,7 @@ impl RTCRtpSender { rid: &str, ) -> Result<(Vec>, Attributes)> { if let Some(encoding) = self.encoding_for_rid(rid).await { - return self.internal.read_rtcp(&encoding, self.receive_mtu).await; + self.internal.read_rtcp(&encoding, self.receive_mtu).await } else { Err(Error::ErrInterceptorNotBind) } From 25cedc0fa9680cb47c75c446b22a67d440c8023e Mon Sep 17 00:00:00 2001 From: robashton Date: Mon, 10 Oct 2022 13:36:51 +0100 Subject: [PATCH 19/37] Re-instate changes from #217 --- webrtc/src/peer_connection/mod.rs | 22 ++++---- webrtc/src/peer_connection/sdp/mod.rs | 59 +++++++++++++------- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 11 ++++ 3 files changed, 59 insertions(+), 33 deletions(-) diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs index 9fe7f75b6..e16d92e51 100644 --- a/webrtc/src/peer_connection/mod.rs +++ b/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 => { diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 4f685532d..d3d4660dc 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -537,6 +537,7 @@ 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, @@ -544,26 +545,9 @@ pub(crate) async fn add_transceiver_sdp( 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 = vec![]; @@ -578,10 +562,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; } } diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 23cf5952d..c45ea237a 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/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>, + /// AssociatedMediaStreamIds from the WebRTC specifcations + pub(crate) associated_media_stream_ids: std::sync::Mutex>, + rtp_transceiver: Mutex>>, send_called_tx: Mutex>>, @@ -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), @@ -651,6 +656,12 @@ impl RTCRtpSender { lock.clone() } + pub(crate) fn associated_media_stream_ids(&self) -> Vec { + 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(); From 2972dd5541555375730c36a64d43870598410e86 Mon Sep 17 00:00:00 2001 From: robashton Date: Mon, 10 Oct 2022 13:49:25 +0100 Subject: [PATCH 20/37] Replace the mutex on track_encodings with a RwLock (most of our operations are reads) --- .../peer_connection_internal.rs | 2 +- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 54 +++++-------------- .../rtp_sender/rtp_sender_test.rs | 2 +- 3 files changed, 16 insertions(+), 42 deletions(-) diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 3efb1b729..5c9a9619f 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1542,7 +1542,7 @@ impl PeerConnectionInternal { RTPCodecType::Video => "video", }; - let encodings = sender.track_encodings.lock().await; + let encodings = sender.track_encodings.read().await; for e in encodings.iter() { let track_id = track.id().to_string(); track_infos.push(TrackInfo { diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index c45ea237a..ce9ba9eb2 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -20,7 +20,7 @@ use interceptor::stream_info::StreamInfo; use interceptor::{Attributes, Interceptor, RTCPReader, RTPWriter}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Weak}; -use tokio::sync::{mpsc, Mutex, Notify}; +use tokio::sync::{mpsc, Mutex, Notify, RwLock}; pub(crate) struct RTPSenderInternal { pub(crate) send_called_rx: Mutex>, @@ -87,7 +87,7 @@ pub struct TrackEncoding { /// RTPSender allows an application to control how a given Track is encoded and transmitted to a remote peer pub struct RTCRtpSender { - pub(crate) track_encodings: Mutex>>, + pub(crate) track_encodings: RwLock>>, pub(crate) transport: Arc, @@ -156,7 +156,7 @@ impl RTCRtpSender { }); let sender = RTCRtpSender { - track_encodings: Mutex::new(vec![]), + track_encodings: RwLock::new(vec![]), transport, payload_type: 0, @@ -258,7 +258,7 @@ impl RTCRtpSender { context: Mutex::new(TrackLocalContext::default()), }; - let mut encodings = self.track_encodings.lock().await; + let mut encodings = self.track_encodings.write().await; encodings.push(Arc::new(track_encoding)); } @@ -297,7 +297,7 @@ impl RTCRtpSender { let mut encodings: Vec = vec![]; { - let track_encodings = self.track_encodings.lock().await; + let track_encodings = self.track_encodings.read().await; for te in track_encodings.iter() { let track = te.track.lock().await; let rid = track @@ -340,7 +340,7 @@ impl RTCRtpSender { /// track returns the RTCRtpTransceiver track, or nil pub async fn track(&self) -> Option> { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; if let Some(t) = encodings.first() { let track = t.track.lock().await; track.clone() @@ -357,7 +357,7 @@ impl RTCRtpSender { track: Option>, ) -> Result<()> { if let Some(t) = &track { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; if encodings.len() > 1 { // return ErrRTPSenderNewTrackHasIncorrectEnvelope return Err(Error::ErrRTPSenderNewTrackHasIncorrectKind); @@ -376,7 +376,7 @@ impl RTCRtpSender { } } - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; if let Some(re) = encodings.first() { if self.has_sent().await { let t = { @@ -405,10 +405,7 @@ impl RTCRtpSender { id: context.id.clone(), params: self .media_engine - .get_rtp_parameters_by_kind( - t.kind(), - RTCRtpTransceiverDirection::Sendonly, - ) + .get_rtp_parameters_by_kind(t.kind(), RTCRtpTransceiverDirection::Sendonly) .await, ssrc: context.ssrc, write_stream: context.write_stream.clone(), @@ -460,30 +457,7 @@ impl RTCRtpSender { return Err(Error::ErrRTPSenderSendAlreadyCalled); } - let write_stream = Arc::new(InterceptorToTrackLocalWriter::new(self.paused.clone())); - let (context, stream_info) = { - let track = self.track.lock().await; - let mut context = TrackLocalContext { - id: self.id.clone(), - params: self - .media_engine - .get_rtp_parameters_by_kind( - if let Some(t) = &*track { - t.kind() - } else { - RTPCodecType::default() - }, - RTCRtpTransceiverDirection::Sendonly, - ) - .await, - ssrc: parameters.encodings[0].ssrc, - write_stream: Some( - Arc::clone(&write_stream) as Arc - ), - paused: self.paused.clone(), - }; - - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; for te in encodings.iter() { let write_stream = Arc::new(InterceptorToTrackLocalWriter::new(self.paused.clone())); let (context, stream_info) = { @@ -498,7 +472,7 @@ impl RTCRtpSender { } else { RTPCodecType::default() }, - RTCRtpTransceiverDirection::Sendonly, + RTCRtpTransceiverDirection::Sendonly, ) .await, ssrc: te.ssrc, @@ -571,7 +545,7 @@ impl RTCRtpSender { self.replace_track(None).await?; - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; for te in encodings.iter() { let stream_info = te.stream_info.lock().await; self.interceptor.unbind_local_stream(&stream_info).await; @@ -593,7 +567,7 @@ impl RTCRtpSender { // These helpers exist because otherwise I accidentally end up locking on blocking calls // because I'm incapable of writing threadsafe code async fn encoding_for_rid(&self, rid: &str) -> Option> { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; for e in encodings.iter() { if let Some(track) = &*e.track.lock().await { if track.rid() == rid { @@ -605,7 +579,7 @@ impl RTCRtpSender { } async fn first_encoding(&self) -> Result>> { - let encodings = self.track_encodings.lock().await; + let encodings = self.track_encodings.read().await; return Ok(encodings.first().map(|x| (*x).clone())); } diff --git a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs index f6ede2006..7c7e2b23f 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/rtp_sender_test.rs @@ -142,7 +142,7 @@ async fn test_rtp_sender_get_parameters() -> Result<()> { signal_pair(&mut offerer, &mut answerer).await?; if let Some(sender) = rtp_transceiver.sender().await { - let encoding = { sender.track_encodings.lock().await[0].clone() }; + let encoding = { sender.track_encodings.read().await[0].clone() }; let parameters = sender.get_parameters().await; assert_ne!(0, parameters.rtp_parameters.codecs.len()); assert_eq!(1, parameters.encodings.len()); From 55bc75cdc1a0ffe4917167da2614716c2f400250 Mon Sep 17 00:00:00 2001 From: robashton Date: Mon, 10 Oct 2022 15:13:56 +0100 Subject: [PATCH 21/37] Initial feedback from PR --- webrtc/src/error.rs | 5 ++- webrtc/src/peer_connection/sdp/mod.rs | 14 +++++---- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 33 +++++++++++++------- webrtc/src/track/track_local/mod.rs | 4 +-- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index 51a7d0e37..37b4c5b2c 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,7 +197,10 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, - #[error("Sender cannot encoding due to RID collision")] + #[error("Sender does not have track for RID")] + ErrRTPSenderNoTrackForRID, + + #[error("Sender cannot add encoding due to RID collision")] ErrRTPSenderRIDCollision, #[error("Sender cannot add encoding as provided track does not match base track")] diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index d3d4660dc..b41cefbf1 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -538,7 +538,7 @@ pub(crate) async fn add_transceiver_sdp( 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() { + for encoding in &send_parameters.encodings { media = media.with_media_source( encoding.ssrc, track.stream_id().to_owned(), /* cname */ @@ -551,11 +551,13 @@ pub(crate) async fn add_transceiver_sdp( if send_parameters.encodings.len() > 1 { let mut send_rids: Vec = vec![]; - for e in send_parameters.encodings.iter() { - let mut s: String = e.rid.clone(); - s.push_str(" send"); - media = media.with_value_attribute("rid".into(), s); - send_rids.push(e.rid.clone()) + for e in &send_parameters.encodings { + if let Some(rid) = e.rid.clone() { + let mut s: String = rid.clone(); + send_rids.push(rid); + s.push_str(" send"); + media = media.with_value_attribute("rid".into(), s); + } } // Simulcast) let mut s: String = "send ".to_owned(); diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index ce9ba9eb2..60d006b81 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -39,13 +39,13 @@ impl RTPSenderInternal { tokio::select! { _ = send_called_rx.recv() =>{ - let rtcp_interceptor = encoding.rtcp_interceptor.clone(); + let rtcp_reader = encoding.rtcp_reader.clone(); let a = Attributes::new(); tokio::select! { _ = self.stop_called_rx.notified() => { Err(Error::ErrClosedPipe) } - result = rtcp_interceptor.read(b, &a) => { + result = rtcp_reader.read(b, &a) => { Ok(result?) } } @@ -77,7 +77,7 @@ pub struct TrackEncoding { pub(crate) srtp_stream: Arc, - pub(crate) rtcp_interceptor: Arc, + pub(crate) rtcp_reader: Arc, pub(crate) stream_info: Mutex, pub(crate) context: Mutex, @@ -247,13 +247,13 @@ impl RTCRtpSender { }); let srtp_rtcp_reader = Arc::clone(&srtp_stream) as Arc; - let rtcp_interceptor = self.interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; + let rtcp_reader = self.interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; let track_encoding = TrackEncoding { track: Mutex::new(Some(track)), srtp_stream, ssrc, - rtcp_interceptor, + rtcp_reader, stream_info: Mutex::new(StreamInfo::default()), context: Mutex::new(TrackLocalContext::default()), }; @@ -409,8 +409,8 @@ impl RTCRtpSender { .await, ssrc: context.ssrc, write_stream: context.write_stream.clone(), - rtcp_intercepter: context.rtcp_intercepter.clone(), paused: self.paused.clone(), + rtcp_reader: context.rtcp_reader.clone(), }; t.bind(&new_context).await @@ -476,7 +476,7 @@ impl RTCRtpSender { ) .await, ssrc: te.ssrc, - rtcp_intercepter: Some(te.rtcp_interceptor.clone()), + rtcp_reader: Some(te.rtcp_reader.clone()), write_stream: Some( Arc::clone(&write_stream) as Arc ), @@ -563,9 +563,6 @@ impl RTCRtpSender { } } - // Having a mutex on that little collection sure does make this whole module fun - // These helpers exist because otherwise I accidentally end up locking on blocking calls - // because I'm incapable of writing threadsafe code async fn encoding_for_rid(&self, rid: &str) -> Option> { let encodings = self.track_encodings.read().await; for e in encodings.iter() { @@ -587,7 +584,7 @@ impl RTCRtpSender { if let Some(encoding) = self.encoding_for_rid(rid).await { self.internal.read(&encoding, b).await } else { - Err(Error::ErrInterceptorNotBind) + Err(Error::ErrRTPSenderNoTrackForRID) } } @@ -598,7 +595,7 @@ impl RTCRtpSender { if let Some(encoding) = self.encoding_for_rid(rid).await { self.internal.read_rtcp(&encoding, self.receive_mtu).await } else { - Err(Error::ErrInterceptorNotBind) + Err(Error::ErrRTPSenderNoTrackForRID) } } @@ -636,6 +633,18 @@ impl RTCRtpSender { lock.clone() } + pub(crate) fn associate_media_stream_id(&self, id: String) -> bool { + let mut lock = self.associated_media_stream_ids.lock().unwrap(); + + if lock.contains(&id) { + return false; + } + + lock.push(id); + + true + } + pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { let mut lock = self.initial_track_id.lock().unwrap(); diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index 2a5fad020..05b9e0ba2 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -36,7 +36,7 @@ pub struct TrackLocalContext { pub(crate) ssrc: SSRC, pub(crate) write_stream: Option>, pub(crate) paused: Arc, - pub(crate) rtcp_intercepter: Option>, + pub(crate) rtcp_reader: Option>, } impl TrackLocalContext { @@ -65,7 +65,7 @@ impl TrackLocalContext { } pub fn rtcp_reader(&self) -> Option> { - self.rtcp_intercepter.clone() + self.rtcp_reader.clone() } /// id is a unique identifier that is used for both bind/unbind From 8a59116e2a670987761f9a1ef422b524359c2919 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:37:34 +0100 Subject: [PATCH 22/37] Make Rid an Option and see how that falls out --- examples/examples/simulcast/simulcast.rs | 6 +- interceptor/src/stream_info.rs | 2 +- webrtc/src/error.rs | 4 ++ .../peer_connection_internal.rs | 10 ++- webrtc/src/rtp_transceiver/mod.rs | 4 +- .../src/rtp_transceiver/rtp_receiver/mod.rs | 12 ++-- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 61 +++++++++---------- webrtc/src/track/track_local/mod.rs | 2 +- .../track_local/track_local_static_rtp.rs | 10 +-- .../track_local/track_local_static_sample.rs | 15 ++--- webrtc/src/track/track_remote/mod.rs | 8 +-- 11 files changed, 68 insertions(+), 66 deletions(-) diff --git a/examples/examples/simulcast/simulcast.rs b/examples/examples/simulcast/simulcast.rs index bd804a2b9..fc0927411 100644 --- a/examples/examples/simulcast/simulcast.rs +++ b/examples/examples/simulcast/simulcast.rs @@ -160,7 +160,7 @@ async fn main() -> Result<()> { if let Some(track) = track { println!("Track has started"); - let rid = track.rid().to_owned(); + let rid = track.rid().unwrap_or("".to_owned()); let output_track = if let Some(output_track) = output_tracks.get(&rid) { Arc::clone(output_track) } else { @@ -199,7 +199,7 @@ async fn main() -> Result<()> { tokio::spawn(async move { // Read RTP packets being sent to webrtc-rs - println!("enter track loop {}", track.rid()); + println!("enter track loop {:?}", track.rid()); while let Ok((rtp, _)) = track.read_rtp().await { if let Err(err) = output_track.write_rtp(&rtp).await { if Error::ErrClosedPipe != err { @@ -210,7 +210,7 @@ async fn main() -> Result<()> { } } } - println!("exit track loop {}", track.rid()); + println!("exit track loop {:?}", track.rid()); }); } Box::pin(async {}) diff --git a/interceptor/src/stream_info.rs b/interceptor/src/stream_info.rs index c4d0b7cb5..95967545e 100644 --- a/interceptor/src/stream_info.rs +++ b/interceptor/src/stream_info.rs @@ -11,7 +11,7 @@ pub struct RTPHeaderExtension { #[derive(Default, Debug, Clone)] pub struct StreamInfo { pub id: String, - pub rid: String, + pub rid: Option, pub attributes: Attributes, pub ssrc: u32, pub payload_type: u8, diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index 37b4c5b2c..4c642a216 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,6 +197,10 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, + // TODO: Delete this once we've added the ones we actually need + // errRTPSenderTrackNil = errors.New("Track must not be nil") + // errRTPSenderDTLSTransportNil = errors.New("DTLSTransport must not be nil") + // errRTPSenderTrackRemoved = errors.New("Sender Track has been removed or replaced to nil") #[error("Sender does not have track for RID")] ErrRTPSenderNoTrackForRID, diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 5c9a9619f..918aac221 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -173,10 +173,8 @@ impl PeerConnectionInternal { let mut receiver_needs_stopped = false; for t in tracks { - if !t.rid().is_empty() { - if let Some(details) = - track_details_for_rid(&track_details, t.rid().to_owned()) - { + if let Some(rid) = t.rid().clone() { + if let Some(details) = track_details_for_rid(&track_details, rid) { t.set_id(details.id.clone()).await; t.set_stream_id(details.stream_id.clone()).await; continue; @@ -1141,7 +1139,7 @@ impl PeerConnectionInternal { let stream_info = create_stream_info( "".to_owned(), ssrc, - "".to_owned(), + None, params.codecs[0].payload_type, params.codecs[0].capability.clone(), ¶ms.header_extensions, @@ -1549,7 +1547,7 @@ impl PeerConnectionInternal { track_id, ssrc: e.ssrc, mid: mid.clone(), - rid: Some(track.rid().to_owned()), + rid: track.rid().clone(), kind, }); } diff --git a/webrtc/src/rtp_transceiver/mod.rs b/webrtc/src/rtp_transceiver/mod.rs index 3d2fcaea5..763ae48d2 100644 --- a/webrtc/src/rtp_transceiver/mod.rs +++ b/webrtc/src/rtp_transceiver/mod.rs @@ -93,7 +93,7 @@ pub struct RTCRtpRtxParameters { /// #[derive(Default, Debug, Clone, Serialize, Deserialize)] pub struct RTCRtpCodingParameters { - pub rid: String, + pub rid: Option, pub ssrc: SSRC, pub payload_type: PayloadType, pub rtx: RTCRtpRtxParameters, @@ -132,7 +132,7 @@ pub struct RTCRtpTransceiverInit { pub(crate) fn create_stream_info( id: String, ssrc: SSRC, - rid: String, + rid: Option, payload_type: PayloadType, codec: RTCRtpCodecCapability, webrtc_header_extensions: &[RTCRtpHeaderExtensionParameters], diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs index fa1c0c139..f80f7b677 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs @@ -198,7 +198,7 @@ impl RTPReceiverInternal { let tracks = self.tracks.read().await; for t in &*tracks { - if t.track.rid() == rid { + if t.track.rid().map_or(false, |r| r == rid) { if let Some(rtcp_interceptor) = &t.stream.rtcp_interceptor { let a = Attributes::new(); @@ -529,7 +529,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), encoding.ssrc, - "".to_owned(), + None, 0, codec.clone(), &global_params.header_extensions, @@ -587,7 +587,7 @@ impl RTCRtpReceiver { let stream_info = create_stream_info( "".to_owned(), rtx_ssrc, - "".to_owned(), + None, 0, codec.clone(), &global_params.header_extensions, @@ -656,7 +656,7 @@ impl RTCRtpReceiver { let mut encodings = vec![RTCRtpDecodingParameters::default(); encoding_size]; for (i, encoding) in encodings.iter_mut().enumerate() { if incoming.rids.len() > i { - encoding.rid = incoming.rids[i].clone(); + encoding.rid = Some(incoming.rids[i].clone()); } if incoming.ssrcs.len() > i { encoding.ssrc = incoming.ssrcs[i]; @@ -751,7 +751,7 @@ impl RTCRtpReceiver { ) -> Result> { let mut tracks = self.internal.tracks.write().await; for t in &mut *tracks { - if t.track.rid() == rid { + if t.track.rid().map_or(false, |r| r == rid) { t.track.set_kind(self.kind); if let Some(codec) = params.codecs.first() { t.track.set_codec(codec.clone()).await; @@ -779,7 +779,7 @@ impl RTCRtpReceiver { let mut tracks = self.internal.tracks.write().await; let l = tracks.len(); for t in &mut *tracks { - if (ssrc != 0 && l == 1) || t.track.rid() == rsid { + if (ssrc != 0 && l == 1) || t.track.rid().map_or(false, |r| r == rsid) { t.repair_stream = repair_stream; let receive_mtu = self.receive_mtu; diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 60d006b81..b33fcb672 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -190,10 +190,6 @@ impl RTCRtpSender { } pub async fn add_encoding(&self, track: Arc) -> Result<()> { - if track.rid() == "" { - return Err(Error::ErrRTPSenderRidNil); - } - if self.has_stopped().await { return Err(Error::ErrRTPSenderStopped); } @@ -202,33 +198,38 @@ impl RTCRtpSender { return Err(Error::ErrRTPSenderSendAlreadyCalled); } - // oops, somebody code-golf this for me - { - let ref_track = if let Some(t) = self.first_encoding().await? { - let t = t.track.lock().await; - if let Some(t) = &*t { - if t.rid() != "" { - t.clone() - } else { - return Err(Error::ErrRTPSenderNoBaseEncoding); - } + if track.rid().is_none() { + return Err(Error::ErrRTPSenderRidNil); + } + + if let Some(rid) = track.rid() { + if self.encoding_for_rid(&rid).await.is_some() { + return Err(Error::ErrRTPSenderRIDCollision); + } + } else { + return Err(Error::ErrRTPSenderRidNil); + } + + let ref_track = if let Some(t) = self.first_encoding().await? { + let t = t.track.lock().await; + if let Some(t) = &*t { + if t.rid().is_some() { + t.clone() } else { return Err(Error::ErrRTPSenderNoBaseEncoding); } } else { return Err(Error::ErrRTPSenderNoBaseEncoding); - }; - - if ref_track.id() != track.id() - || ref_track.stream_id() != track.stream_id() - || ref_track.kind() != track.kind() - { - return Err(Error::ErrRTPSenderBaseEncodingMismatch); } + } else { + return Err(Error::ErrRTPSenderNoBaseEncoding); + }; - if self.encoding_for_rid(track.rid()).await.is_some() { - return Err(Error::ErrRTPSenderRIDCollision); - } + if ref_track.id() != track.id() + || ref_track.stream_id() != track.stream_id() + || ref_track.kind() != track.kind() + { + return Err(Error::ErrRTPSenderBaseEncodingMismatch); } self.add_encoding_internal(track).await; @@ -300,9 +301,7 @@ impl RTCRtpSender { let track_encodings = self.track_encodings.read().await; for te in track_encodings.iter() { let track = te.track.lock().await; - let rid = track - .as_ref() - .map_or(String::from(""), |t| String::from(t.rid())); + let rid = track.as_ref().map_or(None, |t| t.rid().clone()); encodings.push(RTCRtpEncodingParameters { ssrc: te.ssrc, @@ -485,9 +484,9 @@ impl RTCRtpSender { let (codec, rid) = if let Some(t) = &*track { let codec = t.bind(&context).await?; - (codec, t.rid()) + (codec, t.rid().clone()) } else { - (RTCRtpCodecParameters::default(), "") + (RTCRtpCodecParameters::default(), None) }; let payload_type = codec.payload_type; let capability = codec.capability.clone(); @@ -495,7 +494,7 @@ impl RTCRtpSender { let stream_info = create_stream_info( self.id.clone(), te.ssrc, - rid.to_owned(), + rid.clone(), payload_type, capability, ¶meters.rtp_parameters.header_extensions, @@ -567,7 +566,7 @@ impl RTCRtpSender { let encodings = self.track_encodings.read().await; for e in encodings.iter() { if let Some(track) = &*e.track.lock().await { - if track.rid() == rid { + if track.rid().map_or(false, |r| r == rid) { return Some(e.clone()); } }; diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index 05b9e0ba2..431652292 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -93,7 +93,7 @@ pub trait TrackLocal { fn id(&self) -> &str; /// rid is the RTP identifier for this Track - fn rid(&self) -> &str; + fn rid(&self) -> Option; /// stream_id is the group this track belongs too. This must be unique fn stream_id(&self) -> &str; diff --git a/webrtc/src/track/track_local/track_local_static_rtp.rs b/webrtc/src/track/track_local/track_local_static_rtp.rs index d6a35fc16..fec676c8f 100644 --- a/webrtc/src/track/track_local/track_local_static_rtp.rs +++ b/webrtc/src/track/track_local/track_local_static_rtp.rs @@ -10,7 +10,7 @@ pub struct TrackLocalStaticRTP { pub(crate) bindings: Mutex>>, codec: RTCRtpCodecCapability, id: String, - rid: String, + rid: Option, stream_id: String, } @@ -21,13 +21,13 @@ impl TrackLocalStaticRTP { codec, bindings: Mutex::new(vec![]), id, - rid: "".to_owned(), + rid: None, stream_id, } } pub fn set_rid(&mut self, rid: String) { - self.rid = rid; + self.rid = Some(rid); } /// codec gets the Codec of the track @@ -111,8 +111,8 @@ impl TrackLocal for TrackLocalStaticRTP { self.stream_id.as_str() } - fn rid(&self) -> &str { - self.rid.as_str() + fn rid(&self) -> Option { + self.rid.clone() } /// kind controls if this TrackLocal is audio or video diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index 0462073a5..209dd42d6 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -110,12 +110,13 @@ impl TrackLocalStaticSample { if sample.prev_dropped_packets > 0 { packetizer.skip_samples(samples * sample.prev_dropped_packets as u32); } - /*println!( - "clock_rate={}, samples={}, {}", - clock_rate, - samples, - sample.duration.as_secs_f64() - );*/ + // log::info!( + // "clock_rate={}, samples={}, {}", + // clock_rate, + // samples, + // sample.duration.as_secs_f64() + // ); + // packetizer.packetize(&sample.data, samples).await? } else { vec![] @@ -177,7 +178,7 @@ impl TrackLocal for TrackLocalStaticSample { self.rtp_track.id() } - fn rid(&self) -> &str { + fn rid(&self) -> Option { self.rtp_track.rid() } diff --git a/webrtc/src/track/track_remote/mod.rs b/webrtc/src/track/track_remote/mod.rs index 0f6382af3..ae0321b40 100644 --- a/webrtc/src/track/track_remote/mod.rs +++ b/webrtc/src/track/track_remote/mod.rs @@ -48,7 +48,7 @@ pub struct TrackRemote { ssrc: AtomicU32, //SSRC, codec: Mutex, pub(crate) params: Mutex, - rid: String, + rid: Option, media_engine: Arc, interceptor: Arc, @@ -79,7 +79,7 @@ impl TrackRemote { receive_mtu: usize, kind: RTPCodecType, ssrc: SSRC, - rid: String, + rid: Option, receiver: Weak, media_engine: Arc, interceptor: Arc, @@ -135,8 +135,8 @@ impl TrackRemote { /// rid gets the RTP Stream ID of this Track /// With Simulcast you will have multiple tracks with the same ID, but different RID values. /// In many cases a TrackRemote will not have an RID, so it is important to assert it is non-zero - pub fn rid(&self) -> &str { - self.rid.as_str() + pub fn rid(&self) -> Option { + self.rid.clone() } /// payload_type gets the PayloadType of the track From f780884613463ca2c48b3465f04b4f73f10428b1 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:47:35 +0100 Subject: [PATCH 23/37] revert change to log (that I did when I was debugging my own stuff\!) --- webrtc/src/track/track_local/track_local_static_sample.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index 209dd42d6..9e7e175ee 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -110,7 +110,7 @@ impl TrackLocalStaticSample { if sample.prev_dropped_packets > 0 { packetizer.skip_samples(samples * sample.prev_dropped_packets as u32); } - // log::info!( + // println!( // "clock_rate={}, samples={}, {}", // clock_rate, // samples, From 9707863f43a3083744c9c2afa46778be78c9ced9 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:53:37 +0100 Subject: [PATCH 24/37] unique track ids in stats --- webrtc/src/peer_connection/peer_connection_internal.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 918aac221..5cc5dd196 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1542,7 +1542,9 @@ impl PeerConnectionInternal { let encodings = sender.track_encodings.read().await; for e in encodings.iter() { - let track_id = track.id().to_string(); + let track_id = track.rid().map_or(track.id().to_string(), |rid| { + track.id().to_string() + "-" + rid.as_str() + }); track_infos.push(TrackInfo { track_id, ssrc: e.ssrc, From ffad0b5c87520f12c9f52348b86db22e5c6f2e20 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 10:54:01 +0100 Subject: [PATCH 25/37] debug impl for TrackLocalContext --- webrtc/src/track/track_local/mod.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index 431652292..e11961ea8 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -39,6 +39,17 @@ pub struct TrackLocalContext { pub(crate) rtcp_reader: Option>, } +impl std::fmt::Debug for TrackLocalContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TrackLocalContext") + .field("id", &self.id) + .field("params", &self.params) + .field("ssrc", &self.ssrc) + .field("write_stream", &self.write_stream) + .finish() + } +} + impl TrackLocalContext { /// codec_parameters returns the negotiated RTPCodecParameters. These are the codecs supported by both /// PeerConnections and the SSRC/PayloadTypes From 6b2366100f11f9cb8c622d7b7a3a8cd70944afe1 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:02:25 +0100 Subject: [PATCH 26/37] ensure we only do simulcast send directives if we're doing a send on this tranceiver --- webrtc/src/peer_connection/sdp/mod.rs | 48 ++++++++++++++------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index b41cefbf1..2398cd5c0 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -535,34 +535,36 @@ pub(crate) async fn add_transceiver_sdp( // This is equivalent to addSenderSDP in Pion (now) for mt in transceivers { 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 { - media = media.with_media_source( - encoding.ssrc, - track.stream_id().to_owned(), /* cname */ - track.stream_id().to_owned(), /* streamLabel */ - track.id().to_owned(), - ); - } + if mt.direction().has_send() { + let send_parameters = sender.get_parameters().await; + // Get the different encodings expressed first + for encoding in &send_parameters.encodings { + media = media.with_media_source( + encoding.ssrc, + track.stream_id().to_owned(), /* cname */ + track.stream_id().to_owned(), /* streamLabel */ + track.id().to_owned(), + ); + } - // Then tell the world about simulcast - if send_parameters.encodings.len() > 1 { - let mut send_rids: Vec = vec![]; + // Then tell the world about simulcast + if send_parameters.encodings.len() > 1 { + let mut send_rids: Vec = vec![]; - for e in &send_parameters.encodings { - if let Some(rid) = e.rid.clone() { - let mut s: String = rid.clone(); - send_rids.push(rid); - s.push_str(" send"); - media = media.with_value_attribute("rid".into(), s); + for e in &send_parameters.encodings { + if let Some(rid) = e.rid.clone() { + let mut s: String = rid.clone(); + send_rids.push(rid); + s.push_str(" send"); + media = media.with_value_attribute("rid".into(), s); + } } + // Simulcast) + let mut s: String = "send ".to_owned(); + s.push_str(send_rids.join(";").as_ref()); + media = media.with_value_attribute("simulcast".into(), s); } - // Simulcast) - let mut s: String = "send ".to_owned(); - 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 From 7853a8db11ac7fd7fda7f20b82a09a5487b61885 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:08:50 +0100 Subject: [PATCH 27/37] apparently format! is a thing --- webrtc/src/peer_connection/peer_connection_internal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index 5cc5dd196..e6beb7e34 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -1543,7 +1543,7 @@ impl PeerConnectionInternal { let encodings = sender.track_encodings.read().await; for e in encodings.iter() { let track_id = track.rid().map_or(track.id().to_string(), |rid| { - track.id().to_string() + "-" + rid.as_str() + format!("{}-{}", track.id(), rid) }); track_infos.push(TrackInfo { track_id, From afe4dcb9a43ea31ba72cc4c1173c28c9dae4322a Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:24:46 +0100 Subject: [PATCH 28/37] a few things in rtp_sender that we didn't ike --- webrtc/src/error.rs | 7 +- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 117 +++++++++---------- 2 files changed, 60 insertions(+), 64 deletions(-) diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs index 4c642a216..97241f902 100644 --- a/webrtc/src/error.rs +++ b/webrtc/src/error.rs @@ -197,10 +197,9 @@ pub enum Error { #[error("new track must be of the same kind as previous")] ErrRTPSenderNewTrackHasIncorrectKind, - // TODO: Delete this once we've added the ones we actually need - // errRTPSenderTrackNil = errors.New("Track must not be nil") - // errRTPSenderDTLSTransportNil = errors.New("DTLSTransport must not be nil") - // errRTPSenderTrackRemoved = errors.New("Sender Track has been removed or replaced to nil") + #[error("Cannot call replace on a sender with multiple tracks")] + ErrRTPSenderCannotReplaceSimulcast, + #[error("Sender does not have track for RID")] ErrRTPSenderNoTrackForRID, diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index b33fcb672..f82705fa2 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -358,8 +358,7 @@ impl RTCRtpSender { if let Some(t) = &track { let encodings = self.track_encodings.read().await; if encodings.len() > 1 { - // return ErrRTPSenderNewTrackHasIncorrectEnvelope - return Err(Error::ErrRTPSenderNewTrackHasIncorrectKind); + return Err(Error::ErrRTPSenderCannotReplaceSimulcast); } let tr = self.rtp_transceiver.lock().await; if let Some(r) = &*tr { @@ -376,77 +375,75 @@ impl RTCRtpSender { } let encodings = self.track_encodings.read().await; - if let Some(re) = encodings.first() { - if self.has_sent().await { - let t = { - let t = re.track.lock().await; - t.clone() - }; - if let Some(t) = t { - let context = re.context.lock().await; - t.unbind(&context).await?; - } - } + let re = match encodings.first() { + Some(re) => re, + None => return Ok(()), + }; - if !self.has_sent().await || track.is_none() { - let mut t = re.track.lock().await; - *t = track; - return Ok(()); + if self.has_sent().await { + let t = { + let t = re.track.lock().await; + t.clone() + }; + if let Some(t) = t { + let context = re.context.lock().await; + t.unbind(&context).await?; } + } - let context = { - let context = re.context.lock().await; - context.clone() - }; + if !self.has_sent().await || track.is_none() { + let mut t = re.track.lock().await; + *t = track; + return Ok(()); + } - let result = if let Some(t) = &track { - let new_context = TrackLocalContext { - id: context.id.clone(), - params: self - .media_engine - .get_rtp_parameters_by_kind(t.kind(), RTCRtpTransceiverDirection::Sendonly) - .await, - ssrc: context.ssrc, - write_stream: context.write_stream.clone(), - paused: self.paused.clone(), - rtcp_reader: context.rtcp_reader.clone(), - }; + let context = { + let context = re.context.lock().await; + context + }; - t.bind(&new_context).await - } else { - Err(Error::ErrRTPSenderTrackNil) + let result = if let Some(t) = &track { + let new_context = TrackLocalContext { + id: context.id.clone(), + params: self + .media_engine + .get_rtp_parameters_by_kind(t.kind(), RTCRtpTransceiverDirection::Sendonly) + .await, + ssrc: context.ssrc, + write_stream: context.write_stream.clone(), + rtcp_reader: context.rtcp_reader.clone(), + paused: context.paused.clone(), }; - match result { - Err(err) => { - // Re-bind the original track - let track = re.track.lock().await; - if let Some(t) = &*track { - t.bind(&context).await?; - } + t.bind(&new_context).await + } else { + Err(Error::ErrRTPSenderTrackNil) + }; - Err(err) + match result { + Err(err) => { + // Re-bind the original track + let track = re.track.lock().await; + if let Some(t) = &*track { + t.bind(&context).await?; } - Ok(codec) => { - // Codec has changed - if self.payload_type != codec.payload_type { - let mut context = re.context.lock().await; - context.params.codecs = vec![codec]; - } - { - let mut t = re.track.lock().await; - *t = track; - } + Err(err) + } + Ok(codec) => { + // Codec has changed + if self.payload_type != codec.payload_type { + let mut context = re.context.lock().await; + context.params.codecs = vec![codec]; + } - Ok(()) + { + let mut t = re.track.lock().await; + *t = track; } + + Ok(()) } - } else { - // Is it though? - // How do we end up in a state where we don't have at the very least, the default track - // encoding? - Ok(()) } } From b64f163cfd4645be6a6d2377e6982e13aa702200 Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:35:10 +0100 Subject: [PATCH 29/37] unnecessary clone now it's an option --- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index f82705fa2..9588511f8 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -6,7 +6,7 @@ use crate::dtls_transport::RTCDtlsTransport; use crate::error::{Error, Result}; use crate::rtp_transceiver::rtp_codec::{RTCRtpCodecParameters, RTPCodecType}; use crate::rtp_transceiver::rtp_transceiver_direction::RTCRtpTransceiverDirection; -use crate::rtp_transceiver::srtp_writer_future::SrtpWriterFuture; +use crate::rtp_transceiver::trtp_writer_future::SrtpWriterFuture; use crate::rtp_transceiver::{ create_stream_info, PayloadType, RTCRtpEncodingParameters, RTCRtpSendParameters, RTCRtpTransceiver, SSRC, @@ -301,7 +301,7 @@ impl RTCRtpSender { let track_encodings = self.track_encodings.read().await; for te in track_encodings.iter() { let track = te.track.lock().await; - let rid = track.as_ref().map_or(None, |t| t.rid().clone()); + let rid = track.as_ref().map_or(None, |t| t.rid()); encodings.push(RTCRtpEncodingParameters { ssrc: te.ssrc, From 6a242503f474edea7d302b0783a66e445790790e Mon Sep 17 00:00:00 2001 From: robashton Date: Tue, 11 Oct 2022 11:52:17 +0100 Subject: [PATCH 30/37] typo (got vimmed) --- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 9588511f8..ee1c5cbb7 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -6,7 +6,7 @@ use crate::dtls_transport::RTCDtlsTransport; use crate::error::{Error, Result}; use crate::rtp_transceiver::rtp_codec::{RTCRtpCodecParameters, RTPCodecType}; use crate::rtp_transceiver::rtp_transceiver_direction::RTCRtpTransceiverDirection; -use crate::rtp_transceiver::trtp_writer_future::SrtpWriterFuture; +use crate::rtp_transceiver::srtp_writer_future::SrtpWriterFuture; use crate::rtp_transceiver::{ create_stream_info, PayloadType, RTCRtpEncodingParameters, RTCRtpSendParameters, RTCRtpTransceiver, SSRC, From fd84deb90459397a383799b6ae31573f95bc77fe Mon Sep 17 00:00:00 2001 From: robashton Date: Thu, 13 Oct 2022 11:28:22 +0100 Subject: [PATCH 31/37] don't hold onto that lock, or we hang --- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index ee1c5cbb7..716c79f52 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -374,10 +374,12 @@ impl RTCRtpSender { } } - let encodings = self.track_encodings.read().await; - let re = match encodings.first() { - Some(re) => re, - None => return Ok(()), + let re = { + let encodings = self.track_encodings.read().await; + match encodings.first() { + Some(re) => re.clone(), + None => return Ok(()), + } }; if self.has_sent().await { @@ -399,7 +401,7 @@ impl RTCRtpSender { let context = { let context = re.context.lock().await; - context + context.clone() }; let result = if let Some(t) = &track { From f5857dd37c87052ec20ef3047c76f4dd4d8708da Mon Sep 17 00:00:00 2001 From: robashton Date: Thu, 13 Oct 2022 12:35:33 +0100 Subject: [PATCH 32/37] some more tweaks --- webrtc/src/peer_connection/sdp/mod.rs | 2 - webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 40 +++++++++++-------- .../track_local/track_local_static_sample.rs | 7 ---- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 2398cd5c0..434752f1a 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -532,7 +532,6 @@ pub(crate) async fn add_transceiver_sdp( ); } - // This is equivalent to addSenderSDP in Pion (now) for mt in transceivers { if let Some(sender) = mt.sender().await { if let Some(track) = sender.track().await { @@ -569,7 +568,6 @@ pub(crate) async fn add_transceiver_sdp( // 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!( diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 716c79f52..b23a69437 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -247,6 +247,9 @@ impl RTCRtpSender { rtp_write_session: Mutex::new(None), }); + // TODO: Each encoding currently gets its own srtp_stream over the top of a shared + // transport, their own own rtcp_reader, and separate 'bind' calls. + // This is directly lifted from Pion but is perhaps the wrong way to go about things let srtp_rtcp_reader = Arc::clone(&srtp_stream) as Arc; let rtcp_reader = self.interceptor.bind_rtcp_reader(srtp_rtcp_reader).await; @@ -295,13 +298,13 @@ impl RTCRtpSender { /// get_parameters describes the current configuration for the encoding and /// transmission of media on the sender's track. pub async fn get_parameters(&self) -> RTCRtpSendParameters { - let mut encodings: Vec = vec![]; - - { + let encodings = { let track_encodings = self.track_encodings.read().await; + let mut encodings: Vec = + Vec::with_capacity(track_encodings.len()); for te in track_encodings.iter() { let track = te.track.lock().await; - let rid = track.as_ref().map_or(None, |t| t.rid()); + let rid = track.as_ref().and_then(|t| t.rid()); encodings.push(RTCRtpEncodingParameters { ssrc: te.ssrc, @@ -310,7 +313,8 @@ impl RTCRtpSender { ..Default::default() }) } - } + encodings + }; let mut send_parameters = RTCRtpSendParameters { rtp_parameters: self @@ -338,6 +342,8 @@ impl RTCRtpSender { } /// track returns the RTCRtpTransceiver track, or nil + /// In the case of an RTCRtpSender with multiple encodings, this will return the track for the + /// first encoding only pub async fn track(&self) -> Option> { let encodings = self.track_encodings.read().await; if let Some(t) = encodings.first() { @@ -625,10 +631,16 @@ impl RTCRtpSender { lock.clone() } - pub(crate) fn associated_media_stream_ids(&self) -> Vec { - let lock = self.associated_media_stream_ids.lock().unwrap(); + pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { + let mut lock = self.initial_track_id.lock().unwrap(); - lock.clone() + if lock.is_some() { + return Err(Error::ErrSenderInitialTrackIdAlreadySet); + } + + *lock = Some(id); + + Ok(()) } pub(crate) fn associate_media_stream_id(&self, id: String) -> bool { @@ -643,15 +655,9 @@ impl RTCRtpSender { true } - pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { - let mut lock = self.initial_track_id.lock().unwrap(); - - if lock.is_some() { - return Err(Error::ErrSenderInitialTrackIdAlreadySet); - } - - *lock = Some(id); + pub(crate) fn associated_media_stream_ids(&self) -> Vec { + let lock = self.associated_media_stream_ids.lock().unwrap(); - Ok(()) + lock.clone() } } diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index 9e7e175ee..2c6f04f56 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -110,13 +110,6 @@ impl TrackLocalStaticSample { if sample.prev_dropped_packets > 0 { packetizer.skip_samples(samples * sample.prev_dropped_packets as u32); } - // println!( - // "clock_rate={}, samples={}, {}", - // clock_rate, - // samples, - // sample.duration.as_secs_f64() - // ); - // packetizer.packetize(&sample.data, samples).await? } else { vec![] From 5aa6864241e9a11868e3ea4be377782a1b19cc5d Mon Sep 17 00:00:00 2001 From: robashton Date: Thu, 13 Oct 2022 12:45:45 +0100 Subject: [PATCH 33/37] merge from upstream --- webrtc/src/peer_connection/sdp/mod.rs | 2 ++ webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 25 ++++++++++---------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 434752f1a..2398cd5c0 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -532,6 +532,7 @@ pub(crate) async fn add_transceiver_sdp( ); } + // This is equivalent to addSenderSDP in Pion (now) for mt in transceivers { if let Some(sender) = mt.sender().await { if let Some(track) = sender.track().await { @@ -568,6 +569,7 @@ pub(crate) async fn add_transceiver_sdp( // 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!( diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index b23a69437..9024b2bc6 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -231,7 +231,6 @@ impl RTCRtpSender { { return Err(Error::ErrRTPSenderBaseEncodingMismatch); } - self.add_encoding_internal(track).await; Ok(()) } @@ -631,16 +630,10 @@ impl RTCRtpSender { lock.clone() } - pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { - let mut lock = self.initial_track_id.lock().unwrap(); - - if lock.is_some() { - return Err(Error::ErrSenderInitialTrackIdAlreadySet); - } - - *lock = Some(id); + pub(crate) fn associated_media_stream_ids(&self) -> Vec { + let lock = self.associated_media_stream_ids.lock().unwrap(); - Ok(()) + lock.clone() } pub(crate) fn associate_media_stream_id(&self, id: String) -> bool { @@ -655,9 +648,15 @@ impl RTCRtpSender { true } - pub(crate) fn associated_media_stream_ids(&self) -> Vec { - let lock = self.associated_media_stream_ids.lock().unwrap(); + pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { + let mut lock = self.initial_track_id.lock().unwrap(); - lock.clone() + if lock.is_some() { + return Err(Error::ErrSenderInitialTrackIdAlreadySet); + } + + *lock = Some(id); + + Ok(()) } } From 7da6936b43511d8a8a567027532c98a019ed30db Mon Sep 17 00:00:00 2001 From: robashton Date: Thu, 13 Oct 2022 12:57:47 +0100 Subject: [PATCH 34/37] removed comments --- webrtc/src/peer_connection/sdp/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 2398cd5c0..434752f1a 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -532,7 +532,6 @@ pub(crate) async fn add_transceiver_sdp( ); } - // This is equivalent to addSenderSDP in Pion (now) for mt in transceivers { if let Some(sender) = mt.sender().await { if let Some(track) = sender.track().await { @@ -569,7 +568,6 @@ pub(crate) async fn add_transceiver_sdp( // 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!( From 500709349b544769a85a3bc09fbefae11559935d Mon Sep 17 00:00:00 2001 From: robashton Date: Thu, 13 Oct 2022 13:01:42 +0100 Subject: [PATCH 35/37] revert method order --- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 24 +++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 9024b2bc6..40a423b6a 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -630,10 +630,16 @@ impl RTCRtpSender { lock.clone() } - pub(crate) fn associated_media_stream_ids(&self) -> Vec { - let lock = self.associated_media_stream_ids.lock().unwrap(); + pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { + let mut lock = self.initial_track_id.lock().unwrap(); - lock.clone() + if lock.is_some() { + return Err(Error::ErrSenderInitialTrackIdAlreadySet); + } + + *lock = Some(id); + + Ok(()) } pub(crate) fn associate_media_stream_id(&self, id: String) -> bool { @@ -648,15 +654,11 @@ impl RTCRtpSender { true } - pub(crate) fn set_initial_track_id(&self, id: String) -> Result<()> { - let mut lock = self.initial_track_id.lock().unwrap(); + pub(crate) fn associated_media_stream_ids(&self) -> Vec { + let lock = self.associated_media_stream_ids.lock().unwrap(); - if lock.is_some() { - return Err(Error::ErrSenderInitialTrackIdAlreadySet); - } + lock.clone() + } - *lock = Some(id); - Ok(()) - } } From 7114d5e52d6678115ff626b6c2693042f387086c Mon Sep 17 00:00:00 2001 From: robashton Date: Thu, 13 Oct 2022 14:28:49 +0100 Subject: [PATCH 36/37] remove read_simulcast_rtcp as we can't see a reason why it needs to exist --- webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 40a423b6a..2f780dd4d 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -591,17 +591,6 @@ impl RTCRtpSender { } } - pub async fn read_simulcast_rtcp( - &self, - rid: &str, - ) -> Result<(Vec>, Attributes)> { - if let Some(encoding) = self.encoding_for_rid(rid).await { - self.internal.read_rtcp(&encoding, self.receive_mtu).await - } else { - Err(Error::ErrRTPSenderNoTrackForRID) - } - } - /// read_rtcp is a convenience method that wraps Read and unmarshals for you. pub async fn read_rtcp( &self, @@ -659,6 +648,4 @@ impl RTCRtpSender { lock.clone() } - - } From 7c03a1162be552c34a9d07ab2835c6a28d0e3246 Mon Sep 17 00:00:00 2001 From: robashton Date: Mon, 17 Oct 2022 10:39:24 +0100 Subject: [PATCH 37/37] Option on structs, Option<&str> on traits (for rid anyway) --- examples/examples/simulcast/simulcast.rs | 2 +- .../peer_connection/peer_connection_internal.rs | 14 ++++++++------ webrtc/src/peer_connection/sdp/mod.rs | 4 ++-- webrtc/src/rtp_transceiver/mod.rs | 4 ++-- webrtc/src/rtp_transceiver/rtp_receiver/mod.rs | 12 ++++++------ webrtc/src/rtp_transceiver/rtp_sender/mod.rs | 8 ++++---- webrtc/src/track/track_local/mod.rs | 2 +- .../track/track_local/track_local_static_rtp.rs | 4 ++-- .../track/track_local/track_local_static_sample.rs | 2 +- webrtc/src/track/track_remote/mod.rs | 4 ++-- 10 files changed, 29 insertions(+), 27 deletions(-) diff --git a/examples/examples/simulcast/simulcast.rs b/examples/examples/simulcast/simulcast.rs index fc0927411..8e652c608 100644 --- a/examples/examples/simulcast/simulcast.rs +++ b/examples/examples/simulcast/simulcast.rs @@ -160,7 +160,7 @@ async fn main() -> Result<()> { if let Some(track) = track { println!("Track has started"); - let rid = track.rid().unwrap_or("".to_owned()); + let rid = track.rid().map_or(String::from(""), String::from); let output_track = if let Some(output_track) = output_tracks.get(&rid) { Arc::clone(output_track) } else { diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs index e6beb7e34..34037e59a 100644 --- a/webrtc/src/peer_connection/peer_connection_internal.rs +++ b/webrtc/src/peer_connection/peer_connection_internal.rs @@ -173,8 +173,10 @@ impl PeerConnectionInternal { let mut receiver_needs_stopped = false; for t in tracks { - if let Some(rid) = t.rid().clone() { - if let Some(details) = track_details_for_rid(&track_details, rid) { + if let Some(rid) = t.rid() { + if let Some(details) = + track_details_for_rid(&track_details, String::from(rid)) + { t.set_id(details.id.clone()).await; t.set_stream_id(details.stream_id.clone()).await; continue; @@ -1181,7 +1183,7 @@ impl PeerConnectionInternal { return receiver .receive_for_rtx( 0, - rsid, + &rsid, TrackStream { stream_info: Some(stream_info.clone()), rtp_read_stream, @@ -1195,7 +1197,7 @@ impl PeerConnectionInternal { let track = receiver .receive_for_rid( - rid, + &rid, params, TrackStream { stream_info: Some(stream_info.clone()), @@ -1549,7 +1551,7 @@ impl PeerConnectionInternal { track_id, ssrc: e.ssrc, mid: mid.clone(), - rid: track.rid().clone(), + rid: track.rid().map(String::from), kind, }); } @@ -1614,7 +1616,7 @@ impl PeerConnectionInternal { kind, packets_sent, mid, - rid, + rid: rid.map(|a| a.to_owned()), header_bytes_sent, bytes_sent, nack_count, diff --git a/webrtc/src/peer_connection/sdp/mod.rs b/webrtc/src/peer_connection/sdp/mod.rs index 434752f1a..aed5ddc8c 100644 --- a/webrtc/src/peer_connection/sdp/mod.rs +++ b/webrtc/src/peer_connection/sdp/mod.rs @@ -552,9 +552,9 @@ pub(crate) async fn add_transceiver_sdp( let mut send_rids: Vec = vec![]; for e in &send_parameters.encodings { - if let Some(rid) = e.rid.clone() { + if let Some(rid) = &e.rid { let mut s: String = rid.clone(); - send_rids.push(rid); + send_rids.push(rid.clone()); s.push_str(" send"); media = media.with_value_attribute("rid".into(), s); } diff --git a/webrtc/src/rtp_transceiver/mod.rs b/webrtc/src/rtp_transceiver/mod.rs index 763ae48d2..dc94a0cf7 100644 --- a/webrtc/src/rtp_transceiver/mod.rs +++ b/webrtc/src/rtp_transceiver/mod.rs @@ -132,7 +132,7 @@ pub struct RTCRtpTransceiverInit { pub(crate) fn create_stream_info( id: String, ssrc: SSRC, - rid: Option, + rid: Option<&str>, payload_type: PayloadType, codec: RTCRtpCodecCapability, webrtc_header_extensions: &[RTCRtpHeaderExtensionParameters], @@ -155,7 +155,7 @@ pub(crate) fn create_stream_info( StreamInfo { id, - rid, + rid: rid.map(String::from), attributes: Attributes::new(), ssrc, payload_type, diff --git a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs index f80f7b677..28be08529 100644 --- a/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_receiver/mod.rs @@ -198,7 +198,7 @@ impl RTPReceiverInternal { let tracks = self.tracks.read().await; for t in &*tracks { - if t.track.rid().map_or(false, |r| r == rid) { + if Some(rid) == t.track.rid() { if let Some(rtcp_interceptor) = &t.stream.rtcp_interceptor { let a = Attributes::new(); @@ -599,7 +599,7 @@ impl RTCRtpReceiver { self.receive_for_rtx( rtx_ssrc, - "".to_owned(), + "", TrackStream { stream_info: Some(stream_info), rtp_read_stream, @@ -745,13 +745,13 @@ impl RTCRtpReceiver { /// It populates all the internal state for the given RID pub(crate) async fn receive_for_rid( &self, - rid: String, + rid: &str, params: RTCRtpParameters, stream: TrackStream, ) -> Result> { let mut tracks = self.internal.tracks.write().await; for t in &mut *tracks { - if t.track.rid().map_or(false, |r| r == rid) { + if Some(rid) == t.track.rid() { t.track.set_kind(self.kind); if let Some(codec) = params.codecs.first() { t.track.set_codec(codec.clone()).await; @@ -773,13 +773,13 @@ impl RTCRtpReceiver { pub(crate) async fn receive_for_rtx( &self, ssrc: SSRC, - rsid: String, + rsid: &str, repair_stream: TrackStream, ) -> Result<()> { let mut tracks = self.internal.tracks.write().await; let l = tracks.len(); for t in &mut *tracks { - if (ssrc != 0 && l == 1) || t.track.rid().map_or(false, |r| r == rsid) { + if (ssrc != 0 && l == 1) || Some(rsid) == t.track.rid() { t.repair_stream = repair_stream; let receive_mtu = self.receive_mtu; diff --git a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs index 2f780dd4d..4aca6fc27 100644 --- a/webrtc/src/rtp_transceiver/rtp_sender/mod.rs +++ b/webrtc/src/rtp_transceiver/rtp_sender/mod.rs @@ -303,7 +303,7 @@ impl RTCRtpSender { Vec::with_capacity(track_encodings.len()); for te in track_encodings.iter() { let track = te.track.lock().await; - let rid = track.as_ref().and_then(|t| t.rid()); + let rid = track.as_ref().and_then(|t| t.rid().map(String::from)); encodings.push(RTCRtpEncodingParameters { ssrc: te.ssrc, @@ -488,7 +488,7 @@ impl RTCRtpSender { let (codec, rid) = if let Some(t) = &*track { let codec = t.bind(&context).await?; - (codec, t.rid().clone()) + (codec, t.rid()) } else { (RTCRtpCodecParameters::default(), None) }; @@ -498,7 +498,7 @@ impl RTCRtpSender { let stream_info = create_stream_info( self.id.clone(), te.ssrc, - rid.clone(), + rid, payload_type, capability, ¶meters.rtp_parameters.header_extensions, @@ -570,7 +570,7 @@ impl RTCRtpSender { let encodings = self.track_encodings.read().await; for e in encodings.iter() { if let Some(track) = &*e.track.lock().await { - if track.rid().map_or(false, |r| r == rid) { + if Some(rid) == track.rid() { return Some(e.clone()); } }; diff --git a/webrtc/src/track/track_local/mod.rs b/webrtc/src/track/track_local/mod.rs index e11961ea8..597824f98 100644 --- a/webrtc/src/track/track_local/mod.rs +++ b/webrtc/src/track/track_local/mod.rs @@ -104,7 +104,7 @@ pub trait TrackLocal { fn id(&self) -> &str; /// rid is the RTP identifier for this Track - fn rid(&self) -> Option; + fn rid(&self) -> Option<&str>; /// stream_id is the group this track belongs too. This must be unique fn stream_id(&self) -> &str; diff --git a/webrtc/src/track/track_local/track_local_static_rtp.rs b/webrtc/src/track/track_local/track_local_static_rtp.rs index fec676c8f..24e0c4b3d 100644 --- a/webrtc/src/track/track_local/track_local_static_rtp.rs +++ b/webrtc/src/track/track_local/track_local_static_rtp.rs @@ -111,8 +111,8 @@ impl TrackLocal for TrackLocalStaticRTP { self.stream_id.as_str() } - fn rid(&self) -> Option { - self.rid.clone() + fn rid(&self) -> Option<&str> { + self.rid.as_deref() } /// kind controls if this TrackLocal is audio or video diff --git a/webrtc/src/track/track_local/track_local_static_sample.rs b/webrtc/src/track/track_local/track_local_static_sample.rs index 2c6f04f56..dfb00636a 100644 --- a/webrtc/src/track/track_local/track_local_static_sample.rs +++ b/webrtc/src/track/track_local/track_local_static_sample.rs @@ -171,7 +171,7 @@ impl TrackLocal for TrackLocalStaticSample { self.rtp_track.id() } - fn rid(&self) -> Option { + fn rid(&self) -> Option<&str> { self.rtp_track.rid() } diff --git a/webrtc/src/track/track_remote/mod.rs b/webrtc/src/track/track_remote/mod.rs index ae0321b40..9ec722586 100644 --- a/webrtc/src/track/track_remote/mod.rs +++ b/webrtc/src/track/track_remote/mod.rs @@ -135,8 +135,8 @@ impl TrackRemote { /// rid gets the RTP Stream ID of this Track /// With Simulcast you will have multiple tracks with the same ID, but different RID values. /// In many cases a TrackRemote will not have an RID, so it is important to assert it is non-zero - pub fn rid(&self) -> Option { - self.rid.clone() + pub fn rid(&self) -> Option<&str> { + self.rid.as_deref() } /// payload_type gets the PayloadType of the track