From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1JChbc-0005nx-3F for mharc-grub-devel@gnu.org; Wed, 09 Jan 2008 15:32:56 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JChba-0005nE-N8 for grub-devel@gnu.org; Wed, 09 Jan 2008 15:32:54 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JChbZ-0005mJ-HE for grub-devel@gnu.org; Wed, 09 Jan 2008 15:32:54 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JChbZ-0005m9-DL for grub-devel@gnu.org; Wed, 09 Jan 2008 15:32:53 -0500 Received: from pne-smtpout4-sn1.fre.skanova.net ([81.228.11.168]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1JChbY-0006Pb-KQ for grub-devel@gnu.org; Wed, 09 Jan 2008 15:32:53 -0500 Received: from [127.0.0.1] (88.193.32.97) by pne-smtpout4-sn1.fre.skanova.net (7.3.129) id 474FD021001DF04F for grub-devel@gnu.org; Wed, 9 Jan 2008 21:32:29 +0100 Message-ID: <47852F7C.6030407@nic.fi> Date: Wed, 09 Jan 2008 22:33:00 +0200 From: =?ISO-8859-1?Q?Vesa_J=E4=E4skel=E4inen?= User-Agent: Thunderbird 2.0.0.9 (Windows/20071031) MIME-Version: 1.0 To: The development of GRUB 2 References: In-Reply-To: X-Enigmail-Version: 0.95.6 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-detected-kernel: by monty-python.gnu.org: Solaris 10 (beta) Subject: Re: [PATCH] jpeg image reader X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Jan 2008 20:32:54 -0000 Bean wrote: > Hi, > > I just fix a small bug that can cause color distortion, here is the new patch. Nice job for implementing JPEG. This is the major reason I made the bitmap loader interface, to allow other people to write loaders... now if we would just have PNG support ;) I will not comment on the JPEG format or any possible IPR issues as I am not too clear on that myself, just the patch itself. I would have hoped a bit more comments. Check your tabs and spaces. Re-check array handling. I know lots of JPEG loaders seems to use longjmp for error handling, personally I don't like it, but I suppose it is ok. In grub2 you could just check if grub_errno != GRUB_ERR_NONE and then just return and at that level do the same check. This mechanism works a bit like c++'s exceptions. Another thing is that if there is a problem loading JPEG, user gets a message like "invalid marker: 1234", this doesn't really tell user a thing. So I would propose to use jpeg prefix like "jpeg: invalid marker...", or more descriptive from user's perspective. > diff --git a/conf/i386-pc.rmk b/conf/i386-pc.rmk > index a46bc74..4beeeff 100644 > --- a/conf/i386-pc.rmk > +++ b/conf/i386-pc.rmk > @@ -136,7 +136,7 @@ pkglib_MODULES = biosdisk.mod _chain.mod > _linux.mod linux.mod normal.mod \ > _multiboot.mod chain.mod multiboot.mod reboot.mod halt.mod \ > vbe.mod vbetest.mod vbeinfo.mod video.mod gfxterm.mod \ > videotest.mod play.mod bitmap.mod tga.mod cpuid.mod serial.mod \ > - ata.mod vga.mod > + ata.mod vga.mod jpeg.mod > > # For biosdisk.mod. > biosdisk_mod_SOURCES = disk/i386/pc/biosdisk.c > @@ -249,6 +249,11 @@ tga_mod_SOURCES = video/readers/tga.c > tga_mod_CFLAGS = $(COMMON_CFLAGS) > tga_mod_LDFLAGS = $(COMMON_LDFLAGS) > > +# For jpeg.mod > +jpeg_mod_SOURCES = video/readers/jpeg.c > +jpeg_mod_CFLAGS = $(COMMON_CFLAGS) > +jpeg_mod_LDFLAGS = $(COMMON_LDFLAGS) > + > # For cpuid.mod. > cpuid_mod_SOURCES = commands/i386/cpuid.c > cpuid_mod_CFLAGS = $(COMMON_CFLAGS) > diff --git a/video/readers/jpeg.c b/video/readers/jpeg.c > new file mode 100755 > index 0000000..bcbf820 > --- /dev/null > +++ b/video/readers/jpeg.c > @@ -0,0 +1,734 @@ > +/* > + * 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 . > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* Uncomment following define to enable JPEG debug. */ > +//#define JPEG_DEBUG > + > +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]; > + > + int ydu[4][64], cbdu[64], crdu[64]; > + > + int Cr_r_tab[256], Cb_b_tab[256], Cr_g_tab[256], Cb_g_tab[256]; > + > + 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) > +{ > + grub_uint8_t r; > + > + if (grub_file_read (data->file, (char *) &r, 1) != 1) > + grub_longjmp (data->jumper, 1); > + > + return r; > +} > + > +static unsigned unsigned -> grub_uint16_t > +get_word (struct grub_jpeg_data *data) > +{ > + unsigned aa, bb; > + > + aa = get_byte (data); > + bb = get_byte (data); > + return (aa << 8) + bb; > +} > + > +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, > + "invalid 0xFF in data stream"); > + grub_longjmp (data->jumper, 1); > + } > + } > + data->bit_mask = 0x80; > + } > + > + ret = (data->bit_save & data->bit_mask); > + data->bit_mask >>= 1; > + return ret; > +} > + > +static int > +get_huff_leng (struct grub_jpeg_data *data, int num) _length? > +{ > + 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); > + if (!bit) > + value += 1 - (1 << num); > + > + return value; > +} > + > +static int > +get_huff_base (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, "huffman decode fails"); > + grub_longjmp (data->jumper, 1); > + return 0; > +} > + > +static void > +decode_huff_table (struct grub_jpeg_data *data) > +{ > + unsigned id; unsigned -> grub_uint8_t > + int ac, i, n, base, ofs; > + grub_uint8_t count[16]; > + > + get_word (data); not important word? > + id = get_byte (data); > + ac = (id >> 4); > + id &= 0xF; > + if (id > 1) > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "too many huff table"); > + grub_longjmp (data->jumper, 1); > + } > + > + 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]; > + > + id += ac * 2; > + if ((data->huff_value[id] = grub_malloc (n)) == NULL) try to keep malloc on own line and then check for pointer or for grub_errno (or both). > + grub_longjmp (data->jumper, 1); > + > + 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; > + } > +} > + > +static void > +decode_quan_table (struct grub_jpeg_data *data) > +{ > + unsigned id; > + > + get_word (data); > + id = get_byte (data); > + if (id >= 0x10) 0x10? > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, > + "only 8-bit precision is supported"); > + grub_longjmp (data->jumper, 1); > + } > + > + if (id > 1) > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "too many quantization table"); > + grub_longjmp (data->jumper, 1); > + } > + > + if (grub_file_read (data->file, (char *) &data->quan_table[id], 64) != 64) > + grub_longjmp (data->jumper, 1); > +} > + > +static void > +decode_sof (struct grub_jpeg_data *data) > +{ > + int nc, i; > + > + get_word (data); > + if (get_byte (data) != 8) > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, > + "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, "invalid image size"); > + grub_longjmp (data->jumper, 1); > + } > + > + nc = get_byte (data); > + if (nc != 3) > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "component count must be 3"); > + grub_longjmp (data->jumper, 1); > + } > + > + for (i = 0; i < nc; i++) > + { > + int id, ss; > + > + id = get_byte (data) - 1; > + ss = get_byte (data); > + if (((!id) && (ss != 0x22)) || ((id) && (ss != 0x11))) > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "only support 2:1:1 sampling"); > + grub_longjmp (data->jumper, 1); > + } > + data->comp_index[id][0] = get_byte (data); Buffer overflow possibility. > + } > +} > + > +#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 unsigned char natural_order[64] = { grub_uint8_t, or just (unsigned) int > + 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 short aanscales[64] = { use just int (as it is anyway casted to int, and I don't think it is necessary to save some bytes on here from module. > + /* 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 (int *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. */ Extra space after last dot. > + > + 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, int *du) > +{ > + int pos, h1, h2, qt; > + > + grub_memset (du, 0, sizeof (int) * 64); du is always that size ? perhaps add warning about it to prototype, or provide length? I didn't check for indexes below, verify them. > + > + qt = data->comp_index[id][0]; > + h1 = data->comp_index[id][1]; > + h2 = data->comp_index[id][2]; > + > + data->dc_value[id] += get_huff_leng (data, get_huff_base (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_base (data, h2); > + if (!num) > + break; > + val = get_huff_leng (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); > + 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< + > +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 nc, i, r1, c1; > + grub_uint8_t *ptr; > + > + get_word (data); No use for first word? > + nc = get_byte (data); > + > + if (nc != 3) > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "component count must be 3"); > + grub_longjmp (data->jumper, 1); > + } > + > + for (i = 0; i < nc; i++) > + { > + int id, ht; > + > + id = get_byte (data) - 1; > + ht = get_byte (data); > + data->comp_index[id][1] = (ht >> 4); > + data->comp_index[id][2] = (ht & 0xF) + 2; Buffer overflows. > + } > + > + get_byte (data); > + get_word (data); > + > + if (grub_video_bitmap_create (data->bitmap, data->image_width, > + data->image_height, > + GRUB_VIDEO_BLIT_FORMAT_R8G8B8)) > + grub_longjmp (data->jumper, 1); > + > + ptr = (*data->bitmap)->data; > + data->bit_mask = 0x0; > + > + for (r1 = 0; r1 < (data->image_height >> 4); r1++) > + for (c1 = 0; c1 < (data->image_width >> 4); c1++) > + { > + int r2, c2, r3, c3; > + > + decode_du (data, 0, &data->ydu[0][0]); > + decode_du (data, 0, &data->ydu[1][0]); > + > + decode_du (data, 0, &data->ydu[2][0]); > + decode_du (data, 0, &data->ydu[3][0]); > + > + decode_du (data, 1, &data->cbdu[0]); > + decode_du (data, 2, &data->crdu[0]); > + > + for (r2 = 0; r2 < 2; r2++) > + for (c2 = 0; c2 < 2; c2++) > + for (r3 = 0; r3 < 8; r3++) > + for (c3 = 0; c3 < 8; c3++) > + { > + int i0, i1, i2, cr, cb, dd; > + > + i0 = > + ((r1 * 16 + r2 * 8 + r3) * (data->image_width) + c1 * 16 + > + c2 * 8 + c3) * 3; > + i1 = r3 * 8 + c3; > + i2 = (r2 * 32 + c2 * 4) + (r3 >> 1) * 8 + (c3 >> 1); I have following indexes are correct.... (didn't check them) > + > + cr = data->crdu[i2]; > + cb = data->cbdu[i2]; > + > + // Red Try to use C comments. /* ... > + dd = data->ydu[r2 * 2 + c2][i1] + data->Cr_r_tab[cr]; > + if (dd < 0) > + dd = 0; > + ptr[i0] = dd; > + > + // Green > + dd = > + data->ydu[r2 * 2 + c2][i1] + > + ((data->Cb_g_tab[cb] + data->Cr_g_tab[cr]) >> SCALEBITS); > + if (dd < 0) > + dd = 0; > + ptr[i0 + 1] = dd; > + > + // Blue > + dd = data->ydu[r2 * 2 + c2][i1] + data->Cb_b_tab[cb]; > + if (dd < 0) > + dd = 0; > + ptr[i0 + 2] = dd; > + } > + } > +} > + > + > +static grub_uint8_t > +get_marker (struct grub_jpeg_data *data) > +{ > + grub_uint8_t r; > + > + r = get_byte (data); > + > + if (r != 0xFF) 0xFF? > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "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) != 0xD8) 0xD8? > + { > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "invalid jpeg file"); > + grub_longjmp (data->jumper, 1); > + } > + > + while (1) > + { > + grub_uint8_t marker; > + > + marker = get_marker (data); > +#ifdef JPEG_DEBUG > + grub_printf ("marker: %x\n", marker); > +#endif > + switch (marker) > + { > + case 0xc4: 0xC4 ? > + decode_huff_table (data); > + break; > + case 0xdb: Same... > + decode_quan_table (data); > + break; > + case 0xc0: > + decode_sof (data); > + break; > + case 0xda: > + decode_sos (data); > + break; > + case 0xd9: > + return; > + case 0xe0: > + case 0xfe: > + { > + unsigned sz; unsigned -> grub_uint16_t > + > + sz = get_word (data); > + grub_file_seek (data->file, data->file->offset + sz - 2); > + break; > + } > + default: > + grub_error (GRUB_ERR_BAD_FILE_TYPE, "unrecognized marker %x", > + marker); > + grub_longjmp (data->jumper, 1); > + } > + } > +} > + > +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; > + > + if ((data = grub_malloc (sizeof (*data))) != NULL) Separate as two lines. > + { > + 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) && (*bitmap)) No need to check for *bitmap > + { > + 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); > +} > >