Skip to content

Commit c57ea63

Browse files
committed
fix: off-by-one nack reports
NACK is not generated on a packet that is on a NACK report boundary, Last pid + 17 will be lost.
1 parent 2014cbb commit c57ea63

File tree

1 file changed

+32
-5
lines changed

1 file changed

+32
-5
lines changed

src/streams/register_nack.rs

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ use crate::rtp_::{Nack, NackEntry, ReportList, SeqNo};
55
/// Number of out of order packets we keep track of for reports
66
const MAX_MISORDER: u64 = 100;
77

8-
const U16_MAX: u64 = u16::MAX as u64 + 1_u64;
9-
108
/// The max number of NACKs we perform for a single packet
119
const MAX_NACKS: u8 = 5;
1210

@@ -59,7 +57,7 @@ impl<'a> Iterator for NackIterator<'a> {
5957
(self.next..=self.end).find(|s| self.reg.packet_mut((*s).into()).needs_nack())?;
6058

6159
let mut entry = NackEntry {
62-
pid: (self.next % U16_MAX) as u16,
60+
pid: self.next as u16,
6361
blp: 0,
6462
};
6563

@@ -75,8 +73,6 @@ impl<'a> Iterator for NackIterator<'a> {
7573
self.next += 1;
7674
}
7775

78-
self.next += 1;
79-
8076
Some(entry)
8177
}
8278
}
@@ -619,4 +615,35 @@ mod test {
619615
let active = reg.active.clone().expect("nack range");
620616
assert_eq!(*active.start, 65666);
621617
}
618+
619+
#[test]
620+
fn nack_reports_on_boundaries() {
621+
let mut reg = NackRegister::new(None);
622+
623+
for i in 0..=20 {
624+
// gap must be at least 17 packets to separate the reports.
625+
if i == 2 || i == 19 {
626+
continue;
627+
}
628+
reg.update(i.into());
629+
}
630+
631+
let reports: Vec<_> = reg
632+
.nack_reports()
633+
.expect("should generate reports")
634+
.flat_map(|nack| nack.reports)
635+
.collect();
636+
637+
assert_eq!(reports.len(), 2, "Should have found two NACK entries");
638+
assert_eq!(reports[0].pid, 2, "First missing packet should be 2");
639+
assert_eq!(
640+
reports[0].blp, 0,
641+
"No missing packets in blp bits following 2"
642+
);
643+
assert_eq!(reports[1].pid, 19, "Second missing packet should be 19");
644+
assert_eq!(
645+
reports[1].blp, 0,
646+
"No missing packets in blp bits following 19"
647+
);
648+
}
622649
}

0 commit comments

Comments
 (0)