Skip to content
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

Support kCMPixelFormat_32BGRA and add github action for running tests #1

Open
wants to merge 18 commits into
base: 0.10
Choose a base branch
from

Conversation

darioalessandro
Copy link
Member

@darioalessandro darioalessandro commented Dec 19, 2024

Summary

This pull request introduces a new function buf_bgra_to_rgb that converts image data from BGRA (Blue, Green, Red, Alpha) format to RGB format. The
function now supports the specific pixel format kCMPixelFormat_32BGRA, enhancing its versatility for handling images in Apple's Core Media framework.

Key Changes

  • Support for kCMPixelFormat_32BGRA: Added functionality to process images with this specific pixel format, expanding the library's capability to
    handle diverse image types.
  • Input Validation: Enhanced checks to ensure width and height are even numbers, crucial for formats requiring even dimensions.
  • Buffer Size Verification: Ensured correct buffer lengths based on given resolution, critical for accurate data processing.
  • Efficient Pixel Conversion: Implemented conversion of BGRA pixel data to RGB format using chunks_exact for efficient handling.

New Features

  • Pixel Format Compatibility: The function now recognizes and processes the kCMPixelFormat_32BGRA pixel format, aligning with Core Media
    standards.
  • Dimension Check: Verifies that width and height are even numbers, ensuring compatibility with image formats that require even
    dimensions.
  • Efficient Buffer Handling: Processes data in chunks of four bytes, minimizing overhead for improved performance.

Bug Fixes

  • Resolved potential issues related to incorrect buffer sizes through validation checks.
  • Ensured proper handling of alpha values (ignored in conversion).

Known Issues

  • Function assumes correct output buffer sizing; mismatches may cause runtime errors.
  • Performance optimization opportunities exist for very large images, though current implementation is efficient.

Documentation Improvements

  • Added comments explaining each step for clarity and ease of understanding.
  • Updated API documentation to reflect support for kCMPixelFormat_32BGRA.

Testing

  • Comprehensive test cases include checks for invalid dimensions, incorrect buffer sizes, and compatibility with kCMPixelFormat_32BGRA.
  • Additional tests ensure functionality and robustness under various scenarios.

Integration

  • Function seamlessly integrates into the existing image processing pipeline, enhancing the library's capabilities without disrupting current workflows.

This implementation strengthens the image processing library by adding support for the kCMPixelFormat_32BGRA pixel format while maintaining efficiency and safety.

@darioalessandro darioalessandro changed the title [DO NOT REVIEW YET][WIP] Rename Yuyv to i420 [DO NOT REVIEW YET][WIP] Rename Yuyv to i420 and support kCMPixelFormat_32BGRA Dec 19, 2024
@@ -405,9 +496,10 @@ impl FormatDecoder for LumaAFormat {
/// let image: ImageBuffer<Rgb<u8>, Vec<u8>> = buffer.to_image::<YuyvFormat>();
/// ```
#[derive(Copy, Clone, Debug, Default, Hash, Ord, PartialOrd, Eq, PartialEq)]
pub struct YuyvFormat;
pub struct I420Format;
Copy link
Member Author

@darioalessandro darioalessandro Dec 19, 2024

Choose a reason for hiding this comment

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

This was never YUYV it was always meant to be i420, this is a breaking change but it is the right thing to do

@darioalessandro darioalessandro changed the title [DO NOT REVIEW YET][WIP] Rename Yuyv to i420 and support kCMPixelFormat_32BGRA [DO NOT REVIEW YET][WIP] support kCMPixelFormat_32BGRA Dec 26, 2024
@darioalessandro darioalessandro changed the title [DO NOT REVIEW YET][WIP] support kCMPixelFormat_32BGRA Support kCMPixelFormat_32BGRA and add github action for running tests Dec 26, 2024
@@ -29,4 +29,4 @@ jobs:
- name: Cargo Check
run: |
cd examples/threaded-capture
cargo check
Copy link
Member Author

Choose a reason for hiding this comment

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

revert

2 => Mat::new_rows_cols_with_data::<GA8>(resolution.width() as i32, resolution.height() as i32, cast_slice(data)),
3 => Mat::new_rows_cols_with_data::<RGB8>(resolution.width() as i32, resolution.height() as i32, cast_slice(data)),
4 => Mat::new_rows_cols_with_data::<RGBA8>(resolution.width() as i32, resolution.height() as i32, cast_slice(data)),
1 => Mat::new_rows_cols_with_data::<G8>(
Copy link
Member Author

Choose a reason for hiding this comment

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

this is just cargo fmt

@@ -77,6 +77,16 @@ impl FormatDecoder for RgbFormat {
.collect()),
FrameFormat::RAWRGB => Ok(data.to_vec()),
FrameFormat::NV12 => nv12_to_rgb(resolution, data, false),
FrameFormat::BGRA => {
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the meat of the change

@@ -0,0 +1,38 @@

# Assets
Copy link
Member Author

Choose a reason for hiding this comment

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

Documenting how I added the test images and the license for the pyramid picture

@@ -32,7 +32,7 @@ fn main() {

let format = RequestedFormat::new::<RgbFormat>(RequestedFormatType::AbsoluteHighestFrameRate);

let first_camera = cameras.first().unwrap();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not needed, remove

/// let image: ImageBuffer<Rgb<u8>, Vec<u8>> = buffer.to_image::<YuyvFormat>();
/// ```
#[derive(Copy, Clone, Debug, Default, Hash, Ord, PartialOrd, Eq, PartialEq)]
pub struct YuyvFormat;

impl FormatDecoder for YuyvFormat {
// YUV 4:2:0 planar colors. but we need to change the image crate to use this format
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be a TODO

}

/// Converts an image in YUYV format to I420 (YUV 4:2:0) format.
Copy link
Member Author

Choose a reason for hiding this comment

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

Ensure that this is a proper rust doc comment

@@ -1548,21 +1557,21 @@ pub fn buf_mjpeg_to_rgb(data: &[u8], dest: &mut [u8], rgba: bool) -> Result<(),
});
}

jpeg_decompress.read_scanlines_into::<u8>(dest).map_err(|why| {
NokhwaError::ProcessFrameError {
jpeg_decompress
Copy link
Member Author

Choose a reason for hiding this comment

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

cargo fmt

return Err(NokhwaError::ProcessFrameError {
src: FrameFormat::NV12,
destination: "RGB".to_string(),
error: "bad input buffer size".to_string(),
error: format!("bad input buffer size, expected {} but got {}", expected_len, data.len()),
Copy link
Member Author

Choose a reason for hiding this comment

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

Add better error

let b = chunk[0];
let g = chunk[1];
let r = chunk[2];
// let _a = chunk[3]; // Alpha is ignored for RGB
Copy link
Member Author

Choose a reason for hiding this comment

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

remove commented code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant