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<[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?


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:[email protected]
tel:86-010-82450803


_______________________________________________
SeaBIOS mailing list
[email protected]
http://www.seabios.org/mailman/listinfo/seabios

Reply via email to