[PATCH v2 1/2] Boot up with a splash picture configurable

[Wayne Xia]

added a bmp decoder, changed jpeg decoder to fix some problem in 24 bpp mode

Signed-off-by: Wayne Xia 
---
 Makefile         |    4 +-
 src/bmp.c        |   90 ++++++++++++++++++++++++++++++++++++++++++++
 src/bmp.h        |   78 ++++++++++++++++++++++++++++++++++++++
 src/bootsplash.c |  110 ++++++++++++++++++++++++++++++++++++++++-------------
 src/jpeg.c       |   31 +++++++++++----
 src/jpeg.h       |    4 +-
 6 files changed, 277 insertions(+), 40 deletions(-)
 create mode 100644 src/bmp.c
 create mode 100644 src/bmp.h

diff --git a/Makefile b/Makefile
index a96c89d..2786c9e 100644
--- a/Makefile
+++ b/Makefile
@@ -15,12 +15,12 @@ SRCBOTH=misc.c pmm.c stacks.c output.c util.c block.c floppy.c ata.c mouse.c \
         kbd.c pci.c serial.c clock.c pic.c cdrom.c ps2port.c smp.c resume.c \
         pnpbios.c pirtable.c vgahooks.c ramdisk.c pcibios.c blockcmd.c \
         usb.c usb-uhci.c usb-ohci.c usb-ehci.c usb-hid.c usb-msc.c \
-        virtio-ring.c virtio-pci.c virtio-blk.c apm.c ahci.c
+        virtio-ring.c virtio-pci.c virtio-blk.c apm.c ahci.c bmp.c
 SRC16=$(SRCBOTH) system.c disk.c font.c
 SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \
       acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \
       lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c \
-      pci_region.c biostables.c xen.c
+      pci_region.c biostables.c xen.c bmp.c
 SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c

cc-option = $(shell if test -z "`$(1) $(2) -S -o /dev/null -xc \
diff --git a/src/bmp.c b/src/bmp.c
new file mode 100644
index 0000000..7bbfa01
--- /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 
+*
+* 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)
+{
+    int cy = height;
+    u8 *pd, *ps, *pd_valid, *ps_lastline;
+    u8 t;
+    u8 rgb_switch_flag = switch_flag & SWITCH_RGB;
+    u8 line_switch_flag = switch_flag & SWITCH_LINE;
+    while (cy > 0) {
+        ps = bytes_per_line_src*cy + src - 1;
+        if (line_switch_flag)
+            pd = bytes_per_line_dest * (height - cy + 1) + dest - 1;
+        else
+            pd = bytes_per_line_dest * cy + dest - 1;
+        ps_lastline = ps-bytes_per_line_src;
+        pd_valid = pd - (bytes_per_line_dest - bytes_per_line_src);
+        while (pd > pd_valid) {
+            *pd = 0;
+            pd--;
+        }
+        while (ps > ps_lastline) {
+            if (rgb_switch_flag) {
+                t = *ps;
+                *ps = *(ps-2);
+                *(ps-2) = t;
+            }
+            *(pd--) = *(ps--);
+            *(pd--) = *(ps--);
+            *(pd--) = *(ps--);
+        }
+        cy--;
+    }
+}
+
+struct bmp_decdata *bmp_alloc(void)
+{
+    struct bmp_decdata *bmp = malloc_tmphigh(sizeof(*bmp));
+    return bmp;
+}
+
+int bmp_decode(struct bmp_decdata *bmp, unsigned char *data, int data_size)
+{
+    if (data_size < 54)
+        return 1;
+
+    u16 bmp_filehead = bmp_load2byte(data + 0);
+    if (bmp_filehead != 0x4d42)
+        return 2;
+    u32 bmp_recordsize = bmp_load4byte(data + 2);
+    if (bmp_recordsize != data_size)
+        return 3;
+    u32 bmp_dataoffset = bmp_load4byte(data + 10);
+    bmp->datap = (unsigned char *)data + bmp_dataoffset;
+    bmp->width = bmp_load4byte(data + 18);
+    bmp->height = bmp_load4byte(data + 22);
+    bmp->bpp = bmp_load2byte(data + 28);
+    return 0;
+}
+
+void bmp_get_size(struct bmp_decdata *bmp, int *width, int *height)
+{
+    *width = bmp->width;
+    *height = bmp->height;
+}
+
+
+int bmp_show(struct bmp_decdata *bmp, unsigned char *pic, int width
+             , int height, int depth, int bytes_per_line_dest)
+{
+    if (bmp->datap == pic)
+        return 0;
+    /* now only support 24bpp bmp file */
+    if ((depth == 24) && (bmp->bpp == 24)) {
+        raw_data_format_adjust_24bpp(bmp->datap, pic, width, height,
+            3 * width, bytes_per_line_dest, SWITCH_LINE);
+        return 0;
+    }
+    return 1;
+}
diff --git a/src/bmp.h b/src/bmp.h
new file mode 100644
index 0000000..576628c
--- /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;
+
+typedef struct tagBITMAPINFOHEADER {
+    u8 biSize[4];
+    u8 biWidth[4];
+    u8 biHeight[4];
+    u8 biPlanes[2];
+    u8 biBitCount[2];
+    u8 biCompression[4];
+    u8 biSizeImage[4];
+    u8 biXPelsPerMeter[4];
+    u8 biYPelsPerMeter[4];
+    u8 biClrUsed[4];
+    u8 biClrImportant[4];
+} BITMAPINFOHEADER, tagBITMAPINFOHEADER;
+
+typedef struct tagRGBQUAD {
+    u8 rgbBlue;
+    u8 rgbGreen;
+    u8 rgbRed;
+    u8 rgbReserved;
+} RGBQUAD, tagRGBQUAD;
+
+struct bmp_decdata {
+    struct tagRGBQUAD *quadp;
+    unsigned char *datap;
+    int width;
+    int height;
+    int bpp;
+};
+
+#define bmp_load4byte(addr) ((addr)[0] + ((addr)[1]<<8) \
+                              + ((addr)[2]<<16) + ((addr)[3]<<24))
+#define bmp_load2byte(addr) ((addr)[0] + ((addr)[1]<<8))
+
+#define SWITCH_RGB 0x01
+#define SWITCH_LINE 0x02
+
+/* flat picture data adjusting function
+* description:
+*   switch the pixel RGB data sequence if flag is on
+*   switch the vertical line sequence if flag is on
+*   arrange horizontal pixel data, add extra space in the dest buffer
+*       for every line
+* requirement:
+*   bytes_per_line_src <= bytes_per_line_dest
+* note:
+*   src could be equal to dest, if did not want to switch vertical line
+*       sequence
+*/
+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);
+
+/* allocate decdata struct */
+struct bmp_decdata *bmp_alloc(void);
+
+/* extract information from bmp file data */
+int bmp_decode(struct bmp_decdata *bmp, unsigned char *data, int data_size);
+
+/* get bmp properties */
+void bmp_get_size(struct bmp_decdata *bmp, int *width, int *height);
+
+/* flush flat picture data to *pc */
+int bmp_show(struct bmp_decdata *bmp, unsigned char *pic, int width
+             , int height, int depth, int bytes_per_line_dest);
+#endif
diff --git a/src/bootsplash.c b/src/bootsplash.c
index cf1a603..e627365 100644
--- a/src/bootsplash.c
+++ b/src/bootsplash.c
@@ -12,7 +12,7 @@
 #include "jpeg.h" // splash
 #include "biosvar.h" // SET_EBDA
 #include "paravirt.h" // romfile_find
-
+#include "bmp.h"

/****************************************************************
  * VESA structures
@@ -109,7 +109,7 @@ enable_vga_console(void)

static int
 find_videomode(struct vesa_info *vesa_info, struct vesa_mode_info *mode_info
-               , int width, int height)
+               , int width, int height, int bpp_req)
 {
     dprintf(3, "Finding vesa mode with dimensions %d/%d\n", width, height);
     u16 *videomodes = SEGOFF_TO_FLATPTR(vesa_info->video_mode_ptr);
@@ -135,8 +135,13 @@ find_videomode(struct vesa_info *vesa_info, struct vesa_mode_info *mode_info
             || mode_info->y_resolution != height)
             continue;
         u8 depth = mode_info->bits_per_pixel;
-        if (depth != 16 && depth != 24 && depth != 32)
-            continue;
+        if (bpp_req == 0) {
+            if (depth != 16 && depth != 24 && depth != 32)
+                continue;
+        } else {
+            if (depth != bpp_req)
+                continue;
+        }
         return videomode;
     }
 }
@@ -148,17 +153,25 @@ enable_bootsplash(void)
 {
     if (!CONFIG_BOOTSPLASH)
         return;
+    /* splash picture can be bmp or jpeg file */
     dprintf(3, "Checking for bootsplash\n");
+    u8 type = 0; /* 0 means jpg, 1 means bmp, default is 0=jpg */
     int filesize;
     u8 *filedata = romfile_loadfile("bootsplash.jpg", &filesize);
-    if (!filedata)
-        return;
+    if (!filedata) {
+        filedata = romfile_loadfile("bootsplash.bmp", &filesize);
+        if (!filedata)
+            return;
+        type = 1;
+    }
+    dprintf(3, "start showing bootsplash\n");

-    u8 *picture = NULL;
+    u8 *picture = NULL; /* data buff used to be flushded to the video buff */
+    struct jpeg_decdata *jpeg = NULL;
+    struct bmp_decdata *bmp = NULL;
     struct vesa_info *vesa_info = malloc_tmplow(sizeof(*vesa_info));
     struct vesa_mode_info *mode_info = malloc_tmplow(sizeof(*mode_info));
-    struct jpeg_decdata *jpeg = jpeg_alloc();
-    if (!jpeg || !vesa_info || !mode_info) {
+    if (!vesa_info || !mode_info) {
         warn_noalloc();
         goto done;
     }
@@ -184,20 +197,48 @@ enable_bootsplash(void)
             vesa_info->vesa_version>>8, vesa_info->vesa_version&0xff,
             vendor, product);

-    // Parse jpeg and get image size.
-    dprintf(5, "Decoding bootsplash.jpg\n");
-    int ret = jpeg_decode(jpeg, filedata);
-    if (ret) {
-        dprintf(1, "jpeg_decode failed with return code %d...\n", ret);
-        goto done;
+    int ret, width, height;
+    int bpp_require = 0;
+    if (type == 0) {
+        jpeg = jpeg_alloc();
+        if (!jpeg) {
+            warn_noalloc();
+            goto done;
+        }
+        /* Parse jpeg and get image size. */
+        dprintf(5, "Decoding bootsplash.jpg\n");
+        ret = jpeg_decode(jpeg, filedata);
+        if (ret) {
+            dprintf(1, "jpeg_decode failed with return code %d...\n", ret);
+            goto done;
+        }
+        jpeg_get_size(jpeg, &width, &height);
+    } else {
+        bmp = bmp_alloc();
+        if (!bmp) {
+            warn_noalloc();
+            goto done;
+        }
+        /* Parse bmp and get image size. */
+        dprintf(5, "Decoding bootsplash.bmp\n");
+        ret = bmp_decode(bmp, filedata, filesize);
+        if (ret) {
+            dprintf(1, "bmp_decode failed with return code %d...\n", ret);
+            goto done;
+        }
+        bmp_get_size(bmp, &width, &height);
+        bpp_require = 24;
     }
-    int width, height;
-    jpeg_get_size(jpeg, &width, &height);
+    /* jpeg would use 16 or 24 bpp video mode, BMP use 24bpp mode only */

// Try to find a graphics mode with the corresponding dimensions.
-    int videomode = find_videomode(vesa_info, mode_info, width, height);
-    if (videomode < 0)
+    int videomode = find_videomode(vesa_info, mode_info, width, height,
+                                       bpp_require);
+    if (videomode < 0) {
+        dprintf(1, "failed to find a videomode with %dx%d %dbpp (0=any).\n",
+                    width, height, bpp_require);
         goto done;
+    }
     void *framebuffer = mode_info->phys_base_ptr;
     int depth = mode_info->bits_per_pixel;
     dprintf(3, "mode: %04x\n", videomode);
@@ -206,17 +247,29 @@ enable_bootsplash(void)
     dprintf(3, "bits per pixel: %d\n", depth);

// Allocate space for image and decompress it.
-    int imagesize = width * height * (depth / 8);
+    int imagesize = height * mode_info->bytes_per_scanline;
     picture = malloc_tmphigh(imagesize);
     if (!picture) {
         warn_noalloc();
         goto done;
     }
-    dprintf(5, "Decompressing bootsplash.jpg\n");
-    ret = jpeg_show(jpeg, picture, width, height, depth);
-    if (ret) {
-        dprintf(1, "jpeg_show failed with return code %d...\n", ret);
-        goto done;
+
+    if (type == 0) {
+        dprintf(5, "Decompressing bootsplash.jpg\n");
+        ret = jpeg_show(jpeg, picture, width, height, depth,
+                          mode_info->bytes_per_scanline);
+        if (ret) {
+            dprintf(1, "jpeg_show failed with return code %d...\n", ret);
+            goto done;
+        }
+    } else {
+        dprintf(5, "Decompressing bootsplash.bmp\n");
+        ret = bmp_show(bmp, picture, width, height, depth,
+                           mode_info->bytes_per_scanline);
+        if (ret) {
+            dprintf(1, "bmp_show failed with return code %d...\n", ret);
+            goto done;
+        }
     }

/* Switch to graphics mode */
@@ -231,7 +284,7 @@ enable_bootsplash(void)
     }

/* Show the picture */
-    dprintf(5, "Showing bootsplash.jpg\n");
+    dprintf(5, "Showing bootsplash picture\n");
     iomemcpy(framebuffer, picture, imagesize);
     dprintf(5, "Bootsplash copy complete\n");
     BootsplashActive = 1;
@@ -241,7 +294,10 @@ done:
     free(picture);
     free(vesa_info);
     free(mode_info);
-    free(jpeg);
+    if (jpeg != NULL)
+        free(jpeg);
+    if (bmp != NULL)
+        free(bmp);
     return;
 }

diff --git a/src/jpeg.c b/src/jpeg.c
index 1068291..a96a9d7 100644
--- 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;
     int max[6];

if (jpeg->height != height)
@@ -405,6 +405,9 @@ int jpeg_show(struct jpeg_decdata *jpeg, unsigned char *pic
     if (jpeg->width != width)
         return ERR_WIDTH_MISMATCH;

+    jpgbpl = width * depth / 8;
+    mloffset = bytes_per_line_dest > jpgbpl ? bytes_per_line_dest : jpgbpl;
+
     mcusx = jpeg->width >> 4;
     mcusy = jpeg->height >> 4;

@@ -434,18 +437,18 @@ int jpeg_show(struct jpeg_decdata *jpeg, unsigned char *pic
             switch (depth) {
             case 32:
                 col221111_32(jpeg->out,
-                             pic + (my * 16 * mcusx + mx) * 16 * 4,
-                             mcusx * 16 * 4);
+                             pic + (my * 16 * mloffset + mx * 16 * 4),
+                             mloffset);
                 break;
             case 24:
                 col221111(jpeg->out,
-                          pic + (my * 16 * mcusx + mx) * 16 * 3,
-                          mcusx * 16 * 3);
+                          pic + (my * 16 * mloffset + mx * 16 * 3),
+                          mloffset);
                 break;
             case 16:
                 col221111_16(jpeg->out,
-                             pic + (my * 16 * mcusx + mx) * (16 * 2),
-                             mcusx * (16 * 2));
+                             pic + (my * 16 * mloffset + mx * 16 * 2),
+                             mloffset);
                 break;
             default:
                 return ERR_DEPTH_MISMATCH;
@@ -887,6 +890,15 @@ static void initcol(PREC q[][64])

#endif

+#ifdef __LITTLE_ENDIAN
+#define PIC(yin, xin, p, xout)                   \
+(                                                \
+  y = outy[(yin) * 8 + xin],                     \
+  STORECLAMP(p[(xout) * 3 + 2], y + cr),         \
+  STORECLAMP(p[(xout) * 3 + 1], y - cg),         \
+  STORECLAMP(p[(xout) * 3 + 0], y + cb)          \
+)
+#else
 #define PIC(yin, xin, p, xout)                   \
 (                                                \
   y = outy[(yin) * 8 + xin],                     \
@@ -894,6 +906,7 @@ static void initcol(PREC q[][64])
   STORECLAMP(p[(xout) * 3 + 1], y - cg),         \
   STORECLAMP(p[(xout) * 3 + 2], y + cb)          \
 )
+#endif

#ifdef __LITTLE_ENDIAN
 #define PIC_16(yin, xin, p, xout, add)           \
diff --git a/src/jpeg.h b/src/jpeg.h
index a2ac501..2d08f45 100644
--- a/src/jpeg.h
+++ b/src/jpeg.h
@@ -5,7 +5,7 @@ struct jpeg_decdata;
 struct jpeg_decdata *jpeg_alloc(void);
 int jpeg_decode(struct jpeg_decdata *jpeg, unsigned char *buf);
 void jpeg_get_size(struct jpeg_decdata *jpeg, int *width, int *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);

#endif

[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 
> +*
> +* 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

[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
> > +*
> > +* 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

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

[Kevin O'Connor]

On Mon, Jul 04, 2011 at 04:11:11PM +0800, Wayne Xia wrote:
> 2011-7-3 6:57, Kevin O'Connor:
> > On Mon, Jun 27, 2011 at 03:10:38PM +0800, Wayne Xia wrote:
> > > +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.

My point is that the two parameters "bytes_per_line_dest" and
"switch_flag" are no longer needed, and therefore the function can be
simplified.
> > 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.

Thanks.

-Kevin

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

[Wayne Xia]

thanks for you patience, following is my comments.

2011-7-5 3:32, Kevin O'Connor :
> On Mon, Jul 04, 2011 at 04:11:11PM +0800, Wayne Xia wrote:
> > 2011-7-3 6:57, Kevin O'Connor:
> > > On Mon, Jun 27, 2011 at 03:10:38PM +0800, Wayne Xia wrote:
> > > > +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.
>
> My point is that the two parameters "bytes_per_line_dest" and
> "switch_flag" are no longer needed, and therefore the function can be
> simplified.

removed the switch_flag and other logic currently not needed. but
the "bytes_per_line_dest" was kept because the bmp decoder need it to
adjust the data location properly.
     also modified bmp.c a bit to optimize code size.
> > > 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.
>
> Thanks.
>
> -Kevin