refactor!: add supported codecs in BitmapConfig

"session" has a fixed set of supported codecs with associated IDs.

"connector" must expose the set of codecs during capabilities exchange.
It currently uses hard-codes codec IDs in different places.

Move the BitmapCodecs set to ironrdp-pdu. Shared code will be used by
the server, so this is a suitable common place.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

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 {
@@ -617,3 +619,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-session/src/utils.rs

@@ -1,16 +0,0 @@
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-#[repr(u8)]
-pub enum CodecId {
-    None = 0x0,
-    RemoteFx = 0x3,
-}
-
-impl CodecId {
-    pub const fn from_u8(value: u8) -> Option<Self> {
-        match value {
-            0 => Some(Self::None),
-            3 => Some(Self::RemoteFx),
-            _ => None,
-        }
-    }
-}

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"))