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 Encoder: write with progress #1695

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
29 changes: 23 additions & 6 deletions src/codecs/farbfeld.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,12 @@ impl<W: Write> FarbfeldEncoder<W> {

/// Encodes the image ```data``` (native endian)
/// that has dimensions ```width``` and ```height```
pub fn encode(self, data: &[u8], width: u32, height: u32) -> ImageResult<()> {
self.encode_impl(data, width, height)?;
pub fn encode<F: FnMut(Progress)>(self, data: &[u8], width: u32, height: u32, progress: F) -> ImageResult<()> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this an API breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure seems like it. We could rewrite this part such that only new methods are added and none of the existing are modified

self.encode_impl(data, width, height, progress)?;
Ok(())
}

fn encode_impl(mut self, data: &[u8], width: u32, height: u32) -> io::Result<()> {
fn encode_impl<F: FnMut(Progress)>(mut self, data: &[u8], width: u32, height: u32, mut progress: F) -> io::Result<()> {
self.w.write_all(b"farbfeld")?;

let mut buf = [0u8; 4];
Expand All @@ -274,22 +274,29 @@ impl<W: Write> FarbfeldEncoder<W> {
BigEndian::write_u32(&mut buf, height);
self.w.write_all(&buf)?;

for channel in data.chunks_exact(2) {
let u16_chunks = data.chunks_exact(2);
let chunk_count = u16_chunks.len() as u64;

for (chunk_index, channel) in u16_chunks.enumerate() {
progress(Progress::new(chunk_index as u64, chunk_count));

BigEndian::write_u16(&mut buf, NativeEndian::read_u16(channel));
self.w.write_all(&buf[..2])?;
}

progress(Progress::new(chunk_count, chunk_count));
Ok(())
}
}

impl<W: Write> ImageEncoder for FarbfeldEncoder<W> {
fn write_image(
fn write_image_with_progress<F: FnMut(Progress)>(
self,
buf: &[u8],
width: u32,
height: u32,
color_type: ColorType,
progress: F,
) -> ImageResult<()> {
if color_type != ColorType::Rgba16 {
return Err(ImageError::Unsupported(
Expand All @@ -300,7 +307,17 @@ impl<W: Write> ImageEncoder for FarbfeldEncoder<W> {
));
}

self.encode(buf, width, height)
self.encode(buf, width, height, progress)
}

fn write_image(
self,
buf: &[u8],
width: u32,
height: u32,
color_type: ColorType,
) -> ImageResult<()> {
self.write_image_with_progress(buf, width, height, color_type, |_|{})
}
}

Expand Down
44 changes: 35 additions & 9 deletions src/codecs/openexr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder<R> {
unaligned_bytes: &mut [u8],
progress_callback: F,
) -> ImageResult<()> {
let blocks_in_header = self.selected_exr_header().chunk_count as u64;
let blocks_in_header = self.selected_exr_header().chunk_count;
let channel_count = self.color_type().channel_count() as usize;

let display_window = self.selected_exr_header().shared_attributes.display_window;
Expand Down Expand Up @@ -217,10 +217,7 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder<R> {
.all_attributes()
.on_progress(|progress| {
progress_callback(
Progress::new(
(progress * blocks_in_header as f64) as u64,
blocks_in_header,
), // TODO precision errors?
progress_from_f32(blocks_in_header, progress),
);
})
.from_chunks(self.exr_reader)
Expand All @@ -237,18 +234,20 @@ impl<'a, R: 'a + Read + Seek> ImageDecoder<'a> for OpenExrDecoder<R> {
}
}


/// Write a raw byte buffer of pixels,
/// returning an Error if it has an invalid length.
///
/// Assumes the writer is buffered. In most cases,
/// you should wrap your writer in a `BufWriter` for best performance.
// private. access via `OpenExrEncoder`
fn write_buffer(
fn write_buffer<F: FnMut(Progress)>(
mut buffered_write: impl Write + Seek,
unaligned_bytes: &[u8],
width: u32,
height: u32,
color_type: ColorType,
mut progress: F,
) -> ImageResult<()> {
let width = width as usize;
let height = height as usize;
Expand Down Expand Up @@ -298,7 +297,7 @@ fn write_buffer(
}),
)
.write()
// .on_progress(|progress| todo!())
.on_progress(|progress_f32| progress(progress_from_f32(unaligned_bytes.len(), progress_f32)))
.to_buffered(&mut buffered_write)
.map_err(to_image_err)?;
}
Expand All @@ -318,7 +317,7 @@ fn write_buffer(
}),
)
.write()
// .on_progress(|progress| todo!())
.on_progress(|progress_f32| progress(progress_from_f32(unaligned_bytes.len(), progress_f32)))
.to_buffered(&mut buffered_write)
.map_err(to_image_err)?;
}
Expand Down Expand Up @@ -355,6 +354,22 @@ impl<W> ImageEncoder for OpenExrEncoder<W>
where
W: Write + Seek,
{
/// Writes the complete image.
///
/// Returns an Error if it has an invalid length.
/// Assumes the writer is buffered. In most cases,
/// you should wrap your writer in a `BufWriter` for best performance.
fn write_image_with_progress<F: FnMut(Progress)>(
self,
buf: &[u8],
width: u32,
height: u32,
color_type: ColorType,
progress: F,
) -> ImageResult<()> {
write_buffer(self.0, buf, width, height, color_type, progress)
}

/// Writes the complete image.
///
/// Returns an Error if it has an invalid length.
Expand All @@ -367,7 +382,7 @@ where
height: u32,
color_type: ColorType,
) -> ImageResult<()> {
write_buffer(self.0, buf, width, height, color_type)
write_buffer(self.0, buf, width, height, color_type, |_|{})
}
}

Expand Down Expand Up @@ -593,3 +608,14 @@ mod test {
assert!(original.pixels().zip(cropped.pixels()).all(|(a, b)| a == b));
}
}

fn progress_from_f32(granularity: usize, progress: f64) -> Progress {
// avoid float precision in this special case:
if progress == 1.0 { return Progress::new(granularity as u64, granularity as u64) }

Progress::new(
(progress * granularity as f64) as u64,
granularity as u64,
)
}

76 changes: 72 additions & 4 deletions src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,8 @@ pub trait ImageDecoder<'a>: Sized {
self.read_image_with_progress(buf, |_| {})
}

/// Same as `read_image` but periodically calls the provided callback to give updates on loading
/// progress.
/// Same as `read_image_with_progress_mut`, but the callback cannot mutate state.
/// Use `read_image_with_progress_mut` if you want to mutate your state from within the callback.
fn read_image_with_progress<F: Fn(Progress)>(
self,
buf: &mut [u8],
Expand Down Expand Up @@ -720,6 +720,22 @@ pub trait ImageDecoder<'a>: Sized {
Ok(())
}

/// Same as `read_image` but periodically calls the provided callback to give updates on loading
/// progress.
// TODO find a way to offer only _mut versions, eliminating refcells
fn read_image_with_progress_mut<F: FnMut(Progress)>(
self,
buf: &mut [u8],
progress_callback: F,
) -> ImageResult<()> {
let mutable_callback_cell = std::cell::RefCell::new(progress_callback);
self.read_image_with_progress(buf, |progress| {
if let Ok(mut progress_callback) = mutable_callback_cell.try_borrow_mut() {
progress_callback(progress)
}
})
}

/// Set decoding limits for this decoder. See [`Limits`] for the different kinds of
/// limits that is possible to set.
///
Expand Down Expand Up @@ -755,6 +771,17 @@ pub trait ImageDecoderRect<'a>: ImageDecoder<'a> + Sized {
self.read_rect_with_progress(x, y, width, height, buf, |_| {})
}

/// Same as `read_rect_with_progress_mut`, but the callback is not mutable.
fn read_rect_with_progress<F: Fn(Progress)>(
&mut self,
x: u32,
y: u32,
width: u32,
height: u32,
buf: &mut [u8],
progress_callback: F,
) -> ImageResult<()>;

/// Decode a rectangular section of the image, periodically reporting progress.
///
/// The output buffer will be filled with fields specified by
Expand All @@ -767,15 +794,23 @@ pub trait ImageDecoderRect<'a>: ImageDecoder<'a> + Sized {
///
/// This function will panic if the output buffer isn't at least
/// `color_type().bytes_per_pixel() * color_type().channel_count() * width * height` bytes long.
fn read_rect_with_progress<F: Fn(Progress)>(
// TODO find a way to offer only _mut versions, eliminating refcells
fn read_rect_with_progress_mut<F: FnMut(Progress)>(
&mut self,
x: u32,
y: u32,
width: u32,
height: u32,
buf: &mut [u8],
progress_callback: F,
) -> ImageResult<()>;
) -> ImageResult<()> {
let mutable_callback_cell = std::cell::RefCell::new(progress_callback);
self.read_rect_with_progress(x, y, width, height, buf,|progress| {
if let Ok(mut progress_callback) = mutable_callback_cell.try_borrow_mut() {
progress_callback(progress)
}
})
}
}

/// AnimationDecoder trait
Expand Down Expand Up @@ -803,6 +838,39 @@ pub trait ImageEncoder {
height: u32,
color_type: ColorType,
) -> ImageResult<()>;

/// Writes all the bytes in an image to the encoder.
///
/// This function takes a slice of bytes of the pixel data of the image
/// and encodes them. Unlike particular format encoders inherent impl encode
/// methods where endianness is not specified, here image data bytes should
/// always be in native endian. The implementor will reorder the endianess
/// as necessary for the target encoding format.
///
/// See also `ImageDecoder::read_image` which reads byte buffers into
/// native endian.
///
/// The progress callback is a function that is called occasionally,
/// and can be used to update, for example, a progress bar.
/// The granularity of those updates is implementation-defined
/// and might not be very accurate in some implementations.
fn write_image_with_progress<F: FnMut(Progress)>(
self,
buf: &[u8],
width: u32,
height: u32,
color_type: ColorType,
mut progress: F,
) -> ImageResult<()> where Self:Sized {
let max_progress = buf.len() as u64;
// TODO force implementations to implement this function instead of write_image()?
progress(Progress::new(0, max_progress));

self.write_image(buf, width, height, color_type);

progress(Progress::new(max_progress, max_progress));
Ok(())
}
}

/// Immutable pixel iterator
Expand Down