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..5096607ffc 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), }, @@ -1037,9 +1068,9 @@ pub struct GraphicsPipelineServer { /// [`GraphicsPipelineServer::send_mixed_frame()`] to pack multiple codec /// types into a single `StartFrame`/`EndFrame` pair. /// -/// Marked `#[non_exhaustive]` so future EGFX codec additions (for example, -/// Avc444 or hardware-accelerated paths) can land without a SemVer break -/// for downstream consumers that pattern-match on this enum. +/// Marked `#[non_exhaustive]` so future EGFX codec additions can land +/// without a SemVer break for downstream consumers that pattern-match on +/// this enum. #[non_exhaustive] pub enum MixedTilePayload { /// Lossless ClearCodec tile (text, UI elements, icons). @@ -1062,6 +1093,45 @@ pub enum MixedTilePayload { regions: Vec, h264_data: Vec, }, + /// H.264 AVC444 tile (`RFX_AVC444_BITMAP_STREAM`, MS-RDPEGFX 2.2.4.5). + Avc444(Avc444Tile), + /// H.264 AVC444v2 tile (`RFX_AVC444V2_BITMAP_STREAM`, MS-RDPEGFX 2.2.4.6). + /// + /// Identical on the wire to [`MixedTilePayload::Avc444`] except for how + /// the client combines the two views, so it takes the same [`Avc444Tile`]. + Avc444v2(Avc444Tile), +} + +/// One H.264 sub-stream of an [`Avc444Tile`]: the regions it covers and its +/// data, which together are one `RFX_AVC420_BITMAP_STREAM`. +pub struct Avc444SubStream { + pub regions: Vec, + pub data: Vec, +} + +/// The fields of an AVC444 or AVC444v2 tile in a mixed-codec frame. +/// +/// They mirror [`GraphicsPipelineServer::send_avc444v2_frame()`]: `encoding` is +/// the LC value, and the second sub-stream, `stream2`, is present exactly when it +/// is `LUMA_AND_CHROMA`. The regions and the data of the second sub-stream travel +/// together in one [`Avc444SubStream`], so a tile can't hold one without the +/// other. Each sub-stream keeps its own regions because each is a full +/// `RFX_AVC420_BITMAP_STREAM`. +/// +/// MS-RDPEGFX 2.2.3.7 guarantees same-frame mixing only for AVC in YUV420 +/// mode, so no capability version promises that a client decodes AVC444 next to +/// other codecs. [`GraphicsPipelineServer::send_mixed_frame()`] still holds it +/// to the 10.4 floor that applies to AVC420, which you can check beforehand with +/// [`CodecCapabilities::avc420_in_mixed_frames`] from +/// [`GraphicsPipelineServer::codec_capabilities()`], and you should test the +/// clients you target. An LC `CHROMA` update is combined with previously decoded luma +/// (2.2.4.5), so it must not cover an area another codec has repainted since +/// that area's last luma update. +pub struct Avc444Tile { + pub encoding: Encoding, + pub stream1_regions: Vec, + pub stream1_data: Vec, + pub stream2: Option, } impl GraphicsPipelineServer { @@ -1432,6 +1502,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, @@ -1484,6 +1559,12 @@ impl GraphicsPipelineServer { /// AVC444 uses two streams: luma (Y) and chroma (UV). Set `chroma_data` to /// `None` for luma-only transmission. /// + /// Unlike [`Self::send_mixed_frame()`], this does not check the regions. Each + /// must be non-empty and inside the surface, with a QP of at most 51 and a + /// quality of at most 100 (MS-RDPEGFX 2.2.4.4.2). A `None` from this sender has + /// only meant not ready, not supported or backpressure, and callers read it that + /// way. + /// /// Returns `Some(frame_id)` if queued, `None` if not supported or backpressured. pub fn send_avc444_frame( &mut self, @@ -1519,6 +1600,12 @@ impl GraphicsPipelineServer { /// stream 1 and chroma in stream 2, LC=1 carries luma in stream 1, and LC=2 /// carries chroma in stream 1. /// + /// Unlike [`Self::send_mixed_frame()`], this does not check the regions. Each + /// must be non-empty and inside the surface, with a QP of at most 51 and a + /// quality of at most 100 (MS-RDPEGFX 2.2.4.4.2). A `None` from this sender has + /// only meant not ready, not supported or backpressure, and callers read it that + /// way. + /// /// Returns `Some(frame_id)` if queued, `None` if the stream shape is /// invalid, AVC444 is not supported, or backpressure is active. /// @@ -1564,14 +1651,14 @@ impl GraphicsPipelineServer { stream2_regions: Option<&[Avc420Region]>, timestamp_ms: u32, ) -> Option { - let valid_stream_shape = if encoding == Encoding::LUMA_AND_CHROMA { - stream2_data.is_some() && stream2_regions.is_some() - } else if encoding == Encoding::LUMA || encoding == Encoding::CHROMA { - stream2_data.is_none() && stream2_regions.is_none() - } else { - false + // A second sub-stream is its regions and its data together; one without the + // other is never valid. + let stream2 = match (stream2_regions, stream2_data) { + (Some(regions), Some(data)) => Some((regions, data)), + (None, None) => None, + _ => return None, }; - if !valid_stream_shape { + if !Self::avc444_stream_shape_is_valid(encoding, stream2.is_some()) { return None; } @@ -1591,57 +1678,102 @@ impl GraphicsPipelineServer { let timestamp = Self::make_timestamp(timestamp_ms); let frame_id = self.frames.begin_frame(timestamp); - let stream1_rectangles: Vec<_> = stream1_regions.iter().map(Avc420Region::to_rectangle).collect(); - let stream1_quant_vals: Vec<_> = stream1_regions.iter().map(Avc420Region::to_quant_quality).collect(); + let wire_pdu = Self::avc444_wire_pdu(codec_id, surface, encoding, (stream1_regions, stream1_data), stream2); - let stream1 = Avc420BitmapStream { - rectangles: stream1_rectangles, - quant_qual_vals: stream1_quant_vals, - data: stream1_data, - }; + self.output_queue + .push_back(GfxPdu::StartFrame(StartFramePdu { timestamp, frame_id })); + self.output_queue.push_back(GfxPdu::WireToSurface1(wire_pdu)); + self.output_queue.push_back(GfxPdu::EndFrame(EndFramePdu { frame_id })); - let stream2 = if encoding == Encoding::LUMA_AND_CHROMA { - let stream2_data = stream2_data?; - let stream2_regions = stream2_regions?; - let stream2_rectangles: Vec<_> = stream2_regions.iter().map(Avc420Region::to_rectangle).collect(); - let stream2_quant_vals: Vec<_> = stream2_regions.iter().map(Avc420Region::to_quant_quality).collect(); + Some(frame_id) + } - Some(Avc420BitmapStream { - rectangles: stream2_rectangles, - quant_qual_vals: stream2_quant_vals, - data: stream2_data, - }) + /// Whether the second AVC444 sub-stream is present exactly when the LC + /// value calls for it; LC 3 is invalid (MS-RDPEGFX 2.2.4.5). + fn avc444_stream_shape_is_valid(encoding: Encoding, has_stream2: bool) -> bool { + if encoding == Encoding::LUMA_AND_CHROMA { + has_stream2 + } else if encoding == Encoding::LUMA || encoding == Encoding::CHROMA { + !has_stream2 } else { - None + false + } + } + + /// Build the `WireToSurface1` PDU for an AVC444 or AVC444v2 update whose + /// stream shape has already been checked. + fn avc444_wire_pdu<'a>( + codec_id: Codec1Type, + surface: &Surface, + encoding: Encoding, + stream1: (&[Avc420Region], &'a [u8]), + stream2: Option<(&[Avc420Region], &'a [u8])>, + ) -> WireToSurface1Pdu { + let substream = |(regions, data): (&[Avc420Region], &'a [u8])| Avc420BitmapStream { + rectangles: regions.iter().map(Avc420Region::to_rectangle).collect(), + quant_qual_vals: regions.iter().map(Avc420Region::to_quant_quality).collect(), + data, }; let avc444_stream = Avc444BitmapStream { encoding, - stream1, - stream2, + stream1: substream(stream1), + stream2: stream2.map(substream), }; - let encoded_stream = encode_avc444_bitmap_stream(&avc444_stream); - let target_rect = if let Some(stream2_regions) = stream2_regions { - Self::compute_dest_rect_for_streams(stream1_regions, stream2_regions, surface.width, surface.height) - } else { - Self::compute_dest_rect(stream1_regions, surface.width, surface.height) + // destRect is the bounding rectangle of both sub-streams (MS-RDPEGFX 2.2.2.1). + let destination_rectangle = match stream2 { + Some((stream2_regions, _)) => { + Self::compute_dest_rect_for_streams(stream1.0, stream2_regions, surface.width, surface.height) + } + None => Self::compute_dest_rect(stream1.0, surface.width, surface.height), }; - self.output_queue - .push_back(GfxPdu::StartFrame(StartFramePdu { timestamp, frame_id })); - - self.output_queue.push_back(GfxPdu::WireToSurface1(WireToSurface1Pdu { - surface_id, + WireToSurface1Pdu { + surface_id: surface.id, codec_id, pixel_format: surface.pixel_format, - destination_rectangle: target_rect, - bitmap_data: encoded_stream, - })); + destination_rectangle, + bitmap_data: encode_avc444_bitmap_stream(&avc444_stream), + } + } - self.output_queue.push_back(GfxPdu::EndFrame(EndFramePdu { frame_id })); + /// Build the `WireToSurface1` PDU for an AVC444 or AVC444v2 mixed-frame tile + /// whose shape has already been checked. + fn avc444_tile_pdu(codec_id: Codec1Type, surface: &Surface, tile: &Avc444Tile) -> WireToSurface1Pdu { + Self::avc444_wire_pdu( + codec_id, + surface, + tile.encoding, + (&tile.stream1_regions, &tile.stream1_data), + tile.stream2 + .as_ref() + .map(|stream2| (stream2.regions.as_slice(), stream2.data.as_slice())), + ) + } - Some(frame_id) + /// Why the regions of one AVC420 tile or one AVC444 sub-stream can't be sent + /// in a mixed frame, if they can't: the list is empty, a region is empty or + /// outside the surface, or a QP or quality is outside the ranges of + /// MS-RDPEGFX 2.2.4.4.2. + fn avc_regions_refusal(surface: &Surface, regions: &[Avc420Region]) -> Option<&'static str> { + if regions.is_empty() { + return Some("AVC tile has no regions"); + } + if regions + .iter() + .any(|region| !Self::rect_fits_surface(surface, ®ion.to_rectangle())) + { + return Some("AVC 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("AVC QP or quality is out of range"); + } + + None } fn compute_dest_rect_for_streams( @@ -1789,6 +1921,7 @@ impl GraphicsPipelineServer { return None; } if self.should_backpressure() { + self.qoe.record_backpressure(); return None; } @@ -1869,10 +2002,43 @@ 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 AVC444 tiles need + /// AVC444 support; an AVC444 tile's second sub-stream must be present + /// exactly when its LC is `LUMA_AND_CHROMA`. A frame that mixes an AVC tile + /// (AVC420, AVC444 or AVC444v2) with a tile of another codec needs + /// capability version 10.4 or later. The spec promises that only for + /// AVC420, and only from there; AVC444 beside another codec is held to the + /// same floor because no version promises it at all. A frame whose tiles + /// all use one codec is not covered by it. Each AVC tile needs at least one + /// region, and so does the second sub-stream of an AVC444 tile, unlike + /// [`Self::send_avc420_frame()`], which reads an empty list as the whole + /// surface: a whole-surface tile would cover the tiles sent before it. Each + /// AVC region and ClearCodec destination must be non-empty and inside the + /// surface. AVC 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 +2049,69 @@ 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) && !self.supports_avc420() { + trace!( + reason = "AVC420 is not supported by the negotiated capabilities", + "Mixed frame refused" + ); + return None; + } + + // Tiles of one codec family are never a mix; an AVC tile beside a tile of + // another family needs the 10.4 floor (AVC444 and AVC444v2 count as one family). + let family = |tile: &MixedTilePayload| match tile { + MixedTilePayload::ClearCodec { .. } => 0, + MixedTilePayload::RemoteFxProgressive { .. } => 1, + MixedTilePayload::Avc420 { .. } => 2, + MixedTilePayload::Avc444(_) | MixedTilePayload::Avc444v2(_) => 3, + }; + let has_avc = tiles.iter().any(|tile| 2 <= family(tile)); + let mixes_codecs = tiles.iter().any(|tile| family(tile) != family(&tiles[0])); + if has_avc && mixes_codecs && !self.codec_caps.avc420_in_mixed_frames { + trace!( + reason = "AVC beside another codec is not promised by the negotiated capabilities", + "Mixed frame refused" + ); + return None; + } + + let supports_avc444 = self.supports_avc444(); 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, .. } => Self::avc_regions_refusal(surface, regions), + MixedTilePayload::Avc444(tile) | MixedTilePayload::Avc444v2(tile) => { + if !supports_avc444 { + return Some("AVC444 is not supported by the negotiated capabilities"); + } + if !Self::avc444_stream_shape_is_valid(tile.encoding, tile.stream2.is_some()) { + return Some("AVC444 second sub-stream does not match the LC value"); + } + + Self::avc_regions_refusal(surface, &tile.stream1_regions).or_else(|| { + tile.stream2 + .as_ref() + .and_then(|stream2| Self::avc_regions_refusal(surface, &stream2.regions)) + }) + } + }); + 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); @@ -1936,6 +2156,22 @@ impl GraphicsPipelineServer { bitmap_data: encoded_stream, })); } + MixedTilePayload::Avc444(tile) => { + self.output_queue + .push_back(GfxPdu::WireToSurface1(Self::avc444_tile_pdu( + Codec1Type::Avc444, + surface, + &tile, + ))); + } + MixedTilePayload::Avc444v2(tile) => { + self.output_queue + .push_back(GfxPdu::WireToSurface1(Self::avc444_tile_pdu( + Codec1Type::Avc444v2, + surface, + &tile, + ))); + } } } 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..91da952d4b 100644 --- a/crates/ironrdp-testsuite-core/tests/egfx/server.rs +++ b/crates/ironrdp-testsuite-core/tests/egfx/server.rs @@ -2,10 +2,15 @@ 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::{ + Avc444SubStream, Avc444Tile, GraphicsPipelineHandler, GraphicsPipelineServer, MixedTilePayload, QoeMetrics, Surface, }; -use ironrdp_egfx::server::{GraphicsPipelineHandler, GraphicsPipelineServer, QoeMetrics, Surface}; use ironrdp_graphics::zgfx::Decompressor; +use ironrdp_pdu::geometry::ExclusiveRectangle; +use rstest::rstest; // ============================================================================ // Test Handler @@ -323,6 +328,8 @@ fn avc444v2_sender_rejects_invalid_stream_shapes_without_queueing() { (Encoding::LUMA_AND_CHROMA, None, None), (Encoding::LUMA, Some(data.as_slice()), Some(regions.as_slice())), (Encoding::CHROMA, Some(data.as_slice()), Some(regions.as_slice())), + (Encoding::LUMA, Some(data.as_slice()), None), + (Encoding::LUMA, None, Some(regions.as_slice())), (Encoding::from_bits_retain(3), None, None), ] { assert!( @@ -335,6 +342,463 @@ 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)); +} + +fn avc444_tile( + v2: bool, + encoding: Encoding, + stream1_regions: Vec, + stream2_regions: Option>, +) -> MixedTilePayload { + let stream1_data = vec![0x00, 0x00, 0x00, 0x01, 0x67]; + let stream2 = stream2_regions.map(|regions| Avc444SubStream { + regions, + data: vec![0x00, 0x00, 0x00, 0x01, 0x68], + }); + let tile = Avc444Tile { + encoding, + stream1_regions, + stream1_data, + stream2, + }; + if v2 { + MixedTilePayload::Avc444v2(tile) + } else { + MixedTilePayload::Avc444(tile) + } +} + +fn avc444_region() -> Vec { + vec![Avc420Region::new(0, 0, 32, 32, 22, 78)] +} + +/// A server whose client negotiated 10.4, where an AVC tile may share a frame with another codec. +fn server_at_10_4() -> (GraphicsPipelineServer, u16) { + mixed_frame_server(CapabilitySet::V10_4 { + flags: CapabilitiesV104Flags::empty(), + }) +} + +#[rstest] +fn mixed_frame_carries_avc444_tiles_next_to_clearcodec( + #[values(false, true)] v2: bool, + #[values(Encoding::LUMA_AND_CHROMA, Encoding::LUMA, Encoding::CHROMA)] encoding: Encoding, +) { + use ironrdp_graphics::clearcodec::{ClearCodecDecoder, ClearCodecEncoder}; + + let pixels: Vec = (0..=u8::MAX) + .flat_map(|i| [0x10 ^ i, 0x20 ^ i, 0x30 ^ i, 0xFF]) + .collect(); + let clearcodec_data = ClearCodecEncoder::new().encode(&pixels, 16, 16); + + let (mut server, surface_id) = server_at_10_4(); + let stream2_regions = + (encoding == Encoding::LUMA_AND_CHROMA).then(|| vec![Avc420Region::new(16, 8, 64, 48, 24, 76)]); + + let tiles = vec![ + avc444_tile(v2, encoding, avc444_region(), stream2_regions), + MixedTilePayload::ClearCodec { + destination: ExclusiveRectangle { + left: 48, + top: 48, + right: 64, + bottom: 64, + }, + bitmap_data: clearcodec_data, + }, + ]; + server + .send_mixed_frame(surface_id, tiles, 42) + .expect("queue mixed frame"); + + let pdus = decode_drained_pdus(&mut server); + assert_eq!(pdus.len(), 4); + assert!(matches!(pdus[0], GfxPdu::StartFrame(_))); + assert!(matches!(pdus[3], GfxPdu::EndFrame(_))); + + let GfxPdu::WireToSurface1(avc444) = &pdus[1] else { + panic!("expected AVC444 WireToSurface1"); + }; + let expected_codec = if v2 { Codec1Type::Avc444v2 } else { Codec1Type::Avc444 }; + assert_eq!(avc444.codec_id, expected_codec); + + let stream = Avc444BitmapStream::decode(&mut ReadCursor::new(&avc444.bitmap_data)).expect("decode AVC444"); + assert_eq!(stream.encoding, encoding); + assert_eq!(stream.stream1.rectangles[0].right, 32); + assert_eq!(stream.stream1.rectangles[0].bottom, 32); + if encoding == Encoding::LUMA_AND_CHROMA { + let stream2 = stream.stream2.expect("second sub-stream"); + assert_eq!(stream2.rectangles[0].left, 16); + assert_eq!(stream2.rectangles[0].bottom, 48); + // destRect is the bounding box of both sub-streams. + assert_eq!(avc444.destination_rectangle.right, 64); + assert_eq!(avc444.destination_rectangle.bottom, 48); + } else { + assert!(stream.stream2.is_none()); + assert_eq!(avc444.destination_rectangle.right, 32); + assert_eq!(avc444.destination_rectangle.bottom, 32); + } + + let GfxPdu::WireToSurface1(clearcodec) = &pdus[2] else { + panic!("expected ClearCodec WireToSurface1"); + }; + assert_eq!(clearcodec.codec_id, Codec1Type::ClearCodec); + let decoded = ClearCodecDecoder::new() + .decode(&clearcodec.bitmap_data, 16, 16) + .expect("decode ClearCodec tile"); + assert_eq!(decoded.len(), 16 * 16 * 4); +} + +#[rstest] +#[case::lc0_without_a_second_sub_stream(avc444_tile(false, Encoding::LUMA_AND_CHROMA, avc444_region(), None))] +#[case::lc1_with_a_second_sub_stream(avc444_tile(true, Encoding::LUMA, avc444_region(), Some(avc444_region())))] +#[case::lc2_with_a_second_sub_stream(avc444_tile(false, Encoding::CHROMA, avc444_region(), Some(avc444_region())))] +#[case::lc3(avc444_tile(true, Encoding::from_bits_retain(3), avc444_region(), None))] +#[case::second_sub_stream_region_past_the_surface(avc444_tile( + true, + Encoding::LUMA_AND_CHROMA, + avc444_region(), + Some(vec![Avc420Region::new(0, 0, 80, 32, 22, 78)]), +))] +#[case::degenerate_first_sub_stream_region(avc444_tile( + false, + Encoding::LUMA, + vec![Avc420Region::new(4, 4, 4, 32, 22, 78)], + None, +))] +#[case::no_first_sub_stream_regions(avc444_tile(false, Encoding::LUMA, Vec::new(), None))] +#[case::no_second_sub_stream_regions(avc444_tile(true, Encoding::LUMA_AND_CHROMA, avc444_region(), Some(Vec::new())))] +#[case::qp_above_51_in_the_second_sub_stream(avc444_tile( + false, + Encoding::LUMA_AND_CHROMA, + avc444_region(), + Some(vec![Avc420Region::new(0, 0, 32, 32, 52, 78)]), +))] +#[case::quality_above_100_in_the_first_sub_stream(avc444_tile( + false, + Encoding::LUMA, + vec![Avc420Region::new(0, 0, 32, 32, 22, 101)], + None, +))] +fn mixed_frame_rejects_invalid_avc444_tiles_without_queueing(#[case] bad_tile: MixedTilePayload) { + let (mut server, surface_id) = server_at_10_4(); + 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_avc444_tiles_without_avc444_support() { + let (mut server, surface_id) = mixed_frame_server(CapabilitySet::V8_1 { + flags: CapabilitiesV81Flags::AVC420_ENABLED, + }); + assert!(server.supports_avc420()); + assert!(!server.supports_avc444()); + + let tiles = vec![avc444_tile(true, Encoding::LUMA, avc444_region(), None)]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_none()); + assert!(!server.has_pending_output()); + assert_eq!(server.frames_in_flight(), 0); +} + +/// MS-RDPEGFX 2.2.3.7 promises no version at which AVC444 decodes next to other codecs, so +/// the mixed path holds it to the 10.4 floor it applies to AVC420. Each case allows AVC444 +/// and AVC420, so the refusal is for the version and not because AVC is off. +#[rstest] +#[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_avc444_and_another_codec_is_refused_before_10_4( + #[case] caps: CapabilitySet, + #[values(false, true)] v2: bool, +) { + for other_tile in [ + clearcodec_tile(48, 48, 64, 64), + avc420_tile(Avc420Region::new(32, 32, 64, 64, 22, 78)), + ] { + let (mut server, surface_id) = mixed_frame_server(caps.clone()); + assert!(server.supports_avc444()); + assert!(server.supports_avc420()); + + let tiles = vec![avc444_tile(v2, Encoding::LUMA, avc444_region(), None), other_tile]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_none()); + assert!(!server.has_pending_output()); + assert_eq!(server.frames_in_flight(), 0); + } +} + +/// Tiles that all use one codec are no mix, so on an earlier version a frame of AVC444 tiles +/// is queued. +#[test] +fn mixed_frame_of_avc444_tiles_alone_is_allowed_before_10_4() { + let (mut server, surface_id) = mixed_frame_server(CapabilitySet::V10 { + flags: CapabilitiesV10Flags::SMALL_CACHE, + }); + let tiles = vec![ + avc444_tile(false, Encoding::LUMA, avc444_region(), None), + avc444_tile( + true, + Encoding::LUMA, + vec![Avc420Region::new(32, 32, 64, 64, 22, 78)], + None, + ), + ]; + assert!(server.send_mixed_frame(surface_id, tiles, 42).is_some()); +} + // ============================================================================ // Planar Frame Tests // ============================================================================ @@ -645,6 +1109,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 // ============================================================================