Some preliminary refactoring for #670

crates/ironrdp-client/src/config.rs

@@ -5,7 +5,7 @@ use anyhow::Context as _;
 use clap::clap_derive::ValueEnum;
 use clap::Parser;
 use ironrdp::connector::{self, Credentials};
-use ironrdp::pdu::rdp::capability_sets::MajorPlatformType;
+use ironrdp::pdu::rdp::capability_sets::{client_codecs_capabilities, MajorPlatformType};
 use ironrdp::pdu::rdp::client_info::PerformanceFlags;
 use tap::prelude::*;
 use url::Url;
@@ -271,6 +271,7 @@ impl Config {
             Some(connector::BitmapConfig {
                 color_depth,
                 lossy_compression: true,
+                codecs: client_codecs_capabilities(),
             })
         } else {
             None

crates/ironrdp-connector/src/connection_activation.rs

@@ -355,16 +355,13 @@ fn create_client_confirm_active(
         CapabilitySet::SurfaceCommands(SurfaceCommands {
             flags: CmdFlags::SET_SURFACE_BITS | CmdFlags::STREAM_SURFACE_BITS | CmdFlags::FRAME_MARKER,
         }),
-        CapabilitySet::BitmapCodecs(BitmapCodecs(vec![Codec {
-            id: 0x03, // RemoteFX
-            property: CodecProperty::RemoteFx(RemoteFxContainer::ClientContainer(RfxClientCapsContainer {
-                capture_flags: CaptureFlags::empty(),
-                caps_data: RfxCaps(RfxCapset(vec![RfxICap {
-                    flags: RfxICapFlags::empty(),
-                    entropy_bits: EntropyBits::Rlgr3,
-                }])),
-            })),
-        }])),
+        CapabilitySet::BitmapCodecs(
+            config
+                .bitmap
+                .as_ref()
+                .map(|b| b.codecs.clone())
+                .unwrap_or_else(client_codecs_capabilities),
+        ),
         CapabilitySet::FrameAcknowledge(FrameAcknowledge {
             // FIXME(#447): Revert this to 2 per FreeRDP.
             // This is a temporary hack to fix a resize bug, see:

crates/ironrdp-connector/src/lib.rs

@@ -23,7 +23,7 @@ use std::sync::Arc;
 
 use ironrdp_core::{encode_buf, encode_vec, Encode, WriteBuf};
 use ironrdp_pdu::nego::NegoRequestData;
-use ironrdp_pdu::rdp::capability_sets;
+use ironrdp_pdu::rdp::capability_sets::{self, BitmapCodecs};
 use ironrdp_pdu::rdp::client_info::PerformanceFlags;
 use ironrdp_pdu::x224::X224;
 use ironrdp_pdu::{gcc, x224, PduHint};
@@ -43,11 +43,12 @@ pub struct DesktopSize {
     pub height: u16,
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct BitmapConfig {
     pub lossy_compression: bool,
     pub color_depth: u32,
+    pub codecs: BitmapCodecs,
 }
 
 #[derive(Debug, Clone)]

crates/ironrdp-pdu/src/rdp/capability_sets.rs

@@ -32,8 +32,9 @@ pub use self::bitmap_cache::{
     BitmapCache, BitmapCacheRev2, CacheEntry, CacheFlags, CellInfo, BITMAP_CACHE_ENTRIES_NUM,
 };
 pub use self::bitmap_codecs::{
-    BitmapCodecs, CaptureFlags, Codec, CodecProperty, EntropyBits, Guid, NsCodec, RemoteFxContainer, RfxCaps,
-    RfxCapset, RfxClientCapsContainer, RfxICap, RfxICapFlags,
+    client_codecs_capabilities, BitmapCodecs, CaptureFlags, Codec, CodecId, CodecProperty, EntropyBits, Guid, NsCodec,
+    RemoteFxContainer, RfxCaps, RfxCapset, RfxClientCapsContainer, RfxICap, RfxICapFlags, CODEC_ID_NONE,
+    CODEC_ID_REMOTEFX,
 };
 pub use self::brush::{Brush, SupportLevel};
 pub use self::frame_acknowledge::FrameAcknowledge;

crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs

@@ -1,6 +1,8 @@
 #[cfg(test)]
 mod tests;
 
+use core::fmt::{self, Debug};
+
 use bitflags::bitflags;
 use ironrdp_core::{
     cast_length, decode, ensure_fixed_part_size, ensure_size, invalid_field_err, other_err, Decode, DecodeResult,
@@ -97,7 +99,7 @@ impl<'de> Decode<'de> for Guid {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone, Default)]
 pub struct BitmapCodecs(pub Vec<Codec>);
 
 impl BitmapCodecs {
@@ -239,39 +241,29 @@ impl<'de> Decode<'de> for Codec {
         let id = src.read_u8();
         let codec_properties_len = usize::from(src.read_u16());
 
-        let property = if codec_properties_len != 0 {
-            ensure_size!(in: src, size: codec_properties_len);
-            let property_buffer = src.read_slice(codec_properties_len);
-
-            match guid {
-                GUID_NSCODEC => CodecProperty::NsCodec(decode(property_buffer)?),
-                GUID_REMOTEFX | GUID_IMAGE_REMOTEFX => {
-                    let property = if property_buffer[0] == 0 {
-                        RemoteFxContainer::ServerContainer(codec_properties_len)
-                    } else {
-                        RemoteFxContainer::ClientContainer(decode(property_buffer)?)
-                    };
-
-                    match guid {
-                        GUID_REMOTEFX => CodecProperty::RemoteFx(property),
-                        GUID_IMAGE_REMOTEFX => CodecProperty::ImageRemoteFx(property),
-                        _ => unreachable!(),
-                    }
-                }
-                GUID_IGNORE => CodecProperty::Ignore,
-                _ => CodecProperty::None,
-            }
-        } else {
-            match guid {
-                GUID_NSCODEC | GUID_REMOTEFX | GUID_IMAGE_REMOTEFX => {
-                    return Err(invalid_field_err!(
-                        "codecPropertiesLen",
-                        "invalid codec property length"
-                    ));
+        ensure_size!(in: src, size: codec_properties_len);
+        let property_buffer = src.read_slice(codec_properties_len);
+
+        let property = match guid {
+            GUID_NSCODEC => CodecProperty::NsCodec(decode(property_buffer)?),
+            GUID_REMOTEFX | GUID_IMAGE_REMOTEFX => {
+                let byte = property_buffer
+                    .first()
+                    .ok_or_else(|| invalid_field_err!("remotefx property", "must not be empty"))?;
+                let property = if *byte == 0 {
+                    RemoteFxContainer::ServerContainer(codec_properties_len)
+                } else {
+                    RemoteFxContainer::ClientContainer(decode(property_buffer)?)
+                };
+
+                match guid {
+                    GUID_REMOTEFX => CodecProperty::RemoteFx(property),
+                    GUID_IMAGE_REMOTEFX => CodecProperty::ImageRemoteFx(property),
+                    _ => unreachable!(),
                 }
-                GUID_IGNORE => CodecProperty::Ignore,
-                _ => CodecProperty::None,
             }
+            GUID_IGNORE => CodecProperty::Ignore,
+            _ => CodecProperty::None,
         };
 
         Ok(Self { id, property })
@@ -391,6 +383,8 @@ impl Encode for RfxClientCapsContainer {
 
 impl<'de> Decode<'de> for RfxClientCapsContainer {
     fn decode(src: &mut ReadCursor<'de>) -> DecodeResult<Self> {
+        ensure_fixed_part_size!(in: src);
+
         let _length = src.read_u32();
         let capture_flags = CaptureFlags::from_bits_truncate(src.read_u32());
         let _caps_length = src.read_u32();
@@ -617,3 +611,47 @@ bitflags! {
         const CODEC_MODE = 2;
     }
 }
+
+// Those IDs are hard-coded for practical reasons, they are implementation
+// details of the IronRDP client. The server should respect the client IDs.
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub struct CodecId(u8);
+
+pub const CODEC_ID_NONE: CodecId = CodecId(0);
+pub const CODEC_ID_REMOTEFX: CodecId = CodecId(3);
+
+impl Debug for CodecId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let name = match self.0 {
+            0 => "None",
+            3 => "RemoteFx",
+            _ => "unknown",
+        };
+        write!(f, "CodecId({})", name)
+    }
+}
+
+impl CodecId {
+    pub const fn from_u8(value: u8) -> Option<Self> {
+        match value {
+            0 => Some(CODEC_ID_NONE),
+            3 => Some(CODEC_ID_REMOTEFX),
+            _ => None,
+        }
+    }
+}
+
+pub fn client_codecs_capabilities() -> BitmapCodecs {
+    let codecs = vec![Codec {
+        id: CODEC_ID_REMOTEFX.0,
+        property: CodecProperty::RemoteFx(RemoteFxContainer::ClientContainer(RfxClientCapsContainer {
+            capture_flags: CaptureFlags::empty(),
+            caps_data: RfxCaps(RfxCapset(vec![RfxICap {
+                flags: RfxICapFlags::empty(),
+                entropy_bits: EntropyBits::Rlgr3,
+            }])),
+        })),
+    }];
+
+    BitmapCodecs(codecs)
+}

crates/ironrdp-session/src/fast_path.rs

@@ -9,12 +9,12 @@ use ironrdp_pdu::codecs::rfx::FrameAcknowledgePdu;
 use ironrdp_pdu::fast_path::{FastPathHeader, FastPathUpdate, FastPathUpdatePdu, Fragmentation};
 use ironrdp_pdu::geometry::{InclusiveRectangle, Rectangle as _};
 use ironrdp_pdu::pointer::PointerUpdateData;
+use ironrdp_pdu::rdp::capability_sets::{CodecId, CODEC_ID_NONE, CODEC_ID_REMOTEFX};
 use ironrdp_pdu::rdp::headers::ShareDataPdu;
 use ironrdp_pdu::surface_commands::{FrameAction, FrameMarkerPdu, SurfaceCommand};
 
 use crate::image::DecodedImage;
 use crate::pointer::PointerCache;
-use crate::utils::CodecId;
 use crate::{rfx, SessionError, SessionErrorExt, SessionResult};
 
 #[derive(Debug)]
@@ -337,7 +337,7 @@ impl Processor {
                         bottom: destination.bottom - 1,
                     };
                     match codec_id {
-                        CodecId::None => {
+                        CODEC_ID_NONE => {
                             let ext_data = bits.extended_bitmap_data;
                             match ext_data.bpp {
                                 32 => {
@@ -352,7 +352,7 @@ impl Processor {
                                 }
                             }
                         }
-                        CodecId::RemoteFx => {
+                        CODEC_ID_REMOTEFX => {
                             let mut data = ReadCursor::new(bits.extended_bitmap_data.data);
                             while !data.is_empty() {
                                 let (_frame_id, rectangle) = self.rfx_handler.decode(image, &destination, &mut data)?;
@@ -361,6 +361,9 @@ impl Processor {
                                     .or(Some(rectangle));
                             }
                         }
+                        _ => {
+                            warn!("Unsupported codec ID: {}", bits.extended_bitmap_data.codec_id);
+                        }
                     }
                 }
                 SurfaceCommand::FrameMarker(marker) => {

crates/ironrdp-session/src/lib.rs

@@ -13,7 +13,6 @@ pub mod image;
 pub mod legacy;
 pub mod pointer;
 pub mod rfx; // FIXME: maybe this module should not be in this crate
-pub mod utils;
 pub mod x224;
 
 mod active_stage;

crates/ironrdp-web/src/session.rs

@@ -22,6 +22,7 @@ use ironrdp::displaycontrol::client::DisplayControlClient;
 use ironrdp::dvc::DrdynvcClient;
 use ironrdp::graphics::image_processing::PixelFormat;
 use ironrdp::pdu::input::fast_path::FastPathInputEvent;
+use ironrdp::pdu::rdp::capability_sets::client_codecs_capabilities;
 use ironrdp::pdu::rdp::client_info::PerformanceFlags;
 use ironrdp::session::image::DecodedImage;
 use ironrdp::session::{fast_path, ActiveStage, ActiveStageOutput, GracefulDisconnectReason};
@@ -838,6 +839,7 @@ fn build_config(
         bitmap: Some(connector::BitmapConfig {
             color_depth: 16,
             lossy_compression: true,
+            codecs: client_codecs_capabilities(),
         }),
         #[allow(clippy::arithmetic_side_effects)] // fine unless we end up with an insanely big version
         client_build: semver::Version::parse(env!("CARGO_PKG_VERSION"))

ffi/src/connector/config.rs

@@ -125,7 +125,7 @@ pub mod ffi {
         }
 
         pub fn set_bitmap_config(&mut self, bitmap: &BitmapConfig) {
-            self.bitmap = Some(bitmap.0);
+            self.bitmap = Some(bitmap.0.clone());
         }
 
         pub fn set_client_build(&mut self, client_build: u32) {