Skip to content

fix: Add handling for missing VideoFormat on Linux #169

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
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Add handling for missing VideoFormat on Linux #169

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
6 changes: 6 additions & 0 deletions src/capturer/engine/linux/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ fn process_callback(stream: &StreamRef, user_data: &mut ListenerUserData) {
height: frame_size.height as i32,
data: frame_data,
})),
VideoFormat::RGBA => user_data.tx.send(Frame::RGBx(RGBxFrame {
display_time: timestamp as u64,
width: frame_size.width as i32,
height: frame_size.height as i32,
data: frame_data,
})),
Comment on lines +161 to +166
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Consider adding a dedicated Frame::RGBA variant for semantic clarity.

The fix correctly addresses the missing handling for VideoFormat::RGBA and follows the established pattern. However, mapping RGBA frames to Frame::RGBx might be semantically confusing since RGBA includes meaningful alpha channel data while RGBx typically has undefined alpha values.

Consider these alternatives:

  1. Add a dedicated Frame::RGBA variant in the frame module
  2. If Frame::RGBx is intended to handle both RGBx and RGBA formats, consider renaming it to be more generic

Verify that downstream frame processing code can handle RGBA data correctly when it receives Frame::RGBx frames that actually contain alpha channel information.

🏁 Script executed:

#!/bin/bash
# Description: Check if Frame enum has RGBA variant and how RGBx frames are processed downstream

# Check Frame enum definition
ast-grep --pattern 'enum Frame {
  $$$
}'

# Check how Frame::RGBx is processed in the codebase
rg -A 10 "Frame::RGBx" --type rust

Length of output: 2362

#!/bin/bash
set -e

echo "### linux mod imports ###"
sed -n '1,50p' src/capturer/engine/linux/mod.rs

echo
echo "### VideoFrame enum ###"
rg -A 20 "enum VideoFrame" --type rust

echo
echo "### Frame alias ###"
rg "type Frame" -A 5 --type rust

echo
echo "### RGBxFrame struct ###"
rg -A 10 "struct RGBxFrame" --type rust

echo
echo "### RGBAFrame struct ###"
rg -A 10 "struct RGBAFrame" --type rust

🏁 Script executed:

#!/bin/bash
set -e

echo "### Imports in src/capturer/engine/linux/mod.rs ###"
sed -n '1,50p' src/capturer/engine/linux/mod.rs || echo "File not found"

echo
echo "### Searching for 'enum Frame' definitions ###"
rg -n "enum Frame" --type rust || echo "enum Frame not found"

echo
echo "### Searching for 'type Frame' alias ###"
rg -n "type Frame" --type rust || echo "type Frame alias not found"

echo
echo "### Searching for 'struct RGBxFrame' ###"
rg -n "struct RGBxFrame" --type rust || echo "RGBxFrame struct not found"

echo
echo "### Searching for 'struct RGBAFrame' ###"
rg -n "struct RGBAFrame" --type rust || echo "RGBAFrame struct not found"

Length of output: 2189

🏁 Script executed:

#!/bin/bash
set -e

echo "### Displaying Frame enum in src/frame/mod.rs ###"
sed -n '1,200p' src/frame/mod.rs

Length of output: 275

🏁 Script executed:

#!/bin/bash
set -e

echo "### Displaying src/frame/video.rs ###"
sed -n '1,200p' src/frame/video.rs || echo "src/frame/video.rs not found"

Length of output: 4357

Implement dedicated RGBA frame variant for clarity

The current change handles the missing VideoFormat::RGBA case, but mapping true RGBA frames into the RGBx variant conflates two distinct formats. RGBA carries a meaningful alpha channel, while RGBx implies a padded or ignored fourth byte. To avoid downstream confusion, we should:

  • Define a new RGBAFrame struct and corresponding VideoFrame::RGBA(RGBAFrame) variant in src/frame/video.rs.
  • Update the match arm in src/capturer/engine/linux/mod.rs to emit Frame::Video(VideoFrame::RGBA(...)) instead of Frame::RGBx.
  • If alpha really isn’t used downstream, explicitly strip it (e.g. via remove_alpha_channel) and map to RGB or RGBx, making the drop deliberate.

Suggested diffs:

// src/frame/video.rs
 use std::time::SystemTime;
 
+#[derive(Debug, Clone)]
+pub struct RGBAFrame {
+    pub display_time: SystemTime,
+    pub width: i32,
+    pub height: i32,
+    pub data: Vec<u8>, // Red, Green, Blue, Alpha
+}

 #[derive(Debug, Clone)]
 pub enum VideoFrame {
     YUVFrame(YUVFrame),
     RGB(RGBFrame),
     RGBx(RGBxFrame),
     XBGR(XBGRFrame),
     BGRx(BGRxFrame),
     BGR0(BGRFrame),
     BGRA(BGRAFrame),
+    RGBA(RGBAFrame),
 }
// src/capturer/engine/linux/mod.rs
 match video_info.format {
     VideoFormat::RGBA => user_data.tx.send(
-        Frame::RGBx(RGBxFrame { 
+        Frame::Video(VideoFrame::RGBA(RGBAFrame {
             display_time: timestamp as u64,
             width: frame_size.width as i32,
             height: frame_size.height as i32,
             data: frame_data,
-        }))
+        })),
     ),
     _ => panic!("Unsupported frame format received"),
 }

If alpha is guaranteed unused, explicitly drop it:

let rgb_data = remove_alpha_channel(frame_data);
user_data.tx.send(Frame::Video(VideoFrame::RGB(RGBFrame {
    display_time: timestamp as u64,
    width: frame_size.width as i32,
    height: frame_size.height as i32,
    data: rgb_data,
})))?;

This preserves clear semantics around alpha handling and keeps downstream frame consumers unambiguous.

🤖 Prompt for AI Agents 
In src/capturer/engine/linux/mod.rs around lines 161 to 166, the code
incorrectly maps VideoFormat::RGBA frames to the RGBx variant, which conflates
distinct formats and can cause confusion about alpha channel usage. To fix this,
define a new RGBAFrame struct and add a corresponding VideoFrame::RGBA variant
in src/frame/video.rs. Then update the match arm in
src/capturer/engine/linux/mod.rs to send Frame::Video(VideoFrame::RGBA(...))
with the RGBAFrame data. If the alpha channel is not used downstream, explicitly
remove it before mapping to RGB or RGBx to make the alpha drop deliberate and
clear.

_ => panic!("Unsupported frame format received"),
} {
eprintln!("{e}");
Expand Down