diff --git a/crates/ironrdp-egfx/src/pdu/avc.rs b/crates/ironrdp-egfx/src/pdu/avc.rs index 67088c2d04..84077e1dc1 100644 --- a/crates/ironrdp-egfx/src/pdu/avc.rs +++ b/crates/ironrdp-egfx/src/pdu/avc.rs @@ -17,9 +17,9 @@ pub struct QuantQuality { } // Manual `Arbitrary` impl: the encoder packs `quantization_parameter` into bits 0..6 -// via `set_bits`, which panics when the value exceeds 6 bits. Mask the field to its -// wire-allowed range so fuzz inputs always round-trip through `Encode`. The other -// fields use their full type range. +// and fails when the value exceeds 6 bits. Mask the field to its wire-allowed range +// so fuzz inputs always round-trip through `Encode`. The other fields use their full +// type range. #[cfg(feature = "arbitrary")] impl<'a> arbitrary::Arbitrary<'a> for QuantQuality { fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { @@ -41,6 +41,10 @@ impl Encode for QuantQuality { fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { ensure_fixed_part_size!(in: dst); + if 0x3F < self.quantization_parameter { + return Err(invalid_field_err!("quantization_parameter", "must fit in 6 bits", in: dst)); + } + let mut data = 0u8; data.set_bits(0..6, self.quantization_parameter); data.set_bit(7, self.progressive); @@ -565,7 +569,8 @@ pub const fn align_to_16(dimension: u32) -> u32 { /// /// # Panics /// -/// Panics if internal encoding fails (should not happen with valid inputs). +/// Panics if a region's quantization parameter is above 63, the most the 6-bit +/// `qp` field holds. #[must_use] pub fn encode_avc420_bitmap_stream(regions: &[Avc420Region], h264_data: &[u8]) -> Vec { let rectangles: Vec = regions.iter().map(Avc420Region::to_rectangle).collect(); diff --git a/crates/ironrdp-egfx/src/server.rs b/crates/ironrdp-egfx/src/server.rs index e8b60d8ad2..ed46af0144 100644 --- a/crates/ironrdp-egfx/src/server.rs +++ b/crates/ironrdp-egfx/src/server.rs @@ -86,6 +86,18 @@ const DEFAULT_MAX_FRAMES_IN_FLIGHT: u32 = 3; /// Special queue depth value indicating client has disabled acknowledgments const SUSPEND_FRAME_ACK_QUEUE_DEPTH: u32 = 0xFFFFFFFF; +/// Highest QP in the H.264 range that MS-RDPEGFX [2.2.4.4.2] +/// (`RDPGFX_AVC420_QUANT_QUALITY`) requires for the `qp` field of an AVC420 +/// region, for 8-bit video. +/// +/// [2.2.4.4.2]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/bc54993f-2e3c-4285-b32b-58a4bf4cb02e +const MAX_AVC_QP: u8 = 51; + +/// Highest `qualityVal` of an AVC420 region (MS-RDPEGFX [2.2.4.4.2]). +/// +/// [2.2.4.4.2]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/bc54993f-2e3c-4285-b32b-58a4bf4cb02e +const MAX_AVC_QUALITY: u8 = 100; + /// Pre-encoded ZGFX-wrapped bytes for DVC transmission. /// /// `Encode::encode()` takes `&self`, but ZGFX wrapping is done in `drain_output()` @@ -719,6 +731,18 @@ pub struct CodecCapabilities { pub avc420: bool, /// AVC444 (H.264 4:4:4) is available pub avc444: bool, + /// The negotiated capability set promises AVC420 in the same frame as other codecs + /// + /// MS-RDPEGFX [2.2.3.7] (`RDPGFX_CAPSET_VERSION104`) requires that of a + /// client that did not set `AVC_DISABLED` at capability version 10.4, and + /// [2.2.3.8], [2.2.3.9] and [2.2.3.10] carry it to 10.5, 10.6 and 10.7. + /// Earlier versions make no such promise. + /// + /// [2.2.3.7]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/be5ea8da-44db-478d-b55c-d42d82f11d26 + /// [2.2.3.8]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/8fc20f1e-e63e-4b13-a546-22fba213ad83 + /// [2.2.3.9]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/8d489900-e903-4778-bb83-691c5ab719d5 + /// [2.2.3.10]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/ba94595b-04de-4fbd-8ee4-89d8ff8f5cf1 + pub avc420_in_mixed_frames: bool, /// Small cache mode pub small_cache: bool, /// Thin client mode @@ -732,24 +756,28 @@ impl CodecCapabilities { CapabilitySet::V8 { flags } => Self { avc420: false, avc444: false, + avc420_in_mixed_frames: false, small_cache: flags.contains(CapabilitiesV8Flags::SMALL_CACHE), thin_client: flags.contains(CapabilitiesV8Flags::THIN_CLIENT), }, CapabilitySet::V8_1 { flags } => Self { avc420: flags.contains(CapabilitiesV81Flags::AVC420_ENABLED), avc444: false, + avc420_in_mixed_frames: false, small_cache: flags.contains(CapabilitiesV81Flags::SMALL_CACHE), thin_client: flags.contains(CapabilitiesV81Flags::THIN_CLIENT), }, CapabilitySet::V10 { flags } | CapabilitySet::V10_2 { flags } => Self { avc420: !flags.contains(CapabilitiesV10Flags::AVC_DISABLED), avc444: !flags.contains(CapabilitiesV10Flags::AVC_DISABLED), + avc420_in_mixed_frames: false, small_cache: flags.contains(CapabilitiesV10Flags::SMALL_CACHE), thin_client: false, }, CapabilitySet::V10_1 => Self { avc420: true, avc444: true, + avc420_in_mixed_frames: false, small_cache: false, thin_client: false, }, @@ -757,6 +785,7 @@ impl CodecCapabilities { // V10.3 lacks SMALL_CACHE flag avc420: !flags.contains(CapabilitiesV103Flags::AVC_DISABLED), avc444: !flags.contains(CapabilitiesV103Flags::AVC_DISABLED), + avc420_in_mixed_frames: false, small_cache: false, thin_client: flags.contains(CapabilitiesV103Flags::AVC_THIN_CLIENT), }, @@ -766,12 +795,14 @@ impl CodecCapabilities { | CapabilitySet::V10_6Err { flags } => Self { avc420: !flags.contains(CapabilitiesV104Flags::AVC_DISABLED), avc444: !flags.contains(CapabilitiesV104Flags::AVC_DISABLED), + avc420_in_mixed_frames: !flags.contains(CapabilitiesV104Flags::AVC_DISABLED), small_cache: flags.contains(CapabilitiesV104Flags::SMALL_CACHE), thin_client: flags.contains(CapabilitiesV104Flags::AVC_THIN_CLIENT), }, CapabilitySet::V10_7 { flags } => Self { avc420: !flags.contains(CapabilitiesV107Flags::AVC_DISABLED), avc444: !flags.contains(CapabilitiesV107Flags::AVC_DISABLED), + avc420_in_mixed_frames: !flags.contains(CapabilitiesV107Flags::AVC_DISABLED), small_cache: flags.contains(CapabilitiesV107Flags::SMALL_CACHE), thin_client: flags.contains(CapabilitiesV107Flags::AVC_THIN_CLIENT), }, @@ -1432,6 +1463,11 @@ impl GraphicsPipelineServer { } } + /// Whether an exclusive rectangle is non-empty and lies inside the surface. + fn rect_fits_surface(surface: &Surface, rect: &ExclusiveRectangle) -> bool { + rect.left < rect.right && rect.top < rect.bottom && rect.right <= surface.width && rect.bottom <= surface.height + } + /// Queue an H.264 AVC420 frame for transmission /// /// Returns `Some(frame_id)` if queued, `None` if backpressure is active, @@ -1789,6 +1825,7 @@ impl GraphicsPipelineServer { return None; } if self.should_backpressure() { + self.qoe.record_backpressure(); return None; } @@ -1869,10 +1906,39 @@ impl GraphicsPipelineServer { /// ClearCodec tiles (lossless text), Progressive tiles (photos), and H.264 /// tiles (video), all sent between one `StartFrame`/`EndFrame` pair. /// - /// This matches how Azure VDI achieves its visual quality — each tile uses - /// the codec best suited to its content type. + /// MS-RDPEGFX [3.3.5.1] (processing an `RDPGFX_WIRE_TO_SURFACE_PDU_1` + /// message) has the client copy a decoded ClearCodec or AVC420 tile to the + /// surface when it processes the PDU, so where two such tiles overlap the + /// one sent later is what remains. A Progressive tile follows [3.3.5.2] + /// instead, which only says it SHOULD be copied once enough of it is + /// decoded and updated as later PDUs arrive, so the spec doesn't order it + /// against an overlapping tile. MS-RDPEGFX [2.2.3.7] requires a client that + /// did not set `AVC_DISABLED` at capability version 10.4 or later to process + /// AVC420 in the same frame as other codecs; earlier capability versions + /// make no such promise. [`CodecCapabilities::avc420_in_mixed_frames`] + /// reports whether the negotiated set makes it. /// - /// Returns `Some(frame_id)` if queued, `None` if not ready or backpressured. + /// Every tile is checked before the frame is queued. AVC420 tiles need + /// AVC420 support in the negotiated capabilities, and a frame that mixes an + /// AVC420 tile with another codec needs capability version 10.4 or later, + /// the versions that make that promise. A frame of AVC420 tiles alone is + /// not covered by it. Each AVC420 tile needs at least one region, unlike + /// [`Self::send_avc420_frame()`], which reads an empty list as the whole + /// surface: a whole-surface tile would cover the ClearCodec and AVC420 + /// tiles sent before it. Each AVC420 region and ClearCodec destination must + /// be non-empty and inside the surface. + /// AVC420 regions also need a QP of at most 51 (for 8-bit video) and a + /// quality of at most 100 (MS-RDPEGFX [2.2.4.4.2]). If any check fails + /// nothing is queued and no frame is tracked. + /// + /// Returns `Some(frame_id)` if queued, `None` if not ready, backpressured, + /// or a tile failed its checks. A refusal because of the tiles or the + /// negotiated capabilities logs its reason at trace level. + /// + /// [3.3.5.1]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/3fdfcd8d-ec4f-4b7a-8e0d-0385a840ec81 + /// [3.3.5.2]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/9791fc34-7644-4279-844f-7728ae9959c2 + /// [2.2.3.7]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/be5ea8da-44db-478d-b55c-d42d82f11d26 + /// [2.2.4.4.2]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/bc54993f-2e3c-4285-b32b-58a4bf4cb02e pub fn send_mixed_frame( &mut self, surface_id: u16, @@ -1883,15 +1949,63 @@ impl GraphicsPipelineServer { return None; } if self.should_backpressure() { + self.qoe.record_backpressure(); return None; } if tiles.is_empty() { return None; } + let is_avc420 = |tile: &MixedTilePayload| matches!(tile, MixedTilePayload::Avc420 { .. }); + if tiles.iter().any(is_avc420) { + if !self.supports_avc420() { + trace!( + reason = "AVC420 is not supported by the negotiated capabilities", + "Mixed frame refused" + ); + return None; + } + if !self.codec_caps.avc420_in_mixed_frames && tiles.iter().any(|tile| !is_avc420(tile)) { + trace!( + reason = "AVC420 beside another codec is not promised by the negotiated capabilities", + "Mixed frame refused" + ); + return None; + } + } + let surface = self.surfaces.get(surface_id)?; let pixel_format = surface.pixel_format; + let refusal = tiles.iter().find_map(|tile| match tile { + MixedTilePayload::ClearCodec { destination, .. } => (!Self::rect_fits_surface(surface, destination)) + .then_some("ClearCodec destination is empty or outside the surface"), + MixedTilePayload::RemoteFxProgressive { .. } => None, + MixedTilePayload::Avc420 { regions, .. } => { + if regions.is_empty() { + return Some("AVC420 tile has no regions"); + } + if regions + .iter() + .any(|region| !Self::rect_fits_surface(surface, ®ion.to_rectangle())) + { + return Some("AVC420 region is empty or outside the surface"); + } + if regions + .iter() + .any(|region| MAX_AVC_QP < region.quantization_parameter || MAX_AVC_QUALITY < region.quality) + { + return Some("AVC420 QP or quality is out of range"); + } + + None + } + }); + if let Some(reason) = refusal { + trace!(reason, "Mixed frame refused"); + return None; + } + let timestamp = Self::make_timestamp(timestamp_ms); let frame_id = self.frames.begin_frame(timestamp); diff --git a/crates/ironrdp-testsuite-core/tests/egfx/avc.rs b/crates/ironrdp-testsuite-core/tests/egfx/avc.rs index 6540b1f4f5..6b17c5fb80 100644 --- a/crates/ironrdp-testsuite-core/tests/egfx/avc.rs +++ b/crates/ironrdp-testsuite-core/tests/egfx/avc.rs @@ -1,5 +1,8 @@ -use ironrdp_core::{Decode as _, ReadCursor}; -use ironrdp_egfx::pdu::{Avc420BitmapStream, Avc420Region, align_to_16, annex_b_to_avc, encode_avc420_bitmap_stream}; +use ironrdp_core::{Decode as _, EncodeErrorKind, ReadCursor, encode_vec}; +use ironrdp_egfx::pdu::{ + Avc420BitmapStream, Avc420Region, QuantQuality, align_to_16, annex_b_to_avc, encode_avc420_bitmap_stream, +}; +use rstest::rstest; #[test] fn avc420_region_full_frame() { @@ -84,3 +87,38 @@ fn encode_avc420_bitmap_stream_round_trips_through_decode() { assert_eq!(decoded.quant_qual_vals.len(), 1); assert_eq!(decoded.data, &h264_data); } + +#[test] +fn quant_quality_encodes_the_largest_qp_the_field_holds() { + let quant_quality = QuantQuality { + quantization_parameter: 63, + progressive: true, + quality: 255, + }; + + let encoded = encode_vec(&quant_quality).expect("encode QuantQuality"); + + // Progressive flag in bit 7, reserved bit 6 clear, QP in bits 0 to 5. + assert_eq!(encoded, [0b1011_1111, 255]); +} + +#[rstest] +#[case::first_value_past_the_field(64)] +#[case::largest_value_of_the_type(255)] +fn quant_quality_encode_rejects_a_qp_that_does_not_fit_the_field(#[case] quantization_parameter: u8) { + let quant_quality = QuantQuality { + quantization_parameter, + progressive: false, + quality: 100, + }; + + let error = encode_vec(&quant_quality).expect_err("a QP above 63 does not fit the 6-bit field"); + + assert!(matches!( + error.kind(), + EncodeErrorKind::InvalidField { + field: "quantization_parameter", + .. + } + )); +} diff --git a/crates/ironrdp-testsuite-core/tests/egfx/server.rs b/crates/ironrdp-testsuite-core/tests/egfx/server.rs index 6bf6f5a226..8e0f034e5a 100644 --- a/crates/ironrdp-testsuite-core/tests/egfx/server.rs +++ b/crates/ironrdp-testsuite-core/tests/egfx/server.rs @@ -2,10 +2,13 @@ use ironrdp_core::{Decode as _, Encode, ReadCursor, WriteCursor, encode_vec}; use ironrdp_dvc::DvcProcessor as _; use ironrdp_egfx::pdu::{ Avc420Region, Avc444BitmapStream, CapabilitiesAdvertisePdu, CapabilitiesV8Flags, CapabilitiesV10Flags, - CapabilitiesV81Flags, CapabilitySet, Codec1Type, Encoding, FrameAcknowledgePdu, GfxPdu, PixelFormat, QueueDepth, + CapabilitiesV81Flags, CapabilitiesV103Flags, CapabilitiesV104Flags, CapabilitiesV107Flags, CapabilitySet, + Codec1Type, Codec2Type, Encoding, FrameAcknowledgePdu, GfxPdu, PixelFormat, QueueDepth, }; -use ironrdp_egfx::server::{GraphicsPipelineHandler, GraphicsPipelineServer, QoeMetrics, Surface}; +use ironrdp_egfx::server::{GraphicsPipelineHandler, GraphicsPipelineServer, MixedTilePayload, QoeMetrics, Surface}; use ironrdp_graphics::zgfx::Decompressor; +use ironrdp_pdu::geometry::ExclusiveRectangle; +use rstest::rstest; // ============================================================================ // Test Handler @@ -335,6 +338,258 @@ fn avc444v2_sender_rejects_invalid_stream_shapes_without_queueing() { } } +// ============================================================================ +// Mixed-Codec Frame Tests +// ============================================================================ + +/// A server whose client negotiated `caps`. +/// +/// Mixed frames that carry AVC420 beside another codec need capability version +/// 10.4 or later, so the tests of the per-tile checks negotiate 10.4. +fn mixed_frame_server(caps: CapabilitySet) -> (GraphicsPipelineServer, u16) { + let handler = Box::new(TestHandler::new()); + let mut server = GraphicsPipelineServer::new(handler); + let client_caps_pdu = GfxPdu::CapabilitiesAdvertise(CapabilitiesAdvertisePdu::from_typed(&[caps])); + server + .process(0, &encode_pdu(&client_caps_pdu)) + .expect("process capabilities"); + let surface_id = server.create_surface(64, 64).expect("create surface"); + server.drain_output(); + (server, surface_id) +} + +fn decode_drained_pdus(server: &mut GraphicsPipelineServer) -> Vec { + let mut decompressor = Decompressor::new(); + server + .drain_output() + .iter() + .map(|message| { + let encoded = encode_vec(message.as_ref()).expect("encode DVC message"); + let mut decoded = Vec::new(); + decompressor.decompress(&encoded, &mut decoded).expect("decompress PDU"); + GfxPdu::decode(&mut ReadCursor::new(&decoded)).expect("decode PDU") + }) + .collect() +} + +fn clearcodec_tile(left: u16, top: u16, right: u16, bottom: u16) -> MixedTilePayload { + MixedTilePayload::ClearCodec { + destination: ExclusiveRectangle { + left, + top, + right, + bottom, + }, + bitmap_data: vec![0x00, 0x00, 0x00, 0x00], + } +} + +fn avc420_tile(region: Avc420Region) -> MixedTilePayload { + MixedTilePayload::Avc420 { + regions: vec![region], + h264_data: vec![0x00, 0x00, 0x00, 0x01, 0x67], + } +} + +#[test] +fn mixed_frame_sends_each_tile_in_order_inside_one_frame() { + let (mut server, surface_id) = mixed_frame_server(CapabilitySet::V10_4 { + flags: CapabilitiesV104Flags::SMALL_CACHE, + }); + + let tiles = vec![ + clearcodec_tile(0, 0, 16, 16), + MixedTilePayload::RemoteFxProgressive { + codec_context_id: 7, + progressive_data: vec![0xCC, 0xC0, 0x06, 0x00, 0x00, 0x00], + }, + avc420_tile(Avc420Region::new(16, 8, 48, 40, 22, 78)), + ]; + let frame_id = server + .send_mixed_frame(surface_id, tiles, 42) + .expect("queue mixed frame"); + + let pdus = decode_drained_pdus(&mut server); + assert_eq!(pdus.len(), 5); + let GfxPdu::StartFrame(start) = &pdus[0] else { + panic!("expected StartFrame, got {:?}", pdus[0]); + }; + assert_eq!(start.frame_id, frame_id); + + let GfxPdu::WireToSurface1(clearcodec) = &pdus[1] else { + panic!("expected ClearCodec WireToSurface1"); + }; + assert_eq!(clearcodec.codec_id, Codec1Type::ClearCodec); + assert_eq!(clearcodec.destination_rectangle.right, 16); + assert_eq!(clearcodec.destination_rectangle.bottom, 16); + + let GfxPdu::WireToSurface2(progressive) = &pdus[2] else { + panic!("expected Progressive WireToSurface2"); + }; + assert_eq!(progressive.codec_id, Codec2Type::RemoteFxProgressive); + assert_eq!(progressive.codec_context_id, 7); + + let GfxPdu::WireToSurface1(avc420) = &pdus[3] else { + panic!("expected AVC420 WireToSurface1"); + }; + assert_eq!(avc420.codec_id, Codec1Type::Avc420); + assert_eq!(avc420.destination_rectangle.left, 16); + assert_eq!(avc420.destination_rectangle.top, 8); + assert_eq!(avc420.destination_rectangle.right, 48); + assert_eq!(avc420.destination_rectangle.bottom, 40); + + let GfxPdu::EndFrame(end) = &pdus[4] else { + panic!("expected EndFrame, got {:?}", pdus[4]); + }; + assert_eq!(end.frame_id, frame_id); + assert_eq!(server.frames_in_flight(), 1); +} + +#[rstest] +#[case::empty_clearcodec_destination(clearcodec_tile(8, 8, 8, 16))] +#[case::clearcodec_destination_past_the_surface(clearcodec_tile(0, 0, 65, 16))] +#[case::empty_avc420_region(avc420_tile(Avc420Region::new(0, 8, 32, 8, 22, 78)))] +#[case::avc420_region_past_the_surface(avc420_tile(Avc420Region::new(0, 0, 32, 65, 22, 78)))] +#[case::avc420_tile_with_no_regions(MixedTilePayload::Avc420 { + regions: Vec::new(), + h264_data: vec![0x00, 0x00, 0x00, 0x01, 0x67], +})] +#[case::avc420_qp_above_51(avc420_tile(Avc420Region::new(0, 0, 32, 32, 52, 78)))] +#[case::avc420_qp_too_large_for_the_field(avc420_tile(Avc420Region::new(0, 0, 32, 32, 64, 78)))] +#[case::avc420_quality_above_100(avc420_tile(Avc420Region::new(0, 0, 32, 32, 22, 101)))] +fn mixed_frame_rejects_invalid_tiles_without_queueing(#[case] bad_tile: MixedTilePayload) { + let (mut server, surface_id) = mixed_frame_server(CapabilitySet::V10_4 { + flags: CapabilitiesV104Flags::SMALL_CACHE, + }); + let tiles = vec![clearcodec_tile(0, 0, 16, 16), bad_tile]; + + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_none()); + assert!(!server.has_pending_output()); + assert_eq!(server.frames_in_flight(), 0); +} + +#[test] +fn mixed_frame_rejects_avc420_tiles_when_avc_is_disabled() { + let (mut server, surface_id) = mixed_frame_server(CapabilitySet::V10_4 { + flags: CapabilitiesV104Flags::AVC_DISABLED, + }); + assert!(!server.supports_avc420()); + + let tiles = vec![ + clearcodec_tile(0, 0, 16, 16), + avc420_tile(Avc420Region::new(0, 0, 32, 32, 22, 78)), + ]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_none()); + assert!(!server.has_pending_output()); + assert_eq!(server.frames_in_flight(), 0); + + // With no other codec in the frame the codec mix rule is out of the way, so + // the AVC420 support check is what refuses it. + let tiles = vec![avc420_tile(Avc420Region::new(0, 0, 32, 32, 22, 78))]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_none()); + assert!(!server.has_pending_output()); + assert_eq!(server.frames_in_flight(), 0); + + let tiles = vec![clearcodec_tile(0, 0, 16, 16)]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_some()); +} + +/// MS-RDPEGFX 2.2.3.7 promises AVC420 in the same frame as other codecs only from +/// capability version 10.4, so an earlier client gets no such frame. Each case +/// allows AVC420, so it's refused for the reason under test and not because AVC is +/// off. +#[rstest] +#[case::v8_1(CapabilitySet::V8_1 { flags: CapabilitiesV81Flags::AVC420_ENABLED })] +#[case::v10(CapabilitySet::V10 { flags: CapabilitiesV10Flags::SMALL_CACHE })] +#[case::v10_1(CapabilitySet::V10_1)] +#[case::v10_2(CapabilitySet::V10_2 { flags: CapabilitiesV10Flags::SMALL_CACHE })] +#[case::v10_3(CapabilitySet::V10_3 { flags: CapabilitiesV103Flags::empty() })] +fn mixed_frame_with_avc420_and_another_codec_is_refused_before_10_4(#[case] caps: CapabilitySet) { + let (mut server, surface_id) = mixed_frame_server(caps); + assert!(server.supports_avc420()); + + let tiles = vec![ + clearcodec_tile(0, 0, 16, 16), + avc420_tile(Avc420Region::new(0, 0, 32, 32, 22, 78)), + ]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_none()); + assert!(!server.has_pending_output()); + assert_eq!(server.frames_in_flight(), 0); +} + +/// From capability version 10.4 the same mix is queued. +#[test] +fn mixed_frame_with_avc420_and_another_codec_is_allowed_at_10_4() { + let (mut server, surface_id) = mixed_frame_server(CapabilitySet::V10_4 { + flags: CapabilitiesV104Flags::empty(), + }); + let tiles = vec![ + clearcodec_tile(0, 0, 16, 16), + avc420_tile(Avc420Region::new(0, 0, 32, 32, 22, 78)), + ]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_some()); +} + +/// The promise is about AVC420 beside another codec, so tiles that are all AVC420, +/// or none of them AVC420, are not held to it on an earlier version. +#[rstest] +#[case::v8_1(CapabilitySet::V8_1 { flags: CapabilitiesV81Flags::AVC420_ENABLED })] +#[case::v10(CapabilitySet::V10 { flags: CapabilitiesV10Flags::SMALL_CACHE })] +#[case::v10_1(CapabilitySet::V10_1)] +#[case::v10_2(CapabilitySet::V10_2 { flags: CapabilitiesV10Flags::SMALL_CACHE })] +#[case::v10_3(CapabilitySet::V10_3 { flags: CapabilitiesV103Flags::empty() })] +fn mixed_frame_without_a_codec_mix_is_allowed_before_10_4(#[case] caps: CapabilitySet) { + let (mut server, surface_id) = mixed_frame_server(caps.clone()); + let tiles = vec![ + avc420_tile(Avc420Region::new(0, 0, 32, 32, 22, 78)), + avc420_tile(Avc420Region::new(32, 32, 64, 64, 22, 78)), + ]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_some()); + + let (mut server, surface_id) = mixed_frame_server(caps); + let tiles = vec![clearcodec_tile(0, 0, 16, 16)]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_some()); +} + +/// What the server reports for `caps`, after checking that it confirmed exactly them. +fn negotiated_avc420_in_mixed_frames(caps: CapabilitySet) -> bool { + let (server, _surface_id) = mixed_frame_server(caps.clone()); + assert_eq!(server.negotiated_capabilities(), Some(&caps)); + + server.codec_capabilities().avc420_in_mixed_frames +} + +#[rstest] +#[case::v8(CapabilitySet::V8 { flags: CapabilitiesV8Flags::empty() })] +#[case::v8_1(CapabilitySet::V8_1 { flags: CapabilitiesV81Flags::AVC420_ENABLED })] +#[case::v10(CapabilitySet::V10 { flags: CapabilitiesV10Flags::empty() })] +#[case::v10_1(CapabilitySet::V10_1)] +#[case::v10_2(CapabilitySet::V10_2 { flags: CapabilitiesV10Flags::empty() })] +#[case::v10_3(CapabilitySet::V10_3 { flags: CapabilitiesV103Flags::empty() })] +fn avc420_is_not_promised_beside_other_codecs_before_10_4(#[case] caps: CapabilitySet) { + assert!(!negotiated_avc420_in_mixed_frames(caps)); +} + +#[rstest] +#[case::v10_4(CapabilitySet::V10_4 { flags: CapabilitiesV104Flags::empty() })] +#[case::v10_5(CapabilitySet::V10_5 { flags: CapabilitiesV104Flags::empty() })] +#[case::v10_6(CapabilitySet::V10_6 { flags: CapabilitiesV104Flags::empty() })] +#[case::v10_6_err(CapabilitySet::V10_6Err { flags: CapabilitiesV104Flags::empty() })] +#[case::v10_7(CapabilitySet::V10_7 { flags: CapabilitiesV107Flags::empty() })] +fn avc420_is_promised_beside_other_codecs_from_10_4(#[case] caps: CapabilitySet) { + assert!(negotiated_avc420_in_mixed_frames(caps)); +} + +#[rstest] +#[case::v10_4(CapabilitySet::V10_4 { flags: CapabilitiesV104Flags::AVC_DISABLED })] +#[case::v10_5(CapabilitySet::V10_5 { flags: CapabilitiesV104Flags::AVC_DISABLED })] +#[case::v10_6(CapabilitySet::V10_6 { flags: CapabilitiesV104Flags::AVC_DISABLED })] +#[case::v10_6_err(CapabilitySet::V10_6Err { flags: CapabilitiesV104Flags::AVC_DISABLED })] +#[case::v10_7(CapabilitySet::V10_7 { flags: CapabilitiesV107Flags::AVC_DISABLED })] +fn avc420_is_not_promised_beside_other_codecs_when_the_client_disabled_avc(#[case] caps: CapabilitySet) { + assert!(!negotiated_avc420_in_mixed_frames(caps)); +} + // ============================================================================ // Planar Frame Tests // ============================================================================ @@ -645,6 +900,42 @@ fn test_qoe_reset() { assert!(server.qoe_snapshot().is_none()); } +#[test] +fn mixed_and_progressive_senders_count_backpressure_in_qoe() { + use ironrdp_egfx::pdu::QoeFrameAcknowledgePdu; + + let (mut server, surface_id) = mixed_frame_server(CapabilitySet::V10_4 { + flags: CapabilitiesV104Flags::empty(), + }); + server.set_max_frames_in_flight(1); + + // The snapshot only exists once there is a QoE report or an RTT sample. + let qoe_pdu = GfxPdu::QoeFrameAcknowledge(QoeFrameAcknowledgePdu { + frame_id: 0, + timestamp: 1000, + time_diff_se: 50, + time_diff_dr: 3000, + }); + server.process(0, &encode_pdu(&qoe_pdu)).expect("process QoE report"); + assert_eq!(server.qoe_snapshot().expect("QoE snapshot").backpressure_count, 0); + + // One frame in flight is the limit, so the senders refuse everything after it. + let tiles = vec![clearcodec_tile(0, 0, 16, 16)]; + assert!(server.send_mixed_frame(surface_id, tiles, 0).is_some()); + assert!(server.should_backpressure()); + + let tiles = vec![clearcodec_tile(0, 0, 16, 16)]; + assert!(server.send_mixed_frame(surface_id, tiles, 16).is_none()); + assert_eq!(server.qoe_snapshot().expect("QoE snapshot").backpressure_count, 1); + + assert!( + server + .send_remotefx_progressive_frame(surface_id, 1, vec![0; 4], 33) + .is_none() + ); + assert_eq!(server.qoe_snapshot().expect("QoE snapshot").backpressure_count, 2); +} + // ============================================================================ // Uncompressed Frame Tests // ============================================================================