Skip to content

feat(client): add --codecs #783

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.

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

Merged
merged 2 commits into from
May 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 22 additions & 10 deletions crates/ironrdp-client/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::print_stdout)]
Copy link
Member

@CBenoit CBenoit May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick:

Suggested change
#![allow(clippy::print_stdout)]
#![allow(clippy::print_stdout)]

use core::num::ParseIntError;
use core::str::FromStr;

Expand Down Expand Up @@ -233,6 +234,10 @@ struct Args {
/// The clipboard type
#[clap(long, value_enum, value_parser, default_value_t = ClipboardType::Default)]
clipboard_type: ClipboardType,

/// The bitmap codecs to use (remotefx:on, ...)
#[clap(long, value_parser, num_args = 1.., value_delimiter = ',')]
codecs: Vec<String>,
}

impl Config {
Expand Down Expand Up @@ -263,18 +268,25 @@ impl Config {
.context("Password prompt")?
};

let bitmap = if let Some(color_depth) = args.color_depth {
let codecs: Vec<_> = args.codecs.iter().map(|s| s.as_str()).collect();
let codecs = match client_codecs_capabilities(&codecs) {
Ok(codecs) => codecs,
Err(help) => {
print!("{}", help);
std::process::exit(0);
}
};
let mut bitmap = connector::BitmapConfig {
color_depth: 32,
lossy_compression: true,
codecs,
};

if let Some(color_depth) = args.color_depth {
if color_depth != 16 && color_depth != 32 {
anyhow::bail!("Invalid color depth. Only 16 and 32 bit color depths are supported.");
}

Some(connector::BitmapConfig {
color_depth,
lossy_compression: true,
codecs: client_codecs_capabilities(),
})
} else {
None
bitmap.color_depth = color_depth;
};

let clipboard_type = if args.clipboard_type == ClipboardType::Default {
Expand Down Expand Up @@ -306,7 +318,7 @@ impl Config {
height: DEFAULT_HEIGHT,
},
desktop_scale_factor: 0, // Default to 0 per FreeRDP
bitmap,
bitmap: Some(bitmap),
client_build: semver::Version::parse(env!("CARGO_PKG_VERSION"))
.map(|version| version.major * 100 + version.minor * 10 + version.patch)
.unwrap_or(0)
Expand Down
2 changes: 1 addition & 1 deletion crates/ironrdp-connector/src/connection_activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ fn create_client_confirm_active(
.bitmap
.as_ref()
.map(|b| b.codecs.clone())
.unwrap_or_else(client_codecs_capabilities),
.unwrap_or_else(|| client_codecs_capabilities(&[]).unwrap()),
),
CapabilitySet::FrameAcknowledge(FrameAcknowledge {
// FIXME(#447): Revert this to 2 per FreeRDP.
Expand Down
86 changes: 73 additions & 13 deletions crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,17 +641,77 @@ impl CodecId {
}
}

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)
/// This function generates a list of client codec capabilities based on the
/// provided configuration.
///
/// # Arguments
///
/// * `config` - A slice of string slices that specifies which codecs to include
/// in the capabilities. Codecs can be explicitly turned on ("codec:on") or
/// off ("codec:off").
///
/// # List of codecs
///
/// * `remotefx` (on by default)
///
/// # Returns
///
/// A vector of `Codec` structs representing the codec capabilities, or an error
/// suitable for CLI.
pub fn client_codecs_capabilities(config: &[&str]) -> Result<BitmapCodecs, String> {
use std::collections::HashMap;
Comment on lines +644 to +662
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I think this kind of CLI-related logic does not belong to ironrdp-pdu. But I have local changes to improve config handling and support the .RDP file format. Maybe we could revisit afterwards.


fn parse_codecs_config<'a>(codecs: &'a [&'a str]) -> Result<HashMap<&'a str, bool>, String> {
let mut result = HashMap::new();

for &codec_str in codecs {
if let Some(colon_index) = codec_str.find(':') {
let codec_name = &codec_str[0..colon_index];
let state_str = &codec_str[colon_index + 1..];

let state = match state_str {
"on" => true,
"off" => false,
_ => return Err(format!("Unhandled configuration: {}", state_str)),
};

result.insert(codec_name, state);
} else {
// No colon found, assume it's "on"
result.insert(codec_str, true);
}
}

Ok(result)
}

if config.contains(&"help") {
return Err(r#"
List of codecs:
- `remotefx` (on by default)
"#
.to_owned());
}
let mut config = parse_codecs_config(config)?;
let mut codecs = vec![];

if config.remove("remotefx").unwrap_or(true) {
codecs.push(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,
}])),
})),
});
}

let codec_names = config.keys().copied().collect::<Vec<_>>().join(", ");
if !codec_names.is_empty() {
return Err(format!("Unknown codecs: {}", codec_names));
}

Ok(BitmapCodecs(codecs))
}
23 changes: 23 additions & 0 deletions crates/ironrdp-testsuite-core/tests/session/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,24 @@
mod rfx;

#[cfg(test)]
mod tests {
use ironrdp_pdu::rdp::capability_sets::{client_codecs_capabilities, CodecProperty};

#[test]
fn test_codecs_capabilities() {
let config = &[];
let _capabilities = client_codecs_capabilities(config).unwrap();

let config = &["badcodec"];
assert!(client_codecs_capabilities(config).is_err());

let config = &["remotefx:on"];
let capabilities = client_codecs_capabilities(config).unwrap();
assert_eq!(capabilities.0.len(), 1);
assert!(matches!(capabilities.0[0].property, CodecProperty::RemoteFx(_)));

let config = &["remotefx:off"];
let capabilities = client_codecs_capabilities(config).unwrap();
assert_eq!(capabilities.0.len(), 0);
}
}
2 changes: 1 addition & 1 deletion crates/ironrdp-web/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ fn build_config(
bitmap: Some(connector::BitmapConfig {
color_depth: 16,
lossy_compression: true,
codecs: client_codecs_capabilities(),
codecs: client_codecs_capabilities(&[]).unwrap(),
}),
#[allow(clippy::arithmetic_side_effects)] // fine unless we end up with an insanely big version
client_build: semver::Version::parse(env!("CARGO_PKG_VERSION"))
Expand Down
Loading