Skip to content

Commit

Permalink
Add Reader.read_row for decoding into caller-provided buffer.
Browse files Browse the repository at this point in the history
`Reader.read_row` is an alternative to `Reader.next_row`.

* `Reader.next_row` is convenient when the caller doesn't want to manage
  their own buffer, and doesn't mind having extended borrows on `Reader`
  to accommodate `Row` data.
* OTOH the new `Reader.read_row` avoids an extra copy in some scanarios
  which may lead to better runtime performance - see the benchmark
  results below.

The commit results in the followin performance gains seen in the
recently introduced `row-by-row/128x128-4k-idat` benchmark:

time:   [-16.414% -16.196% -15.969%] (p = 0.00 < 0.05)
time:   [-15.570% -15.218% -14.856%] (p = 0.00 < 0.05)
time:   [-16.101% -15.864% -15.629%] (p = 0.00 < 0.05)
  • Loading branch information
anforowicz authored and kornelski committed Dec 22, 2024
1 parent 0da72dc commit 4cd4455
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 23 deletions.
11 changes: 5 additions & 6 deletions benches/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ fn load_all(c: &mut Criterion) {
bench_noncompressed_png(&mut g, 12288, 0x7fffffff); // 576 MB
g.finish();

// Incremental decoding via `next_row`
// Incremental decoding via `read_row`
let mut g = c.benchmark_group("row-by-row");
let mut data = Vec::new();
test_utils::write_noncompressed_png(&mut data, 128, 4096);
bench_next_row(&mut g, data, "128x128-4k-idat");
bench_read_row(&mut g, data, "128x128-4k-idat");
g.finish();
}

Expand Down Expand Up @@ -78,8 +78,8 @@ fn bench_file(g: &mut BenchmarkGroup<WallTime>, data: Vec<u8>, name: String) {
});
}

/// This benchmarks decoding via a sequence of `Reader::next_row` calls.
fn bench_next_row(g: &mut BenchmarkGroup<WallTime>, data: Vec<u8>, name: &str) {
/// This benchmarks decoding via a sequence of `Reader::read_row` calls.
fn bench_read_row(g: &mut BenchmarkGroup<WallTime>, data: Vec<u8>, name: &str) {
let reader = create_reader(data.as_slice());
let mut image = vec![0; reader.output_buffer_size()];
let bytes_per_row = reader.output_line_size(reader.info().width);
Expand All @@ -89,8 +89,7 @@ fn bench_next_row(g: &mut BenchmarkGroup<WallTime>, data: Vec<u8>, name: &str) {
let mut reader = create_reader(data.as_slice());

for output_row in image.chunks_exact_mut(bytes_per_row) {
let decoded_row = reader.next_row().unwrap().unwrap();
output_row.copy_from_slice(decoded_row.data());
reader.read_row(output_row).unwrap().unwrap();
}
})
});
Expand Down
62 changes: 45 additions & 17 deletions src/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,14 +474,45 @@ impl<R: Read> Reader<R> {
Ok(())
}

/// Returns the next processed row of the image
/// Returns the next processed row of the image (discarding `InterlaceInfo`).
///
/// See also [`Reader.read_row`], which reads into a caller-provided buffer.
pub fn next_row(&mut self) -> Result<Option<Row>, DecodingError> {
self.next_interlaced_row()
.map(|v| v.map(|v| Row { data: v.data }))
}

/// Returns the next processed row of the image
/// Returns the next processed row of the image.
///
/// See also [`Reader.read_row`], which reads into a caller-provided buffer.
pub fn next_interlaced_row(&mut self) -> Result<Option<InterlacedRow>, DecodingError> {
let mut output_buffer = mem::take(&mut self.scratch_buffer);
output_buffer.resize(self.output_line_size(self.info().width), 0u8);
let result = self.read_row(&mut output_buffer);
self.scratch_buffer = output_buffer;
result.map(move |option| {
option.map(move |interlace| {
let output_line_size = self.output_line_size_for_interlace_info(&interlace);
InterlacedRow {
data: &self.scratch_buffer[..output_line_size],
interlace,
}
})
})
}

/// Reads the next row of the image into the provided `output_buffer`.
/// `Ok(None)` will be returned if the current image frame has no more rows.
///
/// `output_buffer` needs to be long enough to accommodate [`Reader.output_line_size`] for
/// [`Info.width`] (initial interlaced rows may need less than that).
///
/// See also [`Reader.next_row`] and [`Reader.next_interlaced_row`], which read into a
/// `Reader`-owned buffer.
pub fn read_row(
&mut self,
output_buffer: &mut [u8],
) -> Result<Option<InterlaceInfo>, DecodingError> {
let interlace = match self.subframe.current_interlace_info.as_ref() {
None => {
self.finish_decoding()?;
Expand All @@ -498,24 +529,21 @@ impl<R: Read> Reader<R> {
self.info().raw_row_length_from_width(width)
}
};

let output_line_size = self.output_line_size_for_interlace_info(&interlace);
let output_buffer = &mut output_buffer[..output_line_size];

self.next_interlaced_row_impl(rowlen, output_buffer)?;

Ok(Some(interlace))
}

fn output_line_size_for_interlace_info(&self, interlace: &InterlaceInfo) -> usize {
let width = match interlace {
InterlaceInfo::Adam7(Adam7Info { width, .. }) => width,
InterlaceInfo::Adam7(Adam7Info { width, .. }) => *width,
InterlaceInfo::Null(_) => self.subframe.width,
};
let output_line_size = self.output_line_size(width);

// TODO: change the interface of `next_interlaced_row` to take an output buffer instead of
// making us return a reference to a buffer that we own.
let mut output_buffer = mem::take(&mut self.scratch_buffer);
output_buffer.resize(output_line_size, 0u8);
let ret = self.next_interlaced_row_impl(rowlen, &mut output_buffer);
self.scratch_buffer = output_buffer;
ret?;

Ok(Some(InterlacedRow {
data: &self.scratch_buffer[..output_line_size],
interlace,
}))
self.output_line_size(width)
}

/// Read the rest of the image and chunks and finish up, including text chunks or others
Expand Down

0 comments on commit 4cd4455

Please sign in to comment.