Skip to content
Open
121 changes: 121 additions & 0 deletions rtc-rtp/src/codec/h264/h264_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,124 @@ fn test_h264_payloader_payload_sps_and_pps_handling() -> Result<()> {

Ok(())
}

/// When the combined STAP-A of SPS + PPS exceeds the MTU, both should still
/// be emitted as individual (possibly FU-A fragmented) packets instead of
/// being silently dropped.
#[test]
fn test_h264_stap_a_exceeds_mtu_emits_individually() -> Result<()> {
let mut pck = H264Payloader::default();

// SPS: 3 bytes (NALU type 7)
let sps = Bytes::from_static(&[0x07, 0xAA, 0xBB]);
// PPS: 3 bytes (NALU type 8)
let pps = Bytes::from_static(&[0x08, 0xCC, 0xDD]);

let res = pck.payload(1500, &sps)?;
assert!(res.is_empty(), "SPS alone should be stashed, not emitted");

let res = pck.payload(1500, &pps)?;
assert!(res.is_empty(), "PPS alone should be stashed, not emitted");

// Use a tiny MTU so the STAP-A (1 + 2+3 + 2+3 = 11 bytes) exceeds it.
// SPS and PPS individually are 3 bytes each, which fits in MTU=5.
let result = pck.payload(5, &Bytes::from_static(&[0x05, 0x01, 0x02]))?;

// Expect: SPS (3 bytes, fits), PPS (3 bytes, fits), then the IDR NALU (3 bytes, fits)
assert_eq!(result.len(), 3, "expected SPS + PPS + IDR = 3 packets");
assert_eq!(result[0], sps, "first packet should be the SPS NALU");
assert_eq!(result[1], pps, "second packet should be the PPS NALU");
assert_eq!(
result[2],
Bytes::from_static(&[0x05, 0x01, 0x02]),
"third packet should be the IDR NALU"
);

Ok(())
}

/// When SPS or PPS are too large for a u16 length field (>65535 bytes), they
/// should be emitted via FU-A fragmentation using emit_single_or_fragment
/// rather than being packed into a STAP-A.
#[test]
fn test_h264_oversized_sps_uses_fua_fragmentation() -> Result<()> {
let mut pck = H264Payloader::default();

// Build a large SPS that genuinely exceeds u16::MAX (65535 bytes).
// This ensures we actually hit the `sps_nalu.len() > u16::MAX` branch.
let mut sps_data = vec![0x67]; // NALU type 7, with ref_idc bits set
sps_data.extend(vec![0xAA; 70_000]);
let sps = Bytes::from(sps_data); // 70001 bytes, well above u16::MAX threshold

let pps = Bytes::from_static(&[0x68, 0xCC, 0xDD]); // NALU type 8, with ref_idc bits

let res = pck.payload(1500, &sps)?;
assert!(res.is_empty(), "SPS alone should be stashed");

let res = pck.payload(1500, &pps)?;
assert!(res.is_empty(), "PPS alone should be stashed");

// Trigger emission with a small non-SPS/PPS NALU
let result = pck.payload(1500, &Bytes::from_static(&[0x65, 0x01, 0x02]))?;

// SPS (70001 bytes) exceeds u16::MAX, so it should be FU-A fragmented.
// PPS (3 bytes) fits in a single packet.
// IDR (3 bytes) fits in a single packet.
assert!(
result.len() >= 3,
"expected at least 3 packets (fragmented SPS + PPS + IDR), got {}",
result.len()
);

// Verify the first packet is a FU-A start fragment of the SPS
assert_eq!(
result[0][0] & NALU_TYPE_BITMASK,
FUA_NALU_TYPE,
"first packet should be a FU-A fragment"
);
assert_ne!(
result[0][1] & FU_START_BITMASK,
0,
"first FU-A fragment should have start bit set"
);

Ok(())
}

/// The emit_single_or_fragment helper should pass through small NALUs directly
/// and fragment large ones via FU-A.
#[test]
fn test_h264_emit_single_or_fragment_small_nalu() {
let nalu = Bytes::from_static(&[0x65, 0x01, 0x02, 0x03]);
let mut payloads = vec![];
H264Payloader::emit_single_or_fragment(&nalu, 10, &mut payloads);
assert_eq!(payloads.len(), 1, "small NALU should emit as single packet");
assert_eq!(payloads[0], nalu);
}

#[test]
fn test_h264_emit_single_or_fragment_large_nalu() {
let mut data = vec![0x65]; // IDR NALU type
data.extend(vec![0xBB; 20]);
let nalu = Bytes::from(data);
let mut payloads = vec![];
H264Payloader::emit_single_or_fragment(&nalu, 10, &mut payloads);
assert!(
payloads.len() > 1,
"large NALU should be FU-A fragmented into multiple packets"
);
// First fragment should have FU-A type and start bit
assert_eq!(payloads[0][0] & NALU_TYPE_BITMASK, FUA_NALU_TYPE);
assert_ne!(payloads[0][1] & FU_START_BITMASK, 0);
// Last fragment should have end bit
let last = payloads.last().unwrap();
assert_ne!(last[1] & FU_END_BITMASK, 0);
}

#[test]
fn test_h264_emit_single_or_fragment_empty_nalu() {
let nalu = Bytes::new();
let mut payloads = vec![];
H264Payloader::emit_single_or_fragment(&nalu, 10, &mut payloads);
assert!(payloads.is_empty(), "empty NALU should produce no packets");
}
95 changes: 81 additions & 14 deletions rtc-rtp/src/codec/h264/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,52 @@ impl H264Payloader {
(-1, -1)
}

/// Emit a NALU as a single packet or fragment it via FU-A, without
/// special-casing SPS/PPS (which `emit()` would re-stash).
fn emit_single_or_fragment(nalu: &Bytes, mtu: usize, payloads: &mut Vec<Bytes>) {
if nalu.is_empty() {
return;
}

let nalu_ref_idc = nalu[0] & NALU_REF_IDC_BITMASK;
let nalu_type = nalu[0] & NALU_TYPE_BITMASK;

// Single NALU
if nalu.len() <= mtu {
payloads.push(nalu.clone());
return;
}

// FU-A fragmentation
let max_fragment_size = mtu as isize - FUA_HEADER_SIZE as isize;
let mut nalu_data_index: isize = 1;
let nalu_data_length = nalu.len() as isize - nalu_data_index;
let mut nalu_data_remaining = nalu_data_length;

if std::cmp::min(max_fragment_size, nalu_data_remaining) <= 0 {
return;
}

while nalu_data_remaining > 0 {
let current_fragment_size = std::cmp::min(max_fragment_size, nalu_data_remaining);
let mut out = BytesMut::with_capacity(FUA_HEADER_SIZE + current_fragment_size as usize);
out.put_u8(FUA_NALU_TYPE | nalu_ref_idc);
let mut b1 = nalu_type;
if nalu_data_remaining == nalu_data_length {
b1 |= 1 << 7; // start bit
} else if nalu_data_remaining - current_fragment_size == 0 {
b1 |= 1 << 6; // end bit
}
out.put_u8(b1);
out.put(
&nalu[nalu_data_index as usize..(nalu_data_index + current_fragment_size) as usize],
);
payloads.push(out.freeze());
nalu_data_remaining -= current_fragment_size;
nalu_data_index += current_fragment_size;
}
}

fn emit(&mut self, nalu: &Bytes, mtu: usize, payloads: &mut Vec<Bytes>) {
if nalu.is_empty() {
return;
Expand All @@ -66,21 +112,42 @@ impl H264Payloader {
} else if nalu_type == PPS_NALU_TYPE {
self.pps_nalu = Some(nalu.clone());
return;
} else if let (Some(sps_nalu), Some(pps_nalu)) = (&self.sps_nalu, &self.pps_nalu) {
// Pack current NALU with SPS and PPS as STAP-A
let sps_len = (sps_nalu.len() as u16).to_be_bytes();
let pps_len = (pps_nalu.len() as u16).to_be_bytes();

let mut stap_a_nalu = Vec::with_capacity(1 + 2 + sps_nalu.len() + 2 + pps_nalu.len());
stap_a_nalu.push(OUTPUT_STAP_AHEADER);
stap_a_nalu.extend(sps_len);
stap_a_nalu.extend_from_slice(sps_nalu);
stap_a_nalu.extend(pps_len);
stap_a_nalu.extend_from_slice(pps_nalu);
if stap_a_nalu.len() <= mtu {
payloads.push(Bytes::from(stap_a_nalu));
} else if self.sps_nalu.is_some() && self.pps_nalu.is_some() {
// Clone to release the borrow on self so we can call self.emit() below if needed.
let sps_nalu = self.sps_nalu.clone().unwrap();
let pps_nalu = self.pps_nalu.clone().unwrap();

// Pack the cached SPS and PPS into a STAP-A.
// STAP-A length fields are u16; only pack if both NALUs fit within 65535 bytes.
if sps_nalu.len() <= u16::MAX as usize && pps_nalu.len() <= u16::MAX as usize {
let sps_len = (sps_nalu.len() as u16).to_be_bytes();
let pps_len = (pps_nalu.len() as u16).to_be_bytes();

let mut stap_a_nalu =
Vec::with_capacity(1 + 2 + sps_nalu.len() + 2 + pps_nalu.len());
stap_a_nalu.push(OUTPUT_STAP_AHEADER);
stap_a_nalu.extend(sps_len);
stap_a_nalu.extend_from_slice(&sps_nalu);
stap_a_nalu.extend(pps_len);
stap_a_nalu.extend_from_slice(&pps_nalu);
if stap_a_nalu.len() <= mtu {
// STAP-A fits within MTU, emit as aggregate
payloads.push(Bytes::from(stap_a_nalu));
Comment thread
nightness marked this conversation as resolved.
} else {
// STAP-A does not fit within the MTU; fall back to emitting
// SPS and PPS separately so they are not silently dropped.
Self::emit_single_or_fragment(&sps_nalu, mtu, payloads);
Self::emit_single_or_fragment(&pps_nalu, mtu, payloads);
}
} else {
// SPS or PPS exceeds u16::MAX; fall back to emitting them as
// separate NALUs (which will be fragmented via FU-A if needed).
// We cannot use self.emit() here because it would re-stash SPS/PPS
// NALUs instead of actually outputting them. Push single or fragment directly.
Self::emit_single_or_fragment(&sps_nalu, mtu, payloads);
Self::emit_single_or_fragment(&pps_nalu, mtu, payloads);
}
}
} // end SPS+PPS STAP-A / fallback emission block

if self.sps_nalu.is_some() && self.pps_nalu.is_some() {
self.sps_nalu = None;
Expand Down
127 changes: 127 additions & 0 deletions rtc-rtp/src/codec/h265/h265_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,3 +982,130 @@ fn test_h265_packet_real() -> Result<()> {

Ok(())
}

/// When an aggregation buffer contains a NALU that exceeds u16::MAX, the
/// oversized NALU must be emitted individually (via FU fragmentation) while
/// normal-sized NALUs are still packed into the aggregation packet.
#[test]
fn test_h265_oversized_nalu_in_aggregation_buffer() -> Result<()> {
let mut pck = HevcPayloader::default();

// Build a payload with two NALUs: one normal-sized and one genuinely
// oversized (> u16::MAX = 65535 bytes) to hit the oversized NALU path.
let mut payload = vec![];
// NALU 1 with 3-byte start code: 5 bytes, type=PFR
payload.extend_from_slice(&[0x00, 0x00, 0x01]);
payload.extend_from_slice(&[0x02, 0x01, 0xAA, 0xBB, 0xCC]); // 5 bytes
// NALU 2 with 3-byte start code: >65535 bytes, type=PFR
payload.extend_from_slice(&[0x00, 0x00, 0x01]);
let mut nalu2 = vec![0x02, 0x01]; // header: type=PFR
nalu2.extend(vec![0xDD; 70_000]); // 70002 bytes total, exceeds u16::MAX
payload.extend_from_slice(&nalu2);

let result = pck.payload(1500, &Bytes::from(payload))?; // packetize mixed-size NALUs

Comment on lines +986 to +1006
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test claims to cover the "NALU exceeds u16::MAX" path, but the constructed NALU is only 20 bytes. As a result it never exercises the new oversize filtering in flush_aggregation_buffer (the >u16::MAX check), and will pass regardless of that logic.

To validate the new behavior, build a NALU with length (u16::MAX as usize + 1) and set MTU larger than that so it actually enters the aggregation buffer and triggers the oversize split during flush.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been addressed in a subsequent commit. The fix is in the current code but GitHub's outdated detection did not trigger because the modification was on adjacent lines rather than this exact line.

// NALU 1 (5 bytes) fits in MTU, emitted as a single RTP packet.
// NALU 2 (70002 bytes) exceeds u16::MAX and cannot fit in an AP size field,
// so it must be FU-fragmented into multiple RTP packets.
assert!(
result.len() >= 2,
"expected at least 2 packets (single + fragments), got {}",
result.len()
);

// First packet should be the small NALU (single packet, emitted directly)
assert_eq!(result[0].len(), 5, "first packet should be the 5-byte NALU");

// Remaining packets should be FU fragments of the oversized NALU
for i in 1..result.len() {
let header = H265NALUHeader::new(result[i][0], result[i][1]);
assert!(
header.is_fragmentation_unit(),
"packet {} should be a fragmentation unit",
i
);
}

Ok(())
}

/// The aggregation_payload_header should only be computed from normal-sized
/// NALUs, not oversized ones that get split out. This ensures the header
/// fields (F, layer_id, tid) reflect only the NALUs actually in the packet.
#[test]
fn test_h265_aggregation_header_excludes_oversized_nalus() -> Result<()> {
let mut pck = HevcPayloader::default();

// Three NALUs: two small ones and one oversized (> u16::MAX).
// The oversized NALU should be excluded from the AP and emitted separately.
let mut payload = vec![];
// NALU 1: type=PFR, tid=1, 3 bytes (normal)
payload.extend_from_slice(&[0x00, 0x00, 0x01]);
payload.extend_from_slice(&[0x02, 0x01, 0xAA]);
// NALU 2: type=PFR, tid=1, 3 bytes (normal)
payload.extend_from_slice(&[0x00, 0x00, 0x01]);
payload.extend_from_slice(&[0x02, 0x01, 0xBB]);
// NALU 3: type=PFR, tid=1, >65535 bytes (oversized)
payload.extend_from_slice(&[0x00, 0x00, 0x01]);
let mut nalu3 = vec![0x02, 0x01]; // header
nalu3.extend(vec![0xCC; 70_000]); // 70002 bytes total
payload.extend_from_slice(&nalu3);

let result = pck.payload(1500, &Bytes::from(payload))?;

// The two small NALUs should be aggregated into a single AP packet,
// and the oversized NALU should be FU-fragmented separately.
assert!(
result.len() >= 2,
"expected at least 2 packets (AP + FU fragments), got {}",
result.len()
);
let header = H265NALUHeader::new(result[0][0], result[0][1]);
assert!(
header.is_aggregation_packet(),
Comment on lines +1032 to +1065
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the aggregation header should exclude oversized NALUs, but this test only uses two small NALUs and doesn’t include any >u16::MAX NALU. It currently verifies aggregation behavior in general, not the new oversize-exclusion logic.

Either adjust the test inputs to include an actual oversized NALU (and validate the header against only the aggregated subset), or update the test name/comment to match what’s being asserted.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been addressed in a subsequent commit. The fix is in the current code but GitHub's outdated detection did not trigger because the modification was on adjacent lines rather than this exact line.

"first packet should be an AP containing only the two normal-sized NALUs"
);

// Remaining packets should be FU fragments of the oversized NALU
for i in 1..result.len() {
let fu_header = H265NALUHeader::new(result[i][0], result[i][1]);
assert!(
fu_header.is_fragmentation_unit(),
"packet {} should be a fragmentation unit",
i
);
}

Ok(())
}

/// A single oversized NALU (> MTU) passed through flush_aggregation_buffer
/// should be FU-fragmented into multiple packets via emit().
#[test]
fn test_h265_flush_single_oversized_nalu_fu_fragmentation() -> Result<()> {
let mut pck = HevcPayloader::default();

// Single NALU larger than MTU to trigger FU fragmentation. This tests
// that flush_aggregation_buffer with a single oversized NALU correctly
// emits it (which then gets fragmented via emit()).
let mut nalu_data = vec![0x02, 0x01]; // header: type=PFR
nalu_data.extend(vec![0xAA; 70_000]); // 70002 bytes total, exceeds u16::MAX
let mut payload = vec![0x00, 0x00, 0x01]; // 3-byte start code
payload.extend_from_slice(&nalu_data);

let result = pck.payload(1500, &Bytes::from(payload))?;

// Single oversized NALU should be FU-fragmented into multiple packets
assert!(
result.len() > 1,
"oversized NALU should be FU-fragmented, got {} packets",
result.len()
);
let header = H265NALUHeader::new(result[0][0], result[0][1]);
assert!(
header.is_fragmentation_unit(),
"first packet should be a fragmentation unit"
);

Ok(())
}
Loading