Re: [PATCH] jpeg image reader

[Marco Gerards]

Bean <address@hidden> writes:

Hi,
> change the handling of width/height that's not aligned, now it use the
> exact size instead of padding.

You forgot the changelog entry.

I'll do a quick review.

Especially the longjmp stuff should be changed, IMO.  Otherwise the
code is of good quality and I hope it can be committed soon.  Please
see the comments below.

--
Marco


> /*
>  *  GRUB  --  GRand Unified Bootloader
>  *  Copyright (C) 2008  Free Software Foundation, Inc.

Did you write all the code yourself?

>  *  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>
> #include <grub/setjmp.h>

setjmp!?

> /* Uncomment following define to enable JPEG debug.  */
> //#define JPEG_DEBUG
>
> typedef enum
> {                             /* JPEG marker codes  */
>   M_SOF0 = 0xc0,
>   M_SOF1 = 0xc1,
>   M_SOF2 = 0xc2,
>   M_SOF3 = 0xc3,
>
>   M_SOF5 = 0xc5,
>   M_SOF6 = 0xc6,
>   M_SOF7 = 0xc7,
>
>   M_JPG = 0xc8,
>   M_SOF9 = 0xc9,
>   M_SOF10 = 0xca,
>   M_SOF11 = 0xcb,
>
>   M_SOF13 = 0xcd,
>   M_SOF14 = 0xce,
>   M_SOF15 = 0xcf,
>
>   M_DHT = 0xc4,
>
>   M_DAC = 0xcc,
>
>   M_RST0 = 0xd0,
>   M_RST1 = 0xd1,
>   M_RST2 = 0xd2,
>   M_RST3 = 0xd3,
>   M_RST4 = 0xd4,
>   M_RST5 = 0xd5,
>   M_RST6 = 0xd6,
>   M_RST7 = 0xd7,
>
>   M_SOI = 0xd8,
>   M_EOI = 0xd9,
>   M_SOS = 0xda,
>   M_DQT = 0xdb,
>   M_DNL = 0xdc,
>   M_DRI = 0xdd,
>   M_DHP = 0xde,
>   M_EXP = 0xdf,
>
>   M_APP0 = 0xe0,
>   M_APP1 = 0xe1,
>   M_APP2 = 0xe2,
>   M_APP3 = 0xe3,
>   M_APP4 = 0xe4,
>   M_APP5 = 0xe5,
>   M_APP6 = 0xe6,
>   M_APP7 = 0xe7,
>   M_APP8 = 0xe8,
>   M_APP9 = 0xe9,
>   M_APP10 = 0xea,
>   M_APP11 = 0xeb,
>   M_APP12 = 0xec,
>   M_APP13 = 0xed,
>   M_APP14 = 0xee,
>   M_APP15 = 0xef,
>
>   M_JPG0 = 0xf0,
>   M_JPG13 = 0xfd,
>   M_COM = 0xfe,
>
>   M_TEM = 0x01,
>
>   M_ERROR = 0x100
> } JPEG_MARKER;

Please use jpeg_marker_t.

> typedef int jpeg_data_unit[64];

jpeg_data_unit_t

> struct grub_jpeg_data
> {
>   grub_file_t file;
>   grub_jmp_buf jumper;
>
>   int image_width;
>   int image_height;
>
>   grub_uint8_t *huff_value[4];
>   int huff_offset[4][16];
>   int huff_maxval[4][16];
>
>   grub_uint8_t quan_table[2][64];
>   int comp_index[3][3];
>
>   jpeg_data_unit ydu[4];
>   jpeg_data_unit crdu;
>   jpeg_data_unit cbdu;
>
>   int Cr_r_tab[256], Cb_b_tab[256], Cr_g_tab[256], Cb_g_tab[256];

I do not like using uppercase for members.

>   int vs, hs;
>
>   int dc_value[3];
>
>   int bit_mask, bit_save;
>
>   struct grub_video_bitmap **bitmap;
> };
>
>
> static grub_uint8_t
> get_byte (struct grub_jpeg_data *data)

How about jpeg_get_byte or so?  Or even add grub_.  I have the feeling
this will clash someday.

> {
>   grub_uint8_t r;
>
>   if (grub_file_read (data->file, (char *) &r, 1) != 1)
>     grub_longjmp (data->jumper, 1);

I don't like having longjmp here, please use the error handling as is
used in other parts of GRUB 2.

>   return r;
> }
>
> static grub_uint16_t
> get_word (struct grub_jpeg_data *data)
> {
>   grub_uint16_t r;
>
>   if (grub_file_read (data->file, (char *) &r, 2) != 2)
>     grub_longjmp (data->jumper, 1);

please replace the 2's by sizeof (grub_uint16_t).  As for the error
handling, see the previous comment.

>   return grub_be_to_cpu16 (r);
> }
>
> static int
> get_bit (struct grub_jpeg_data *data)
> {
>   int ret;
>
>   if (data->bit_mask == 0)
>     {
>       data->bit_save = get_byte (data);
>       if (data->bit_save == 0xFF)
>       {
>         if (get_byte (data) != 0)
>           {
>             grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                         "jpeg: invalid 0xFF in data stream");
>             grub_longjmp (data->jumper, 1);

jongjmp...

>           }
>       }
>       data->bit_mask = 0x80;
>     }
>
>   ret = (data->bit_save & data->bit_mask);
>   data->bit_mask >>= 1;
>   return ret;

How about "return !!ret" so you can directly use this bit in the other
functions that call this one?

> }
>
> static int
> get_number (struct grub_jpeg_data *data, int num)
> {
>   int value, i, bit;
>
>   if (num == 0)
>     return 0;
>
>   bit = get_bit (data);
>   value = (bit != 0);
>   for (i = 1; i < num; i++)
>     value = value * 2 + (get_bit (data) != 0);

(value << 1)

>   if (!bit)
>     value += 1 - (1 << num);
>
>   return value;
> }
>
> static int
> get_huff_code (struct grub_jpeg_data *data, int id)
> {
>   int code, i;
>
>   code = 0;
>   for (i = 0; i < 16; i++)
>     {
>       code <<= 1;
>       if (get_bit (data))
>       code++;
>       if (code < data->huff_maxval[id][i])
>       return data->huff_value[id][code + data->huff_offset[id][i]];
>     }
>   grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: huffman decode fails");
>   grub_longjmp (data->jumper, 1);

...

>   return 0;
> }
>
> static void
> decode_huff_table (struct grub_jpeg_data *data)
> {
>   int id, ac, i, n, base, ofs;
>   grub_uint32_t next_marker;
>   grub_uint8_t count[16];
>
>   next_marker = data->file->offset;
>   next_marker += get_word (data);
>
>   id = get_byte (data);
>   ac = (id >> 4);
>   id &= 0xF;
>   if (id > 1)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: too many huffman table");
>       grub_longjmp (data->jumper, 1);
>     }

How many are normally supported?  Max 4, IIRC?

>   if (grub_file_read (data->file, (char *) &count, sizeof (count)) !=
>       sizeof (count))
>     grub_longjmp (data->jumper, 1);
>
>   n = 0;
>   for (i = 0; i < 16; i++)
>     n += count[i];

What does this do?

>   id += ac * 2;
>   data->huff_value[id] = grub_malloc (n);
>   if (data->huff_value[id] == NULL)
>     grub_longjmp (data->jumper, 1);

How about "goto fail"? or a return?

>
>   if (grub_file_read (data->file, (char *) data->huff_value[id], n) != n)
>     grub_longjmp (data->jumper, 1);

...

>   base = 0;
>   ofs = 0;
>   for (i = 0; i < 16; i++)
>     {
>       base += count[i];
>       ofs += count[i];
>
>       data->huff_maxval[id][i] = base;
>       data->huff_offset[id][i] = ofs - base;
>
>       base <<= 1;
>     }
>
>   if (data->file->offset != next_marker)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                 "jpeg: extra byte in huffman table");
>       grub_longjmp (data->jumper, 1);
>     }
> }
>
> static void
> decode_quan_table (struct grub_jpeg_data *data)
> {
>   int id;
>   grub_uint32_t next_marker;
>
>   next_marker = data->file->offset;
>   next_marker += get_word (data);
>
>   id = get_byte (data);
>   if (id >= 0x10)             /* upper 4-bit is precision  */
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                 "jpeg: only 8-bit precision is supported");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   if (id > 1)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                 "jpeg: too many quantization table");

table -> tables

>       grub_longjmp (data->jumper, 1);
>     }
>
>   if (grub_file_read (data->file, (char *) &data->quan_table[id], 64) != 64)
>     grub_longjmp (data->jumper, 1);
>
>   if (data->file->offset != next_marker)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                 "jpeg: extra byte in quantization table");
>       grub_longjmp (data->jumper, 1);
>     }
> }
>
> static void
> decode_sof (struct grub_jpeg_data *data)
> {
>   int i, cc;
>   grub_uint32_t next_marker;
>
>   next_marker = data->file->offset;
>   next_marker += get_word (data);
>
>   if (get_byte (data) != 8)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                 "jpeg: only 8-bit precision is supported");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   data->image_height = get_word (data);
>   data->image_width = get_word (data);
>
>   if ((!data->image_height) || (!data->image_width))
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid image size");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   cc = get_byte (data);
>   if (cc != 3)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: component count must be 3");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   for (i = 0; i < cc; i++)
>     {
>       int id, ss;
>
>       id = get_byte (data) - 1;
>       if ((id < 0) || (id >= 3))
>       {
>         grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid index");
>         grub_longjmp (data->jumper, 1);
>       }
>       ss = get_byte (data);
>       if (!id)
>       {
>         data->vs = ss & 0xF;
>         data->hs = ss >> 4;

What's ss, vs, hs?

>         if ((data->vs > 2) || (data->hs > 2))
>           {
>             grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                         "jpeg: sampling method nor supported");
>             grub_longjmp (data->jumper, 1);
>           }
>       }
>       else if (ss != 0x11)

Magic value...  Can you use a macro for this or so?  I do not know
what this means.

>       {
>         grub_error (GRUB_ERR_BAD_FILE_TYPE,
>                     "jpeg: sampling method nor supported");
>         grub_longjmp (data->jumper, 1);
>       }
>       data->comp_index[id][0] = get_byte (data);
>     }
>
>   if (data->file->offset != next_marker)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: extra byte in sof");
>       grub_longjmp (data->jumper, 1);
>     }
> }
>
> #define DCTSIZE     8
>
> #define CONST_BITS  8
> #define PASS1_BITS  2
>
> #define FIX_1_082392200  ((int)  277) /* FIX(1.082392200)  */
> #define FIX_1_414213562  ((int)  362) /* FIX(1.414213562)  */
> #define FIX_1_847759065  ((int)  473) /* FIX(1.847759065)  */
> #define FIX_2_613125930  ((int)  669) /* FIX(2.613125930)  */
>
> #define ONE   ((long) 1)
> #define DESCALE(x,n)  (((x) + (ONE << ((n)-1))) >> (n))
>
> #define MULTIPLY(var,const)  DESCALE((var) * (const), CONST_BITS)
>
> static const grub_uint8_t natural_order[64] = {
>   0, 1, 8, 16, 9, 2, 3, 10,
>   17, 24, 32, 25, 18, 11, 4, 5,
>   12, 19, 26, 33, 40, 48, 41, 34,
>   27, 20, 13, 6, 7, 14, 21, 28,
>   35, 42, 49, 56, 57, 50, 43, 36,
>   29, 22, 15, 23, 30, 37, 44, 51,
>   58, 59, 52, 45, 38, 31, 39, 46,
>   53, 60, 61, 54, 47, 55, 62, 63
> };
>
> static const int aanscales[64] = {
>   /* precomputed values scaled up by 14 bits  */
>   16384, 22725, 21407, 19266, 16384, 12873, 8867, 4520,
>   22725, 31521, 29692, 26722, 22725, 17855, 12299, 6270,
>   21407, 29692, 27969, 25172, 21407, 16819, 11585, 5906,
>   19266, 26722, 25172, 22654, 19266, 15137, 10426, 5315,
>   16384, 22725, 21407, 19266, 16384, 12873, 8867, 4520,
>   12873, 17855, 16819, 15137, 12873, 10114, 6967, 3552,
>   8867, 12299, 11585, 10426, 8867, 6967, 4799, 2446,
>   4520, 6270, 5906, 5315, 4520, 3552, 2446, 1247
> };
>
> static void
> idct_transform (jpeg_data_unit du)
> {
>   int *pd;
>   int ctr;
>   int tmp0, tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7;
>   int tmp10, tmp11, tmp12, tmp13;
>   int z5, z10, z11, z12, z13;
>
>   /* Pass 1: process columns from input, store into work array.  */
>
>   pd = du;
>   for (ctr = DCTSIZE; ctr > 0; ctr--)
>     {
>       if ((pd[DCTSIZE * 1] | pd[DCTSIZE * 2] | pd[DCTSIZE * 3] |
>          pd[DCTSIZE * 4] | pd[DCTSIZE * 5] | pd[DCTSIZE * 6] |
>          pd[DCTSIZE * 7]) == 0)
>       {
>         pd[DCTSIZE * 1] = pd[DCTSIZE * 2]
>           = pd[DCTSIZE * 3] = pd[DCTSIZE * 4]
>           = pd[DCTSIZE * 5] = pd[DCTSIZE * 6]
>           = pd[DCTSIZE * 7] = pd[DCTSIZE * 0];
>
>         pd++;                 /* advance pointers to next column  */
>         continue;
>       }
>
>       /* Even part */
>
>       tmp0 = pd[DCTSIZE * 0];
>       tmp1 = pd[DCTSIZE * 2];
>       tmp2 = pd[DCTSIZE * 4];
>       tmp3 = pd[DCTSIZE * 6];
>
>       tmp10 = tmp0 + tmp2;    /* phase 3  */
>       tmp11 = tmp0 - tmp2;
>
>       tmp13 = tmp1 + tmp3;    /* phases 5-3  */
>       tmp12 = MULTIPLY (tmp1 - tmp3, FIX_1_414213562) - tmp13;        /* 2*c4 
>  */
>
>       tmp0 = tmp10 + tmp13;   /* phase 2  */
>       tmp3 = tmp10 - tmp13;
>       tmp1 = tmp11 + tmp12;
>       tmp2 = tmp11 - tmp12;
>
>       /* Odd part  */
>
>       tmp4 = pd[DCTSIZE * 1];
>       tmp5 = pd[DCTSIZE * 3];
>       tmp6 = pd[DCTSIZE * 5];
>       tmp7 = pd[DCTSIZE * 7];
>
>       z13 = tmp6 + tmp5;      /* phase 6  */
>       z10 = tmp6 - tmp5;
>       z11 = tmp4 + tmp7;
>       z12 = tmp4 - tmp7;
>
>       tmp7 = z11 + z13;               /* phase 5  */
>       tmp11 = MULTIPLY (z11 - z13, FIX_1_414213562);  /* 2*c4  */
>
>       z5 = MULTIPLY (z10 + z12, FIX_1_847759065);     /* 2*c2  */
>       tmp10 = MULTIPLY (z12, FIX_1_082392200) - z5;   /* 2*(c2-c6)  */
>       tmp12 = MULTIPLY (z10, -FIX_2_613125930) + z5;  /* -2*(c2+c6)  */
>
>
>       tmp6 = tmp12 - tmp7;    /* phase 2  */
>       tmp5 = tmp11 - tmp6;
>       tmp4 = tmp10 + tmp5;
>
>       pd[DCTSIZE * 0] = (int) (tmp0 + tmp7);
>       pd[DCTSIZE * 7] = (int) (tmp0 - tmp7);
>       pd[DCTSIZE * 1] = (int) (tmp1 + tmp6);
>       pd[DCTSIZE * 6] = (int) (tmp1 - tmp6);
>       pd[DCTSIZE * 2] = (int) (tmp2 + tmp5);
>       pd[DCTSIZE * 5] = (int) (tmp2 - tmp5);
>       pd[DCTSIZE * 4] = (int) (tmp3 + tmp4);
>       pd[DCTSIZE * 3] = (int) (tmp3 - tmp4);
>
>       pd++;                   /* advance pointers to next column  */
>     }
>
>   /* Pass 2: process rows from work array, store into output array.  */
>   /* Note that we must descale the results by a factor of 8 == 2**3,  */
>   /* and also undo the PASS1_BITS scaling.  */
>
>   pd = du;
>   for (ctr = 0; ctr < DCTSIZE; ctr++)
>     {
>       /* Even part  */
>
>       tmp10 = pd[0] + pd[4];
>       tmp11 = pd[0] - pd[4];
>
>       tmp13 = pd[2] + pd[6];
>       tmp12 = MULTIPLY (pd[2] - pd[6], FIX_1_414213562) - tmp13;
>
>       tmp0 = tmp10 + tmp13;
>       tmp3 = tmp10 - tmp13;
>       tmp1 = tmp11 + tmp12;
>       tmp2 = tmp11 - tmp12;
>
>       /* Odd part  */
>
>       z13 = pd[5] + pd[3];
>       z10 = pd[5] - pd[3];
>       z11 = pd[1] + pd[7];
>       z12 = pd[1] - pd[7];
>
>       tmp7 = z11 + z13;               /* phase 5  */
>       tmp11 = MULTIPLY (z11 - z13, FIX_1_414213562);  /* 2*c4  */
>
>       z5 = MULTIPLY (z10 + z12, FIX_1_847759065);     /* 2*c2  */
>       tmp10 = MULTIPLY (z12, FIX_1_082392200) - z5;   /* 2*(c2-c6)  */
>       tmp12 = MULTIPLY (z10, -FIX_2_613125930) + z5;  /* -2*(c2+c6)  */
>
>       tmp6 = tmp12 - tmp7;    /* phase 2  */
>       tmp5 = tmp11 - tmp6;
>       tmp4 = tmp10 + tmp5;
>
>       /* Final output stage: scale down by a factor of 8 and range-limit  */
>
>       pd[0] = DESCALE (tmp0 + tmp7, PASS1_BITS + 3) + 128;
>       pd[7] = DESCALE (tmp0 - tmp7, PASS1_BITS + 3) + 128;
>       pd[1] = DESCALE (tmp1 + tmp6, PASS1_BITS + 3) + 128;
>       pd[6] = DESCALE (tmp1 - tmp6, PASS1_BITS + 3) + 128;
>       pd[2] = DESCALE (tmp2 + tmp5, PASS1_BITS + 3) + 128;
>       pd[5] = DESCALE (tmp2 - tmp5, PASS1_BITS + 3) + 128;
>       pd[4] = DESCALE (tmp3 + tmp4, PASS1_BITS + 3) + 128;
>       pd[3] = DESCALE (tmp3 - tmp4, PASS1_BITS + 3) + 128;
>
>       pd += DCTSIZE;          /* advance pointer to next row  */
>     }
>
>   for (ctr = 0; ctr < 64; ctr++)
>     {
>       if (du[ctr] < 0)
>       du[ctr] = 0;
>       if (du[ctr] > 255)
>       du[ctr] = 255;
>     }
> }
>
> static void
> decode_du (struct grub_jpeg_data *data, int id, jpeg_data_unit du)
> {
>   int pos, h1, h2, qt;
>
>   grub_memset (du, 0, sizeof (jpeg_data_unit));
>
>   qt = data->comp_index[id][0];
>   h1 = data->comp_index[id][1];
>   h2 = data->comp_index[id][2];
>
>   data->dc_value[id] += get_number (data, get_huff_code (data, h1));
>
>   du[0] =
>     data->dc_value[id] * DESCALE ((int) data->quan_table[qt][0] *
>                                 (int) aanscales[0], 14 - 2);
>   pos = 1;
>   while (pos < 64)
>     {
>       int num, val;
>
>       num = get_huff_code (data, h2);
>       if (!num)
>       break;
>       val = get_number (data, num & 0xF);
>       num >>= 4;
>       pos += num;
>       du[natural_order[pos]] =
>       val * DESCALE ((int) data->quan_table[qt][pos] *
>                      (int) aanscales[natural_order[pos]], 14 - 2);

14 - 2?

>       pos++;
>     }
>
>   idct_transform (du);
> }
>
> #define SCALEBITS     16      /* speediest right-shift on some machines */
> #define ONE_HALF      ((int) 1 << (SCALEBITS-1))
> #define FIX(x)                ((int) ((x) * (1L<<SCALEBITS) + 0.5))
>
> static void
> build_color_table (struct grub_jpeg_data *data)
> {
>   int i, x;
>
>   for (i = 0, x = -128; i <= 255; i++, x++)
>     {
>       data->Cr_r_tab[i] = (int) (FIX (1.40200) * x + ONE_HALF) >> SCALEBITS;
>       data->Cb_b_tab[i] = (int) (FIX (1.77200) * x + ONE_HALF) >> SCALEBITS;
>       data->Cr_g_tab[i] = (-FIX (0.71414)) * x;
>       data->Cb_g_tab[i] = (-FIX (0.34414)) * x + ONE_HALF;
>     }
> }
>
> static void
> decode_sos (struct grub_jpeg_data *data)
> {
>   int i, cc, r1, c1, nr1, nc1, vb, hb;
>   grub_uint8_t *ptr1;
>   grub_uint32_t data_offset;
>
>   data_offset = data->file->offset;
>   data_offset += get_word (data);
>
>   cc = get_byte (data);
>
>   if (cc != 3)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: component count must be 3");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   for (i = 0; i < cc; i++)
>     {
>       int id, ht;
>
>       id = get_byte (data) - 1;
>       if ((id < 0) || (id >= 3))
>       {
>         grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid index");
>         grub_longjmp (data->jumper, 1);
>       }
>
>       ht = get_byte (data);
>       data->comp_index[id][1] = (ht >> 4);
>       data->comp_index[id][2] = (ht & 0xF) + 2;
>     }
>
>   get_byte (data);            /* skip 3 unused bytes  */
>   get_word (data);
>
>   if (data->file->offset != data_offset)
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: extra byte in sos");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   if (grub_video_bitmap_create (data->bitmap, data->image_width,
>                               data->image_height,
>                               GRUB_VIDEO_BLIT_FORMAT_R8G8B8))
>     grub_longjmp (data->jumper, 1);
>
>   data->bit_mask = 0x0;
>
>   vb = data->vs * 8;
>   hb = data->hs * 8;
>   nr1 = (data->image_height + vb - 1) / vb;
>   nc1 = (data->image_width + hb - 1) / hb;
>
>   ptr1 = (*data->bitmap)->data;
>   for (r1 = 0; r1 < nr1; r1++, ptr1 += (vb * data->image_width - hb * nc1) * 
> 3)
>     for (c1 = 0; c1 < nc1; c1++, ptr1 += hb * 3)
>       {
>       int r2, c2, nr2, nc2;
>         grub_uint8_t *ptr2;
>
>       for (r2 = 0; r2 < data->vs; r2++)
>         for (c2 = 0; c2 < data->hs; c2++)
>           decode_du (data, 0, data->ydu[r2 * 2 + c2]);
>
>       decode_du (data, 1, data->cbdu);
>       decode_du (data, 2, data->crdu);
>
>         nr2 = (r1 == nr1 - 1) ? (data->image_height - r1 * vb) : vb;
>         nc2 = (c1 == nc1 - 1) ? (data->image_width - c1 * hb) : hb;
>
>         ptr2 = ptr1;
>       for (r2 = 0; r2 < nr2; r2++, ptr2 += (data->image_width - nc2) * 3)
>         for (c2 = 0; c2 < nc2; c2++)
>           {
>             int i0, yy, cr, cb, dd;
>
>               i0 = (r2 / data->vs) * 8 + (c2 / data->hs);
>             cr = data->crdu[i0];
>             cb = data->cbdu[i0];
>
>               yy = data->ydu[(r2 / 8) * 2 + (c2 / 8)][(r2 % 8) * 8 + (c2 % 
> 8)];
>
>             /* Red  */
>             dd = yy + data->Cr_r_tab[cr];
>             if (dd < 0)
>               dd = 0;
>             if (dd > 255)
>               dd = 255;
>             *(ptr2++) = dd;
>
>             /* Green  */
>             dd = yy + ((data->Cb_g_tab[cb] + data->Cr_g_tab[cr]) >> 
> SCALEBITS);
>             if (dd < 0)
>               dd = 0;
>             if (dd > 255)
>               dd = 255;
>             *(ptr2++) = dd;
>
>             /* Blue  */
>             dd = yy + data->Cb_b_tab[cb];
>             if (dd < 0)
>               dd = 0;
>             if (dd > 255)
>               dd = 255;
>             *(ptr2++) = dd;

Perhaps it's useful to make a separate function for colorspace
conversion?  (that's what's happening here, right?)  Other loaders can
use it too.

>           }
>       }
> }
>
>
> static grub_uint8_t
> get_marker (struct grub_jpeg_data *data)
> {
>   grub_uint8_t r;
>
>   r = get_byte (data);
>
>   if (r != 0xFF)              /* escape character  */
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid maker");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   return get_byte (data);
> }
>
> static void
> decode_jpeg (struct grub_jpeg_data *data)
> {
>   build_color_table (data);
>
>   if (get_marker (data) != M_SOI)     /* Start Of Image  */
>     {
>       grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid jpeg file");
>       grub_longjmp (data->jumper, 1);
>     }
>
>   while (1)
>     {
>       grub_uint8_t marker;
>
>       marker = get_marker (data);
> #ifdef JPEG_DEBUG
>       grub_printf ("jpeg marker: %x\n", marker);
> #endif
>       switch (marker)
>       {
>       case M_DHT:             /* Define Huffman Table  */
>         decode_huff_table (data);
>         break;
>       case M_DQT:             /* Define Quantization Table  */
>         decode_quan_table (data);
>         break;
>       case M_SOF0:            /* Start Of Frame 0  */
>         decode_sof (data);
>         break;
>       case M_SOS:             /* Start Of Scan  */
>         decode_sos (data);
>         break;
>       case M_EOI:             /* End Of Image  */
>         return;
>       default:                /* Skip unrecognized marker  */
>         {
>           grub_uint16_t sz;
>
>           sz = get_word (data);
>           grub_file_seek (data->file, data->file->offset + sz - 2);
>         }
>       }
>     }
> }
>
> static grub_err_t
> grub_video_reader_jpeg (struct grub_video_bitmap **bitmap,
>                       const char *filename)
> {
>   grub_file_t file;
>   struct grub_jpeg_data *data;
>
>   file = grub_file_open (filename);
>   if (!file)
>     return grub_errno;
>
>   data = grub_malloc (sizeof (*data));
>   if (data != NULL)
>     {
>       int i;
>
>       grub_memset (data, 0, sizeof (*data));
>       data->file = file;
>       data->bitmap = bitmap;
>       if (!grub_setjmp (data->jumper))
>       decode_jpeg (data);
>
>       for (i = 0; i < 4; i++)
>       if (data->huff_value[i])
>         grub_free (data->huff_value[i]);
>       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(JPEG_DEBUG)
> static grub_err_t
> grub_cmd_jpegtest (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_jpeg (&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 jpg_reader = {
>   .extension = ".jpg",
>   .reader = grub_video_reader_jpeg,
>   .next = 0
> };
>
> static struct grub_video_bitmap_reader jpeg_reader = {
>   .extension = ".jpeg",
>   .reader = grub_video_reader_jpeg,
>   .next = 0
> };
>
> GRUB_MOD_INIT (video_reader_jpeg)
> {
>   grub_video_bitmap_reader_register (&jpg_reader);
>   grub_video_bitmap_reader_register (&jpeg_reader);
> #if defined(JPEG_DEBUG)
>   grub_register_command ("jpegtest", grub_cmd_jpegtest,
>                        GRUB_COMMAND_FLAG_BOTH, "jpegtest FILE",
>                        "Tests loading of JPEG bitmap.", 0);
> #endif
> }
>
> GRUB_MOD_FINI (video_reader_jpeg)
> {
> #if defined(JPEG_DEBUG)
>   grub_unregister_command ("jpegtest");
> #endif
>   grub_video_bitmap_reader_unregister (&jpeg_reader);
>   grub_video_bitmap_reader_unregister (&jpg_reader);
> }
>
>
> -- 
> Bean
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel