Skip to content

Where to go from Bytes to DecodingResult? #87

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
feefladder opened this issue Apr 3, 2025 · 2 comments
Open

Where to go from Bytes to DecodingResult? #87

feefladder opened this issue Apr 3, 2025 · 2 comments

Comments

@feefladder
Copy link
Contributor

feefladder commented Apr 3, 2025

In decoding tiles, there is a question of where to go from network-provided Bytes to an output typed array (DecodingResult).

This emerged in #86 and is important, but may need some more discussion/other changes. See there for more info.

@feefladder
Copy link
Contributor Author

feefladder commented Apr 3, 2025

  1. From added predictors #86, better code snippet:
impl Tile {
pub fn decode(&self, registry: &DecoderRegistry) -> AsyncTiffResult<DecodingResult> {
  let mut res = DecodingResult::from_format_and_bit_depth(self.SampleFormat,  self.bits_per_sample(), self.n_output_samples(),)?;
  self.decode_into(registry, res.buf_mut())?
  Ok(res)
}

/// decode into the **properly sized** buffer
pub fn decode_into(&self, registry: &DecoderRegistry, buf: &mut[u8]) -> AsyncTiffResult {
  let decoder = registry[&self.compression_method];
  match self.predictor {
    Predictor::None => {
      decoder.decode_tile(self.compressed_bytes, buf,  ..)?;
      fix_endianness(buf, ...);
    },
    Predictor::Horizontal => {
      decoder.decode_tile(self.compressed_bytes, buf, ..)?;
      unpredict_hdiff(buf, &self.predictor_info, self.x as _);
    }
    Predictor::Float => {
      let mut temp_buf = vec![0u8; buf.len()]
      decoder.decode_tile(self.compressed_bytes, &mut temp_buf, ..)?;
      unpredict_float(temp_buf, buf, &self.predictor_info, self.x as _, self.y as _)
    }
  }
}
  1. For True Zero-Copy™, I did a bufs: &mut[&mut[u8]] in tiff2, which was possible because different decoders implemented std::io::Read and then a
for buf in bufs {
  decoder.read_exact(buf)
}

was possible. The benefit is that a user can directly mosaic an entire tile-based image. There's just some bookkeeping involved with splitting the result buffer for the entire image. For a single tile, it is as much as:

let bufs: Vec<_> = res.chunks_exact_mut(self.predictor_info.output_row_stride()).collect()
self.decode_into(self, registry, bufs)

I think 2 is a lot of effort for marginal speed/memory savings, but haven't benched

and then needing to change the decoding trait.

I mainly wanted to get these thoughts out on a nice place, can still put it into #86

@feefladder
Copy link
Contributor Author

feefladder commented Apr 3, 2025

So the three questions in here are:

  1. Is above the right place to go to a typed array?
  2. do we want a &mut[u8], &mut[&mut[u8]]1 or something different?
  3. what should the Decoder trait look like?
  4. What should the DecodingResult struct look like

Some options for 2 and 3 (assuming 1 was a yes):

  1. &mut[u8] with Decoder trait becoming fn decode_tile(.. out_buf: &mut [u8]) -> void
    • easiest less-copy implementation
  2. &mut[u8] with Decoder being anything implementing std::io::Read/BufRead
    • this is what image-tiff uses: Free implementations :)
    • and allows for greater (future) flexibility, such as
  3. &mut[&mut[u8]] -> maybe in the future?

...maybe options 2 and 3 sidetrack this conversation, we just choose option 1 and go on discussing questions 1 and 4?

For Question 4, I thought:

  • giant (non-exhaustive?) enum covering possible output types (ui8-64, f?16?-64)
  • is DecodingResult a good name? geotiff calls it RasterData
  • nice from_format_and_bit_depth(format: SampleFormat, bit_depth: ?u16?, n_samples: usize) creation function, could also just be named new(..)
  • buf_mut or similar function returning a &mut[u8] to underlying data
  • no storage of photometric interpretation and planar config, that logic is for the user :)

Footnotes

  1. This would also allow Planar people to directly split the planes...

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

No branches or pull requests

1 participant