FS#5697 - bmp resize patch
Attached to Project:
Rockbox
Opened by takka (takka) - Sunday, 23 July 2006, 01:05 GMT
Last edited by Dave Chapman (linuxstb) - Thursday, 02 October 2008, 20:45 GMT
Opened by takka (takka) - Sunday, 23 July 2006, 01:05 GMT
Last edited by Dave Chapman (linuxstb) - Thursday, 02 October 2008, 20:45 GMT
|
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
Closed by Dave Chapman (linuxstb)
Thursday, 02 October 2008, 20:45 GMT
Reason for closing: Rejected
Additional comments about closing: The general view amongst the Rockbox developers is that this feature can and should be implemented by resizing the bitmap as it is being loaded.
This patch implements the feature by loading the complete bitmap into RAM and then resizing it, hence it's being rejected.
Discussion concerning the decision to close this specific patch can be found in the IRC logs here:
http://www.rockbox.org/irc/log-20081002# 22:09:30
Thursday, 02 October 2008, 20:45 GMT
Reason for closing: Rejected
Additional comments about closing: The general view amongst the Rockbox developers is that this feature can and should be implemented by resizing the bitmap as it is being loaded.
This patch implements the feature by loading the complete bitmap into RAM and then resizing it, hence it's being rejected.
Discussion concerning the decision to close this specific patch can be found in the IRC logs here:
http://www.rockbox.org/irc/log-20081002# 22:09:30
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.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.is this on purpose or a bug?
so i wonder if its on purpose :-)
No functionality changes.
If you try to use this patch,
please also apply the "smooth_resize.patch" at
FS#8308- Port of a imlib2 based smooth scaling algorithm for bitmaps.Question/suggestion though: maybe the "s" flag can be applied by default for .wps that lacks the flag.
So please also apply above patch...
Anyway works great on theme without "s" flag preloaded. Tested all three of those patches on pretty big album art 176x176 resized to 100x100 and the picture still good.
So please apply this one.
#And I found aa align flags bug...
and add "n" (previous default) flag...
- d: Decreasing
- i: Increasing
- s: Sizing (decrease or increase) (default)
- n: No sizing (previous default)
FS#8308patching this file (with the old name).If you use sansa e200 or something else, please try that...
I hope that this patch will resolve the slowness of the WPS...
Noticeable improvement - Don't stuck at all anymore, at least for me.
If the AA is smaller than 100px:
• 's' flag - no AA at all.
• 'd,i,n' flags - real size AA (cuts the AA if the .wps states smaller) no decreasing.
I hope that the accidental buggy pixelization will be gone...
@sanusart:
I guess that you change WPSes heavily.
Until a new WPS's album art buffer is loaded,
the previous WPS's album art buffer is used.
(Or if the previous WPS has no album art tag, no album art is drawn
until a new song's album art image is buffered.)
This is the "original" Album Art draw behavior.
To reproduce my experience:
All my AA is exactly 176x176 pixel. The size of AA stated on WPS is 90x90 pixel.
I 99.5% sure my WPS syntax is fine (0.5% not though). Theme file used is there: http://www.sanusart.com/sansa/ThePanel_lists.zip
@sanusart:
I see... That's maybe original aa align flags bug
that I found at coding "apply s flag patch".
If you want to use "b/t" or "r/l" flags,
please apply the "aa_align.patch" at
FS#8517- Fix Album art align flagshttp://www.rockbox.org/tracker/task/8517
CC bookmark.c
In file included from bookmark.c:37:
recorder/icons.h:30:25: error: rockboxlogo.h: No such file or directory
make[1]: *** [/home/jacob/rockbox/build/apps/bookmark.o] Error 1
make: *** [build] Error 2
@jac0b:
Please refer to the above comment
http://www.rockbox.org/tracker/task/5697#comment21000
To reproduce (Sansa e200): Take a 176x176 album art file, and try to load it into Azure_Ultimate (found on rockbox-themes.org).
albumart-smooth_resize-080207.patch is the last working version.
Thanks it compiles now.
The WPS buffer is no longer used.
So I hope that this patch will fix the problem
related to the lack of the WPS buffer.
@TO ALL
If you want to use "apply_s_flag-3.patch"
with album art align flags(|t|c|b| / |r|c|l|),
Please also apply the "aa_align.patch" at
http://www.rockbox.org/tracker/task/8517?getfile=15795
FS#8517- Fix Album art align flagsIt's a clear rockbox's bug.
But why no one wants to commit this patch ASAP?
"albumart-smooth_resize", "aa_align", "apply_s_flag-3" and "smooth_resize"
but the "AA smaller than 100x100 scaling issue" that i mentioned earlier here is still present.
Besides that, there's an issue with the apply_s_flag patch. It does indeed apply s the s flag by default. But only unless you have a align fag assigned. In the case of sanusart:
%Cl|17|86|c90|c90| <-- in this case the s flag doesn't apply and have to be added manually, since the c flag is there.
IMO, the aligning flags are obsolete with bmp resizes, we might consider removing them.
And revert "N" flag, it is for debugging.
This patch is no need to apply "apply_s_flag-3.patch" (and aa_align.patch)
if i understand right the AA data is put into the buffer in full size and is then rescaled when displayed, correct?
if so, wouldnt it make more sense to resize the AA when putting it into the buffer (to save buffer space) and thus allow the support of bigger AA without draining the buffer-space?
this also would save some time when displaying the AA from buffer (as it is smaller and can be put to the screen unscaled)
Thank you for commenting.
> if i understand right the AA data is put into the buffer in full size and is
> then rescaled when displayed, correct?
No, it's the old approach.
> if so, wouldnt it make more sense to resize the AA when putting it into the
> buffer (to save buffer space) and thus allow the support of bigger AA without
> draining the buffer-space?
> this also would save some time when displaying the AA from buffer (as it is
> smaller and can be put to the screen unscaled)
The recent patches which I call "MoB version" exactly do the above
things, I believe ;).
(mind you, not when pulling the bmp from the MoB to display it).
i ask because i remember someone saying that the size is limited to 150 (or 300 when patched) because otherwise too much space is required in the buffer.
so technically you dont need to restrict the size of the bmp, is that correct? if so then you should propably remove the size restriction.
> from disk to the MoB?
> (mind you, not when pulling the bmp from the MoB to display it).
That's right.
> i ask because i remember someone saying that the size is limited to 150 (or
> 300 when patched) because otherwise too much space is required in the buffer.
> so technically you dont need to restrict the size of the bmp, is that correct?
> if so then you should propably remove the size restriction.
First of all, that restriction (bitmap width limitation) does NOT depend on my
album art resize patch.
It is the current rockbox's restriction (at loading bmp function).
And takka's bmpresize patch changes that restriction
(It changes bmp width limitation from LCD_WIDTH to "1280").
The following patch also changes bmp width limitation from LCD_WIDTH to
"1280" (So no need to apply bmp_wide_width.patch).
# But, I do NOT think that it is the right approach.
and thus: can it be left out completely when using bmp-resize?
this doesn't appear to be compiling on current SVN.
Not sure why it patches fine however it errors out during make
it finishes with the following error message:
In file included from bookmark.c:37:
recorder/icons.h:30:25: error: rockboxlogo.h: No such file or directory
make[1]: *** [/home/johannes/rockbox/build/apps/bookmark.o] Error 1
make: *** [build] Fehler 2
No functionality changes.
@Ubuntuxer:
Please refer to the above comment
http://www.rockbox.org/tracker/task/5697#comment21554
Now, I can compile it. Thank you very much.:-)
but unfortunately it seems to me, that it doesn't work well on my Sansa e250.
The covers are doesn't shown always;
Although I play the same album, so the same cover should shown, just sometimes at the same songs the covers are shown. This is a bit strange, isn't it.
But the covers, which have a resolution of 130x130, are always shown.
Perhaps I still have done something wrong, so I tell you briefly what I've done.
I'm using the latest svn version, so I needn't Patch
FS#3045, right?Also I needn't the bmp_wide_width.patch not any more.
So I just have to install the Paches album-smooth_resize and smooth_rezise_Patch.
I'm using the Themes Azure_Ultimate and the default Theme cabbiev2, which support cover display and I needn't configure somthing there.
I hope you bear with me.
I have included that patch in my build, but many users reported "*Panic* Stkov", crashes and freezes with bigger bitmaps. Also, there's actually no reason to have bitmaps that are bigger than the LCD resolution. It only reduces the shown information and they need alot of more disk space.
I think so too (I think that removing the restriction of bmp width is a _bad_ approach).
This patch reverts that restriction (bmp width restriction is LCD_WIDTH).
If someone want to change that restriction, please also apply the above patch.
http://www.rockbox.org/tracker/task/5697#comment21118
You can see it yourself: Take a SVN, build and note the file size of the rockbox.<target>, then apply this patch and do the same.
Nico_P made some effort bring smooth scaling into the core, but he stopped for some reason (he probably had to focus on playback bugs).
(smooth_resize_bitmap() and simple_resize_bitmap())
into the core,
and makes plugins use that core resize function.
The file size of the core (ex. rockbox.ipod) increases about 4KB,
but each file size of plugins using smooth_resize_bitmap() decreases about 4KB.
(ex. sliding_puzzle.rock and ppmviewer.rock )
Second patch is almost the same as old one.
So please apply both patches.
Also, every theme that works with the svn, will not contain any of the [d|i|s|n] tags, which means they're resized by default anyway.
4KB seems a lot of code for one feature - in my opinion you should look at the code and try and see if you can optimise it for size (without sacrificing speed). For example, the RGB_UNPACK_* macros will generate a fair amount of code for each invocation.
My obvious question is that the resize function is really 4 functions - upscaling, downscaling, downscaling in vertical direction only, and downscaling in horizontal direction only. Do we need the two special cases for downscaling in one direction only (which I can't imagine being needed often enough to deserve special-cases) ? Would the generic downscaling break in that case?
This patch does not move (but rather remove it all) simple_resize for color targets, since smooth scaling is much better IMHO. By doing this, I unified the usage (the functions are now called resize_bmp). This should also save a couple of bytes for color targets.
I edited test_resize, so that PLA_MENU doesn't change the algorithm (which isn't possible anymore), but changes the scaling direction, so that you can now also resize vertically.
Also, I'd like to mention, that this will not work with bmp resize patch idak posted today, since I renamed the functions to resize_bitmap.
I could imagine the aligning options could be removed too.
With both patches (my versions), I get a ~3KB bin size increase, while idak's patches add 5,5KB.
I think there's still room for more optimizing, but I doubt it'll be much. Further optimization will probably not save more than 300 (assuming the resize algorithm itself cannot be optimized more).
Here's the statistic I collected today (e200) I compared the rockbox in build/apps, not the rockbox.mi4:
svn's bin size: 571928 Bytes
smooth scaling in the core (idak's version): +4720
smooth scaling in the core + albumart resize (both idak's versions): 5632
smooth scaling in the core without special cases (my version): 2736
smooth scaling in the core without spacial cases + album art resize without options (both my versions): 3120
I'll keep the bins (and the .map files) for reference.
Note: the attached patch is meant to be applied with my patch, since it also has smooth_resize_bmp renamed to resize_bmp, but it should generally also work with idaks resize_into_core (and vice versa).
kugel:
Album art tag has the possibility that is not the square (ie %Cl|0|0|80|100|).
And I think the following code is maybe bad.
+ /* keep aspect ratio, discuss whether this is wanted or not */
+ if (orig_bmp.width > orig_bmp.height)
+ aa_bmp->height = aa_bmp->height * orig_bmp.height / orig_bmp.width;
+ if (orig_bmp.width < orig_bmp.height)
+ aa_bmp->width = aa_bmp->width * orig_bmp.width / orig_bmp.height;
I'm struggling to follow the history of this patch (so apologies if I've missed it), but can't see an explanation for why you changed the way this feature is implemented. i.e. from being part of the read_bmp() function (resizing-on-load) to a standalone function that requires the whole of the original image to be loaded first.
The general view of the Rockbox devs seems to be that resize-on-load is how this should work - see the discussion in IRC today, starting here:
http://www.rockbox.org/irc/log-20080803#19:24:10
we could do in Album Art resizing feature.
(2) Related to (1), I thought it was easy to maintain and change the code
separated the bitmap resizing function from the bitmap loading function.
#At that time, I didn't know that the pre-allocated buffer got some problems.
If we can resize-on-load with smooth scaling, it's more suitable than my patch.