Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

* Support NACK retransmission without RTX, resending in-band on the main SSRC
* Add `PayloadParams::set_resend` to configure or disable RTX
* Fix `Simulcast::add_recv_layer` to push to recv instead of send #968
* Accept any non-empty SDP session name (`s=`), not just `s=-` (RFC 8866 section 5.3)

Expand Down
8 changes: 6 additions & 2 deletions src/change/sdp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,9 +1385,13 @@ fn update_media(
.find(|r| r.repairs == Some(i.ssrc))
.map(|r| r.ssrc);

// If remote communicated a main a=ssrc, but no RTX, we will not send nacks.
// NACK gated on negotiated `nack` feedback; RTX is just the transport (as in libwebrtc).
let fb_nack = config
.params()
.iter()
.any(|p| media.remote_pts().contains(&p.pt) && p.fb_nack());
let midrid = MidRid(media.mid(), None);
let suppress_nack = repair_ssrc.is_none();
let suppress_nack = !fb_nack;
streams.expect_stream_rx(i.ssrc, repair_ssrc, midrid, suppress_nack);
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/format/payload_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ impl PayloadParams {
self.resend
}

/// Sets the RTX resend payload type. Use `None` to disable RTX, in which
/// case retransmissions (if NACK is enabled) are sent in-band on the main SSRC.
pub fn set_resend(&mut self, resend: Option<Pt>) {
self.resend = resend;
}

/// The codec with settings for this group of parameters.
pub fn spec(&self) -> CodecSpec {
self.spec
Expand Down Expand Up @@ -578,6 +584,14 @@ impl PayloadParams {
let mut remote_pt = first.pt;
let mut remote_rtx = first.resend;

// The RTX payload type (RFC 4588 §8.6) is a media format, and for unicast
// the answerer selects formats (RFC 3264 §6.1, Unicast Streams) "from
// amongst those listed in the offer". So if we didn't offer RTX for this
// codec, don't adopt the remote's RTX PT.
if self.resend.is_none() {
remote_rtx = None;
}

if self.locked {
// This can happen if the incoming PTs are suggestions (send-direction) rather than demanded
// (receive-direction). We only want to warn if we get receive direction changes.
Expand Down Expand Up @@ -646,6 +660,13 @@ impl PayloadParams {
claimed.assert_claim_once(rtx);
}

// The answerer removes rtcp-fb attributes it does not support, and per
// RFC 4585 §4.2 "both offerer and answerer MUST only use feedback
// mechanisms negotiated in this way". So intersect our local `fb_nack`
// with what the remote signaled (`a=rtcp-fb <pt> nack`). Runs once at
// lock time, when `self.fb_nack` still holds the local desired value.
self.fb_nack = self.fb_nack && first.fb_nack;

// This is now locked.
self.locked = true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/streams/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ impl Streams {
}
}

// If we don't have an RTX PT configured, we don't want NACK.
let suppress_nack = payload.resend.is_none();
// NACK gated on negotiated `nack` feedback; RTX is just the transport (as in libwebrtc).
let suppress_nack = !payload.fb_nack;

// If stream already exists, this might only "fill in" the RTX.
self.expect_stream_rx(ssrc_main, rtx, midrid, suppress_nack);
Expand Down
29 changes: 22 additions & 7 deletions src/streams/send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,12 +505,10 @@ impl StreamTx {
// since the above loop figuring out param needs to be correct also
// for the NextPacketKind::Blank case.
set_pt_for_padding = Some(pt_main);
} else {
// If the PT we're sending on doesn't have a corresponding RTX PT,
// the packet is de-facto not nackable.
//
// This blocks incoming NACK requests and thus ensures there are no
// entries in self.retries without a RTX PT.
} else if !param.fb_nack() {
// No RTX PT and no `fb_nack`: the packet cannot be resent (neither
// via RTX nor in-band), so it is de-facto not nackable. This ensures
// there are no entries in the rtx cache we can't resend.
next.pkt.nackable = false;
}

Expand All @@ -527,6 +525,13 @@ impl StreamTx {

header_ref.clone()
}
NextPacketKind::ResendInband => {
// In-band resend on the main SSRC: reuse the original PT and sequence
// number so SRTP reproduces the original ROC/IV and the encrypted
// payload decrypts at the receiver.
header_ref.ext_vals.rid_repair = None;
header_ref.clone()
}
NextPacketKind::Resend(_) | NextPacketKind::Blank(_) => {
// * For the Resend case, we will not have accepted/cached the packet unless
// we have a RTX PT (see logic setting next.pkt.nackable above).
Expand Down Expand Up @@ -587,7 +592,7 @@ impl StreamTx {
let pkt = &next.pkt;

let body_len = match next.kind {
NextPacketKind::Regular | NextPacketKind::Resend(_) => {
NextPacketKind::Regular | NextPacketKind::Resend(_) | NextPacketKind::ResendInband => {
let body_len = pkt.payload.len();
if let Some(patch) = pkt.vp8_patch.as_ref() {
patch.copy_to(pkt.payload.as_ref(), &mut body_out[..body_len]);
Expand Down Expand Up @@ -753,6 +758,15 @@ impl StreamTx {
h.push(now, len);
}

if self.rtx.is_none() {
let seq_no = pkt.seq_no;
return Some(NextPacket {
kind: NextPacketKind::ResendInband,
seq_no,
pkt,
});
}

let seq_no = self.seq_no_rtx.inc();

let orig_seq_no = pkt.seq_no;
Expand Down Expand Up @@ -1203,6 +1217,7 @@ struct NextPacket<'a> {
enum NextPacketKind {
Regular,
Resend(SeqNo),
ResendInband,
Blank(u8),
}

Expand Down
Loading
Loading