[PATCH v2 0/2] Boot up with a splash picture configurable, comments

[Wayne Xia]

Sorry, forgot to sent this mail, following is my comments.
> On Wed, Jun 22, 2011 at 08:24:16PM +0800, Wayne Xia wrote:
> > Signed-off-by: Wayne Xia
>
> Thanks.  See my comments below.
>
> [...]
> > +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)
> >
> > +{
>
> It's still not clear to me what this function does.  It seems like it
> is adjusting for extra space in the framebuffer at the end of every
> horizontal line.  However, it's unclear why BMP would use SWITCH_LINE
> and JPEG would use SWITCH_RGB or why either is needed.

in my test, the line sequence of contents from the BMP file need to be
reversed to show correctly, and the jpg decoder's output have a
different RGB order also. And some video mode have some more bytes per
line than the picture data, so the function adjusted it.
> The purpose of jpeg_show (and bmp_show) is to decode the picture - if
> the picture isn't decoded properly then I think the xxx_show()
> function should be fixed instead of re-copying the picture to correct
> the decoding.

now modified the bmp and jpeg decoder instead of re-copying it.
> [...]
> > +typedef struct tagBITMAPFILEHEADER {
> > +u8 bfType[2];
> > +u8 bfSize[4];
>
> Indentation.

sorry for that..., changed
> > void
> >   enable_bootsplash(void)
> >   {
> > if (!CONFIG_BOOTSPLASH)
> > if (!CONFIG_BOOTSPLASH) {
> >      return;
> > }
>
> Unneeded style change.

changed.
> [...]
> > int width, height;
> > jpeg_get_size(jpeg,&width,&height);
> > bpp_require = 24;/* for better vision effect, use 24 bpp mode */
>
> This could break existing users that use 16bit or 32bit modes.

changed, now jpg file remains the same process as before the patch, but
I modified the jpg decoder a little to handle the situation that line
size of video buffer is different with the image buff's, and reversed
the RGB sequence in 24 bpp video mode.
> -Kevin