Building a fast PNG encoder issues

Question

I am trying to build a fast 8-bit greyscale PNG encoder. Unfortunately I must be misunderstanding part of the spec. Smaller image sizes seem to work, but the larger ones will only open in some image viewers. This image (with multiple DEFLATE Blocks) gives a "Decompression error in IDAT" error in my image viewer but opens fine in my browser:

This image has just one DEFLATE block but also gives an error:

Below I will outline what I put in my IDAT chunk in case you can easily spot any mistakes (note, images and steps have been modified based on answers, but there is still a problem):

1. IDAT length

2. "IDAT" in ascii (literally the bytes 0x49 0x44 0x41 0x54)

3. Zlib header 0x78 0x01

Steps 4-7 are for every deflate block, as the data may need to be broken up:

4. The byte 0x00 or 0x01, depending on if it is a middle or the last block.

5. Number of bytes in block (up to 2^16-1) stored as a little endian 16-bit integer

6. The 1's complement of this integer representation.

7. Image data (each scan-line is starts with a zero-byte for the no filter option in PNG, and is followed by width bytes of greyscale pixel data)

8. An adler-32 checksum of all the image data

9. A CRC of all the IDAT data

I've tried pngcheck on linux, an it does not spot any errors. If nobody can see what is wrong, can you point me in the right direction for a debugging tool?

My last resort is to use the libpng library to make my own decoder, and debug from there.

Some people have suggested it may be my adler-32 function calculation:

static uint32_t adler32(uint32_t height, uint32_t width, char** pixel_array)
{
  uint32_t a=1,b=0,w,h;
  for(h=0;h<height;h++)
    {
      b+=a;
      for(w=0;w<width;w++)
        {
          a+=pixel_array[h][w];
          b+=a;
        }
    }
  return (uint32_t)(((b%65521)*65536)|(a%65521));
}

Note that because the pixel_array passed to the function does not contain the zero-byte at the beginning of each scanline (needed for PNG) there is an extra b+=a (and implicit a+=0) at the beginning of each iteration of the outer loop.

Answer 1

4. The byte 0x00 or 0x80, depending on if it is a middle or the last block.

Change the 0x80 to 0x01 and all will be well.

The 0x80 is appearing as a stored block that is not the last block. All that's being looked at is the low bit, which is zero, indicating a middle block. All of the data is in that "middle" block, so a decoder will recover the full image. Some liberal PNG decoders may then ignore the errors it gets when it tries to decode the next block, which isn't there, and then ignore the missing check values (Adler-32 and CRC-32), etc. That's why it shows up ok in browsers, even though it is an invalid PNG file.

There are two things wrong with your Adler-32 code. First, you are accessing the data from a char array. char is signed, so your 0xff bytes are being added not as 255, but rather as -127. You need to make the array unsigned char or cast it to that before extracting byte values from it.

Second, you are doing the modulo operation too late. You must do the % 65521 before the uint32_t overflows. Otherwise you don't get the modulo of the sum as required by the algorithm. A simple fix would be to do the % 65521 to a and b right after the width loop, inside the height loop. This will work so long as you can guarantee that the width will be less than 5551 bytes. (Why 5551 is left as an exercise for the reader.) If you cannot guarantee that, then you will need to embed a another loop to consume bytes from the line until you get to 5551 of them, do the modulo, and then continue with the line. Or, a smidge slower, just run a counter and do the modulo when it gets to the limit.

Here is an example of a version that works for any width:

static uint32_t adler32(uint32_t height, uint32_t width, unsigned char ** pixel_array)
{
    uint32_t a = 1, b = 0, w, h, k;
    for (h = 0; h < height; h++)
    {
        b += a;
        w = k = 0;
        while (k < width) {
            k += 5551;
            if (k > width)
                k = width;
            while (w < k) {
                a += pixel_array[h][w++];
                b += a;
            }
            a %= 65521;
            b %= 65521;
        }
    }
    return (b << 16) | a;
}