[SeaBIOS] [PATCH v3 0/3] Boot up with a splash picture configurable

[Wayne Xia]

     thanks for the comments.
     for the lzma decompressor, to enable compressing the qemu/kvm needs
8 files from lzma SDK, I am afraid this is a bit hard to be accepted by
qemu for such logo showing feature.

Following is some comments about the code.

2011-7-3 6:57, Kevin O'Connor:
> 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<xiawenc at cn.ibm.com>
>> +*
>> +* 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?
>

changed to static. But I am afraid this function need to be called by
the "bmp_show" to switch vertical line sequence and adjust data
location.

> [...]
>> --- /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.

moved to 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.
>

split it as a single patch.

> -Kevin
>


-- 
Best Regards

Wayne Xia
mail:xiawenc at linux.vnet.ibm.com
tel:86-010-82450803