parse JFIF APP0 sections and expose Density with DensityUnits

image-rs / jpeg-decoder

JPEG decoder written in Rust -- currently in maintenance mode

Apache License 2.0

148 stars 89 forks source link

parse JFIF APP0 sections and expose Density with DensityUnits #115

Open cormacrelf opened 5 years ago

cormacrelf commented 5 years ago

See https://github.com/image-rs/image/issues/1067

Possible improvements

Rename to match png::PixelDimensions, which is not really an accurate name because it doesn't represent the size of a pixel, but the density of an image.
Remove the thumbnail stuff completely (I added it because I was there, and maybe someone wants it? I drew the line at allocating a buffer for the packed thumbnail data.)
Add tests? The current infrastructure doesn't appear capable of testing the whole range of possible inputs as PNG's density information has fewer units options. But I did verify with a dbg!() that the reftest images with JFIF APP0 sections could decode the density information.

cormacrelf commented 5 years ago

The one failure on stable-1.28.0 is because cfg-if has an edition key in its Cargo.toml, which appears like so:

jpeg-decoder v0.1.16 (/Users/cormac/git/jpeg-decoder)
├── byteorder v1.3.2
└── rayon v1.2.0
    ├── crossbeam-deque v0.7.1
    │   ├── crossbeam-epoch v0.7.2
    │   │   ├── arrayvec v0.4.12
    │   │   │   └── nodrop v0.1.14
    │   │   ├── cfg-if v0.1.10
    │   │   ├── crossbeam-utils v0.6.6
    │   │   │   ├── cfg-if v0.1.10 (*)

Seems pretty weird to me as I thought the edition flag was stable already. Maybe it's a bad message because using edition = "2018" wasn't stable until 1.32.

fintelia commented 4 years ago

This PR generally looks good to me, though I agree that it should probably either fully decode thumbnail images or just ignore them.