Skip to content

Update tiff reading - writing methods for f32 and u8 datatype #345

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions crates/kornia-io/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ kornia-image = { workspace = true }
png = "0.17"
jpeg-encoder = "0.6"
jpeg-decoder = "0.3"
tiff = "0.8"

log = { workspace = true }
thiserror = { workspace = true }

Expand Down
10 changes: 10 additions & 0 deletions crates/kornia-io/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ pub enum IoError {
#[error("Error with Jpeg encoding")]
JpegEncodingError(#[from] jpeg_encoder::EncodingError),


/// Error decoding the TIFF image.
#[error(transparent)]
TiffError(#[from] tiff::TiffError),

/// Error to create the TIFF image.
#[error("Error with Tiff encoding")]
TiffEncodingError(String),

/// Error to create the image.
#[error("Failed to create image")]
ImageCreationError(#[from] kornia_image::ImageError),
Expand All @@ -37,4 +46,5 @@ pub enum IoError {
/// Error to decode the PNG image.
#[error("Failed to decode the image")]
PngDecodeError(String),

}
10 changes: 10 additions & 0 deletions crates/kornia-io/src/jpeg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ pub fn read_image_jpeg_mono8(file_path: impl AsRef<Path>) -> Result<Image<u8, 1>
read_image_jpeg_internal(file_path)
}



fn read_image_jpeg_internal<const N: usize>(
file_path: impl AsRef<Path>,
) -> Result<Image<u8, N>, IoError> {
Expand Down Expand Up @@ -119,6 +121,14 @@ mod tests {
Ok(())
}

#[test]
fn read_jpeg_f32() -> Result<(), IoError> {
let image = read_image_jpeg_mono8("../../tests/data/dog.jpeg")?;
assert_eq!(image.cols(), 258);
assert_eq!(image.rows(), 195);
Ok(())
}

Comment on lines +124 to +131
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[test]
fn read_jpeg_f32() -> Result<(), IoError> {
let image = read_image_jpeg_mono8("../../tests/data/dog.jpeg")?;
assert_eq!(image.cols(), 258);
assert_eq!(image.rows(), 195);
Ok(())
}

Firstly, the name of the test doesn't make sense as jpeg doesn't support f32 and you are using read_image_jpeg_mono8 in it.

And secondly, this doesn't work. It will give ImageCreationError(InvalidChannelShape(150930, 50310)) error.

Make sure to run the tests first.

#[test]
fn read_write_jpeg() -> Result<(), IoError> {
let tmp_dir = tempfile::tempdir()?;
Expand Down
2 changes: 2 additions & 0 deletions crates/kornia-io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ pub mod jpeg;
#[cfg(feature = "gstreamer")]
pub mod stream;

/// Tiff image encoding and decoding.
pub mod tiff;
pub use crate::error::IoError;
228 changes: 228 additions & 0 deletions crates/kornia-io/src/tiff.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
use crate::error::IoError;

use kornia_image::{Image, ImageSize};
use tiff::decoder::{Decoder,DecodingResult};
use tiff::encoder::{
colortype::{ColorType,RGB8,RGBA8,Gray8},
TiffEncoder,
TiffValue,
};
use std::fs::File;
use std::path::Path;
use std::io::BufWriter;
Copy link
Member

Choose a reason for hiding this comment

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

group properly the imports, probably cargo fmt will do this for you


/// Writes the given TIFF (RGB8) data to the specified file path.
///
/// # Arguments
///
/// * `file_path` - The path to the TIFF file to write.
/// * `image` - The image tensor containing TIFF image data (3 channels).
pub fn write_image_tiff_rgb8(
file_path: impl AsRef<Path>,
image: &Image<u8, 3>,
) -> Result<(), IoError> {
write_image_tiff_internal::<RGB8,3>(file_path, image)
}

/// Writes the given TIFF (Grayscale) data to the specified file path.
///
/// # Arguments
///
/// * `file_path` - The path to the TIFF file to write.
/// * `image` - The image tensor containing TIFF image data (1 channel).
pub fn write_image_tiff_gray8(
file_path: impl AsRef<Path>,
image: &Image<u8, 1>,
) -> Result<(), IoError> {
write_image_tiff_internal::<Gray8,1>(file_path, image)
}

/// Writes the given TIFF (RGBA8) data to the specified file path.
///
/// # Arguments
///
/// * `file_path` - The path to the TIFF file to write.
/// * * `image` - The image tensor containing TIFF image data (4 channels).
pub fn write_image_tiff_rgba8(
file_path: impl AsRef<Path>,
image: &Image<u8, 4>,
) -> Result<(), IoError> {
write_image_tiff_internal::<RGBA8,4>(file_path, image)
}


/// Writes the given TIFF (Grayscale) data to the specified file path.
///
/// # Arguments
///
/// * `file_path` - The path to the TIFF file to write.
/// * `image` - The image tensor containing TIFF image data (1 channel).
fn write_image_tiff_internal<T, const C:usize>(
file_path: impl AsRef<Path>,
image: &Image<T::Inner, C>,
) -> Result<(), IoError>
where
T: ColorType,
[T::Inner]:TiffValue,
{
let file = File::create(file_path.as_ref())?;
let writer = BufWriter::new(file);
let mut tiff = TiffEncoder::new(writer)?;
let image_encoder = tiff.new_image::<T>(image.width() as u32, image.height() as u32)?;
image_encoder.write_data(image.as_slice())?;
Ok(())
}

/// Reads a TIFF image with a single channel (rgb8).
/// #Arguments
/// file_path - The path to the TIFF file.
///
Comment on lines +76 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Reads a TIFF image with a single channel (rgb8).
/// #Arguments
/// file_path - The path to the TIFF file.
///
/// Reads a TIFF image with a three channel (rgb8).
///
/// # Arguments
///
/// - `file_path` - The path to the TIFF file.

Make it in markdown format, so it render properly on docs.rs

pub fn read_image_tiff_rgb8(
file_path: impl AsRef<Path>,
) -> Result<Image<u8, 3>, IoError> {
read_image_tiff_internal::<u8, 3>(file_path)
}

/// Reads a TIFF image with a single channel (gray8).
/// #Arguments
/// file_path - The path to the TIFF file.
Comment on lines +86 to +88
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Reads a TIFF image with a single channel (gray8).
/// #Arguments
/// file_path - The path to the TIFF file.
/// Reads a TIFF image with a single channel (gray8).
///
/// #Arguments
///
/// - `file_path` - The path to the TIFF file.

Same.

pub fn read_image_tiff_gray8(
file_path: impl AsRef<Path>,
) -> Result<Image<u8, 1>, IoError> {
read_image_tiff_internal::<u8,1>(file_path)
}

/// Reads a TIFF image with RGBA8 channels
/// #Arguments
/// file_path - The path to the TIFF file.
Comment on lines +95 to +97
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Reads a TIFF image with RGBA8 channels
/// #Arguments
/// file_path - The path to the TIFF file.
/// Reads a TIFF image with four channels (rgba8).
///
/// # Arguments
///
/// - `file_path` - The path to the TIFF file.

Same

pub fn read_image_tiff_rgba8(
file_path: impl AsRef<Path>,
) -> Result<Image<u8, 4>, IoError> {
read_image_tiff_internal::<u8,4>(file_path)
}

/// A trait to convert TIFF values to a specific type.
///
/// This trait is used to convert TIFF values (u8 or f32) to a specific type (e.g., u8 or f32).
/// It provides two methods:
pub trait TiffValueConverter: Sized {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should have a simple trait (could be internal?) with only one function like type_ and should have it's implementation for all numeric primitive types that could be bound to generic T (P in your case, change it to T so that it's globally same).
The type_ function could be used to verify if the decoded image is of same type, if not return error.

This could be used to to only allow integer primitive types. Also, we should not deal with any conversion and make it up to the user if they want to covert the type.

Then your functions should look something like:

pub trait NumericTrait {
    fn type_() -> NumericType;
}

pub fn read_image_tiff_rgba8<T: NumericTrait>(file_path: impl AsRef<Path>) -> Result<Image<T, 4>, IoError> {
    todo!()
}

pub fn read_image_tiff_internal<T: NumericTrait>(/* arguments */) -> /* ... */

What do you say @edgarriba ?

Copy link
Member

Choose a reason for hiding this comment

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

sounds good to me

Copy link
Author

Choose a reason for hiding this comment

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

Hello, I think we should provide more details when implementing images for our application. It could be a marker type instead of struct u8 in this context, for example:
let rgb_image: Image<u8, 4> = read_image_any::<Rgba8>("image.jpg")?;
From my vision, this could be more sustainable in case we would provide more features for the fourth channel, like depth or heat instead of alpha.

Copy link
Author

Choose a reason for hiding this comment

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

@edgarriba , @AS1100K
Would it be a good one in this case? If it's good to go, i'm gonna include these for the next pull request

Copy link
Member

Choose a reason for hiding this comment

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

for now we decided in a previous conversation in discord to hold on the read_any and leave generalizations for later

/// * `from_u8`: Converts a u8 value to the specific type.
fn from_u8(value: u8) -> Self;
/// * `from_f32`: Converts a f32 value to the specific type.
fn from_f32(value: f32) -> Self;
}

impl TiffValueConverter for u8 {
fn from_u8(value: u8) -> Self { value }
fn from_f32(value: f32) -> Self { (value * 255.0) as u8 }
}

impl TiffValueConverter for f32 {
fn from_u8(value: u8) -> Self { (value as f32) / 255.0 }
fn from_f32(value: f32) -> Self { value }
}



// Read a TIFF image with a single channel (rgb8).
//
fn read_image_tiff_internal<T:Clone,const N: usize>(
file_path: impl AsRef<Path>,
) -> Result<Image<T,N>, IoError>
where
T: TiffValueConverter,
{
let file_path = file_path.as_ref().to_owned();

if !file_path.exists() {
return Err(IoError::FileDoesNotExist(file_path.to_path_buf()));
}

if file_path.extension().map_or(true, |ext| {
!ext.eq_ignore_ascii_case("tif") && !ext.eq_ignore_ascii_case("tiff")
}) {
return Err(IoError::InvalidFileExtension(file_path.to_path_buf()));
}

let file = File::open(&file_path)?;
let mut decoder = Decoder::new(file).map_err(IoError::TiffError)?;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let mut decoder = Decoder::new(file).map_err(IoError::TiffError)?;
let mut decoder = Decoder::new(file)?;

forward error diectly like this

// read the image data
let decoding_result = decoder.read_image().map_err(IoError::TiffError)?;
let vec_data:Vec<T> = match decoding_result {
DecodingResult::U8(data) => data.iter().map(|&x| T::from_u8(x)).collect(),
DecodingResult::F32(data) => data.iter().map(|&x| T::from_f32(x)).collect(),
_ => return Err(IoError::TiffEncodingError("Unsupported data type".to_string())),
};
let (width, height) = decoder.dimensions()?;
// check the number of channels
let image = Image::new(
ImageSize {
width: width as usize,
height: height as usize,
},
vec_data,
)?;

Ok(image)
}



#[cfg(test)]
mod tests {
use super::*;
use std::fs;

use std::path::PathBuf;
use kornia_image::ImageSize;
#[test]
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, add a read_write test

Refer

fn read_write_jpeg() -> Result<(), IoError> {
let tmp_dir = tempfile::tempdir()?;
create_dir_all(tmp_dir.path())?;
let file_path = tmp_dir.path().join("dog.jpeg");
let image_data = read_image_jpeg_rgb8("../../tests/data/dog.jpeg")?;
write_image_jpeg_rgb8(&file_path, &image_data)?;
let image_data_back = read_image_jpeg_rgb8(&file_path)?;
assert!(file_path.exists(), "File does not exist: {:?}", file_path);
assert_eq!(image_data_back.cols(), 258);
assert_eq!(image_data_back.rows(), 195);
assert_eq!(image_data_back.num_channels(), 3);
Ok(())
}

fn test_write_image_tiff_rgb8() {
let image = Image::new(ImageSize { width: 100, height: 100 }, vec![0; 30000]).unwrap();
let file_path = PathBuf::from("test_rgb8.tiff");
assert!(write_image_tiff_rgb8(&file_path, &image).is_ok());
fs::remove_file(file_path).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use tempfile crate for this, as all the similar tests in this crate uses it

}

#[test]
fn test_write_image_tiff_gray8() {
use std::fs;

let image = Image::new(ImageSize { width: 100, height: 100 }, vec![0; 10000]).unwrap();
let tmp_dir = tempfile::tempdir().expect("Failed to create temp directory");
fs::create_dir_all(tmp_dir.path()).expect("Failed to create directory");

let file_path = tmp_dir.path().join("example.tiff");
assert!(write_image_tiff_gray8(&file_path, &image).is_ok(), "Failed to write TIFF file");
assert!(file_path.exists(), "TIFF file was not created");

let file_size = fs::metadata(&file_path).unwrap().len();
assert!(file_size > 0, "TIFF file is empty");
}

#[test]
fn test_read_image_tiff_rgba8() {
let file_path = PathBuf::from("../../tests/data/example.tiff");
let image = read_image_tiff_rgba8(&file_path).unwrap();
assert_eq!(image.width(), 1400);
assert_eq!(image.height(), 934);
}

#[test]
fn read_write_tiff() -> Result<(), IoError> {
let tmp_dir = tempfile::tempdir()?;
fs::create_dir_all(tmp_dir.path())?;

let file_path = tmp_dir.path().join("example.tiff");
let image_data = read_image_tiff_rgba8("../../tests/data/example.tiff")?;
write_image_tiff_rgba8(&file_path, &image_data)?;

let image_data_back = read_image_tiff_rgba8(&file_path)?;
assert!(file_path.exists(), "File does not exist: {:?}", file_path);

assert_eq!(image_data_back.cols(), 1400);
assert_eq!(image_data_back.rows(), 934);
assert_eq!(image_data_back.num_channels(), 4);

Ok(())
}
}
Binary file added tests/data/example.tiff
Binary file not shown.