jpeg-decoder icon indicating copy to clipboard operation
jpeg-decoder copied to clipboard

parse JFIF APP0 sections and expose Density with DensityUnits

Open cormacrelf opened this issue 6 years ago • 4 comments

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 avatar Oct 21 '19 13:10 cormacrelf

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.

cormacrelf avatar Oct 21 '19 13:10 cormacrelf

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

fintelia avatar Nov 23 '19 19:11 fintelia