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

Image FastFormat DataType #1

Merged
merged 12 commits into from
Aug 6, 2024
Merged

Image FastFormat DataType #1

merged 12 commits into from
Aug 6, 2024

Conversation

Hennzau
Copy link
Collaborator

@Hennzau Hennzau commented Aug 2, 2024

First FastFormat DataType

This PR adds the first FastFormat DataType, the Image DataType, and the project structure.

Representation

An Image is represented by a struct with two Enum Variant with the corresponding data and encoding

pub enum DataContainer {
    U8Data(Vec<u8>),
}

pub enum Encoding {
    RGB8,
    BGR8,
    GRAY8,
}

pub struct Image {
    pub data: DataContainer,

    pub width: u32,
    pub height: u32,

    pub encoding: Encoding,

    pub name: Option<String>,
}

Usage

You can then create an Image from flat data:

use fastformat::image::Image;

let pixels = vec![0; 27]; // 3x3 image with 3 bytes per pixel
let image = Image::new_bgr8(pixels, 3, 3, Some("example")).unwrap();

Features

  • You can convert RGB8 <-> BGR8 in place
use fastformat::image::Image;

let pixels = vec![0; 27]; // 3x3 image with 3 bytes per pixel
let bgr8_image = Image::new_bgr8(pixels, 3, 3, Some("example")).unwrap();
let rgb8_image = bgr8_image.to_rgb8().unwrap();
  • You can convert Image <-> NDArray with ZeroCopy
use crate::image::Image;

let pixels = vec![0; 27]; // 3x3 image with 3 bytes per pixel
let bgr8_image = Image::new_bgr8(pixels, 3, 3, Some("example")).unwrap();
let bgr8_ndarray = bgr8_image.bgr8_to_ndarray().unwrap();
let bgr8_image = Image::bgr8_from_ndarray(bgr8_ndarray, None);
  • You can convert Image <-> Arrow with ZeroCopy
/*
- **Image** as a **UnionArray**,
    - Field "pixels': UintXArray (e.g [0, 255, 0, 255, 0, 255, ...])
    - Field "width": Uint32Array (e.g [1280])
    - Field "height": Uint32Array (e.g [720])
    - Field "encoding": StringArray (e.g ["RGB8"])
    - Field "name" (Optional): StringArray (e.g,["image.front_camera"] or [None])
*/

use crate::image::Image;

let pixels = vec![0; 27]; // 3x3 image with 3 bytes per pixel
let bgr8_image = Image::new_bgr8(pixels, 3, 3, Some("example")).unwrap();
let arrow_image = bgr8_image.to_arrow().unwrap();
let bgr8_image = Image::from_arrow(arrow_image).unwrap();

RoadMap to finish this PR

  • separate the image.rs file into submodules for each datatype so everyone can simply add a module like rgba16.rs to add its owm Implementation for the Image Enum
image.rs/
├── bgr8.rs
├── rgb8.rs
├── gray8rs
└── rgba16.rs
  • add tests inside each image submodule directly

@Hennzau Hennzau requested a review from haixuanTao August 3, 2024 09:13
@Hennzau Hennzau marked this pull request as ready for review August 3, 2024 17:24
Copy link
Contributor

@haixuanTao haixuanTao left a comment

Choose a reason for hiding this comment

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

Looks really good overall!

I left some comments as well as I would recommend checking the clippy warnings that you can see here: https://github.com/dora-rs/fastformat/pull/1/files

I think you can merge it after the couple of comments.

Looks very very solid!

src/image.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/image/arrow.rs Outdated Show resolved Hide resolved
@Hennzau Hennzau merged commit 0827e04 into main Aug 6, 2024
24 checks passed
@Hennzau Hennzau deleted the image_datatype branch August 6, 2024 10:46
@phil-opp
Copy link

phil-opp commented Aug 7, 2024

Thanks for the PR, looks good to me too!

Two nits:

  • You can convert RGB8 <-> BGR8 with ZeroCopy

    I think the term "in place" fits better here than "zero copy", given that the data is processed and changed. "In place" makes it clear that no additional allocations are created. When I see "zero copy", I think that the original data is left untouched without any processing (just the interpretation changes) and that the operation has negligible cost (i.e. O(1)).

  • Rust's API guidelines recommend naming conversion methods that take self by value into_*. This way, it is clear that the original data is no longer available after using the method. Methods named to_* should only take &self (with the exception of Copy types).
    This PR introduces some methods named to_* that take self by value. For example, to_arrow or to_bgr8. It would be better to rename these to into_arrow/into_bgr8/etc.

@Hennzau
Copy link
Collaborator Author

Hennzau commented Aug 7, 2024

Thanks for the PR, looks good to me too!

Two nits:

  • You can convert RGB8 <-> BGR8 with ZeroCopy

    I think the term "in place" fits better here than "zero copy", given that the data is processed and changed. "In place" makes it clear that no additional allocations are created. When I see "zero copy", I think that the original data is left untouched without any processing (just the interpretation changes) and that the operation has negligible cost (i.e. O(1)).

  • Rust's API guidelines recommend naming conversion methods that take self by value into_*. This way, it is clear that the original data is no longer available after using the method. Methods named to_* should only take &self (with the exception of Copy types).

    This PR introduces some methods named to_* that take self by value. For example, to_arrow or to_bgr8. It would be better to rename these to into_arrow/into_bgr8/etc.

Hi! Thanks for your review!

I will make sure to change those to_ for into_ in the next PR! 😄

Hennzau added a commit that referenced this pull request Sep 9, 2024
# Objective

This PR adds a new datatype for **BBox** to ensure that everything work
well and it's easy to add a new datatype.

# Datatypes & Usage

- [x] Image

```Rust
use crate::image::Image;

let flat_image = (0..27).collect::<Vec<u8>>();
let image = Image::new_rgb8(flat_image, 3, 3, Some("camera.test")).unwrap();

let final_image = image.into_bgr8().unwrap();
let final_image_data = final_image.data.as_u8().unwrap();

let expected_image = vec![
    2, 1, 0, 5, 4, 3, 8, 7, 6, 11, 10, 9, 14, 13, 12, 17, 16, 15, 20, 19, 18, 23, 22, 21,
    26, 25, 24,
];

assert_eq!(&expected_image, final_image_data);

use crate::image::Image;

let flat_image = vec![0; 27];
let original_buffer_address = flat_image.as_ptr();

let bgr8_image = Image::new_bgr8(flat_image, 3, 3, None).unwrap();
let image_buffer_address = bgr8_image.as_ptr();

let arrow_image = bgr8_image.into_arrow().unwrap();

let new_image = Image::from_arrow(arrow_image).unwrap();
let final_image_buffer = new_image.as_ptr();

assert_eq!(original_buffer_address, image_buffer_address);
assert_eq!(image_buffer_address, final_image_buffer);
```

- [x] BBox

```Rust
use crate::bbox::BBox;

let flat_bbox = vec![1.0, 1.0, 2.0, 2.0];
let confidence = vec![0.98];
let label = vec!["cat".to_string()];

let bbox = BBox::new_xyxy(flat_bbox, confidence, label).unwrap();
let final_bbox = bbox.into_xywh().unwrap();
let final_bbox_data = final_bbox.data;

let expected_bbox = vec![1.0, 1.0, 1.0, 1.0];

assert_eq!(expected_bbox, final_bbox_data);

use crate::bbox::BBox;

let flat_bbox = vec![1.0, 1.0, 2.0, 2.0];
let original_buffer_address = flat_bbox.as_ptr();

let confidence = vec![0.98];
let label = vec!["cat".to_string()];

let xyxy_bbox = BBox::new_xyxy(flat_bbox, confidence, label).unwrap();
let bbox_buffer_address = xyxy_bbox.data.as_ptr();

let arrow_bbox = xyxy_bbox.into_arrow().unwrap();

let new_bbox = BBox::from_arrow(arrow_bbox).unwrap();
let final_bbox_buffer = new_bbox.data.as_ptr();

assert_eq!(original_buffer_address, bbox_buffer_address);
assert_eq!(bbox_buffer_address, final_bbox_buffer);
```

# Quick Fixes

- I also improved readability and consistency with Rust formatting,
following the guidelines mentioned in [this
comment](#1 (comment)).
- Fix Arrow Array extraction from Union inside #3 
- Fix Dora compatibility (by viewing objects when it's not possible to
own them) inside #5
- I improved the structure of the library, with separated packages and
with some `features` as one might want to use `fastformat` only with
ndarray/arrow. #6
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.

3 participants