Skip to content

feat(egfx): add ClearCodec client-side decode dispatch #1175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Greg Lamberson (glamberson) wants to merge 1 commit into
base: master
Choose a base branch
from
+388 −41
Open

feat(egfx): add ClearCodec client-side decode dispatch #1175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 85 additions & 8 deletions crates/ironrdp-egfx/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Client-side EGFX implementation
//!
//! This module provides client-side support for the Graphics Pipeline Extension
//! ([MS-RDPEGFX]), including H.264 AVC420 decode and surface management.
//! ([MS-RDPEGFX]), including H.264 AVC420 decode, ClearCodec decode, and surface management.
//!
//! # Protocol Compliance
//!
Expand All @@ -24,7 +24,7 @@
//! | |
//! | (For each frame:) |
//! |--- StartFrame ----------------------->|
//! |--- WireToSurface1 (H.264) ----------->| -> H264Decoder::decode()
//! |--- WireToSurface1 (codec) ----------->| -> H264/ClearCodec decode
//! |--- EndFrame ------------------------->| -> FrameAcknowledge
//! | |
//! |<---------- FrameAcknowledge ----------|
Expand Down Expand Up @@ -57,8 +57,9 @@ use std::collections::BTreeMap;

use ironrdp_core::{Decode as _, ReadCursor, impl_as_any};
use ironrdp_dvc::{DvcClientProcessor, DvcMessage, DvcProcessor};
use ironrdp_graphics::clearcodec::ClearCodecDecoder;
use ironrdp_graphics::zgfx;
use ironrdp_pdu::geometry::{ExclusiveRectangle, Rectangle as _};
use ironrdp_pdu::geometry::ExclusiveRectangle;
use ironrdp_pdu::{PduResult, decode_cursor, decode_err, pdu_other_err};
use tracing::{debug, trace, warn};

Expand Down Expand Up @@ -383,6 +384,7 @@ enum ClientState {
pub struct GraphicsPipelineClient {
handler: Box<dyn GraphicsPipelineHandler>,
h264_decoder: Option<Box<dyn H264Decoder>>,
clearcodec_decoder: ClearCodecDecoder,

decompressor: zgfx::Decompressor,
decompressed_buffer: Vec<u8>,
Expand All @@ -401,10 +403,12 @@ impl GraphicsPipelineClient {
/// Create a new `GraphicsPipelineClient`
///
/// If `h264_decoder` is `None`, AVC420 frames are logged and skipped.
/// ClearCodec decoding is always available (no external decoder required).
pub fn new(handler: Box<dyn GraphicsPipelineHandler>, h264_decoder: Option<Box<dyn H264Decoder>>) -> Self {
Self {
handler,
h264_decoder,
clearcodec_decoder: ClearCodecDecoder::new(),
decompressor: zgfx::Decompressor::new(),
decompressed_buffer: Vec::new(),
state: ClientState::WaitingForConfirm,
Expand Down Expand Up @@ -632,6 +636,9 @@ impl GraphicsPipelineClient {
if let Some(ref mut decoder) = self.h264_decoder {
decoder.reset();
}
// ClearCodec maintains V-bar and glyph caches that must be rebuilt
// after a graphics reset (the server will re-send all needed state).
self.clearcodec_decoder = ClearCodecDecoder::new();

debug!(width, height, "Graphics reset");
self.handler.on_reset_graphics(width, height);
Expand Down Expand Up @@ -720,6 +727,9 @@ impl GraphicsPipelineClient {
debug!("AVC444 codec not yet implemented, forwarding to handler");
self.handler.on_unhandled_pdu(&GfxPdu::WireToSurface1(pdu));
}
Codec1Type::ClearCodec => {
self.decode_clearcodec(pdu.surface_id, &pdu.destination_rectangle, &pdu.bitmap_data)?;
}
Codec1Type::Uncompressed => {
self.handle_uncompressed(pdu);
}
Expand All @@ -745,8 +755,10 @@ impl GraphicsPipelineClient {
.decode(stream.data)
.map_err(|e| pdu_other_err!("H.264 decode", source: e))?;

let dest_width = dest_rect.width();
let dest_height = dest_rect.height();
// MS-RDPEGFX 2.2.1.4.1: RDPGFX_RECT16 right/bottom are exclusive (one-past-end),
// so dimensions are right-left / bottom-top despite the parsed type's name.
let dest_width = dest_rect.right - dest_rect.left;
let dest_height = dest_rect.bottom - dest_rect.top;

// Decoded frame must be at least as large as the destination rectangle.
// Larger is expected (macroblock alignment) and handled by cropping.
Expand Down Expand Up @@ -777,9 +789,41 @@ impl GraphicsPipelineClient {
Ok(())
}

fn decode_clearcodec(
&mut self,
surface_id: u16,
dest_rect: &ExclusiveRectangle,
bitmap_data: &[u8],
) -> PduResult<()> {
// MS-RDPEGFX 2.2.1.4.1: see decode_avc420 above for wire-format note.
let dest_width = dest_rect.right - dest_rect.left;
let dest_height = dest_rect.bottom - dest_rect.top;

let bgra = self
.clearcodec_decoder
.decode(bitmap_data, dest_width, dest_height)
.map_err(|e| pdu_other_err!("ClearCodec decode", source: e))?;

// ClearCodec outputs BGRA; convert to RGBA for the uniform BitmapUpdate format
let rgba = convert_bgra_to_rgba(&bgra);

let update = BitmapUpdate {
surface_id,
destination_rectangle: dest_rect.clone(),
codec_id: Codec1Type::ClearCodec,
data: rgba,
width: dest_width,
height: dest_height,
};

self.handler.on_bitmap_updated(&update);
Ok(())
}

fn handle_uncompressed(&mut self, pdu: crate::pdu::WireToSurface1Pdu) {
let dest_width = pdu.destination_rectangle.width();
let dest_height = pdu.destination_rectangle.height();
// MS-RDPEGFX 2.2.1.4.1: see decode_avc420 above for wire-format note.
let dest_width = pdu.destination_rectangle.right - pdu.destination_rectangle.left;
let dest_height = pdu.destination_rectangle.bottom - pdu.destination_rectangle.top;

// Convert wire-format pixels to RGBA.
// BitmapUpdate.data is always RGBA8888 regardless of codec -- this is
Expand Down Expand Up @@ -807,7 +851,9 @@ impl GraphicsPipelineClient {

self.handler.on_frame_complete(frame_id);

// Per [3.3.5.12]: client MUST send FrameAcknowledge after EndFrame
// Per [3.3.5.12]: client MUST send FrameAcknowledge after EndFrame.
// We send the actual queue depth (not Unavailable / 0xFFFFFFFF as FreeRDP does);
// the real value gives the server backpressure information for frame pacing.
let ack = GfxPdu::FrameAcknowledge(FrameAcknowledgePdu {
queue_depth: QueueDepth::from_u32(self.frames_queued),
frame_id,
Expand Down Expand Up @@ -896,6 +942,19 @@ impl DvcClientProcessor for GraphicsPipelineClient {}
// Frame Cropping
// ============================================================================

/// Convert BGRA pixel data to RGBA8888
///
/// ClearCodec produces BGRA output per [MS-RDPEGFX 2.2.4.1]. Reorder to
/// [R, G, B, A] for the uniform `BitmapUpdate` pixel format.
fn convert_bgra_to_rgba(src: &[u8]) -> Vec<u8> {
debug_assert!(src.len() % 4 == 0, "BGRA input length not aligned to 4 bytes");
let mut dst = Vec::with_capacity(src.len());
for pixel in src.chunks_exact(4) {
dst.extend_from_slice(&[pixel[2], pixel[1], pixel[0], pixel[3]]);
}
dst
Comment thread
glamberson marked this conversation as resolved.
}

/// Convert uncompressed 32bpp little-endian pixels to RGBA8888
///
/// The wire format for uncompressed graphics is 0xAARRGGBB in a 32-bit
Expand Down Expand Up @@ -1057,6 +1116,24 @@ mod tests {
assert_eq!(cropped.len(), 1920 * 1080 * 4);
}

#[test]
fn convert_bgra_to_rgba_reorders_channels() {
// BGRA input: [B, G, R, A] per pixel
let bgra = vec![
0xFF, 0x00, 0x00, 0xCC, // B=255, G=0, R=0, A=204 (blue)
0x00, 0xFF, 0x00, 0x80, // B=0, G=255, R=0, A=128 (green)
];
let rgba = convert_bgra_to_rgba(&bgra);
// Expected: [R, G, B, A] per pixel
assert_eq!(
rgba,
vec![
0x00, 0x00, 0xFF, 0xCC, // R=0, G=0, B=255, A=204
0x00, 0xFF, 0x00, 0x80, // R=0, G=255, B=0, A=128
]
);
}

#[test]
fn convert_uncompressed_bgrx_to_rgba() {
// Wire format: [B, G, R, A] per pixel (0xAARRGGBB little-endian)
Expand Down
46 changes: 46 additions & 0 deletions crates/ironrdp-egfx/src/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,52 @@ impl GraphicsPipelineServer {
Some(frame_id)
}

/// Queue a ClearCodec frame for transmission.
///
/// ClearCodec is a mandatory lossless codec for all EGFX versions. It
/// provides excellent compression for text, UI elements, and icons.
///
/// `bitmap_data` should be a pre-encoded ClearCodec bitmap stream
/// (as produced by `ironrdp_graphics::clearcodec::ClearCodecEncoder`).
///
/// Returns `Some(frame_id)` if queued, `None` if backpressure is active
/// or the server is not ready.
pub fn send_clearcodec_frame(
&mut self,
surface_id: u16,
destination_rectangle: ExclusiveRectangle,
bitmap_data: Vec<u8>,
timestamp_ms: u32,
) -> Option<u32> {
if !self.is_ready() {
return None;
}
if self.should_backpressure() {
Comment thread
glamberson marked this conversation as resolved.
self.qoe.record_backpressure();
return None;
}

let surface = self.surfaces.get(surface_id)?;

let timestamp = Self::make_timestamp(timestamp_ms);
let frame_id = self.frames.begin_frame(timestamp);

self.output_queue
.push_back(GfxPdu::StartFrame(StartFramePdu { timestamp, frame_id }));

self.output_queue.push_back(GfxPdu::WireToSurface1(WireToSurface1Pdu {
surface_id,
codec_id: Codec1Type::ClearCodec,
pixel_format: surface.pixel_format,
destination_rectangle,
bitmap_data,
}));

self.output_queue.push_back(GfxPdu::EndFrame(EndFramePdu { frame_id }));

Some(frame_id)
}

// ========================================================================
// Mixed-Codec Frame Support
// ========================================================================
Expand Down
83 changes: 51 additions & 32 deletions crates/ironrdp-graphics/src/clearcodec/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ impl ClearCodecDecoder {
let max_offset = output.len();
let mut offset = 0;
for seg in &segments {
// Trim each segment's run_length to the bytes remaining in the
// output buffer. When a segment's declared run would exceed
// the buffer, the excess is intentionally dropped (not a
// parse error) so a single malformed segment cannot CPU-spin
// the decoder. Subsequent segments are also skipped via the
// `break` below once the buffer is full.
let pixels_remaining = (max_offset.saturating_sub(offset)) / 4;
let effective_run = u32::try_from(pixels_remaining).unwrap_or(u32::MAX).min(seg.run_length);
for _ in 0..effective_run {
Expand Down Expand Up @@ -220,6 +226,12 @@ impl ClearCodecDecoder {
.vbar_cache
.get_short_vbar(*index)
.ok_or_else(|| invalid_field_err!("shortVbarIndex", "short V-bar cache miss on hit"))?;
if usize::from(*y_on) + usize::from(cached_short.pixel_count) > usize::from(band_height) {
return Err(invalid_field_err!(
"shortVBarYOn",
"y_on + pixel_count exceeds band height"
));
}
// Create a modified short vbar with the y_on from this reference
let modified = ShortVBar {
y_on: *y_on,
Expand Down Expand Up @@ -291,46 +303,51 @@ impl ClearCodecDecoder {
SubcodecId::Rlex => {
let rlex = ironrdp_pdu::codecs::clearcodec::decode_rlex(sub.bitmap_data)?;
let w = usize::from(sub.width);
let region_pixels = usize::from(sub.width) * usize::from(sub.height);
let palette_len = rlex.palette.len();
let h = usize::from(sub.height);
let pixel_budget = w * h;
let mut px = 0usize;

for seg in &rlex.segments {
if usize::from(seg.start_index) >= palette_len {
return Err(invalid_field_err!("rlex", "start_index exceeds palette size"));
}
if usize::from(seg.stop_index) >= palette_len {
return Err(invalid_field_err!("rlex", "stop_index exceeds palette size"));
}

let color = &rlex.palette[usize::from(seg.start_index)];
for _ in 0..seg.run_length {
if px >= region_pixels {
return Err(invalid_field_err!("rlex", "run exceeds region pixel count"));
// Run: repeat start_index color for run_length pixels
if let Some(color) = rlex.palette.get(usize::from(seg.start_index)) {
for _ in 0..seg.run_length {
if px >= pixel_budget {
break;
}
let col = px % w;
let row = px / w;
let x = usize::from(sub.x_start) + col;
let y = usize::from(sub.y_start) + row;
let dst_idx = (y * sw + x) * 4;
if dst_idx + 3 < output.len() {
output[dst_idx] = color[0]; // B
output[dst_idx + 1] = color[1]; // G
output[dst_idx + 2] = color[2]; // R
output[dst_idx + 3] = 0xFF;
}
px += 1;
}
let x = usize::from(sub.x_start) + px % w;
let y = usize::from(sub.y_start) + px / w;
let dst_idx = (y * sw + x) * 4;
output[dst_idx] = color[0];
output[dst_idx + 1] = color[1];
output[dst_idx + 2] = color[2];
output[dst_idx + 3] = 0xFF;
px += 1;
}

// Suite: sequential palette walk from start_index to stop_index
for palette_idx in seg.start_index..=seg.stop_index {
if px >= region_pixels {
return Err(invalid_field_err!("rlex", "suite exceeds region pixel count"));
if px >= pixel_budget {
break;
}
if let Some(color) = rlex.palette.get(usize::from(palette_idx)) {
let col = px % w;
let row = px / w;
let x = usize::from(sub.x_start) + col;
let y = usize::from(sub.y_start) + row;
let dst_idx = (y * sw + x) * 4;
if dst_idx + 3 < output.len() {
output[dst_idx] = color[0];
output[dst_idx + 1] = color[1];
output[dst_idx + 2] = color[2];
output[dst_idx + 3] = 0xFF;
}
px += 1;
}
let color = &rlex.palette[usize::from(palette_idx)];
let x = usize::from(sub.x_start) + px % w;
let y = usize::from(sub.y_start) + px / w;
let dst_idx = (y * sw + x) * 4;
output[dst_idx] = color[0];
output[dst_idx + 1] = color[1];
output[dst_idx + 2] = color[2];
output[dst_idx + 3] = 0xFF;
px += 1;
}
}
}
Comment thread
glamberson marked this conversation as resolved.
Expand Down Expand Up @@ -428,6 +445,8 @@ impl ClearCodecEncoder {
}

// Composite payload: residual only (bands=0, subcodec=0)
// ClearCodec tiles are bounded by EGFX surface limits, so residual
// data for a single tile is always well within u32 range.
let residual_len = u32::try_from(residual_data.len()).unwrap_or(u32::MAX);
out.extend_from_slice(&residual_len.to_le_bytes());
out.extend_from_slice(&0u32.to_le_bytes()); // bandsByteCount
Expand Down
Loading
Loading