From 46509b41fe15b80c344c2f31f6c63e93df353849 Mon Sep 17 00:00:00 2001 From: Willie Abrams Date: Tue, 22 Sep 2026 20:09:53 -0500 Subject: [PATCH 1/2] fix(egfx): decode AVC420 sent as an Annex B byte stream MS-RDPEGFX defines the RFX_AVC420_BITMAP_STREAM bitstream as an Annex B byte stream, but OpenH264Decoder always read its input as 4-byte length-prefixed NAL units. Given Annex B, the first start code (00 00 00 01) was read as a 1-byte NAL and the following bytes as a bogus length, so the frame was dropped. GNOME Remote Desktop sends Annex B, so no AVC420 frame from it ever decoded. The decoder now converts only input that is a complete chain of length-prefixed NAL units ending exactly at the end of the buffer, and passes everything else to OpenH264 unchanged. Docs that said the wire format is length-prefixed are corrected, and the dead spec link is replaced. Co-Authored-By: Claude Opus 5.5 --- crates/ironrdp-egfx/src/decode.rs | 62 ++++++++++--- crates/ironrdp-egfx/src/encode.rs | 8 +- crates/ironrdp-egfx/src/pdu/avc.rs | 18 ++-- crates/ironrdp-fuzzing/src/oracles/mod.rs | 10 +-- .../tests/egfx/decode.rs | 89 ++++++++++++++++--- 5 files changed, 144 insertions(+), 43 deletions(-) diff --git a/crates/ironrdp-egfx/src/decode.rs b/crates/ironrdp-egfx/src/decode.rs index d9b1b3164e..e4b8fe29d7 100644 --- a/crates/ironrdp-egfx/src/decode.rs +++ b/crates/ironrdp-egfx/src/decode.rs @@ -9,11 +9,14 @@ //! # Protocol Context //! //! H.264 data arrives inside [RFX_AVC420_BITMAP_STREAM][1] payloads -//! within `RDPGFX_WIRE_TO_SURFACE_PDU_1` messages. The NAL units -//! are in AVC format (4-byte big-endian length prefix per NAL unit), -//! not Annex B (start code prefix). +//! within `RDPGFX_WIRE_TO_SURFACE_PDU_1` messages. The specification +//! defines it as an Annex B byte stream (start code prefix), which is what +//! servers built on FreeRDP's server library, such as GNOME Remote Desktop, +//! send. Earlier versions of this crate documented AVC format (4-byte +//! big-endian length prefix per NAL unit) instead, so decoders should accept +//! both. //! -//! [1]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/d65c3f9c-2088-4302-90c0-53adc0e11a78 +//! [1]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/5f12c20e-2ea1-4ad1-a2a0-019ee3893731 use core::fmt; @@ -138,8 +141,10 @@ pub type DecoderResult = Result; /// Trait for H.264 (AVC) decoders /// /// Implement this trait to provide H.264 decode capability to the -/// EGFX client. The decoder receives AVC-format NAL units (length-prefixed, -/// not Annex B) from `RFX_AVC420_BITMAP_STREAM` payloads. +/// EGFX client. The decoder receives the H.264 data from +/// `RFX_AVC420_BITMAP_STREAM` payloads: an Annex B byte stream per the +/// specification, or AVC-format NAL units (4-byte BE length prefix) from +/// servers that followed this crate's earlier documentation. /// /// # Thread Safety /// @@ -160,8 +165,8 @@ pub type DecoderResult = Result; /// } /// ``` pub trait H264Decoder: Send { - /// Decode AVC-format H.264 NAL units (4-byte BE length prefix, not Annex B) - /// into an RGBA bitmap. + /// Decode H.264 NAL units, as an Annex B byte stream or in AVC format + /// (4-byte BE length prefix), into an RGBA bitmap. /// /// Frame dimensions may exceed the destination rectangle due to /// macroblock alignment (16x16). The caller crops to fit. @@ -190,9 +195,9 @@ mod openh264_impl { /// H.264 decoder backed by Cisco's OpenH264 library /// - /// This decoder converts AVC-format NAL units to Annex B format - /// (as required by OpenH264), decodes to YUV420p, then converts - /// to RGBA for the client pipeline. + /// This decoder passes Annex B input to OpenH264 as-is and converts + /// AVC-format NAL units to Annex B first (OpenH264 only reads Annex B), + /// decodes to YUV420p, then converts to RGBA for the client pipeline. /// /// # Feature Gates /// @@ -246,11 +251,42 @@ mod openh264_impl { impl H264Decoder for OpenH264Decoder { fn decode(&mut self, data: &[u8]) -> DecoderResult { - crate::pdu::avc_to_annex_b_into(data, &mut self.annex_b_buffer); + // AVC-format input is a chain of 4-byte BE lengths that lands exactly + // on the end of the buffer. Anything else is treated as the Annex B + // byte stream the specification defines. A start-code check alone + // would be ambiguous: an AVC buffer whose first NAL unit is + // 256..=511 bytes long also begins with 00 00 01. + let is_length_prefixed = { + let mut offset = 0usize; + loop { + if offset == data.len() { + break true; + } + let Some(len_bytes) = data.get(offset..offset + 4).and_then(|b| <[u8; 4]>::try_from(b).ok()) else { + break false; + }; + let nal_len = u32::from_be_bytes(len_bytes); + let next = usize::try_from(nal_len) + .ok() + .filter(|&len| len > 0) + .and_then(|len| (offset + 4).checked_add(len)); + match next { + Some(next) if next <= data.len() => offset = next, + _ => break false, + } + } + }; + + let annex_b = if is_length_prefixed { + crate::pdu::avc_to_annex_b_into(data, &mut self.annex_b_buffer); + &self.annex_b_buffer + } else { + data + }; let yuv = self .decoder - .decode(&self.annex_b_buffer) + .decode(annex_b) .map_err(|e| DecoderError::new("OpenH264 decode failed", e))? .ok_or_else(|| DecoderError::msg("OpenH264 returned no picture"))?; diff --git a/crates/ironrdp-egfx/src/encode.rs b/crates/ironrdp-egfx/src/encode.rs index ede4e75f85..ae72c01c96 100644 --- a/crates/ironrdp-egfx/src/encode.rs +++ b/crates/ironrdp-egfx/src/encode.rs @@ -257,10 +257,10 @@ mod tests { use super::*; use crate::decode::H264Decoder as _; - /// Round-trip through the reference encoder and the reference decoder, - /// crossing the documented format boundary explicitly: the encoder - /// produces Annex B (the wire format), the decoder consumes AVC - /// length-prefixed input, and `pdu::annex_b_to_avc` is the bridge. + /// Round-trip through the reference encoder and the reference decoder. + /// The encoder produces Annex B (the wire format). The decoder accepts + /// both Annex B and AVC length-prefixed input; this test converts with + /// `pdu::annex_b_to_avc` to exercise the AVC path. /// /// Run with: `cargo test -p ironrdp-egfx --features openh264-bundled` #[test] diff --git a/crates/ironrdp-egfx/src/pdu/avc.rs b/crates/ironrdp-egfx/src/pdu/avc.rs index 67088c2d04..b858d144a7 100644 --- a/crates/ironrdp-egfx/src/pdu/avc.rs +++ b/crates/ironrdp-egfx/src/pdu/avc.rs @@ -368,10 +368,12 @@ impl Avc420Region { /// Convert H.264 AVC format to Annex B format /// -/// OpenH264 and similar decoders consume Annex B format (start code prefixed), -/// but MS-RDPEGFX delivers AVC format (length-prefixed NAL units). This helper -/// converts between the two by replacing each 4-byte big-endian length prefix -/// with the 4-byte Annex B start code `[0x00, 0x00, 0x00, 0x01]`. +/// OpenH264 and similar decoders consume Annex B format (start code prefixed). +/// MS-RDPEGFX specifies Annex B on the wire, but earlier versions of this +/// crate documented AVC format (length-prefixed NAL units), so senders may +/// still use it. This helper converts AVC input by +/// replacing each 4-byte big-endian length prefix with the 4-byte Annex B +/// start code `[0x00, 0x00, 0x00, 0x01]`. /// /// ```text /// AVC: <4-byte BE length> <4-byte BE length> ... @@ -448,8 +450,9 @@ pub fn avc_to_annex_b_into(data: &[u8], out: &mut Vec) { /// Convert H.264 Annex B format to AVC format /// -/// MS-RDPEGFX requires AVC format (length-prefixed NAL units), -/// but most encoders output Annex B format (start code prefixed). +/// MS-RDPEGFX specifies Annex B (start code prefixed) on the wire, so this is +/// only needed to produce AVC format (length-prefixed NAL units) for +/// consumers that require it. /// /// ```text /// Annex B: 00 00 00 01 00 00 00 01 ... @@ -557,7 +560,8 @@ pub const fn align_to_16(dimension: u32) -> u32 { /// # Arguments /// /// * `regions` - List of regions with their encoding parameters -/// * `h264_data` - H.264 encoded data (should be in AVC format, not Annex B) +/// * `h264_data` - H.264 encoded data as an Annex B byte stream, which is the +/// format MS-RDPEGFX specifies for `RFX_AVC420_BITMAP_STREAM` /// /// # Returns /// diff --git a/crates/ironrdp-fuzzing/src/oracles/mod.rs b/crates/ironrdp-fuzzing/src/oracles/mod.rs index 9de240563a..17de19cd4a 100644 --- a/crates/ironrdp-fuzzing/src/oracles/mod.rs +++ b/crates/ironrdp-fuzzing/src/oracles/mod.rs @@ -396,8 +396,8 @@ pub fn egfx_round_trip(data: &[u8]) { /// /// Fuzzes the IronRDP wrapper layer between a wire `Avc420BitmapStream` and /// the consumer's `H264Decoder`. Specifically targets `avc_to_annex_b`, the -/// AVC-length-prefix to Annex-B conversion that runs before OpenH264 sees -/// any bytes. +/// AVC-length-prefix to Annex-B conversion that runs on length-prefixed input +/// before OpenH264 sees it (Annex B input is passed through unchanged). /// /// The oracle runs two paths on each input: /// @@ -1297,9 +1297,9 @@ pub fn message_decoding_invariants(data: &[u8]) { /// /// Sibling of [`egfx_avc420_decode`]. Fuzzes the IronRDP wrapper layer between /// a wire `Avc444BitmapStream` and the consumer's `H264Decoder`. Targets the -/// AVC-length-prefix to Annex-B conversion that runs on each of the two -/// underlying `Avc420BitmapStream`s (luma plus optional chroma) per -/// MS-RDPEGFX 2.2.4.4. +/// AVC-length-prefix to Annex-B conversion that runs on length-prefixed input +/// in each of the two underlying `Avc420BitmapStream`s (luma plus optional +/// chroma) per MS-RDPEGFX 2.2.4.4. /// /// The oracle runs three paths on each input: /// diff --git a/crates/ironrdp-testsuite-core/tests/egfx/decode.rs b/crates/ironrdp-testsuite-core/tests/egfx/decode.rs index a12821a73b..72f54f1b06 100644 --- a/crates/ironrdp-testsuite-core/tests/egfx/decode.rs +++ b/crates/ironrdp-testsuite-core/tests/egfx/decode.rs @@ -4,12 +4,9 @@ use ironrdp_egfx::decode::{H264Decoder, OpenH264Decoder}; // Test Helpers // ============================================================================ -/// Generate a minimal AVC-format H.264 bitstream by encoding a black 16x16 frame -/// -/// The encoder produces Annex B format (start code prefixed). This function -/// converts the output to AVC format (4-byte BE length prefixed) to exercise -/// the full decode pipeline including AVC-to-Annex-B conversion. -fn generate_test_avc_bitstream() -> Vec { +/// Generate a minimal Annex B H.264 bitstream (start code prefixed) by +/// encoding a black 16x16 frame. This is the format MS-RDPEGFX specifies. +fn generate_test_annex_b_bitstream() -> Vec { use openh264::encoder::Encoder; use openh264::formats::YUVBuffer; @@ -18,9 +15,16 @@ fn generate_test_avc_bitstream() -> Vec { // Black 16x16 YUV420p frame (all zeros) let yuv = YUVBuffer::new(16, 16); let bitstream = encoder.encode(&yuv).expect("encode should succeed"); - let annex_b = bitstream.to_vec(); + bitstream.to_vec() +} - annex_b_to_avc(&annex_b) +/// Generate a minimal AVC-format H.264 bitstream by encoding a black 16x16 frame +/// +/// The encoder produces Annex B format (start code prefixed). This function +/// converts the output to AVC format (4-byte BE length prefixed) to exercise +/// the full decode pipeline including AVC-to-Annex-B conversion. +fn generate_test_avc_bitstream() -> Vec { + annex_b_to_avc(&generate_test_annex_b_bitstream()) } /// Convert Annex B format NAL units to AVC format (4-byte BE length prefix) @@ -120,6 +124,63 @@ fn test_openh264_decoder_reset() { assert_eq!(frame.height(), 16); } +// ============================================================================ +// Annex B Input Tests +// ============================================================================ + +#[test] +fn test_openh264_decode_annex_b() { + // MS-RDPEGFX specifies an Annex B byte stream; it must decode as sent. + let annex_b = generate_test_annex_b_bitstream(); + assert!(annex_b.starts_with(&[0x00, 0x00, 0x00, 0x01])); + + let mut decoder = OpenH264Decoder::new().expect("decoder should initialize"); + let frame = decoder.decode(&annex_b).expect("Annex B decode should succeed"); + assert_eq!(frame.width(), 16); + assert_eq!(frame.height(), 16); +} + +#[test] +fn test_openh264_decode_annex_b_with_access_unit_delimiter() { + // GNOME Remote Desktop starts each frame with an access unit delimiter + // (NAL type 9). Read as AVC format, `00 00 00 01` becomes a 1-byte NAL + // and the bytes after it a bogus length, so the frame never decodes. + let mut annex_b = vec![0x00, 0x00, 0x00, 0x01, 0x09, 0x10]; + annex_b.extend_from_slice(&generate_test_annex_b_bitstream()); + + let mut decoder = OpenH264Decoder::new().expect("decoder should initialize"); + let frame = decoder + .decode(&annex_b) + .expect("Annex B decode with an access unit delimiter should succeed"); + assert_eq!(frame.width(), 16); + assert_eq!(frame.height(), 16); +} + +#[test] +fn test_openh264_decode_annex_b_three_byte_start_codes() { + // Annex B also allows 3-byte start codes (00 00 01). + let annex_b = generate_test_annex_b_bitstream(); + let mut short_codes = Vec::with_capacity(annex_b.len()); + let mut i = 0; + while i < annex_b.len() { + if annex_b[i..].starts_with(&[0x00, 0x00, 0x00, 0x01]) { + short_codes.extend_from_slice(&[0x00, 0x00, 0x01]); + i += 4; + } else { + short_codes.push(annex_b[i]); + i += 1; + } + } + assert!(short_codes.starts_with(&[0x00, 0x00, 0x01])); + + let mut decoder = OpenH264Decoder::new().expect("decoder should initialize"); + let frame = decoder + .decode(&short_codes) + .expect("Annex B decode with 3-byte start codes should succeed"); + assert_eq!(frame.width(), 16); + assert_eq!(frame.height(), 16); +} + // ============================================================================ // Error Path Tests // ============================================================================ @@ -128,8 +189,7 @@ fn test_openh264_decoder_reset() { fn test_decode_empty_input() { let mut decoder = OpenH264Decoder::new().expect("decoder should initialize"); - // Empty input has no NAL units -- the AVC-to-Annex-B converter - // produces nothing, and OpenH264 returns no picture. + // Empty input has no NAL units, so OpenH264 returns no picture. let result = decoder.decode(&[]); assert!(result.is_err(), "decoding empty input should fail"); } @@ -138,8 +198,8 @@ fn test_decode_empty_input() { fn test_decode_truncated_nal_length() { let mut decoder = OpenH264Decoder::new().expect("decoder should initialize"); - // Less than 4 bytes: can't even read the NAL length prefix. - // The converter produces an empty Annex B buffer. + // Less than 4 bytes: can't even read a NAL length prefix, so the input + // is passed to OpenH264 as Annex B, which finds no picture in it. let result = decoder.decode(&[0x00, 0x00]); assert!(result.is_err(), "truncated NAL length should fail"); } @@ -148,8 +208,9 @@ fn test_decode_truncated_nal_length() { fn test_decode_nal_length_exceeds_buffer() { let mut decoder = OpenH264Decoder::new().expect("decoder should initialize"); - // NAL length says 100 bytes but only 2 bytes follow. - // The converter discards the malformed NAL and produces empty output. + // NAL length says 100 bytes but only 2 bytes follow, so the input is not + // valid AVC framing. It is passed to OpenH264 as Annex B, which finds no + // picture in it. let mut data = Vec::new(); data.extend_from_slice(&100u32.to_be_bytes()); data.extend_from_slice(&[0x67, 0x00]); // Partial NAL From 8fac24cd01a49fc8e1faa01cebb00ee0304eead0 Mon Sep 17 00:00:00 2001 From: Willie Abrams Date: Wed, 23 Sep 2026 07:47:40 -0500 Subject: [PATCH 2/2] fix(egfx): prefer Annex B when a buffer starts with a start code A buffer that begins with 00 00 01 can also be a well-formed AVC buffer: 00 00 01 67 followed by 359 bytes reads as one 359-byte NAL unit. The chain check took that as AVC format and corrupted the frame. Since MS-RDPEGFX specifies Annex B, a leading start code now wins, and the chain check applies only to buffers without one. The check moves to a public `pdu::is_avc_format` next to `avc_to_annex_b`, so other H264Decoder implementations can share it, and the egfx_avc420_decode fuzz oracle now makes the same dispatch as OpenH264Decoder. Co-Authored-By: Claude Opus 5.5 --- crates/ironrdp-egfx/src/decode.rs | 28 +-------------- crates/ironrdp-egfx/src/pdu/avc.rs | 34 +++++++++++++++++++ crates/ironrdp-fuzzing/src/oracles/mod.rs | 33 +++++++++++------- .../tests/egfx/decode.rs | 33 ++++++++++++++++++ 4 files changed, 88 insertions(+), 40 deletions(-) diff --git a/crates/ironrdp-egfx/src/decode.rs b/crates/ironrdp-egfx/src/decode.rs index e4b8fe29d7..411de5a4a6 100644 --- a/crates/ironrdp-egfx/src/decode.rs +++ b/crates/ironrdp-egfx/src/decode.rs @@ -251,33 +251,7 @@ mod openh264_impl { impl H264Decoder for OpenH264Decoder { fn decode(&mut self, data: &[u8]) -> DecoderResult { - // AVC-format input is a chain of 4-byte BE lengths that lands exactly - // on the end of the buffer. Anything else is treated as the Annex B - // byte stream the specification defines. A start-code check alone - // would be ambiguous: an AVC buffer whose first NAL unit is - // 256..=511 bytes long also begins with 00 00 01. - let is_length_prefixed = { - let mut offset = 0usize; - loop { - if offset == data.len() { - break true; - } - let Some(len_bytes) = data.get(offset..offset + 4).and_then(|b| <[u8; 4]>::try_from(b).ok()) else { - break false; - }; - let nal_len = u32::from_be_bytes(len_bytes); - let next = usize::try_from(nal_len) - .ok() - .filter(|&len| len > 0) - .and_then(|len| (offset + 4).checked_add(len)); - match next { - Some(next) if next <= data.len() => offset = next, - _ => break false, - } - } - }; - - let annex_b = if is_length_prefixed { + let annex_b = if crate::pdu::is_avc_format(data) { crate::pdu::avc_to_annex_b_into(data, &mut self.annex_b_buffer); &self.annex_b_buffer } else { diff --git a/crates/ironrdp-egfx/src/pdu/avc.rs b/crates/ironrdp-egfx/src/pdu/avc.rs index b858d144a7..46476c393a 100644 --- a/crates/ironrdp-egfx/src/pdu/avc.rs +++ b/crates/ironrdp-egfx/src/pdu/avc.rs @@ -366,6 +366,40 @@ impl Avc420Region { } } +/// Whether `data` is H.264 AVC format (4-byte big-endian length prefixes) +/// rather than an Annex B byte stream +/// +/// MS-RDPEGFX specifies Annex B, so a buffer that starts with a start code +/// (`00 00 01` or `00 00 00 01`) is Annex B. Any other buffer is AVC format +/// if its length prefixes chain exactly to the end of the buffer. +/// +/// Checking the chain first would misread valid Annex B: `00 00 01 67` +/// followed by 359 bytes is also a well-formed one-unit AVC buffer. Checking +/// the start code first moves the ambiguity to AVC senders whose first NAL +/// unit is 1 or 256..=511 bytes long, which don't follow the specification. +pub fn is_avc_format(data: &[u8]) -> bool { + if data.starts_with(&[0x00, 0x00, 0x01]) || data.starts_with(&[0x00, 0x00, 0x00, 0x01]) { + return false; + } + let mut offset = 0usize; + loop { + if offset == data.len() { + return !data.is_empty(); + } + let Some(len_bytes) = data.get(offset..offset + 4).and_then(|b| <[u8; 4]>::try_from(b).ok()) else { + return false; + }; + let next = usize::try_from(u32::from_be_bytes(len_bytes)) + .ok() + .filter(|&len| len > 0) + .and_then(|len| (offset + 4).checked_add(len)); + match next { + Some(next) if next <= data.len() => offset = next, + _ => return false, + } + } +} + /// Convert H.264 AVC format to Annex B format /// /// OpenH264 and similar decoders consume Annex B format (start code prefixed). diff --git a/crates/ironrdp-fuzzing/src/oracles/mod.rs b/crates/ironrdp-fuzzing/src/oracles/mod.rs index 17de19cd4a..2e713e1874 100644 --- a/crates/ironrdp-fuzzing/src/oracles/mod.rs +++ b/crates/ironrdp-fuzzing/src/oracles/mod.rs @@ -395,18 +395,21 @@ pub fn egfx_round_trip(data: &[u8]) { /// AVC420 decode-side wrapper fuzz oracle. /// /// Fuzzes the IronRDP wrapper layer between a wire `Avc420BitmapStream` and -/// the consumer's `H264Decoder`. Specifically targets `avc_to_annex_b`, the -/// AVC-length-prefix to Annex-B conversion that runs on length-prefixed input -/// before OpenH264 sees it (Annex B input is passed through unchanged). +/// the consumer's `H264Decoder`: `is_avc_format`, which decides whether the +/// input is length-prefixed, and `avc_to_annex_b`, the conversion that +/// `OpenH264Decoder` runs on length-prefixed input before OpenH264 sees it +/// (Annex B input is passed through unchanged). /// /// The oracle runs two paths on each input: /// -/// - Direct: call `avc_to_annex_b(data)` on the raw fuzz input. This -/// exercises the wrapper on arbitrary byte distributions, including +/// - Direct: run `is_avc_format` and `avc_to_annex_b` on the raw fuzz input, +/// the conversion unconditionally so malformed length prefixes reach it. +/// This exercises the wrapper on arbitrary byte distributions, including /// inputs that do not parse as `Avc420BitmapStream`. -/// - Decode-chain: try `Avc420BitmapStream::decode(data)`; on success, call -/// `avc_to_annex_b(stream.data)`. This exercises the wrapper on the -/// realistic post-decode payload distribution. +/// - Decode-chain: try `Avc420BitmapStream::decode(data)`; on success, make +/// the same dispatch as `OpenH264Decoder::decode` on the stream's payload. +/// This exercises the wrapper on the realistic post-decode payload +/// distribution. /// /// What this catches: panics in the wrapper, OOM via attacker-controlled /// NAL length encoding, contract violations on the produced Annex-B byte @@ -416,13 +419,16 @@ pub fn egfx_round_trip(data: &[u8]) { /// post-OpenH264 YUV-to-RGBA conversion path in `OpenH264Decoder::decode`, /// AVC444 luma plus chroma split (covered by a sibling target). pub fn egfx_avc420_decode(data: &[u8]) { - use ironrdp_egfx::pdu::{Avc420BitmapStream, avc_to_annex_b}; + use ironrdp_egfx::pdu::{Avc420BitmapStream, avc_to_annex_b, is_avc_format}; + let _ = is_avc_format(data); let _ = avc_to_annex_b(data); let mut cursor = ironrdp_core::ReadCursor::new(data); if let Ok(stream) = ironrdp_core::decode_cursor::>(&mut cursor) { - let _ = avc_to_annex_b(stream.data); + if is_avc_format(stream.data) { + let _ = avc_to_annex_b(stream.data); + } } } @@ -1297,9 +1303,10 @@ pub fn message_decoding_invariants(data: &[u8]) { /// /// Sibling of [`egfx_avc420_decode`]. Fuzzes the IronRDP wrapper layer between /// a wire `Avc444BitmapStream` and the consumer's `H264Decoder`. Targets the -/// AVC-length-prefix to Annex-B conversion that runs on length-prefixed input -/// in each of the two underlying `Avc420BitmapStream`s (luma plus optional -/// chroma) per MS-RDPEGFX 2.2.4.4. +/// AVC-length-prefix to Annex-B conversion, called unconditionally (without +/// the `is_avc_format` dispatch) on the raw input and on each of the two +/// underlying `Avc420BitmapStream`s (luma plus optional chroma) per +/// MS-RDPEGFX 2.2.4.4, so malformed length prefixes reach it. /// /// The oracle runs three paths on each input: /// diff --git a/crates/ironrdp-testsuite-core/tests/egfx/decode.rs b/crates/ironrdp-testsuite-core/tests/egfx/decode.rs index 72f54f1b06..d85231b572 100644 --- a/crates/ironrdp-testsuite-core/tests/egfx/decode.rs +++ b/crates/ironrdp-testsuite-core/tests/egfx/decode.rs @@ -181,6 +181,39 @@ fn test_openh264_decode_annex_b_three_byte_start_codes() { assert_eq!(frame.height(), 16); } +#[test] +fn test_openh264_decode_annex_b_that_also_parses_as_avc() { + // `00 00 01 67` read as a length is 0x167 = 359, so a 363-byte Annex B + // buffer that starts with a 3-byte start code and an SPS is also a + // well-formed one-unit AVC buffer. The start code must win. Trailing + // zero bytes are allowed after the last NAL unit in Annex B. + let mut annex_b = vec![0x00, 0x00, 0x01]; + annex_b.extend_from_slice(&generate_test_annex_b_bitstream()[4..]); + assert!(annex_b.starts_with(&[0x00, 0x00, 0x01, 0x67])); + assert!(annex_b.len() <= 363, "test frame too large: {}", annex_b.len()); + annex_b.resize(363, 0); + + let mut decoder = OpenH264Decoder::new().expect("decoder should initialize"); + let frame = decoder + .decode(&annex_b) + .expect("Annex B that also parses as AVC should decode as Annex B"); + assert_eq!(frame.width(), 16); + assert_eq!(frame.height(), 16); + assert!(!ironrdp_egfx::pdu::is_avc_format(&annex_b)); +} + +#[test] +fn test_is_avc_format() { + use ironrdp_egfx::pdu::is_avc_format; + + let annex_b = generate_test_annex_b_bitstream(); + assert!(!is_avc_format(&annex_b)); + assert!(is_avc_format(&annex_b_to_avc(&annex_b))); + assert!(!is_avc_format(&[])); + // A length that runs past the end is not AVC format. + assert!(!is_avc_format(&[0x00, 0x00, 0x00, 0x09, 0x67])); +} + // ============================================================================ // Error Path Tests // ============================================================================