On Mon, Jun 27, 2011 at 03:10:38PM +0800, Wayne Xia wrote: > added a bmp decoder, changed jpeg decoder to fix some problem in 24 bpp mode
Thanks. In general it looks good to me. I have a few minor comments. [...] > --- /dev/null > +++ b/src/bmp.c > @@ -0,0 +1,90 @@ > +/* > +* Basic BMP data process and Raw picture data handle functions. > +* Could be used to adjust pixel data format, get infomation, etc. > +* > +* Copyright (C) 2011 Wayne Xia <[email protected]> > +* > +* This work is licensed under the terms of the GNU LGPLv3. > +*/ > +#include "util.h" > +#include "bmp.h" > + > +void raw_data_format_adjust_24bpp(u8 *src, u8 *dest, int width, int height, > + int bytes_per_line_src, int bytes_per_line_dest, u8 switch_flag) This should be declared static. Also, doesn't this function just become a series of memcpy calls now that bytes_per_line_dest and switch_flag aren't needed? [...] > --- /dev/null > +++ b/src/bmp.h > @@ -0,0 +1,78 @@ > +#ifndef BMP_H > +#define BMP_H > +#include "types.h" > + > +#define WIDTHBYTES(bits) (((bits)+31)/32*4) > + > +typedef struct tagBITMAPFILEHEADER { > + u8 bfType[2]; > + u8 bfSize[4]; > + u8 bfReserved1[2]; > + u8 bfReserved2[2]; > + u8 bfOffBits[4]; > +} BITMAPFILEHEADER, tagBITMAPFILEHEADER; I think it would be preferable to move all the bmp specific declarations into bmp.c. [...] > --- a/src/jpeg.c > +++ b/src/jpeg.c > @@ -394,10 +394,10 @@ void jpeg_get_size(struct jpeg_decdata *jpeg, int > *width, int *height) > *height = jpeg->height; > } > > -int jpeg_show(struct jpeg_decdata *jpeg, unsigned char *pic > - , int width, int height, int depth) > +int jpeg_show(struct jpeg_decdata *jpeg, unsigned char *pic, int width > + , int height, int depth, int bytes_per_line_dest) > { > - int m, mcusx, mcusy, mx, my; > + int m, mcusx, mcusy, mx, my, mloffset, jpgbpl; Thanks. It would be preferable if the jpeg fixes were in a separate patch from the bmp feature addition. -Kevin _______________________________________________ SeaBIOS mailing list [email protected] http://www.seabios.org/mailman/listinfo/seabios
