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

generic progress callback implementation on a byte-level #2014

Closed

Conversation

johannesvollmer
Copy link
Contributor

this approach would be imprecise, but better than nothing

  • the decoders and encoders could internally use this to add support for progress callbacks (thus pub(crate))
  • they can replace it with real progress tracking implementation any time

@johannesvollmer
Copy link
Contributor Author

johannesvollmer commented Sep 22, 2023

attempting tiff decoder. currently impossible without unsafe code because:

  • we would need to wrap the real byte source upon creation of the tiff reader
  • the progress callback is only available later in load_with_progress, but we cannot mutate the reader at that point, it's immutable

@fintelia
Copy link
Contributor

I wonder if this is something that would make more sense living outside this crate. Given a R: Read + Seek, a user would be able to construct a BufReader<TrackProgressReader<R>> and then pass it when constructing the image decoder. And the stream length could be obtained exactly using Seek::stream_len (or a manual implementation until it stabilizes)

@johannesvollmer
Copy link
Contributor Author

johannesvollmer commented Sep 25, 2023

Using Seek to find the length fits well to the core idea of this PR.

However, this byte-level tracking only works if assumptions about the internal implementation of the decoder holds. So, you can't really rely on it to work in general. I'd rather author this on a per-decoder basis, because it won't work for some decoders.

My goal was to provide a helper that works with the current image api. I consider this experiment a success, the answer is: it's not possible right now.

We would have to change the way that the progress callback works.

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.

2 participants