This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#5697 - bmp resize patch
Attached to Project:
Rockbox
Opened by takka (takka) - Sunday, 23 July 2006, 03:05 GMT+2
Last edited by Matthias Mohr (aka Massa) (mmohr) - Monday, 23 October 2006, 11:10 GMT+2
Opened by takka (takka) - Sunday, 23 July 2006, 03:05 GMT+2
Last edited by Matthias Mohr (aka Massa) (mmohr) - Monday, 23 October 2006, 11:10 GMT+2
|
DetailsThis patch adds the ability to automatically resize bitmaps to a given size when loading from disk.
With this patch the caller of the function "read_bmp_file" is able to tell the function if a resizing is wanted and for which dimension it is wanted. The original patch was made by takka - who doesn't seem to be around anymore.. Since version v0.99.6 I maintain it completely and in that version I rewrote the resizing algorithm. It's now really simple (but with our small LCDs it should not have any noticeable affect): if a bitmap dimension is bigger than the given max-size, it will just skip some pixels. if a bitmap dimension is smaller than the given size, it will duplicate some pixels. If you think that's not good enough, point me to a page where a good resizing algorithm is described in detail (without floating point calculations) and I'll try to implement this - although I assume that algorithms which give better looking results are also more complicated to implement and also consume more CPU time... So I'm not sure if it would be worth the effort! The current implementation limits the maximum bitmap width to 1024 pixel (the height is not limited)! This may change in future (but only if needed)... takka initially also added a BMP viewer as an usage example - I put it in a separate patch because it's only an example which lacks some features, is not necessary for the usage of the resizing feature and it had not been maintained since takka's initial release! Currently the resize patch (but not the bmpviewer) is used (and needed) by the albumart patch ( Enjoy and tell me if something does not work ;-) (the newest patch can always be found at the end of this page...) |
This task depends upon
http://www.rockbox.org/tracker/task/3045
I've just played around a bit with it and it seems to work just fine.
The BMP viewer is pretty nice too, but it could use a few improvements if it were to become Rockbox's main bitmap viewer.
Also, it might be better if the ROCKPAINT_* macros in the bmpviewer would be renamed to BMPVIEWER_x.
clean up source code.
add a few comments.
bmpviewer.c
ROCKPAINT_* macro rename BMPVIEWER_*
I haven't tested it yet but i have some questions/comments:
* does the change in rockpaint.c mean that smaller than screen bmp.c will be resized when opened ?
* your scaling patch seems to work on all targets but the bmpviewer doesn't have button definitions for all targets. You might want to add some.
* if loading the bmp fails in the viewer, you should quit instead of displaying a white screen.
* In the long run it might be better to have 1 image viewer with support for all image formats (jpeg, gif(i saw a patch somewhere in the tracker), bmp, png(i'm planing on porting libpng), ...)
Btw, copyright for rockpaint is 2006 and not 2005 (and please use my real name instead of my nick, or better, keep the real name + the email address).
yes.Please change as follows if you do not want to resize it.
ret = rb->read_bmp_file( file, &bm, ROWS*COLS*sizeof( fb_data ),
FORMAT_NATIVE, 0, 0);
>>* your scaling patch seems to work on all targets but the bmpviewer doesn't have button definitions for all targets. You might want to add some.
>>* if loading the bmp fails in the viewer, you should quit instead of displaying a white screen.
It is a test version of the resize. It will improve it in the future.
>>* In the long run it might be better to have 1 image viewer with support for all image formats (jpeg, gif(i saw a patch somewhere in the tracker), bmp, png(i'm planing on porting libpng), ...)
I also think so.
>>Btw, copyright for rockpaint is 2006 and not 2005 (and please use my real name instead of my nick, or better, keep the real name + the email address).
Sorry.Is it good in this though corrected?
It is thought that it is possible to display it without changing if it is smaller than the screen.
BUG fix:Mono bmp some pixels missing
THANKS Ashen.
buffer over fix.
include buffer_Xtension.
I am newly rewriting the source now.
Please wait for Ver.2
could you please add two additional parameters to read_bmp_file
with which it is possible to tell the resize mechanism to only
resize if
1.) bitmap dimension is bigger than given display dimension
or
2.) bitmap dimension is smaller than given display dimension
or
3.) combination of 1.) and 2.) will resize if smaller or bigger
e.g. define additional flags in gwps.h:
#define WPS_RESIZE_INCREASE 8
#define WPS_RESIZE_DECREASE 16
a call to read_bmp_file could then look like follows:
rc = read_bmp_file(filepath, &temp_bmp, sizeof(temp_bmp),
FORMAT_ANY|FORMAT_TRANSPARENT,
maxwidth, WPS_RESIZE_INCREASE|WPS_RESIZE_DECREASE,
maxheight, WPS_RESIZE_INCREASE|WPS_RESIZE_DECREASE);
(I'm currently in progress of a small update to albumart
and could need such a feature ;-)
IMHO it's not correct to check against LCD_WIDTH/LCD_HEIGHT
inside read_bmp_file.
Why?
Because you could (theoretically) load bitmaps to different
screens with different sizes.
e.g. it should be possible to load a greyscale image to an
iRiver LCD-remote (which definitely has other dimensions
than LCD_WIDTH/LCD_HEIGHT ;-)
Although I don't know if somebody is really loading bitmaps
to the remote screen...
And I second the suggestion of Antoine that we should know
if the bitmap has been resized during load or not
(for each direction)
how about "width_orig" and "height_orig" elements inside
the bitmap structure?
P.S.: maxwen, thanks that you synced the patch to newer
CVS! But IMHO it should only change its version number
if something changed...
So for CVS-syncs it would be much better if you just use a
timestamp addition to the original version.
(e.g. bmp_resize_v0.99.2_20060812.patch)
I will do it correct next time :-)
int read_bmp_file(char* filename, /* file name */
struct bitmap *bm, /* return data */
int maxsize, /* return data size */
int format, /* FORMAT_MONO/FORMAT_NATIVE/FORMAT_ANY | FORMAT_TRANSPARENT */
int src_x1, /* sorce start x */
int src_y1, /* sorce start y */
int src_x2, /* sorce end x */
int src_y2, /* sorce end y */
int dst_width, /* destination width */
int dst_height, /* destination height */
int dst_x1, /* destination start x */
int dst_y1, /* destination start y */
int dst_x2, /* destination end x */
int dst_y2, /* destination end y */
int ratio, /* resize ratio * 1024 (ex. ratio=2.0 -> 2048, ratio=0.5 -> 512) */
int resize_mode /* AUTO(Ratio preservation)/FIT/REDUCE_ONLY/NO_RESIZE */
)
Why should positioning of the bitmap be part of the bitmap loading?
And what sense does the ratio make?
Normally there is a bitmap size and a size to which the bitmap should fit.
I would be able to calculate a ration when I know about both sizes.
But
a) the bitmap dimensions are not known before you load the bitmap.
b) why should I want to calculate a ratio? The loading routine could take care of it.
So I can't see any advantage for a ratio...
Maybe I misinterpreted your idea???
And I still like my idea to differentiate between width resizing and height resizing
and enable the possibility to only resize one dimension...
The read_bmp_file function has now the following signature:
int read_bmp_File(char *filename,
struct bitmap *bm,
int maxsize,
int format,
int dst_xsize,
int dst_x_resize,
int dst_ysize,
int dst_y_resize);
the resize parameters may be set to:
BMP_RESIZE_NONE - don't resize
BMP_RESIZE_INCREASE - only resize if bitmap size is smaller than given size
BMP_RESIZE_DECREASE - only resize if bitmap size is bigger than given size
BMP_RESIZE_INCREASE | BMP_RESIZE_DECREASE - resize if bitmap size != given size
What do you think about?
make[2]: *** No rule to make target `/src/iriver/rockbox/bla/build/apps/plugins/bmpviewer.rock', needed by `all'. Stop.
make[1]: *** [rocks] Error 2
I have
- current cvs
- bmp_resize_v0.99.4-MM.20060820
- wps_preload_tags_20060820
- album_art_v5.1.0_TEST-MM_20060820
Any ideas?
Ciao Norbert
Here is an update version (nothing else changed)
when either dst_x < 0 or dst_y < 0.
Shouldn't be noticed by anybody (except myself :-)
@takka: could it be that the resizing does not work always?
I tested it with several bitmap sizes and destination sizes
and sometimes I got strange behaviour (no resize at all or
a resize to another size as I set in dst_x, dst_y)
The resulting dst_x and dst_y (which will be set in the bmp
structure as width and height) are always set to the given
sizes, even if the bitmap (after resizing) is smaller than
dst_x and dst_y!
BTW, why _IS_ it smaller?
Increasing smaller bitmaps also seems not to work.
Is this on intend or is it a bug?
and takka did not answer I had a deeper look at the sources.
I did not understand takka's code (I'm no graphics specialist)
I decided to rewrite the complete resizing code.
So although I call it v0.99.6 it's my first version with completely
different resize code!
The read_bmp_file function looks as follows:
int read_bmp_file(char* filename,
struct bitmap *bm,
int maxsize,
int format,
int dst_maxwidth,
int dst_width_resize, // BMP_RESIZE_INCREASE | BMP_RESIZE_DECREASE
int dst_maxheight,
int dst_height_resize); // BMP_RESIZE_INCREASE | BMP_RESIZE_DECREASE
(actually the same as in v0.99.5 :-)
the given maxwidth/maxheight are (as the name says) only maximum width/heights which
may not be exceeded when resizing.
It are _not_ the sizes which the resized bitmap will get!
The width/height ratio of the original bitmap will always be kept -
so normally only one dimension of the resized bitmap will be the given maxsize,
the other will calculated accordingly to the original width/height ratio...
Increasing and Decreasing should both work and the correct bitmap dimensions will
be given back in the bitmap structure.
I also hope the source code is easy enough to be understandable by other developers ;)
The only thing missing is to optimize and hopefully combine some code parts.
But that can wait...
Tell me if it's working for you or not...
Synched with today's CVS and splitted into two patches:
One for the resizing code and the other to add the bmpviewer.
The resizing code is (and was) platform independent, but the
bmpviewer code is missing keycode defintions for several platforms.
So it will currently not compile for several platforms (e.g. H10).
The bmpviewer is only example code and not necessary needed so I decided
to put it in an extra patch which has been applied on top of the resize
patch if wanted.
IMHO it should somehow be integrated in the jpegviewer...
(which then would extend it into a "pictureviewer" ;)
bmp_resize_v0.99.3.patch when resizing to a lower resolution
e.g. resizing 120x120 to 66x66
The result is much more "pixelated" (best descriptions I can give :))
- sorry that it took a while - at the moment I'm really busy
with payed work and therefore nearly have no spare time for
rockbox :-(
@maxwen: v0.99.6 uses a complete different algorithm for the resizing.
(as I wrote above, I needed a complete working resizing algorithm
for the albumart; takka's version did not work properly in certain
situations and was unable to understand his algorithm. And it seems
he is no longer available)
I don't have any knowledge of graphical resizing algorithms so I choosed
the easiest which came to my mind: just duplicate pixels when increasing
and remove pixels when decreasing.
I know it's a really lame algorithm but I don't thought it could be noticed
with that small bitmaps...
@Angyman: what do you mean with "Cpu consumption raised"? Compared to what?
If you mean compared to using bitmaps which do not need to be resized it's
normal.
The bigger the original bitmaps are, the more CPU work has to be done...
Maybe another resize algorithm would take less CPU - I don't know one!
@all: point me to the description of resizing algorithms (without floating
point calculations of course) and I'll try to implement them -
although I assume that algorithms which give better looking results are
also more complicated to implement and also consume more CPU time...
So I'm not sure if it would be worth the effort!
Umpf, the change 15 Oct 17:50 Mike Sevakis did break it again. Could you resync onc more? Thanks a lot.
Bye
Norbert
If I read the code correctly pressing the select and menu buttons at the same time should quit but it dose not work here.
It has not been maintained since his initial release...
(that was one of the reasons why I keep it separate from the resize patch)
Sync to todays CVS...
(should now be able to build again...)
I started to do this - but it's more complicated than I thought and my spare
time is currently _very_ limited, so it may still take some time...
@Angyman: it's not your failure, it's currently not possible to use the patch.
base of a newer bmp.c.
Maybe somebody else (Nicolas :-) could find some time to do it...
(or you have to wait a bit longer until I find some more time)
Matthias, if you plan on rewriting the patch, i suggest you ask the main devs how they want it done rather than simply redoing it the same way as before :)
In addition to originally implemented increase/decrease feature flags implemented two more - "fill area" and "ignore aspect ratio". Firest option will make sure that area will be used. Some portions of image can be stripped in process. With "ignore aspect" flag on entire screen area will be used, but image can be stretched(or squezed, if you want :-) in one direction. To see how it works you can use bmpviewer.c, where you can control all that options.
I tried to compile with these patches but found three things:
- First in gwps-common.c two times the read_bmp_file is called in the wrong way, around line 209 and 323
- (out of sync) the bmpviewer.c should remove the flags at line 141 in the call to rb->creat (see http://svn.rockbox.org/viewvc.cgi/trunk/firmware/font.c?r1=12178&r2=12179)
- finally, something is going wrong with the prototyping/headers, because *within* a plugin the read_bmp_file seems to be defined in a different way, compiling bmpviewer.c gives an error: Too many arguments, but they are right.
But thanks a lot for the update and all the best
Norbert
OBJCOPY viewer.rock
make[2]: *** No rule to make target `/home/rockbox/build-video/apps/plugins/
bmpviewer.rock', needed by `all'. Stop.
- I didn't have time to have a look at, but I'll do it sooner or later :-)
Did you talk with the main devs (as Nicolas suggested) what their concerns about my older version were
and how they want it do be done?
@takka: Hey, you're back again - that's really good :-)
Now you could describe your resizing method or even better
write a new patch based on your algorithm...
I also suggest to change the jpegviewer to also support bmp files instead of writing an own bmpviewer
But this is only my suggestion - the main devs should be asked about this, too...
No, I have not talk to anyone in main devs team. I guess they does not care much about album art anyway.
Personally, I need AlbumArt from id3 tags support. So, in background I am moving JPEG decoder in the kernel.
Then BMP reader as is can handle read from mp3 file, because core function accept open file handle from current position, so you can set position in mp3 file to start of image data and ask BMP reader to get theat image.
help me..
with faild message file send it..
24.patch = bmpresize_20070204-broken.patch
25.patch1 = bmpresize_20070204-broken2.patch
um..
and making gwps-common.c.rej
***************
*** 373,379 ****
gwps->state->id3->albumart_path );
rc = read_bmp_file(gwps->state->id3->albumart_path,
&temp_bm, data->img_buf_free,
- FORMAT_ANY|FORMAT_TRANSPARENT);
if (rc <= 0) {
gwps->state->id3->albumart_data = NULL;
strcpy(data->cur_displayed_albumart_path, "");
--- 373,381 ----
gwps->state->id3->albumart_path );
rc = read_bmp_file(gwps->state->id3->albumart_path,
&temp_bm, data->img_buf_free,
+ FORMAT_ANY|FORMAT_TRANSPARENT,
+ gui_wps->data->albumart_max_width, gui_wps->data->albumart_max_height,
+ BMP_RESIZE_INCREASE | BMP_RESIZE_DECREASE);
if (rc <= 0) {
gwps->state->id3->albumart_data = NULL;
strcpy(data->cur_displayed_albumart_path, "");
what's wrong?
I solve it..
but build folder type make.. i see error..
what's wrong?
Or would that be a bug with the album art patch?
Otherwise solution is to inject support for different image formats in kernel. I am fine with this, but am I sure core developers will strongly disagree.
To use the patch first apply album_art.patch from http://www.rockbox.org/tracker/task/3045#comment13690
Then my attached re-synced patch.
This should fix a warning on most targets, and an error on the Gigabeats.
(and I'm a couple of days behind - tested on svn r12944.)
Had to remove two patches to gwps-common.c and added one to wps_parser.c
(be nice, this is my first flyspray upload)
Hunk #4 FAILER at 250
Please look at the patch about details.
I tested customized iconsets, and it works fine...
file: /apps/gui/gwps-common.c
Hunk #1 failed at 596
Is there something I can do about that?
file: /apps/gui/gwps-common.c
Hunk #1 failed at 592
What could I do to fix this?
Anybody else being succesful ?
I'd like to make a build for the gigabeat with both Zune Thelme support and the new videoplayer, this patch is the only thing that still prevents me from succeeding.
Thanks for your help !
No functionality changes for the patch...
The current resizing method makes the album art images appear jaggy.
It can be solved if you resize the images with an image editor, but themes don't use a strict width and height and therefore image editing isn't a solution.
But I'm so busy and I have no time to try it...
if (src_h < 0) { /* Top-down BMP file */
src_h = -src_h;
rowstart = 0;
rowstop = src_h;
rowstep = 1;
} else { /* normal BMP */
rowstart = src_h - 1;
rowstop = -1;
rowstep = -1;
}
and used rowstart etc. as they were originally used in the declaration of the for loop to put it in the buffer in bmp.c.
This patch only impacts albumart resize (and not for all bmp function).
This patch does not have any specific big buffer...
@nico: Thank you for these advices.
I can improve my implementation by that.
No functionality changes.
Additional flags are the following.
maxwidth:
- d: decrease image
- i: increase image
- s: decrease or increase image
maxwidth:
- d: decrease image
- i: increase image
- s: decrease or increase image
ex:
%Cl|0|0|s100|s100|
%C
If you try viewport-build with this patch,
please refer to the latest jClix_Night_VP.zip at
http://www.rockbox.org/tracker/task/8385?histring=viewport
FS#8385on January 5. While I'd agree that it seems to correct the .bmp resizing, the theme has other problems on my Sansa. But I'll poke around in the .wps file for learning purposes. Thanks.