From 818b2c664fad544eed128fbe57de5ed736f93b1e Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Mon, 24 Nov 2025 22:44:46 +0100 Subject: [PATCH 1/6] RTT as Duration --- CHANGELOG.md | 1 + src/stats.rs | 8 ++++---- src/streams/receive.rs | 8 ++++---- src/streams/send_stats.rs | 10 +++++----- src/util/mod.rs | 5 +++-- tests/stats.rs | 7 ++++--- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a07e2ddad..13e2c9a85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased + * RTT as Duration #756 * Network emulation for tests #774 * Apple Crypto backend #763 * Move all crypto backends out of main crate #768 diff --git a/src/stats.rs b/src/stats.rs index 87884ae39..df04fc376 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -130,8 +130,8 @@ pub struct MediaEgressStats { pub plis: u64, /// Number of nacks received. pub nacks: u64, - /// Round-trip-time (ms) extracted from the last RTCP receiver report. - pub rtt: Option, + /// Round-trip-time extracted from the last RTCP receiver report. + pub rtt: Option, /// Fraction of packets lost averaged from the RTCP receiver reports received. /// `None` if no reports have been received since the last event pub loss: Option, @@ -171,8 +171,8 @@ pub struct MediaIngressStats { pub plis: u64, /// Number of nacks sent. pub nacks: u64, - /// Round-trip-time (ms) extracted from the last RTCP XR DLRR report block. - pub rtt: Option, + /// Round-trip-time extracted from the last RTCP XR DLRR report block. + pub rtt: Option, /// Fraction of packets lost extracted from the last RTCP receiver report. pub loss: Option, /// Timestamp when this event was generated. diff --git a/src/streams/receive.rs b/src/streams/receive.rs index 5e3fb4716..0518fca14 100644 --- a/src/streams/receive.rs +++ b/src/streams/receive.rs @@ -10,7 +10,7 @@ use crate::rtp_::{ReportBlock, ReportList, Rid, Rrtr, Rtcp}; use crate::rtp_::{RtcpFb, RtpHeader, SenderInfo, SeqNo}; use crate::rtp_::{SdesType, Ssrc}; use crate::stats::{MediaIngressStats, RemoteEgressStats, StatsSnapshot}; -use crate::util::{already_happened, calculate_rtt_ms}; +use crate::util::{already_happened, calculate_rtt}; use crate::util::{InstantExt, SystemTimeExt}; use super::register::ReceiverRegister; @@ -121,8 +121,8 @@ pub(crate) struct StreamRxStats { plis: u64, /// count of NACKs sent nacks: u64, - /// round trip time (ms) from the last DLRR, if any - rtt: Option, + /// round trip time from the last DLRR, if any + rtt: Option, /// fraction of packets lost from the last RR, if any loss: Option, } @@ -289,7 +289,7 @@ impl StreamRx { fn set_dlrr_item(&mut self, now: Instant, dlrr: DlrrItem) { let ntp_time = now.to_ntp_duration(); - let rtt = calculate_rtt_ms(ntp_time, dlrr.last_rr_delay, dlrr.last_rr_time); + let rtt = calculate_rtt(ntp_time, dlrr.last_rr_delay, dlrr.last_rr_time); self.stats.rtt = rtt; } diff --git a/src/streams/send_stats.rs b/src/streams/send_stats.rs index 79dd5e55f..5fef5cb82 100644 --- a/src/streams/send_stats.rs +++ b/src/streams/send_stats.rs @@ -1,10 +1,10 @@ -use std::time::Instant; +use std::time::{Duration, Instant}; use crate::rtp::SeqNo; use crate::rtp_::{extend_u32, ReceptionReport}; use crate::stats::{MediaEgressStats, RemoteIngressStats, StatsSnapshot}; use crate::util::value_history::ValueHistory; -use crate::util::{calculate_rtt_ms, InstantExt}; +use crate::util::{calculate_rtt, InstantExt}; use super::MidRid; @@ -27,9 +27,9 @@ pub(crate) struct StreamTxStats { plis: u64, /// count of NACKs received nacks: u64, - /// round trip time (ms) + /// round trip time /// Can be null in case of missing or bad reports - rtt: Option, + rtt: Option, /// losses collecter from RR (known packets, lost ratio) losses: Losses, /// The last reception report for the stream, if any. @@ -86,7 +86,7 @@ impl StreamTxStats { pub fn update_with_rr(&mut self, now: Instant, last_sent_seq_no: SeqNo, r: ReceptionReport) { let ntp_time = now.to_ntp_duration(); - let rtt = calculate_rtt_ms(ntp_time, r.last_sr_delay, r.last_sr_time); + let rtt = calculate_rtt(ntp_time, r.last_sr_delay, r.last_sr_time); self.rtt = rtt; // The last_sent_seq_no should be in the vicinity of the rr.max_seq. diff --git a/src/util/mod.rs b/src/util/mod.rs index 59fb6798a..2d40bfb09 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -44,7 +44,7 @@ impl Soonest for (Option, T) { /// - `delay` the delay(`DLSR`) since last sender report expressed as fractions of a second in 32 bits. /// - `last_report` the middle 32 bits of an NTP timestamp for the most recent sender report(LSR) /// or Receiver Report(LRR). -pub(crate) fn calculate_rtt_ms(ntp_time: Duration, delay: u32, last_report: u32) -> Option { +pub(crate) fn calculate_rtt(ntp_time: Duration, delay: u32, last_report: u32) -> Option { // [10 Nov 1995 11:33:25.125 UTC] [10 Nov 1995 11:33:36.5 UTC] // n SR(n) A=b710:8000 (46864.500 s) // ----------------------------------------------------------------> @@ -84,7 +84,8 @@ pub(crate) fn calculate_rtt_ms(ntp_time: Duration, delay: u32, last_report: u32) let rtt_seconds = rtt >> 16; let rtt_fraction = (rtt & (u16::MAX as u32)) as f32 / (u16::MAX as u32) as f32; - Some(rtt_seconds as f32 * 1000.0 + rtt_fraction * 1000.0) + let nanos = (rtt_fraction * 1_000_000_000.0) as u32; + Some(Duration::new(rtt_seconds as u64, nanos)) } pub struct NonCryptographicRng; diff --git a/tests/stats.rs b/tests/stats.rs index 76e4a0e85..e14181816 100644 --- a/tests/stats.rs +++ b/tests/stats.rs @@ -107,7 +107,8 @@ pub fn stats() -> Result<(), RtcError> { egress_stats_l .iter() .filter_map(|egress_stat_l| egress_stat_l.rtt) - .for_each(|rtt| assert!(rtt < 100_f32)); // rtt should be under 100ms in this scenario + // rtt should be under 100ms in this scenario + .for_each(|rtt| assert!(rtt < Duration::from_millis(100))); let egress_stats_r: Vec = l .events @@ -125,8 +126,8 @@ pub fn stats() -> Result<(), RtcError> { egress_stats_r .iter() .filter_map(|egress_stat_l| egress_stat_l.rtt) - .for_each(|rtt| assert!(rtt < 100_f32)); // rtt should be under 100ms in this scenario - + // rtt should be under 100ms in this scenario + .for_each(|rtt| assert!(rtt < Duration::from_millis(100))); assert!( media_count_l > 1100, "Not enough MediaData at L: {}", From ac58fb23d1ca7dfed52e150fef1cdb75d7fddc53 Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Thu, 11 Dec 2025 10:21:54 +0100 Subject: [PATCH 2/6] Bump wincrypto dev-dep --- crypto/wincrypto/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/wincrypto/Cargo.toml b/crypto/wincrypto/Cargo.toml index 382c6e672..3f2e95393 100644 --- a/crypto/wincrypto/Cargo.toml +++ b/crypto/wincrypto/Cargo.toml @@ -22,4 +22,4 @@ windows = { version = "0.58", features = [ ] } [dev-dependencies] -str0m = { version = "0.12.0", path = "../../" } +str0m = { version = "0.13.0", path = "../../" } From da41874572aff965e6c40e5703353f10ee6ed60c Mon Sep 17 00:00:00 2001 From: Hugo Tunius Date: Fri, 12 Dec 2025 15:02:30 +0000 Subject: [PATCH 3/6] Reduce reallocation when depacketizing I noticed we spent a fair amount of time reallocating growing the `out` vector in `Depacketizer::depacketize`. With this change the Depacketizer can provide a hint for the total size required and avoid these reallocations. --- src/packet/buffer_rx.rs | 11 ++++++++++- src/packet/g7xx.rs | 4 ++++ src/packet/h264.rs | 6 ++++++ src/packet/h265.rs | 4 ++++ src/packet/mod.rs | 21 +++++++++++++++++++++ src/packet/null.rs | 4 ++++ src/packet/opus.rs | 4 ++++ src/packet/vp8.rs | 4 ++++ src/packet/vp9.rs | 4 ++++ 9 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/packet/buffer_rx.rs b/src/packet/buffer_rx.rs index f563ed552..a74230ba3 100644 --- a/src/packet/buffer_rx.rs +++ b/src/packet/buffer_rx.rs @@ -254,7 +254,12 @@ impl DepacketizingBuffer { } } - let mut data = Vec::new(); + let packets_size = self.queue.range(start..=stop).map(|p| p.data.len()).sum(); + let mut data = self + .depack + .out_size_hint(packets_size) + .map(Vec::with_capacity) + .unwrap_or_else(Vec::new); let mut codec_extra = CodecExtra::None; let time = self.queue.get(start).expect("first index exist").meta.time; @@ -592,6 +597,10 @@ mod test { struct TestDepack; impl Depacketizer for TestDepack { + fn out_size_hint(&self, packets_size: usize) -> Option { + Some(packets_size) + } + fn depacketize( &mut self, packet: &[u8], diff --git a/src/packet/g7xx.rs b/src/packet/g7xx.rs index 9579f486f..ff72456a1 100644 --- a/src/packet/g7xx.rs +++ b/src/packet/g7xx.rs @@ -42,6 +42,10 @@ impl Packetizer for G7xxPacketizer { pub struct G711Depacketizer; impl Depacketizer for G711Depacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + Some(packets_size) + } + fn depacketize( &mut self, packet: &[u8], diff --git a/src/packet/h264.rs b/src/packet/h264.rs index 43e272f11..4cc7a025f 100644 --- a/src/packet/h264.rs +++ b/src/packet/h264.rs @@ -224,6 +224,12 @@ pub struct H264Depacketizer { } impl Depacketizer for H264Depacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + // Roughly account for Annex B start codes or AVC length prefixes. + let estimated_packets = (packets_size / 1200).saturating_add(1); + Some(packets_size.saturating_add(4usize.saturating_mul(estimated_packets))) + } + /// depacketize parses the passed byte slice and stores the result in the /// H264Packet this method is called upon fn depacketize( diff --git a/src/packet/h265.rs b/src/packet/h265.rs index f6a93ef5f..e37f2481a 100644 --- a/src/packet/h265.rs +++ b/src/packet/h265.rs @@ -756,6 +756,10 @@ impl H265Depacketizer { } impl Depacketizer for H265Depacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + Some(packets_size) + } + /// depacketize parses the passed byte slice and stores the result /// in the H265Packet this method is called upon fn depacketize( diff --git a/src/packet/mod.rs b/src/packet/mod.rs index 005aca581..42815f935 100644 --- a/src/packet/mod.rs +++ b/src/packet/mod.rs @@ -111,6 +111,13 @@ pub enum CodecExtra { /// This trait is not currently versioned according to semver rules. /// Breaking changes may be made in minor or patch releases. pub trait Depacketizer: fmt::Debug { + /// Provide a size hint for the `out: &mut Vec` parameter in [`Depacketizer::depacketize`]. + /// The [`packets_size`] parameter is the sum of the size of all packets that will be processed in + /// subsequent calls to [`Depacketizer::depacketize`]. + /// This is used to allocate the vector with upfront capacity. + /// Return [`None`] if it cannot be determined. + fn out_size_hint(&self, packets_size: usize) -> Option; + /// Unpack the RTP packet into a provided `Vec`. fn depacketize( &mut self, @@ -268,6 +275,20 @@ impl Packetizer for CodecPacketizer { } impl Depacketizer for CodecDepacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + use CodecDepacketizer::*; + match self { + H264(v) => v.out_size_hint(packets_size), + H265(v) => v.out_size_hint(packets_size), + Opus(v) => v.out_size_hint(packets_size), + G711(v) => v.out_size_hint(packets_size), + Vp8(v) => v.out_size_hint(packets_size), + Vp9(v) => v.out_size_hint(packets_size), + Null(v) => v.out_size_hint(packets_size), + Boxed(v) => v.out_size_hint(packets_size), + } + } + fn depacketize( &mut self, packet: &[u8], diff --git a/src/packet/null.rs b/src/packet/null.rs index 0f2a183a1..029a0677b 100644 --- a/src/packet/null.rs +++ b/src/packet/null.rs @@ -17,6 +17,10 @@ impl Packetizer for NullPacketizer { } impl Depacketizer for NullDepacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + Some(packets_size) + } + fn depacketize( &mut self, packet: &[u8], diff --git a/src/packet/opus.rs b/src/packet/opus.rs index fe2f6a60d..4625bfa56 100644 --- a/src/packet/opus.rs +++ b/src/packet/opus.rs @@ -43,6 +43,10 @@ impl Packetizer for OpusPacketizer { pub struct OpusDepacketizer; impl Depacketizer for OpusDepacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + Some(packets_size) + } + fn depacketize( &mut self, packet: &[u8], diff --git a/src/packet/vp8.rs b/src/packet/vp8.rs index 5090f32a6..c9deaf62f 100644 --- a/src/packet/vp8.rs +++ b/src/packet/vp8.rs @@ -183,6 +183,10 @@ pub struct Vp8Depacketizer { } impl Depacketizer for Vp8Depacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + Some(packets_size) + } + /// depacketize parses the passed byte slice and stores the result in the /// VP8Packet this method is called upon fn depacketize( diff --git a/src/packet/vp9.rs b/src/packet/vp9.rs index 92c2282fc..05fb8943d 100644 --- a/src/packet/vp9.rs +++ b/src/packet/vp9.rs @@ -283,6 +283,10 @@ pub struct Vp9Depacketizer { } impl Depacketizer for Vp9Depacketizer { + fn out_size_hint(&self, packets_size: usize) -> Option { + Some(packets_size) + } + /// depacketize parses the passed byte slice and stores the result /// in the Vp9Packet this method is called upon fn depacketize( From 02ad4a66133879f6d173c6446e8912bff65ff290 Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Tue, 16 Dec 2025 12:48:01 +0100 Subject: [PATCH 4/6] Fix max-bundle bug --- docs/SDP.md | 56 ++++++++++++++++++++++++++++ src/change/sdp.rs | 35 ++++++++++++++---- src/sdp/data.rs | 61 +++++++++++++++++++++++++++++++ tests/sdp-negotiation.rs | 79 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 224 insertions(+), 7 deletions(-) diff --git a/docs/SDP.md b/docs/SDP.md index aba87143a..2dfbe5e6e 100644 --- a/docs/SDP.md +++ b/docs/SDP.md @@ -116,5 +116,61 @@ by str0m or the client. For media str0m creates, it refuses to allow the client transition `Inactive` -> `RecvOnly`. All other transitions are fine, i.e. a client can change, also a str0m created media, from `Inactive` to `SendRecv`. +## BUNDLE and Bundle Policies + +str0m only supports a single ICE transport, meaning all m-lines must +be bundled together. The `a=group:BUNDLE` attribute in the SDP lists +which m-lines share transport. + +### max-bundle vs max-compat + +[RFC 8843][rfc8843] defines bundle policies that control how ports are set in SDP offers: + +- **max-bundle**: Only the first m-line in the bundle has a real port + (typically 9). Subsequent bundled m-lines use `port=0` to indicate + they share transport with the first m-line. +- **max-compat**: All m-lines use a non-zero port (typically 9), for + backwards compatibility with endpoints that don't understand + BUNDLE. + +The bundle policy is a local choice made by the offerer and is **not +explicitly stated** in the SDP. It can only be inferred by looking at +the port values on bundled m-lines. + +Example max-bundle offer: + +``` +a=group:BUNDLE 0 1 +m=audio 9 UDP/TLS/RTP/SAVPF 111 <- port 9 +m=video 0 UDP/TLS/RTP/SAVPF 96 <- port 0 (bundled, NOT rejected) +``` + +Example max-compat offer: + +``` +a=group:BUNDLE 0 1 +m=audio 9 UDP/TLS/RTP/SAVPF 111 <- port 9 +m=video 9 UDP/TLS/RTP/SAVPF 96 <- port 9 +``` + +### How str0m handles this + +**Generating SDPs**: str0m produces max-compat style SDPs. All active +m-lines use port 9. Only truly rejected m-lines (e.g., no codec +match) use port 0. + +**Receiving SDPs**: str0m accepts both styles. An m-line with `port=0` +is only considered rejected if it is NOT listed in the +`a=group:BUNDLE` attribute. If it is in the BUNDLE group, `port=0` +simply indicates the m-line shares transport with the first bundled +m-line (max-bundle style). + +| Port | In BUNDLE group | Meaning | +|------|-----------------|-----------------------------------------| +| 0 | Yes | Valid bundled m-line (max-bundle style) | +| 0 | No | Rejected m-line | +| 9 | Yes | Valid bundled m-line (max-compat style) | + [quote]: https://mailarchive.ietf.org/arch/msg/mmusic/2N1_-eUTVrmciX3LpSjkjFH7oCU/ [sdpspec]: https://datatracker.ietf.org/doc/html/rfc3264#section-6.1 +[rfc8843]: https://datatracker.ietf.org/doc/html/rfc8843 diff --git a/src/change/sdp.rs b/src/change/sdp.rs index 29d4931e2..dd7a6eb96 100644 --- a/src/change/sdp.rs +++ b/src/change/sdp.rs @@ -796,9 +796,10 @@ fn apply_offer(session: &mut Session, offer: SdpOffer) -> Result<(), RtcError> { update_session(session, &offer); + let bundle_mids = offer.bundle_mids(); let new_lines = sync_medias(session, &offer).map_err(RtcError::RemoteSdp)?; - add_new_lines(session, &new_lines, true).map_err(RtcError::RemoteSdp)?; + add_new_lines(session, &new_lines, true, bundle_mids).map_err(RtcError::RemoteSdp)?; ensure_stream_tx(session); @@ -814,6 +815,7 @@ fn apply_answer( update_session(session, &answer); + let bundle_mids = answer.bundle_mids(); let new_lines = sync_medias(session, &answer).map_err(RtcError::RemoteSdp)?; // The new_lines from the answer must correspond to what we sent in the offer. @@ -821,7 +823,7 @@ fn apply_answer( return Err(RtcError::RemoteSdp(err)); } - add_new_lines(session, &new_lines, false).map_err(RtcError::RemoteSdp)?; + add_new_lines(session, &new_lines, false, bundle_mids).map_err(RtcError::RemoteSdp)?; // Add all pending changes (since we pre-allocated SSRC communicated in the Offer). add_pending_changes(session, pending); @@ -934,6 +936,7 @@ fn add_pending_changes(session: &mut Session, pending: Changes) { /// * New m-lines are returned to the caller. fn sync_medias<'a>(session: &mut Session, sdp: &'a Sdp) -> Result, String> { let mut new_lines = Vec::with_capacity(sdp.media_lines.len()); + let bundle_mids = sdp.bundle_mids(); for (idx, m) in sdp.media_lines.iter().enumerate() { // First, match existing m-lines. @@ -958,6 +961,7 @@ fn sync_medias<'a>(session: &mut Session, sdp: &'a Sdp) -> Result, ) -> Result<(), String> { for m in new_lines { let idx = session.line_count(); @@ -993,7 +998,12 @@ fn add_new_lines( // For disabled (rejected) m-lines, don't fire open event // and the direction will be set to Inactive by update_media. - media.need_open_event = is_offer && !m.disabled; + // In max-bundle, port=0 with the MID in BUNDLE group is NOT rejected. + let is_in_bundle = bundle_mids + .map(|mids| mids.contains(&m.mid())) + .unwrap_or(false); + let is_rejected = m.disabled && !is_in_bundle; + media.need_open_event = is_offer && !is_rejected; // Match/remap remote params. session @@ -1009,6 +1019,7 @@ fn add_new_lines( &session.codec_config, &session.exts, &mut session.streams, + bundle_mids, ); session.add_media(media); @@ -1067,13 +1078,23 @@ fn update_media( config: &CodecConfig, exts: &ExtensionMap, streams: &mut Streams, + bundle_mids: Option<&[Mid]>, ) { - // If the m-line is disabled (port=0), clear remote_pts and set direction to Inactive. - // This handles the case where the remote rejects an m-line. - if m.disabled { + // If the m-line has port=0, it could mean: + // 1. The m-line is rejected (not in BUNDLE group) + // 2. The m-line is bundled with another m-line (max-bundle format, RFC 8843) + // + // In max-bundle, secondary m-lines use port=0 to indicate they share transport + // with the first m-line. This is NOT a rejection. + let is_in_bundle = bundle_mids + .map(|mids| mids.contains(&m.mid())) + .unwrap_or(false); + let is_rejected = m.disabled && !is_in_bundle; + + if is_rejected { if !media.disabled() { debug!( - "Mid ({}) is rejected (port=0), setting to Inactive", + "Mid ({}) is rejected (port=0, not in BUNDLE), setting to Inactive", media.mid() ); } diff --git a/src/sdp/data.rs b/src/sdp/data.rs index adedf60f3..eac6cf1f6 100644 --- a/src/sdp/data.rs +++ b/src/sdp/data.rs @@ -31,6 +31,18 @@ impl Sdp { .map_err(|e| SdpError::ParseError(e.to_string())) } + /// Get the MIDs listed in the BUNDLE group, if any. + pub(crate) fn bundle_mids(&self) -> Option<&[Mid]> { + self.session.attrs.iter().find_map(|a| { + if let SessionAttribute::Group { typ, mids } = a { + if typ == "BUNDLE" { + return Some(mids.as_slice()); + } + } + None + }) + } + pub(crate) fn assert_consistency(&self) -> Result<(), SdpError> { match self.do_assert_consistency() { None => Ok(()), @@ -1600,4 +1612,53 @@ mod test { a=fmtp:46 apt=45\r\n\ ")); } + + #[test] + fn bundle_mids_returns_correct_mids() { + let input = "v=0\r\n\ + o=- 123456 2 IN IP4 127.0.0.1\r\n\ + s=-\r\n\ + t=0 0\r\n\ + a=group:BUNDLE 0 1 2\r\n\ + a=msid-semantic:WMS *\r\n\ + m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n\ + c=IN IP4 0.0.0.0\r\n\ + a=mid:0\r\n\ + a=sendrecv\r\n\ + a=rtpmap:111 opus/48000/2\r\n\ + a=setup:actpass\r\n\ + a=ice-ufrag:test\r\n\ + a=ice-pwd:testpassword1234\r\n\ + m=video 0 UDP/TLS/RTP/SAVPF 96\r\n\ + c=IN IP4 0.0.0.0\r\n\ + a=mid:1\r\n\ + a=sendrecv\r\n\ + a=rtpmap:96 VP8/90000\r\n\ + a=setup:actpass\r\n\ + a=ice-ufrag:test\r\n\ + a=ice-pwd:testpassword1234\r\n\ + m=application 0 UDP/DTLS/SCTP webrtc-datachannel\r\n\ + c=IN IP4 0.0.0.0\r\n\ + a=mid:2\r\n\ + a=setup:actpass\r\n\ + a=ice-ufrag:test\r\n\ + a=ice-pwd:testpassword1234\r\n\ + a=sctp-port:5000\r\n\ + "; + + let sdp = Sdp::parse(input).expect("should parse"); + + // bundle_mids should return all MIDs from the BUNDLE group + let mids = sdp.bundle_mids().expect("should have BUNDLE group"); + assert_eq!(mids.len(), 3); + assert_eq!(mids[0], "0".into()); + assert_eq!(mids[1], "1".into()); + assert_eq!(mids[2], "2".into()); + + // The m-lines with port=0 should have disabled=true at parsing level + // (interpretation of whether this means "rejected" happens later) + assert!(!sdp.media_lines[0].disabled, "audio has port=9"); + assert!(sdp.media_lines[1].disabled, "video has port=0"); + assert!(sdp.media_lines[2].disabled, "application has port=0"); + } } diff --git a/tests/sdp-negotiation.rs b/tests/sdp-negotiation.rs index 7533f6df9..58f7ef3df 100644 --- a/tests/sdp-negotiation.rs +++ b/tests/sdp-negotiation.rs @@ -417,6 +417,85 @@ fn media_creator_can_change_inactive_to_recvonly() { assert_eq!(m_r.direction(), Direction::SendOnly); } +/// Test that max-bundle offers (where secondary m-lines have port=0) are handled correctly. +/// In max-bundle format (RFC 8843), m-lines after the first one use port=0 to indicate +/// they share transport with the first m-line. This is NOT a rejection. +#[test] +fn max_bundle_offer_accepted() { + init_log(); + init_crypto_default(); + + // Create an Rtc that supports both opus and VP8 + let mut r = TestRtc::new_with_rtc( + info_span!("R"), + Rtc::builder() + .clear_codecs() + .enable_opus(true) + .enable_vp8(true) + .build(), + ); + + // This is a max-bundle offer where the video m-line has port=0 + // to indicate it shares transport with the audio m-line. + let max_bundle_offer = "\ + v=0\r\n\ + o=- 123456789 2 IN IP4 127.0.0.1\r\n\ + s=-\r\n\ + t=0 0\r\n\ + a=group:BUNDLE 0 1\r\n\ + a=msid-semantic:WMS *\r\n\ + a=fingerprint:sha-256 00:00:00:00:00:00:00:00\ + :00:00:00:00:00:00:00:00:00:00:00:00:00:00:00\ + :00:00:00:00:00:00:00:00:00\r\n\ + a=ice-ufrag:testufrag\r\n\ + a=ice-pwd:testpassword12345678\r\n\ + a=setup:actpass\r\n\ + m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n\ + c=IN IP4 0.0.0.0\r\n\ + a=mid:0\r\n\ + a=sendrecv\r\n\ + a=rtcp-mux\r\n\ + a=rtpmap:111 opus/48000/2\r\n\ + a=fmtp:111 minptime=10;useinbandfec=1\r\n\ + m=video 0 UDP/TLS/RTP/SAVPF 96\r\n\ + c=IN IP4 0.0.0.0\r\n\ + a=mid:1\r\n\ + a=sendrecv\r\n\ + a=rtcp-mux\r\n\ + a=rtpmap:96 VP8/90000\r\n\ + "; + + // Parse and accept the max-bundle offer + let offer = SdpOffer::from_sdp_string(max_bundle_offer).expect("should parse"); + let answer = r.rtc.sdp_api().accept_offer(offer).expect("should accept"); + + // Verify the answer includes both m-lines in the BUNDLE group + let answer_sdp = answer.to_sdp_string(); + assert!( + answer_sdp.contains("a=group:BUNDLE 0 1"), + "Answer should have both MIDs in BUNDLE group, got:\n{}", + answer_sdp + ); + + // Both m-lines should be active (not rejected) + let mids = r._mids(); + assert_eq!(mids.len(), 2, "Should have 2 media lines"); + + let audio = r.media(mids[0]).unwrap(); + assert_eq!( + audio.direction(), + Direction::SendRecv, + "Audio should be SendRecv, not disabled" + ); + + let video = r.media(mids[1]).unwrap(); + assert_eq!( + video.direction(), + Direction::SendRecv, + "Video should be SendRecv even though offer had port=0 (max-bundle)" + ); +} + fn with_params( span_l: Span, params_l: &[PayloadParams], From a80f4b2d4b387727c0a1a40ce749fe6c58e304f2 Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Wed, 17 Dec 2025 10:21:22 +0100 Subject: [PATCH 5/6] 0.14 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f8896f33e..419dadfb9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1759,7 +1759,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" [[package]] name = "str0m" -version = "0.13.0" +version = "0.14.0" dependencies = [ "_str0m_test", "combine", diff --git a/Cargo.toml b/Cargo.toml index 805d5e790..8772bee0b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "str0m" -version = "0.13.0" +version = "0.14.0" authors = ["Martin Algesten ", "Hugo Tunius ", "Davide Bertola "] description = "WebRTC library in Sans-IO style" license = "MIT OR Apache-2.0" From 0b0bd844573d65d87f1f1290fe3ea7b713170a82 Mon Sep 17 00:00:00 2001 From: Kostya Vasilyev <112665990+kvasilye@users.noreply.github.com> Date: Wed, 17 Dec 2025 01:31:46 -0800 Subject: [PATCH 6/6] Simulcast Layer API v2 --- src/change/sdp.rs | 91 +++++++++++++++++++++++++--------------------- src/media/event.rs | 75 +++++++++++++++----------------------- 2 files changed, 79 insertions(+), 87 deletions(-) diff --git a/src/change/sdp.rs b/src/change/sdp.rs index dd7a6eb96..50c50d3df 100644 --- a/src/change/sdp.rs +++ b/src/change/sdp.rs @@ -1624,7 +1624,7 @@ mod test { use sdp::SimulcastLayer as SdpSimulcastLayer; use crate::format::Codec; - use crate::media::Simulcast; + use crate::media::{Simulcast, SimulcastLayer}; use crate::sdp::RtpMap; use super::*; @@ -1776,11 +1776,11 @@ mod test { let mut rtc1 = Rtc::new(); - let simulcast = Simulcast::builder() - .add_send_layer("h") - .add_send_layer("m") - .add_send_layer("l") - .build(); + let mut simulcast = Simulcast::new(); + + simulcast.add_send_layer(SimulcastLayer::new("h")); + simulcast.add_send_layer(SimulcastLayer::new("m")); + simulcast.add_send_layer(SimulcastLayer::new("l")); let mut change = rtc1.sdp_api(); change.add_media( @@ -1864,42 +1864,49 @@ mod test { let mut rtc1 = Rtc::new(); - let simulcast_builder = Simulcast::builder() - // High layer - .add_send_layer_with_attributes("high") - .max_width(1280) - .max_height(720) - .max_br(1100000) - .max_br(1300000) - .max_br(1500000) // the last one wins - .max_fps(30) - .finish() - // Medium layer - .add_send_layer_with_attributes("medium") - .max_width(640) - .max_height(360) - .max_br(600000) - // No max_fps - .finish() - // Low layer - .add_send_layer_with_attributes("low") - // No max_width - .max_height(180) - .max_br(200000) - .max_fps(15) - .finish() - // Custom attribute - .add_send_layer_with_attributes("custom") - .custom("foo", "bar") - .finish(); - - // Make sure we can add layers one at a time, e.g. in a loop in a user's application - let simulcast = simulcast_builder - // No attributes - .add_send_layer_with_attributes("no_attrs") - .finish() - // Build the simulcast itself - .build(); + let mut simulcast = Simulcast::new(); + + // High layer + simulcast.add_send_layer( + SimulcastLayer::new_with_attributes("high") + .max_width(1280) + .max_height(720) + .max_br(1100000) + .max_br(1300000) + .max_br(1500000) // the last one wins + .max_fps(30) + .build(), + ); + + // Medium layer + simulcast.add_send_layer( + SimulcastLayer::new_with_attributes("medium") + .max_width(640) + .max_height(360) + .max_br(600000) + // No max_fps + .build(), + ); + + // Low layer + simulcast.add_send_layer( + SimulcastLayer::new_with_attributes("low") + // No max_width + .max_height(180) + .max_br(200000) + .max_fps(15) + .build(), + ); + + // Custom attribute + simulcast.add_send_layer( + SimulcastLayer::new_with_attributes("custom") + .custom("foo", "bar") + .build(), + ); + + // No attributes + simulcast.add_send_layer(SimulcastLayer::new_with_attributes("no_attrs").build()); let mut change = rtc1.sdp_api(); change.add_media( diff --git a/src/media/event.rs b/src/media/event.rs index 7f0991ffe..bcd0cb534 100644 --- a/src/media/event.rs +++ b/src/media/event.rs @@ -88,54 +88,23 @@ pub struct Simulcast { impl Simulcast { /// Create a new struct with no layers - pub fn builder() -> SimulcastBuilder { - SimulcastBuilder { + pub fn new() -> Simulcast { + Simulcast { send: vec![], recv: vec![], } } } -/// A builder for simulcast -pub struct SimulcastBuilder { - send: Vec, - recv: Vec, -} - -impl SimulcastBuilder { - /// Add a new simulcast layer with just the rid name - pub fn add_send_layer(mut self, rid: &str) -> Self { - self.send.push(SimulcastLayer { - rid: Rid::from(rid), - attributes: None, - }); - self - } - - /// Add a new simulcast layer with rid name and attributes - pub fn add_send_layer_with_attributes(self, rid: &str) -> SimulcastLayerBuilder { - SimulcastLayerBuilder { - builder: self, - rid: Rid::from(rid), - attributes: vec![], - } +impl Simulcast { + /// Add a send layer + pub fn add_send_layer(&mut self, layer: SimulcastLayer) { + self.send.push(layer); } - /// Add a new receive layer - pub fn add_receive_layer(mut self, rid: &str) -> Self { - self.recv.push(SimulcastLayer { - rid: Rid::from(rid), - attributes: None, - }); - self - } - - /// Finish the building process - pub fn build(self) -> Simulcast { - Simulcast { - send: self.send, - recv: self.recv, - } + /// Add a receive layer + pub fn add_recv_layer(&mut self, layer: SimulcastLayer) { + self.send.push(layer); } } @@ -148,9 +117,26 @@ pub struct SimulcastLayer { pub attributes: Option>, } +impl SimulcastLayer { + /// Creates a new layer with the rid + pub fn new(rid: &str) -> SimulcastLayer { + SimulcastLayer { + rid: Rid::from(rid), + attributes: None, + } + } + + /// Create a new layer builder for setting attributes + pub fn new_with_attributes(rid: &str) -> SimulcastLayerBuilder { + SimulcastLayerBuilder { + rid: Rid::from(rid), + attributes: vec![], + } + } +} + /// A builder which is used to populate layer attributes pub struct SimulcastLayerBuilder { - builder: SimulcastBuilder, rid: Rid, // We could use a HashMap but that doesn't preserve order - which we need for tests (to validate the // resulting SDP). BTreeMap seems like overkill for 4-6 keys, so does HashMap, actually. A simple @@ -190,16 +176,15 @@ impl SimulcastLayerBuilder { } /// Build the layer - pub fn finish(mut self) -> SimulcastBuilder { - self.builder.send.push(SimulcastLayer { + pub fn build(self) -> SimulcastLayer { + SimulcastLayer { rid: self.rid, attributes: if self.attributes.is_empty() { None } else { Some(self.attributes) }, - }); - self.builder + } } fn update_or_insert(&mut self, key: &str, value: String) {