Update limits to be byte based #115
Conversation
src/decoder/mod.rs
Outdated
| self.processed = vec![0; self.line_size(width)] | ||
| let bytes = self.limits.bytes; | ||
| if bytes < self.line_size(width) { | ||
| // FIXME: DecodingError::Other is used for backwards compatibility. |
There was a problem hiding this comment.
This reasoning doesn't make much sense, as this is a backwards incompatible commit–a public member of Limits was renamed from pixels to bytes.
There was a problem hiding this comment.
I simply copied code from the previous limit stuff, but yeah I realize that this doesn't really make sense. I will add a new error variant.
src/decoder/mod.rs
Outdated
| pub struct Limits { | ||
| /// max number of pixels: `width * height` (default: 67M = 2<sup>26</sup>) | ||
| pub pixels: u64, | ||
| /// maximum number of bytes the decoder is allowed to allocate, default is 64M |
There was a problem hiding this comment.
It might be helpful to clarify that this is Mib, not Mb. See https://en.wikipedia.org/wiki/Mebibyte
There was a problem hiding this comment.
Thanks, I always wondered which was which
There was a problem hiding this comment.
There's also 'the decoder will allocate less than 10K bytes' somewhere below, as 10*1024.
|
Overall I think this is definitely a step in the right direction. CI failure on nightly seems to be a rustc bug, could anyone familiar with the CI setup comment on that? |
Limiting allocated bytes is more effective and useful than image size. Especially when incrementaly decoding an image which may not fit in memory, limiting pixels dosen't make much sense.
|
Since nobody else has commented this I will merge it. |
Limiting allocated bytes is more effective and useful than image
size. Especially when incrementally decoding an image which may not
fit in memory, limiting pixels doesn't make much sense. Also see the discussion in image-rs/image-tiff#29
This will change the public api in that the
Limitsstruct now has a different field.This fixes #105
@HeroicKatora @fintelia @Shnatsel what do you guys think?