search

google / wuffs

Wrangling Untrusted File Formats Safely
Other
4.07k stars 129 forks source link

PNG decoder seems to get pixel format wrong #46

Closed Ono-Sendai closed 3 years ago

Ono-Sendai commented 3 years ago

Running wuffs on the PNG test suite: (PngSuite-2013jan13), hitting some odd reporting of pixel formats by Wuffs:

basn0g16.png (16 bit (64k level) grayscale):

bits_per_pixel is incorrectly reported as 8. (should be 16) likewise with channel_0_bits.

basn2c16.png (3x16 bits rgb color):

bpp incorrectly reported as 64, should be 48. channel_3_bits incorrectly reported as 16, should be 0.

Or am I misunderstanding wuffs, and it's doing some automatic conversion in some weird way?

thanks!

Ono-Sendai commented 3 years ago

There are more incorrect readings but i'll leave it at that for now.

nigeltao commented 3 years ago

basn0g16.png (16 bit (64k level) grayscale): bits_per_pixel is incorrectly reported as 8. (should be 16)

Yeah, that's a known bug / TODO.

basn2c16.png (3x16 bits rgb color): bpp incorrectly reported as 64, should be 48. channel_3_bits incorrectly reported as 16, should be 0.

Or am I misunderstanding wuffs, and it's doing some automatic conversion in some weird way?

It's sort of doing automatic conversion. Wuffs' pixel swizzler supports converting from pixel format S to pixel format D for a variety of S and D pixel formats, but it's not the full set of all possible pairs of pixel formats. The pixel format set by decode_image_config is conventionally one of the supported S pixel formats. Whilst the PNG file format has 3x16 RGB separate from 4x16 RGBA, the former seems rare enough in practice that it wasn't worth dedicated code paths converting from 3x16 RGB to all of the supported D pixel formats. Instead, for Wuffs' PNG decoder, 3x16 RGB is 'rounded up' to a superset S format: 4x16 RGBA (and the Alpha values are always 0xFFFF).

There are more incorrect readings but i'll leave it at that for now.

I'm curious what the others are, although I suspect they're expanded for the same reason. For example, YA (Gray + Alpha) is expanded to RGBA (with R = G = B = Y).

Ono-Sendai commented 3 years ago

Yeah, basn4a08.png, which is 8 bit grayscale + 8 bit alpha-channel, is reported as 32bpp with 4 channels.

nigeltao commented 3 years ago

Closing as Working As Intended.