Skip to content

Commit 1534d1b

Browse files
authored
fix(egfx)!: make DecodedFrame fields private with getters to enforce size invariant (#1331)
1 parent 479a13a commit 1534d1b

3 files changed

Lines changed: 56 additions & 22 deletions

File tree

crates/ironrdp-egfx/src/client.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -751,18 +751,18 @@ impl GraphicsPipelineClient {
751751
// Decoded frame must be at least as large as the destination rectangle.
752752
// Larger is expected (macroblock alignment) and handled by cropping.
753753
// Smaller means the server sent mismatched dimensions.
754-
if frame.width < u32::from(dest_width) || frame.height < u32::from(dest_height) {
754+
if frame.width() < u32::from(dest_width) || frame.height() < u32::from(dest_height) {
755755
warn!(
756-
frame_width = frame.width,
757-
frame_height = frame.height,
756+
frame_width = frame.width(),
757+
frame_height = frame.height(),
758758
dest_width,
759759
dest_height,
760760
"decoded frame smaller than destination rectangle"
761761
);
762762
return Err(pdu_other_err!("decoded frame smaller than destination rectangle"));
763763
}
764764

765-
let cropped_data = crop_decoded_frame(&frame.data, frame.width, frame.height, dest_width, dest_height);
765+
let cropped_data = crop_decoded_frame(frame.data(), frame.width(), frame.height(), dest_width, dest_height);
766766

767767
let update = BitmapUpdate {
768768
surface_id,

crates/ironrdp-egfx/src/decode.rs

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,9 @@ use core::fmt;
2929
#[derive(Clone)]
3030
#[non_exhaustive]
3131
pub struct DecodedFrame {
32-
/// RGBA pixel data (4 bytes per pixel)
33-
pub data: Vec<u8>,
34-
/// Frame width in pixels
35-
pub width: u32,
36-
/// Frame height in pixels
37-
pub height: u32,
32+
data: Vec<u8>,
33+
width: u32,
34+
height: u32,
3835
}
3936

4037
impl DecodedFrame {
@@ -50,6 +47,26 @@ impl DecodedFrame {
5047
);
5148
Self { data, width, height }
5249
}
50+
51+
/// RGBA pixel data (4 bytes per pixel).
52+
pub fn data(&self) -> &[u8] {
53+
&self.data
54+
}
55+
56+
/// Frame width in pixels.
57+
pub fn width(&self) -> u32 {
58+
self.width
59+
}
60+
61+
/// Frame height in pixels.
62+
pub fn height(&self) -> u32 {
63+
self.height
64+
}
65+
66+
/// Consume the frame and return the owned RGBA buffer.
67+
pub fn into_data(self) -> Vec<u8> {
68+
self.data
69+
}
5370
}
5471

5572
impl fmt::Debug for DecodedFrame {
@@ -286,11 +303,7 @@ mod openh264_impl {
286303
let mut rgba = vec![0u8; rgba_size];
287304
yuv.write_rgba8(&mut rgba);
288305

289-
Ok(DecodedFrame {
290-
data: rgba,
291-
width: w32,
292-
height: h32,
293-
})
306+
Ok(DecodedFrame::new(rgba, w32, h32))
294307
}
295308

296309
fn reset(&mut self) {
@@ -309,3 +322,24 @@ mod openh264_impl {
309322

310323
#[cfg(feature = "openh264")]
311324
pub use openh264_impl::OpenH264Decoder;
325+
326+
#[cfg(test)]
327+
mod tests {
328+
use super::DecodedFrame;
329+
330+
#[test]
331+
fn getters_return_constructor_inputs() {
332+
let data = vec![0u8; 2 * 3 * 4];
333+
let frame = DecodedFrame::new(data.clone(), 2, 3);
334+
assert_eq!(frame.data(), data.as_slice());
335+
assert_eq!(frame.width(), 2);
336+
assert_eq!(frame.height(), 3);
337+
}
338+
339+
#[test]
340+
fn into_data_yields_owned_buffer() {
341+
let data = vec![0xAAu8; 4 * 4 * 4];
342+
let frame = DecodedFrame::new(data.clone(), 4, 4);
343+
assert_eq!(frame.into_data(), data);
344+
}
345+
}

crates/ironrdp-testsuite-core/tests/egfx/decode.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ fn test_openh264_decode_sps_pps() {
8686

8787
let mut decoder = OpenH264Decoder::new().expect("decoder should initialize");
8888
let frame = decoder.decode(&avc_data).expect("decode should succeed");
89-
assert!(frame.width >= 16, "decoded width should be at least 16");
90-
assert!(frame.height >= 16, "decoded height should be at least 16");
89+
assert!(frame.width() >= 16, "decoded width should be at least 16");
90+
assert!(frame.height() >= 16, "decoded height should be at least 16");
9191
}
9292

9393
#[test]
@@ -98,9 +98,9 @@ fn test_openh264_decode_iframe() {
9898
let frame = decoder.decode(&avc_data).expect("decode should succeed");
9999

100100
// Verify RGBA output dimensions and data
101-
assert_eq!(frame.width, 16);
102-
assert_eq!(frame.height, 16);
103-
assert_eq!(frame.data.len(), 16 * 16 * 4, "RGBA data should be 16x16x4 bytes");
101+
assert_eq!(frame.width(), 16);
102+
assert_eq!(frame.height(), 16);
103+
assert_eq!(frame.data().len(), 16 * 16 * 4, "RGBA data should be 16x16x4 bytes");
104104
}
105105

106106
#[test]
@@ -116,8 +116,8 @@ fn test_openh264_decoder_reset() {
116116

117117
// Decoder should still be usable after reset
118118
let frame = decoder.decode(&avc_data).expect("decode after reset should succeed");
119-
assert_eq!(frame.width, 16);
120-
assert_eq!(frame.height, 16);
119+
assert_eq!(frame.width(), 16);
120+
assert_eq!(frame.height(), 16);
121121
}
122122

123123
// ============================================================================

0 commit comments

Comments
 (0)