diff --git a/rtc-interceptor/src/report/receiver_stream.rs b/rtc-interceptor/src/report/receiver_stream.rs index 578a1cd7..ec1c538a 100644 --- a/rtc-interceptor/src/report/receiver_stream.rs +++ b/rtc-interceptor/src/report/receiver_stream.rs @@ -3,6 +3,13 @@ use std::time::Instant; /// Number of packets tracked per u64 entry in the bitmap. const PACKETS_PER_ENTRY: usize = 64; +/// Maximum value for `total_lost` per RFC 3550 §6.4.1. +/// +/// The wire format is a signed 24-bit integer. We store it as `u32` but +/// clamp to the signed-positive maximum (`0x7F_FFFF`) so the sign bit +/// stays clear when the value is serialised into the 24-bit RTCP field. +const MAX_TOTAL_LOST: u32 = 0x7F_FFFF; + pub(crate) struct ReceiverStream { ssrc: u32, receiver_ssrc: u32, @@ -137,15 +144,12 @@ impl ReceiverStream { } }; - self.total_lost += total_lost_since_report; - - // allow up to 24 bits - if total_lost_since_report > 0xFFFFFF { - total_lost_since_report = 0xFFFFFF; - } - if self.total_lost > 0xFFFFFF { - self.total_lost = 0xFFFFFF - } + // Clamp before adding to prevent overflow (RFC 3550 §6.4.1: signed 24-bit max) + total_lost_since_report = total_lost_since_report.min(MAX_TOTAL_LOST); + self.total_lost = self + .total_lost + .saturating_add(total_lost_since_report) + .min(MAX_TOTAL_LOST); // Calculate DLSR (Delay Since Last SR) - RFC 3550 // Return 0 if no SR has been received yet @@ -461,9 +465,9 @@ mod tests { #[test] fn test_receiver_stream_24bit_loss_clamping() { - // Test that total_lost is clamped to 24 bits (0xFFFFFF) + // Test that total_lost is clamped to signed 24-bit max (MAX_TOTAL_LOST) let mut stream = ReceiverStream::new(123456, 90000); - stream.total_lost = 0xFFFFFE; // Almost at max + stream.total_lost = MAX_TOTAL_LOST - 1; // Almost at max let now = Instant::now(); @@ -473,7 +477,7 @@ mod tests { let rr = stream.generate_report(now); - // Should be clamped to 0xFFFFFF - assert_eq!(rr.reports[0].total_lost, 0xFFFFFF); + // Should be clamped to MAX_TOTAL_LOST (0x7F_FFFF per RFC 3550 §6.4.1) + assert_eq!(rr.reports[0].total_lost, MAX_TOTAL_LOST); } } diff --git a/rtc-rtcp/src/receiver_report/receiver_report_test.rs b/rtc-rtcp/src/receiver_report/receiver_report_test.rs index 22450a54..9524bd6a 100644 --- a/rtc-rtcp/src/receiver_report/receiver_report_test.rs +++ b/rtc-rtcp/src/receiver_report/receiver_report_test.rs @@ -191,7 +191,31 @@ fn test_receiver_report_roundtrip() { None, ), ( - "totallost overflow", + "totallost at 24-bit max", + ReceiverReport { + ssrc: 1, + reports: vec![ReceptionReport { + total_lost: 0xFF_FFFF, + ..Default::default() + }], + ..Default::default() + }, + None, + ), + ( + "totallost overflow at 24-bit boundary", + ReceiverReport { + ssrc: 1, + reports: vec![ReceptionReport { + total_lost: 0x100_0000, + ..Default::default() + }], + ..Default::default() + }, + Some(Error::InvalidTotalLost), + ), + ( + "totallost overflow large", ReceiverReport { ssrc: 1, reports: vec![ReceptionReport { diff --git a/rtc-rtcp/src/reception_report.rs b/rtc-rtcp/src/reception_report.rs index a873dafa..300f1b69 100644 --- a/rtc-rtcp/src/reception_report.rs +++ b/rtc-rtcp/src/reception_report.rs @@ -29,6 +29,12 @@ pub struct ReceptionReport { pub fraction_lost: u8, /// The total number of RTP data packets from source SSRC that have /// been lost since the beginning of reception. + /// + /// RFC 3550 §6.4.1 defines this as a signed 24-bit integer on the wire + /// (negative values can occur when duplicates arrive). We store it as + /// `u32` and clamp to `0x7F_FFFF` so the sign bit stays clear during + /// serialisation. A future revision may switch to `i32` to fully + /// represent the negative-loss (duplicate) case. pub total_lost: u32, /// The least significant 16 bits contain the highest sequence number received /// in an RTP data packet from source SSRC, and the most significant 16 bits extend @@ -116,8 +122,9 @@ impl Marshal for ReceptionReport { buf.put_u8(self.fraction_lost); - // pack TotalLost into 24 bits - if self.total_lost >= (1 << 25) { + // Pack TotalLost into 24 bits (RFC 3550 §6.4.1). + // Values above 0xFF_FFFF cannot be represented in 24 bits. + if self.total_lost > 0xFF_FFFF { return Err(Error::InvalidTotalLost); }