Re: [PATCH] PNG image reader

[Marco Gerards]

Bean <address@hidden> writes:

Hi,

> remove longjmp, also add grub_png prefix to function names.

Thanks!

A changelog entry is missing, this is a single file.  See the review
below for some comments.

--
Marco



> /*
>  *  GRUB  --  GRand Unified Bootloader
>  *  Copyright (C) 2008  Free Software Foundation, Inc.
>  *
>  *  GRUB is free software: you can redistribute it and/or modify
>  *  it under the terms of the GNU General Public License as published by
>  *  the Free Software Foundation, either version 3 of the License, or
>  *  (at your option) any later version.
>  *
>  *  GRUB is distributed in the hope that it will be useful,
>  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>  *  GNU General Public License for more details.
>  *
>  *  You should have received a copy of the GNU General Public License
>  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>  */
>
> #include <grub/bitmap.h>
> #include <grub/types.h>
> #include <grub/normal.h>
> #include <grub/dl.h>
> #include <grub/mm.h>
> #include <grub/misc.h>
> #include <grub/arg.h>
> #include <grub/file.h>
>
> /* Uncomment following define to enable PNG debug.  */
> //#define PNG_DEBUG
>
> #define PNG_COLOR_MASK_PALETTE    1
> #define PNG_COLOR_MASK_COLOR      2
> #define PNG_COLOR_MASK_ALPHA      4
>
> #define PNG_COLOR_TYPE_GRAY 0
> #define PNG_COLOR_TYPE_PALETTE  (PNG_COLOR_MASK_COLOR | 
> PNG_COLOR_MASK_PALETTE)
> #define PNG_COLOR_TYPE_RGB        (PNG_COLOR_MASK_COLOR)
> #define PNG_COLOR_TYPE_RGB_ALPHA  (PNG_COLOR_MASK_COLOR | 
> PNG_COLOR_MASK_ALPHA)
> #define PNG_COLOR_TYPE_GRAY_ALPHA (PNG_COLOR_MASK_ALPHA)
> #define PNG_COLOR_TYPE_RGBA  PNG_COLOR_TYPE_RGB_ALPHA
> #define PNG_COLOR_TYPE_GA  PNG_COLOR_TYPE_GRAY_ALPHA
>
> #define PNG_COMPRESSION_TYPE_BASE 0   /* Deflate method 8, 32K window  */
>
> #define PNG_FILTER_TYPE_BASE      0   /* Single row per-byte filtering  */
>
> #define PNG_INTERLACE_NONE        0   /* Non-interlaced image  */
> #define PNG_INTERLACE_ADAM7       1   /* Adam7 interlacing  */
>
> #define PNG_FILTER_VALUE_NONE  0
> #define PNG_FILTER_VALUE_SUB   1
> #define PNG_FILTER_VALUE_UP    2
> #define PNG_FILTER_VALUE_AVG   3
> #define PNG_FILTER_VALUE_PAETH 4
> #define PNG_FILTER_VALUE_LAST  5
>
> #define Z_DEFLATED    8
>
> #define Z_FLAG_DICT   32
>
> #define INFLATE_STORED        0
> #define INFLATE_FIXED 1
> #define INFLATE_DYNAMIC       2
>
> #define WSIZE 0x8000
>
> #define CHUNK_IHDR    0x49484452
> #define CHUNK_IDAT    0x49444154
> #define CHUNK_IEND    0x49454e44
>
> struct huff_table
> {
>   int *values, *maxval, *offset;
>   int num_values, max_length;
> };
>
> struct grub_png_data
> {
>   grub_file_t file;
>   struct grub_video_bitmap **bitmap;
>
>   int bit_count, bit_save;
>
>   int image_width, image_height;
>
>   int bpp;
>
>   int inside_idat, idat_remain;
>
>   int code_values[286];
>   int code_maxval[16];
>   int code_offset[16];
>
>   int dist_values[30];
>   int dist_maxval[16];
>   int dist_offset[16];
>
>   struct huff_table code_table;
>   struct huff_table dist_table;
>
>   grub_uint8_t slide[WSIZE];
>   int wp;
>
>   grub_uint8_t *cur_rgb;
>
>   int cur_colume, cur_filter, first_line;
> };
>
> static grub_uint32_t
> grub_png_get_dword (struct grub_png_data *data)
> {
>   grub_uint32_t r;
>
>   r = 0;

Why this?

>   grub_file_read (data->file, (char *) &r, sizeof (grub_uint32_t));
>
>   return grub_be_to_cpu32 (r);
> }
>
> static grub_uint8_t
> grub_png_get_byte (struct grub_png_data *data)
> {
>   grub_uint8_t r;
>
>   if ((data->inside_idat) && (data->idat_remain == 0))
>     {
>       grub_uint32_t len, type;
>
>       do
>       {
>         grub_png_get_dword (data);    /* skip crc checksum  */

Please follow our style of commenting.  Personally I prefer putting a
comment on a separate line.

>         len = grub_png_get_dword (data);
>         type = grub_png_get_dword (data);
>         if (type != CHUNK_IDAT)
>           {
>             grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                         "png: unexpected end of data");
>             return 0;
>           }
>       }
>       while (len == 0);
>       data->idat_remain = len;
>     }
>
>   r = 0;

...

>   grub_file_read (data->file, (char *) &r, 1);
>
>   if (data->inside_idat)
>     data->idat_remain--;
>
>   return r;
> }
>
> static int
> grub_png_get_bits (struct grub_png_data *data, int num)
> {
>   int code, shift;
>
>   if (data->bit_count == 0)
>     {
>       data->bit_save = grub_png_get_byte (data);
>       data->bit_count = 8;
>     }
>
>   code = 0;
>   shift = 0;
>   while (grub_errno == 0)
>     {
>       int n;
>
>       n = data->bit_count;
>       if (n > num)
>       n = num;
>
>       code += (int) (data->bit_save & ((1 << n) - 1)) << shift;
>       num -= n;
>       if (!num)
>       {
>         data->bit_count -= n;
>         data->bit_save >>= n;
>         break;
>       }
>
>       shift += n;
>
>       data->bit_save = grub_png_get_byte (data);
>       data->bit_count = 8;
>     }
>
>   return code;
> }
>
> static grub_err_t
> grub_png_decode_image_header (struct grub_png_data *data)
> {
>   int color_type;
>
>   data->image_width = grub_png_get_dword (data);
>   data->image_height = grub_png_get_dword (data);
>
>   if ((!data->image_height) || (!data->image_width))
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: invalid image size");
>
>   if (grub_png_get_byte (data) != 8)
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: bit depth must be 8");
>
>   color_type = grub_png_get_byte (data);
>   if (color_type == PNG_COLOR_TYPE_RGB)
>     {
>       if (grub_video_bitmap_create (data->bitmap, data->image_width,
>                                   data->image_height,
>                                   GRUB_VIDEO_BLIT_FORMAT_R8G8B8))
>       return grub_errno;

I am not sure, is this indention correct?

>       data->bpp = 3;
>     }
>   else if (color_type == PNG_COLOR_TYPE_RGBA)
>     {
>       if (grub_video_bitmap_create (data->bitmap, data->image_width,
>                                   data->image_height,
>                                   GRUB_VIDEO_BLIT_FORMAT_R8G8B8A8))
>       return grub_errno;
>       data->bpp = 4;
>     }
>   else
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                      "png: color type not supported");

YUV isn't used here?

>   data->cur_rgb = (*data->bitmap)->data;
>   data->cur_colume = 0;
>   data->first_line = 1;
>
>   if (grub_png_get_byte (data) != PNG_COMPRESSION_TYPE_BASE)
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                      "png: compression method not supported");
>
>   if (grub_png_get_byte (data) != PNG_FILTER_TYPE_BASE)
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                      "png: filter method not supported");
>
>   if (grub_png_get_byte (data) != PNG_INTERLACE_NONE)
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                      "png: interlace method not supported");
>
>   grub_png_get_dword (data);  /* skip crc checksum  */

^^ comment

>   return grub_errno;
> }
>
> static grub_uint8_t bitorder[] = {    /* Order of the bit length code lengths 
>  */
>   16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15
> };

You can make this constant.  Please fix the comment.  Same for the
contants below.

> static int cplens[] = {               /* Copy lengths for literal codes 
> 257..285  */
>   3, 4, 5, 6, 7, 8, 9, 10, 11, 13, 15, 17, 19, 23, 27, 31,
>   35, 43, 51, 59, 67, 83, 99, 115, 131, 163, 195, 227, 258, 0, 0
> };
>
> static grub_uint8_t cplext[] = {      /* Extra bits for literal codes 
> 257..285  */
>   0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 2,
>   3, 3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5, 0, 99, 99
> };                            /* 99==invalid  */
>
> static int cpdist[] = {               /* Copy offsets for distance codes 
> 0..29  */
>   1, 2, 3, 4, 5, 7, 9, 13, 17, 25, 33, 49, 65, 97, 129, 193,
>   257, 385, 513, 769, 1025, 1537, 2049, 3073, 4097, 6145,
>   8193, 12289, 16385, 24577
> };
>
> static grub_uint8_t cpdext[] = {      /* Extra bits for distance codes  */
>   0, 0, 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6,
>   7, 7, 8, 8, 9, 9, 10, 10, 11, 11,
>   12, 12, 13, 13
> };
>
> static void
> grub_png_init_huff_table (struct huff_table *ht, int cur_maxlen,
>                         int *cur_values, int *cur_maxval, int *cur_offset)
> {
>   ht->values = cur_values;
>   ht->maxval = cur_maxval;
>   ht->offset = cur_offset;
>   ht->num_values = 0;
>   ht->max_length = cur_maxlen;
>   grub_memset (cur_maxval, 0, sizeof (int) * cur_maxlen);
> }
>
> static void
> grub_png_insert_huff_item (struct huff_table *ht, int code, int len)
> {
>   int i, n;
>
>   if (len == 0)
>     return;
>
>   if (len > ht->max_length)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: invalid code length");
>       return;
>     }
>
>   n = 0;
>   for (i = len; i < ht->max_length; i++)
>     n += ht->maxval[i];
>
>   for (i = 0; i < n; i++)
>     ht->values[ht->num_values - i] = ht->values[ht->num_values - i - 1];
>
>   ht->values[ht->num_values - n] = code;
>   ht->num_values++;
>   ht->maxval[len - 1]++;
> }
>
> static void
> grub_png_build_huff_table (struct huff_table *ht)
> {
>   int base, ofs, i;
>
>   base = 0;
>   ofs = 0;
>   for (i = 0; i < ht->max_length; i++)
>     {
>       base += ht->maxval[i];
>       ofs += ht->maxval[i];
>
>       ht->maxval[i] = base;
>       ht->offset[i] = ofs - base;
>
>       base <<= 1;
>     }
> }
>
> static int
> grub_png_get_huff_code (struct grub_png_data *data, struct huff_table *ht)
> {
>   int code, i;
>
>   code = 0;
>   for (i = 0; i < ht->max_length; i++)
>     {
>       code = (code << 1) + grub_png_get_bits (data, 1);
>       if (code < ht->maxval[i])
>       return ht->values[code + ht->offset[i]];
>     }
>   return 0;
> }

Can this code above be shared with jpeg somehow?

> static grub_err_t
> grub_png_init_dynamic_block (struct grub_png_data *data)
> {
>   int nl, nd, nb, i, prev;
>   struct huff_table cl;
>   int cl_values[sizeof (bitorder)];
>   int cl_maxval[8];
>   int cl_offset[8];
>   grub_uint8_t lens[20];
>
>   nl = 257 + grub_png_get_bits (data, 5);
>   nd = 1 + grub_png_get_bits (data, 5);
>   nb = 4 + grub_png_get_bits (data, 4);
>   if ((nl > 286) || (nd > 30) || (nb > 19))
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: too much data");

Magic values...

>   grub_png_init_huff_table (&cl, 8, cl_values, cl_maxval, cl_offset);
>
>   for (i = 0; i < nb; i++)
>     lens[bitorder[i]] = grub_png_get_bits (data, 3);
>
>   for (; i < 19; i++)
>     lens[bitorder[i]] = 0;
>
>   for (i = 0; i < 19; i++)
>     grub_png_insert_huff_item (&cl, i, lens[i]);
>
>   grub_png_build_huff_table (&cl);
>
>   grub_png_init_huff_table (&data->code_table, 16, data->code_values,
>                           data->code_maxval, data->code_offset);
>   grub_png_init_huff_table (&data->dist_table, 16, data->dist_values,
>                           data->dist_maxval, data->dist_offset);
>
>   prev = 0;
>   for (i = 0; i < nl + nd; i++)
>     {
>       int n, code;
>       struct huff_table *ht;
>
>       if (grub_errno)
>       return grub_errno;
>
>       if (i < nl)
>       {
>         ht = &data->code_table;
>         code = i;
>       }
>       else
>       {
>         ht = &data->dist_table;
>         code = i - nl;
>       }
>
>       n = grub_png_get_huff_code (data, &cl);
>       if (n < 16)
>       {
>         grub_png_insert_huff_item (ht, code, n);
>         prev = n;
>       }
>       else if (n == 16)
>       {
>         int c;
>
>         c = 3 + grub_png_get_bits (data, 2);
>         while (c > 0)
>           {
>             grub_png_insert_huff_item (ht, code++, prev);
>             i++;
>             c--;
>           }
>         i--;
>       }
>       else if (n == 17)
>       i += 3 + grub_png_get_bits (data, 3) - 1;
>       else
>       i += 11 + grub_png_get_bits (data, 7) - 1;
>     }
>
>   grub_png_build_huff_table (&data->code_table);
>   grub_png_build_huff_table (&data->dist_table);
>
>   return grub_errno;
> }
>
> static grub_err_t
> grub_png_output_byte (struct grub_png_data *data, grub_uint8_t n)
> {
>   int row_bytes;
>
>   if (data->cur_colume == 0)
>     {
>       if (n >= PNG_FILTER_VALUE_LAST)
>       return grub_error (GRUB_ERR_BAD_FILE_TYPE, "invalid filter value");
>
>       data->cur_filter = n;
>     }
>   else
>     *(data->cur_rgb++) = n;
>
>   data->cur_colume++;
>   row_bytes = data->image_width * data->bpp;
>   if (data->cur_colume == row_bytes + 1)
>     {
>       grub_uint8_t *blank_line = NULL;
>       grub_uint8_t *cur = data->cur_rgb - row_bytes;
>       grub_uint8_t *left = cur;
>       grub_uint8_t *up;
>
>       if (data->first_line)
>       {
>         blank_line = grub_malloc (row_bytes);
>         if (grub_errno)
>           return grub_errno;
>
>         grub_memset (blank_line, 0, row_bytes);
>         up = blank_line;
>       }
>       else
>       up = cur - row_bytes;
>
>       switch (data->cur_filter)
>       {
>       case PNG_FILTER_VALUE_SUB:
>         {
>           int i;
>
>           cur += data->bpp;
>           for (i = data->bpp; i < row_bytes; i++)
>             *(cur++) += *(left++);
>           break;
>         }
>       case PNG_FILTER_VALUE_UP:
>         {
>           int i;
>
>           for (i = 0; i < row_bytes; i++)
>             *(cur++) += *(up++);
>
>           break;
>         }
>       case PNG_FILTER_VALUE_AVG:
>         {
>           int i;
>
>           for (i = 0; i < data->bpp; i++)
>             *(cur++) += *(up++) >> 1;
>
>           for (; i < row_bytes; i++)
>             *(cur++) += ((int) *(up++) + (int) *(left++)) >> 1;
>
>           break;
>         }
>       case PNG_FILTER_VALUE_PAETH:

PAETH?

>         {
>           int i;
>           grub_uint8_t *upper_left = up;
>
>           for (i = 0; i < data->bpp; i++)
>             *(cur++) += *(up++);
>
>           for (; i < row_bytes; i++)
>             {
>               int a, b, c, pa, pb, pc, p;
>
>               a = *(left++);
>               b = *(up++);
>               c = *(upper_left++);
>
>               p = b - c;
>               pc = a - c;
>
>               pa = p < 0 ? -p : p;
>               pb = pc < 0 ? -pc : pc;
>               pc = (p + pc) < 0 ? -(p + pc) : p + pc;
>
>               p = (pa <= pb && pa <= pc) ? a : (pb <= pc) ? b : c;
>
>               *(cur++) += p;
>             }
>         }
>       }
>
>       if (blank_line)
>       grub_free (blank_line);
>
>       data->cur_colume = 0;
>       data->first_line = 0;
>     }
>
>   return grub_errno;
> }
>
> static grub_err_t
> grub_png_read_dynamic_block (struct grub_png_data *data)
> {
>   while (grub_errno == 0)
>     {
>       int n;
>
>       n = grub_png_get_huff_code (data, &data->code_table);
>       if (n < 256)
>       {
>         data->slide[data->wp] = n;
>         grub_png_output_byte (data, n);
>
>         data->wp++;
>         if (data->wp >= WSIZE)
>           data->wp = 0;
>       }
>       else if (n == 256)
>       break;
>       else
>       {
>         int len, dist, pos;
>
>         n -= 257;
>         len = cplens[n];
>         if (cplext[n])
>           len += grub_png_get_bits (data, cplext[n]);
>
>         n = grub_png_get_huff_code (data, &data->dist_table);
>         dist = cpdist[n];
>         if (cpdext[n])
>           dist += grub_png_get_bits (data, cpdext[n]);
>
>         pos = data->wp - dist;
>         if (pos < 0)
>           pos += WSIZE;
>
>         while (len > 0)
>           {
>             data->slide[data->wp] = data->slide[pos];
>             grub_png_output_byte (data, data->slide[data->wp]);
>
>             data->wp++;
>             if (data->wp >= WSIZE)
>               data->wp = 0;
>
>             pos++;
>             if (pos >= WSIZE)
>               pos = 0;
>
>             len--;
>           }
>       }
>     }
>
>   return grub_errno;
> }
>
> static grub_err_t
> grub_png_decode_image_data (struct grub_png_data *data)
> {
>   grub_uint8_t cmf, flg;
>   int final;
>
>   cmf = grub_png_get_byte (data);
>   flg = grub_png_get_byte (data);
>
>   if ((cmf & 0xF) != Z_DEFLATED)
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                      "png: only support deflate compression method");
>
>   if (flg & Z_FLAG_DICT)
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                      "png: dictionary not supported");
>
>   do
>     {
>       int block_type;
>
>       final = grub_png_get_bits (data, 1);
>       block_type = grub_png_get_bits (data, 2);
>
>       switch (block_type)
>       {
>       case INFLATE_STORED:
>         {
>           grub_uint16_t i, len;
>
>           data->bit_count = 0;
>           len = grub_png_get_byte (data);
>           len += ((grub_uint16_t) grub_png_get_byte (data)) << 8;
>
>           grub_png_get_byte (data);   /* skip NLEN field  */
>           grub_png_get_byte (data);
>
>           for (i = 0; i < len; i++)
>             grub_png_output_byte (data, grub_png_get_byte (data));
>
>           break;
>         }
>
>       case INFLATE_FIXED:
>         return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                            "png: block type fixed not supported");
>
>       case INFLATE_DYNAMIC:
>         grub_png_init_dynamic_block (data);
>         grub_png_read_dynamic_block (data);
>         break;
>
>       default:
>         return grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                            "png: unknown block type");
>       }
>     }
>   while ((!final) && (grub_errno == 0));
>
>   grub_png_get_dword (data);  /* skip adler checksum  */
>   grub_png_get_dword (data);  /* skip crc checksum  */

adler?

Please fix the comments.

>   return grub_errno;
> }
>
> static grub_uint8_t png_magic[8] =
>   { 0x89, 0x50, 0x4e, 0x47, 0xd, 0xa, 0x1a, 0x0a };

Constant?

> static grub_err_t
> grub_png_decode_png (struct grub_png_data *data)
> {
>   grub_uint8_t magic[8];
>
>   if (grub_file_read (data->file, (char *) &magic[0], 8) != 8)
>     return grub_errno;
>
>   if (grub_memcmp (magic, png_magic, sizeof (png_magic)))
>     return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: not a png file");
>
>   while (grub_errno == 0)
>     {
>       grub_uint32_t len, type;
>
>       len = grub_png_get_dword (data);
>       type = grub_png_get_dword (data);
>
>       switch (type)
>       {
>       case CHUNK_IHDR:
>         grub_png_decode_image_header (data);
>         break;
>
>       case CHUNK_IDAT:
>         data->inside_idat = 1;
>         data->idat_remain = len;
>         data->bit_count = 0;
>
>         grub_png_decode_image_data (data);
>
>         data->inside_idat = 0;
>         break;
>
>       case CHUNK_IEND:
>         return grub_errno;
>
>       default:
>         grub_file_seek (data->file, data->file->offset + len + 4);
>       }
>     }
>
>   return grub_errno;
> }
>
> static grub_err_t
> grub_video_reader_png (struct grub_video_bitmap **bitmap,
>                      const char *filename)
> {
>   grub_file_t file;
>   struct grub_png_data *data;
>
>   file = grub_file_open (filename);
>   if (!file)
>     return grub_errno;
>
>   data = grub_malloc (sizeof (*data));
>   if (data != NULL)
>     {
>       grub_memset (data, 0, sizeof (*data));
>       data->file = file;
>       data->bitmap = bitmap;
>
>       grub_png_decode_png (data);
>
>       grub_free (data);
>     }
>
>   if (grub_errno != GRUB_ERR_NONE)
>     {
>       grub_video_bitmap_destroy (*bitmap);
>       *bitmap = 0;
>     }
>
>   grub_file_close (file);
>   return grub_errno;
> }
>
> #if defined(PNG_DEBUG)
> static grub_err_t
> grub_cmd_pngtest (struct grub_arg_list *state __attribute__ ((unused)),
>                 int argc, char **args)
> {
>   struct grub_video_bitmap *bitmap = 0;
>
>   if (argc != 1)
>     return grub_error (GRUB_ERR_BAD_ARGUMENT, "file name required");
>
>   grub_video_reader_png (&bitmap, args[0]);
>   if (grub_errno != GRUB_ERR_NONE)
>     return grub_errno;
>
>   grub_video_bitmap_destroy (bitmap);
>
>   return GRUB_ERR_NONE;
> }
> #endif
>
> static struct grub_video_bitmap_reader png_reader = {
>   .extension = ".png",
>   .reader = grub_video_reader_png,
>   .next = 0
> };
>
> GRUB_MOD_INIT (video_reader_png)
> {
>   grub_video_bitmap_reader_register (&png_reader);
> #if defined(PNG_DEBUG)
>   grub_register_command ("pngtest", grub_cmd_pngtest,
>                        GRUB_COMMAND_FLAG_BOTH, "pngtest FILE",
>                        "Tests loading of PNG bitmap.", 0);
> #endif
> }
>
> GRUB_MOD_FINI (video_reader_png)
> {
> #if defined(PNG_DEBUG)
>   grub_unregister_command ("pngtest");
> #endif
>   grub_video_bitmap_reader_unregister (&png_reader);
> }