Hedroed / png-parser

Analyse PNG file format for CTF, python API and CLI
MIT License
98 stars 11 forks source link

Fail with pal2/pal4/rgb48 images #5

Open AlexGuo1998 opened 2 years ago

AlexGuo1998 commented 2 years ago

I tried png-parser with pal2/pal4/rgb48 images:

pngparser -v -s test.png

... and they all fail. There are mainly 3 problems.


For all images, scanline_width is incorrect, so each scanline read is either too short or too long.

https://github.com/Hedroed/png-parser/blob/4d75f17f158acd6e1f86117bde2a0c5bfceede76/pngparser/imagedata.py#L232-L234

Guess it should be like:

# line_width_bit = pixel_count_per_line * components_per_pixel * bit_per_component
line_width_bit = self.header.width * self.header.pixel_len * self.header.bit_depth
# line_width = round_up(line_width_bit / 8) + the_filter_byte
line_width = (line_width_bit + 7) // 8 + 1

i.e. include bit_per_component(actually self.header.bit_depth) in calculation.

(I suggest renaming pixel_len to component_count, but it's also a little weird: paletted images can have more than 1 component, so maybe a better name?)


For pal2/pal4 images, it failed to convert index to pixel.

https://github.com/Hedroed/png-parser/blob/4d75f17f158acd6e1f86117bde2a0c5bfceede76/pngparser/imagedata.py#L332-L333

The correct way is:

if self.palette:
    bit_depth = self.header.bit_depth  # bit count per sample
    pixels = [self.palette[i] for i in BitArray(row, bit_depth)]

For rgb48 images, it failed to scale pixel value to 0~255 range when displaying.

https://github.com/Hedroed/png-parser/blob/4d75f17f158acd6e1f86117bde2a0c5bfceede76/pngparser/imagedata.py#L337-L341

With rgb48, bit_depth is 16, i.e. val in range(0, 65536). So val should be divided by 256, or be shifted 8 bits to the right:

if bit_depth > 8:
    # shrink value between 0 and 255
    val = val >> (bit_depth - 8)

I can craft a PR for these issues, but I'm not sure about the wording problem. Anyone has an idea?

Hedroed commented 2 years ago

Hi, thanks for all this fixes, I have not actually tested it on these types of images. I'm OK with you about the pixel_len name. I suggest using the name used in the PNG specification (I don't know it). I will be happy to accept your PR.