diff --git a/quest/m1/README.md b/quest/m1/README.md index 8498025f09..59422c6ad9 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -172,7 +172,6 @@ bench all require its crate. - [T-STD TS export](/quest/m1/tstd/README.md) - `moq export ts` is a proper remux that passes the T-STD buffer model, starting with a fixed `--delay` - [Multi-packet PMT](/quest/m1/ts-pmt-split.md) - `export ts` splits a PMT longer than one packet instead of exiting before its first packet - [TS damage log](/quest/m1/ts-damage-log.md) - a burst of damaged TS units logs a first warning and a periodic summary per PID, not one warning per unit -- [TS adaptation field length](/quest/m1/ts-adaptation-only.md) - a TS adaptation field with the wrong length for its packet (183 adaptation-only, at most 182 with a payload) is refused as damage - [Rust non-continuous signal](/quest/m1/rust-continuous.md) - the Rust container consumer reports a frame after a subscribe or discontinuity as non-continuous, like JS, for the tune-in and warmup trims - [Open-GOP leading pictures](/quest/m1/open-gop-leading-pictures.md) - a viewer joining at a recovery point drops the leading pictures it cannot decode; continuous viewers keep them - [Watch decode errors](/quest/m1/watch-decode-error.md) - a WebCodecs error ends the subscription and the element reports it diff --git a/quest/m1/ts-adaptation-only.md b/quest/m1/ts-adaptation-only.md deleted file mode 100644 index 038dbd8a5c..0000000000 --- a/quest/m1/ts-adaptation-only.md +++ /dev/null @@ -1,17 +0,0 @@ -# [XS] A TS adaptation field must fit its packet - -## Goal - -`adaptation_valid` (`rs/moq-mux/src/container/ts/import.rs`) refuses an -adaptation-only packet (`adaptation_field_control == 0b10`) whose adaptation -field length is not 183, and a packet with both (`0b11`) whose field is longer -than 182, as ISO 13818-1 requires, so either is treated as damage like any -other malformed adaptation field. Lands on `main` first, then is -cherry-picked to `release`. - -## Plan - -Found reviewing #5124. Add a regression test covering both boundaries, -on a media PID and on the PCR PID. - -Public API: none. Wire: none. diff --git a/rs/moq-mux/src/container/ts/import.rs b/rs/moq-mux/src/container/ts/import.rs index 56b0e02534..3dd62ef1e8 100644 --- a/rs/moq-mux/src/container/ts/import.rs +++ b/rs/moq-mux/src/container/ts/import.rs @@ -1631,6 +1631,11 @@ pub(super) fn adaptation_valid(pkt: &[u8; TsPacket::SIZE]) -> bool { if pkt[3] & 0x20 == 0 { return true; } + // Without payload the adaptation field fills the packet; with payload it leaves a byte. + let length = usize::from(pkt[4]); + if (pkt[3] & 0x10 == 0 && length != 183) || (pkt[3] & 0x10 != 0 && length > 182) { + return false; + } let Some(field) = pkt.get(5..5 + usize::from(pkt[4])) else { return false; }; @@ -6824,6 +6829,60 @@ pub(super) mod test { damaged_sps_keeps_the_last_good_parameter_sets(StreamType::H265).await; } + #[test] + fn adaptation_lengths_obey_the_packet_control() { + for (control, length, valid) in [ + (0b10, 0, false), + (0b10, 182, false), + (0b10, 183, true), + (0b10, 184, false), + (0b11, 0, true), + (0b11, 182, true), + (0b11, 183, false), + (0b11, 184, false), + ] { + let mut packet: [u8; TsPacket::SIZE] = clock_break_packet(VIDEO).try_into().unwrap(); + packet[3] = control << 4; + packet[4] = length; + packet[5] = 0; + assert_eq!( + super::adaptation_valid(&packet), + valid, + "control={control:b} length={length}" + ); + } + } + + #[test] + fn invalid_adaptation_lengths_damage_media_and_dedicated_clock_pids() { + const PCR: u16 = 0x0200; + for pid in [VIDEO, PCR] { + for (control, length) in [(0b10, 0), (0b10, 182), (0b11, 183)] { + let mut mux = Mux { + out: synth_pmt(&[(StreamType::H264, VIDEO)], false), + ..Default::default() + }; + mux.gop(VIDEO, 90_000); + let mut broadcast = moq_net::broadcast::Info::new().produce(); + let catalog = crate::catalog::Producer::new(&mut broadcast, crate::catalog::Config::default()).unwrap(); + let mut import = super::Import::new(broadcast, catalog.reserve()); + import.decode(&mux.out).unwrap(); + import.pcr_pid = Some(Pid::new(PCR).unwrap()); + + let mut packet = clock_break_packet(pid); + packet[3] = control << 4; + packet[4] = length; + import.decode(&packet).unwrap(); + assert!(import.last_pts.is_some(), "invalid adaptation reset the clock"); + assert_eq!( + import.stats().streams[&pid].damaged, + 1, + "pid={pid} control={control:b} length={length}" + ); + } + } + } + /// A dedicated PCR PID's malformed adaptation field cannot declare a timebase break. #[test] fn damaged_adaptation_on_a_dedicated_pcr_pid_keeps_the_clock() {