Rockbox

Tasklist

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
Task Type Patches
Category LCD
Status Closed
Assigned To Matthias Mohr (aka Massa) (mmohr)
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 5
Private No

Details

This 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 ( FS#3045 )!

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
Comment by takka (takka) - Sunday, 23 July 2006, 20:15 GMT
"album art wps" need album_art_v5.00.patch
http://www.rockbox.org/tracker/task/3045
Comment by Nicolas Pennequin (nicolas_p) - Monday, 24 July 2006, 00:46 GMT
This patch looks great !
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.
Comment by Linus Nielsen Feltzing (linusnielsen) - Monday, 24 July 2006, 08:03 GMT
Looks interesting. I know that the BMP reader is almost uncommented, but I would appreciate if your scaling code would have at least one or two comments telling what the code does.

Also, it might be better if the ROCKPAINT_* macros in the bmpviewer would be renamed to BMPVIEWER_x.
Comment by takka (takka) - Monday, 24 July 2006, 13:00 GMT
bmp resize patch var.0.99
clean up source code.
add a few comments.

bmpviewer.c
ROCKPAINT_* macro rename BMPVIEWER_*
Comment by Antoine Cellerier (dionoea) - Monday, 24 July 2006, 14:01 GMT
Looks neat :)

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).
Comment by takka (takka) - Monday, 24 July 2006, 14:25 GMT
>>* does the change in rockpaint.c mean that smaller than screen bmp.c will be resized when opened ?
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?

Comment by Antoine Cellerier (dionoea) - Monday, 24 July 2006, 14:39 GMT
About the resize thing in rockpaint, i think that the best way to handle it would be to not resize smaller than screen bmps (i have to add code to make it possible to save non screen sized bitmaps) and maybe resize bigger than screen bitmaps ... but in the long run i'd want to handle bigger than screen bitmaps without rescaling in rockpaint (i'm not sure what they'd be usefull for on rockbox though).
Comment by takka (takka) - Monday, 24 July 2006, 14:54 GMT
To the read_bmp_file() function in the upcoming version, ExpansionOnly/ReductionOnly/Automatic select argument add.
It is thought that it is possible to display it without changing if it is smaller than the screen.
Comment by takka (takka) - Monday, 24 July 2006, 17:15 GMT
Ver.0.99.1
BUG fix:Mono bmp some pixels missing

THANKS Ashen.
Comment by takka (takka) - Wednesday, 26 July 2006, 22:55 GMT
Ver.0.99.2
buffer over fix.
include buffer_Xtension.

I am newly rewriting the source now.
Please wait for Ver.2

Comment by Antoine Cellerier (dionoea) - Friday, 28 July 2006, 14:36 GMT
I just though about something that could be usefull: make it possible to know if the bitmap was scaled or not when loading it (like in the return value or a flag in the bitmap structure).
Comment by Max Weninger (maxwen) - Saturday, 12 August 2006, 14:55 GMT
Updated patch to compile against CVS HEAD 12.08.2006
Comment by Matthias Mohr (aka Massa) (mmohr) - Sunday, 13 August 2006, 11:43 GMT
takka, I have also a wish:
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 ;-)
Comment by Matthias Mohr (aka Massa) (mmohr) - Sunday, 13 August 2006, 12:11 GMT
another thing:
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)
Comment by Max Weninger (maxwen) - Sunday, 13 August 2006, 12:17 GMT
Sorry for that!
I will do it correct next time :-)
Comment by takka (takka) - Sunday, 13 August 2006, 19:44 GMT
I have such an idea.

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 */
)
   bmp1.JPG (19.5 KiB)
Comment by Matthias Mohr (aka Massa) (mmohr) - Sunday, 20 August 2006, 09:17 GMT
Sorry to say that - but your idea seems way to complicated to me.

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...
Comment by Matthias Mohr (aka Massa) (mmohr) - Sunday, 20 August 2006, 16:13 GMT
I implemented my suggestion and created a new patch version.
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?
Comment by Norbert Preining (norbusan) - Monday, 21 August 2006, 07:54 GMT
Build failure with current cvs:
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
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 21 August 2006, 09:55 GMT
my failure - I forget to add the new file bmpviewer.c to the patch...

Here is an update version (nothing else changed)
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 21 August 2006, 15:43 GMT
Here is another version which fixes a small bug
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?
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 21 August 2006, 15:47 GMT
Ooops - I forgot to attach the file.
Comment by Matthias Mohr (aka Massa) (mmohr) - Friday, 01 September 2006, 21:58 GMT
Because I needed a completely working bmp_resize patch
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...
Comment by Matthias Mohr (aka Massa) (mmohr) - Wednesday, 13 September 2006, 21:43 GMT
Here's the same version as above.
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" ;)
Comment by david (dave.d.x) - Wednesday, 11 October 2006, 17:28 GMT
bmp_resize_v0.99.6-MM.20060913.patch do not work with current CVS (20061011) :(
Comment by Frederik (freqmod) - Wednesday, 11 October 2006, 20:18 GMT
My bmp.c synced from cvs with bmp_resize patch bmp_resize_v0.99.6-MM.20060913.patch and 64 bit datatype fix.
   bmp.c (18.5 KiB)
Comment by david (dave.d.x) - Thursday, 12 October 2006, 18:54 GMT
Frederik, pls, how apply it? Thanks.
Comment by Max Weninger (maxwen) - Thursday, 12 October 2006, 19:41 GMT
Just replace the bmp.c file after applying the previous patch
Comment by david (dave.d.x) - Thursday, 12 October 2006, 20:33 GMT
Sorry, but do not work... pls, update...
Comment by Sacha (Angyman) - Friday, 13 October 2006, 19:34 GMT
I get strange colors... brown gets purple... Can someone commit this or is it my fault and the patch is working now again???
Comment by Max Weninger (maxwen) - Friday, 13 October 2006, 19:42 GMT
I also discovered that the image quality is lower as with
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 :))

Comment by Frederik (freqmod) - Saturday, 14 October 2006, 14:21 GMT
Patch between bmp.c (cvs) and bmp.c (bmpresize), cd to <rockboxroot> and apply after bmp_resize_v0.99.6-MM.20060913.patch .
   bmp.diff (25.8 KiB)
Comment by Frederik (freqmod) - Saturday, 14 October 2006, 14:21 GMT
tested with iPod
Comment by Sacha (Angyman) - Saturday, 14 October 2006, 19:47 GMT
Commit: iriver H10 works... (but it looks not like 100% perfect. Cpu consumption raised and one WPS isnt able to update the current time anymore)
Comment by Matthias Mohr (aka Massa) (mmohr) - Sunday, 15 October 2006, 10:26 GMT
Here is a sync to today's CVS
- 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!
Comment by Norbert Preining (norbusan) - Monday, 16 October 2006, 09:57 GMT
Hi Matthias!
Umpf, the change 15 Oct 17:50 Mike Sevakis did break it again. Could you resync onc more? Thanks a lot.
Bye
Norbert
Comment by Jon (ace214) - Tuesday, 17 October 2006, 03:30 GMT
sync
Comment by Jon (ace214) - Tuesday, 17 October 2006, 06:03 GMT
sorry i was dumb and sloppy and broke/didn't update some stuff in that one- it patches but doesnt compile. this one works.
Comment by Sacha (Angyman) - Wednesday, 18 October 2006, 22:56 GMT
Sorr for the wrong statement... bmpr_resize works perfect... the higher processing time was produced by a bug in another patch. Sorry.
Comment by Douglas Valentine (Dwyloc) - Friday, 20 October 2006, 01:19 GMT
With my ipod nano no matter what I do I can’t quit the BMPVIEWER without resorting to rebooting my ipod.
If I read the code correctly pressing the select and menu buttons at the same time should quit but it dose not work here.
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 23 October 2006, 08:52 GMT
@Dwyloc: bmpviewer was just a "goodie" from takka - just meant as an example.
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...
Comment by Douglas Valentine (Dwyloc) - Tuesday, 31 October 2006, 17:38 GMT
bmp_resize_v0.99.6-MM.20061023.patch brakes the h120 build.
Comment by Matthias Mohr (aka Massa) (mmohr) - Thursday, 02 November 2006, 09:54 GMT
Sync to todays CVS and small fix for targets with LCD_DEPTH != 16
(should now be able to build again...)
Comment by Jakub Matoušek (kubiix) - Saturday, 25 November 2006, 08:48 GMT
After the complete rework of apps/recorder/bmp.c the patch need to be synced :/
Comment by Sacha (Angyman) - Thursday, 30 November 2006, 00:02 GMT
Yep... the little idiot (sitting here <- ME) is not able to get the bmp.c to get properly patched ;-)
Comment by Matthias Mohr (aka Massa) (mmohr) - Saturday, 02 December 2006, 09:22 GMT
The big changes in bmp.c makes it necessary to reimplement the resize patch :-(
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.
Comment by Tan Yu Sheng (Farpenoodle) - Sunday, 03 December 2006, 09:06 GMT
In the meantime, it's possible to simply download an older version of bmp.c from cvs and use that instead.
Comment by Jakub Matoušek (kubiix) - Saturday, 20 January 2007, 17:10 GMT
any news about syncing ?
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 22 January 2007, 21:51 GMT
Sorry, I still had not enough spare time to reimplement it on
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)
Comment by Nicolas Pennequin (nicolas_p) - Monday, 22 January 2007, 22:26 GMT
I did think about it but I won't have time before a while. Currently there's talk of committing the AA patch and that'll most certainly be without bmp resizing. The main devs don't like the approach used in this patch and they want it done another way so AA will use the best way when it comes. I'm focusing on other things like cuesheet support and trying to get some other patches committed.

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 :)
Comment by Alex Gerchanovsky (shoora) - Friday, 02 February 2007, 09:45 GMT
Here is new bmp reader with resize patch. For Gigabeat owners gbpviewer.c created to read proprietary Gigabeat files.
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.
Comment by Norbert Preining (norbusan) - Friday, 02 February 2007, 12:44 GMT
Hi Alex!
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
Comment by Alex Gerchanovsky (shoora) - Saturday, 03 February 2007, 11:11 GMT
SVN DIFF did not picked up new files. Here is tested version of patch.
Comment by Jon (ace214) - Monday, 05 February 2007, 01:46 GMT
you missed a hunk in gwps-common.c @ line 373. i fixed it in this attachment but i got this error at compiling that i don't know what to do with.

OBJCOPY viewer.rock
make[2]: *** No rule to make target `/home/rockbox/build-video/apps/plugins/
bmpviewer.rock', needed by `all'. Stop.
Comment by takka (takka) - Monday, 05 February 2007, 02:02 GMT
Fix for album art resize.
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 05 February 2007, 21:07 GMT
@shoora: thanks for reimplementing it again. What resize algorithm did you implement?
- 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...
Comment by Alex Gerchanovsky (shoora) - Monday, 05 February 2007, 22:26 GMT
Actually, I made hybrid of new bmp reader and old resizing algorithm. May be this resize algorithm far from perfect because it does not make antialiasing, but it looks reasonably well on small screen.
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.
Comment by leejongchul (crazyman9916) - Thursday, 08 February 2007, 23:44 GMT
Takka I apply your patch 2/4 build... apply faild..

help me..

with faild message file send it..

24.patch = bmpresize_20070204-broken.patch

25.patch1 = bmpresize_20070204-broken2.patch
Comment by leejongchul (crazyman9916) - Friday, 09 February 2007, 00:29 GMT
I apply 2/8 build.. but apply faild..

um..


Comment by Norbert Preining (norbusan) - Friday, 09 February 2007, 00:35 GMT
@leejongchul: first: you cannot apply *both* the broken and broken2 patch. And second, what about checking the created .rej file, maybe it is easy to fix yourself.
Comment by leejongchul (crazyman9916) - Friday, 09 February 2007, 00:45 GMT
Norbert.. now I apply bmprezie_20070204-broken2.patch..

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?
Comment by Norbert Preining (norbusan) - Friday, 09 February 2007, 00:52 GMT
Arggg ..... YOU MUST FIRST APPLY album_art_v5.2_nobmpresize_20061215.patch!!!
Comment by leejongchul (crazyman9916) - Friday, 09 February 2007, 01:45 GMT
Thank you for norbert.... really thank you.. !!!

I solve it..


Comment by leejongchul (crazyman9916) - Friday, 09 February 2007, 03:49 GMT
I succed apply album_art_v5.2_nobmpresize_20061215.patch and bmpresize_20070204-broken2.patch ..

but build folder type make.. i see error..

what's wrong?

Comment by Norbert Preining (norbusan) - Friday, 09 February 2007, 12:04 GMT
Come on, this is NOT a support forum. If you don't know how to do the stuff, please FIRST make yourself confident with the C code ... Take a look at the patches of my build, they maybe will help you!
Comment by Oleh Hradowy (MadCow15) - Monday, 12 February 2007, 20:26 GMT
Just a small suggestion, could you make it resize album art after loading another WPS? What happens to me is it doesn't resize the album art after loading a WPS with a different size album art without reloading the song.

Or would that be a bug with the album art patch?
Comment by Alex Gerchanovsky (shoora) - Wednesday, 14 February 2007, 19:30 GMT
I am thinking about universal picture viewer for BMP, JPEG, PNG and GIF(?). Then we can run viewer plugin from WPS when album art is changing.
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.
Comment by Douglas Valentine (Dwyloc) - Tuesday, 06 March 2007, 11:56 GMT
I have re-synced the patch to work with the SVN and the current album art patch.

To use the patch first apply album_art.patch from http://www.rockbox.org/tracker/task/3045#comment13690

Then my attached re-synced patch.
Comment by David Hall (Soap) - Friday, 30 March 2007, 08:44 GMT
Removed reference to (and attempted use of) non-existant bmpviewer and gbpviewer.

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.)
Comment by Hepdog (007quick) - Tuesday, 10 April 2007, 18:35 GMT
The album art patch has changed, this patch needs a sync. error in apps/gui/gui-common.c Some functions are now missing
Comment by Chris Cooper (Psiuyo) - Friday, 13 April 2007, 18:33 GMT
Updated the patch to work with the newer AA patch. Works so far for me...
Had to remove two patches to gwps-common.c and added one to wps_parser.c
(be nice, this is my first flyspray upload)
Comment by Dave Hooper (stripwax) - Saturday, 14 April 2007, 09:21 GMT
Hi Psiuyo, this patch doesn't apply cleanly for me. It looks like your patch includes an extensive change to gwps-common.c.orig , which is not a regular source file? Did you generate the patch by using the regular "svn diff" or something else? svn diff should generate the right patch files vs the svn repository and ignore non-repository files
Comment by Dave Hooper (stripwax) - Saturday, 14 April 2007, 10:31 GMT
This fixed patch works for me.
Comment by Dave Hooper (stripwax) - Saturday, 14 April 2007, 12:16 GMT
Hm. Seems like albumart displayed using this patch will always be scaled up or down regardless of the I/D/S flags in the %Cl tag. (e.g. %Cl|x|y|d100|d100 should mean the albumart is scaled down if it's bigger than 100x100 but left alone if it's smaller - but currently it gets scaled up if it's smaller)
Comment by TheKind (TheKind) - Monday, 16 April 2007, 18:45 GMT
Not working for me:

Hunk #4 FAILER at 250
Comment by Hepdog (007quick) - Monday, 16 April 2007, 19:25 GMT
They just changed something refereeing to the apps/recorder/icon.c file. I think that they removed the last bit of a function. Unfortunetly, my attempts at syncing didn't work out!
Comment by Akio Idehara (idak) - Tuesday, 17 April 2007, 15:02 GMT
I re-synced the patch.
Comment by TheKind (TheKind) - Tuesday, 17 April 2007, 15:45 GMT
Thx a lot !
Comment by Dave Hooper (stripwax) - Tuesday, 17 April 2007, 23:18 GMT
Akio - Does this sync'ed patch work? It seems that read_bmp_file fails to load customised iconsets because the iconset bitmap is too high. The error I get (on ipod5g sim) is for example: read_bmp_file: error - Bitmap is too big (13 x 384 pixels, screen is 320 x 240). Did I patch incorrectly?
Comment by Akio Idehara (idak) - Wednesday, 18 April 2007, 14:47 GMT
I changed the code that compares the output image size and LCD size.
Please look at the patch about details.
I tested customized iconsets, and it works fine...
Comment by Dave Hooper (stripwax) - Wednesday, 18 April 2007, 21:00 GMT
Thanks Akiro. I see that in the patch, I apologies - that hunk failed on mine due to other changes and I'd not noticed - your patch works great! sorry!
Comment by Travis Tooke (tdtooke) - Friday, 27 April 2007, 03:10 GMT
Needs resync, there isn't a apps/recorder/backdrop.c anymore.
Comment by takka (tfact) - Friday, 27 April 2007, 10:13 GMT
apps/recorder/backdrop.c move to apps/gui/backdrop.c
Comment by Travis Tooke (tdtooke) - Saturday, 28 April 2007, 07:42 GMT
Thanks, feel kinda silly now since that was so simple.
Comment by Hepdog (007quick) - Monday, 30 April 2007, 17:13 GMT
Synced hopefully:) Works for me!
Comment by Peter Nollanger (Elle) - Wednesday, 16 May 2007, 22:39 GMT
i got an error when patching the current daily source (20070516):
file: /apps/gui/gwps-common.c
Hunk #1 failed at 596

Is there something I can do about that?


Comment by Travis Tooke (tdtooke) - Tuesday, 26 June 2007, 00:21 GMT
This might help, this is my personal copy of this patch. Apply it using -p1.
Comment by josh (joshunhappy) - Tuesday, 03 July 2007, 05:33 GMT
hello.. [for every one] maybe i'm about to make a silly quiestion but... i need.. i'm new in this and i'm trying to configure mi sansa e250 y instaled rockbox and DockPodAA WPS.. but it said that i need a patch but i donknow now where to put it.. can somebody helpme [sorry].. PLEASE I NEED HELP... by the way.. as you can see i'm not an english speaker so sorry about that too
Comment by josh (joshunhappy) - Tuesday, 03 July 2007, 05:33 GMT
hello.. [for every one] maybe i'm about to make a silly quiestion but... i need.. i'm new in this and i'm trying to configure mi sansa e250 y instaled rockbox and DockPodAA WPS.. but it said that i need a patch but i donknow now where to put it.. can somebody helpme [sorry].. PLEASE I NEED HELP... by the way.. as you can see i'm not an english speaker so sorry about that too... you can talkme on joshunhappy@hotmail.com thx
Comment by Cam (GreyhoundGuy) - Sunday, 26 August 2007, 01:43 GMT
I got an error while trying to use this patch on today's daily source (20070825):
file: /apps/gui/gwps-common.c
Hunk #1 failed at 592

What could I do to fix this?
Comment by Cam (GreyhoundGuy) - Sunday, 26 August 2007, 04:08 GMT
Ok, I figured out what my problem was. I just needed to apply the album art patch first, as was stated earlier on by Douglas.
Comment by benoit (snk4ever) - Wednesday, 03 October 2007, 15:08 GMT
I can not apply this patch on the recent SVNs (I tried on the 20071003).
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 !
Comment by Jacob Brooks (jac0b) - Wednesday, 03 October 2007, 22:20 GMT
It patched fine for me on SVN 14973
Comment by benoit (snk4ever) - Thursday, 04 October 2007, 09:21 GMT
It patched on today's svn r14976, sorry for the wrong alert, I probably did something wrong yesterday !
Comment by Akio Idehara (idak) - Friday, 05 October 2007, 16:54 GMT
Fix tab mixed indent that brings on a headache.
No functionality changes for the patch...
Comment by Jeton Aliji (jeton) - Tuesday, 30 October 2007, 21:59 GMT
Any plans to improve the resizing algorithm?
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.
Comment by Travis Tooke (tdtooke) - Sunday, 11 November 2007, 03:28 GMT
Didn't do a single thing with any algorithms, just synced it:
Comment by JerryLange (psycho_maniac) - Monday, 12 November 2007, 00:33 GMT
i think this needs to be fixed. Im getting hunk errors.
Comment by Nicolas Pennequin (nicolas_p) - Monday, 12 November 2007, 00:39 GMT
It's no surprise that it's broken due to committing the AA patch... I plan on implementing resizing, but I don't know when I'll do it.
Comment by Travis Tooke (tdtooke) - Monday, 12 November 2007, 06:47 GMT
Quick and dirty sync. I don't like the way I handled this, but it works..
Comment by Michael Sevakis (MikeS) - Tuesday, 13 November 2007, 15:34 GMT
Here's some faster image stretching/squeezing code that uses no division in the loops and is compact. Will be used for thumbnailing in mpegplayer but I thought I'd share it for this purpose. It uses nearest-neighbor resizing (no fancy interpolation/filtering). Should be easy-as-pie to adapt here.
Comment by Travis Tooke (tdtooke) - Wednesday, 14 November 2007, 00:45 GMT
Looks good, thanks! idak, you want to take a stab at this? I just took a good look at bmp.c and my head started hurting.
Comment by Akio Idehara (idak) - Thursday, 15 November 2007, 14:37 GMT
I think it looks good too.
But I'm so busy and I have no time to try it...
Comment by Thomas Martitz (kugel.) - Sunday, 18 November 2007, 13:54 GMT
Is there any chance to have this patch working with the current album art implementation?
Comment by takka (tfact) - Friday, 07 December 2007, 22:01 GMT
sync current album art implementation
Comment by Travis Tooke (tdtooke) - Friday, 07 December 2007, 22:37 GMT
@takka, what did you do for top-down bmp support? On my version I have:
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.
Comment by JerryLange (psycho_maniac) - Sunday, 16 December 2007, 18:56 GMT
I would love to offer testing help with this. I have the sansa e260, ipod5.5g video, and the gigabeat F40. I really think its pointless to convert all my album art to 100x100 when there is a patch in the patch tracker that will automatically resize my art for me. So I would like to help test this. Does the newest patch work in svn and with album art?
Comment by Jacob Brooks (jac0b) - Monday, 17 December 2007, 13:00 GMT
I think pictureflow broke this patch. It will not make with this patch applied.
Comment by JerryLange (psycho_maniac) - Monday, 17 December 2007, 20:08 GMT
I think your right. I tried compiling last night with this patch and it wouldn't get past the "make rocks" portion of compiling.
Comment by David Maliniak (major_works) - Wednesday, 02 January 2008, 00:49 GMT
Anyone up to the task of making this work with current SVN? I would if I could...
Comment by Thomas Martitz (kugel.) - Wednesday, 02 January 2008, 00:59 GMT
You need to add 3 parameters (e.g. PREFERRED_IMG_WIDTH, PREFERRED_IMG_HEIGHT and 0 this is what I use) in the call of read_bmp_file (something like that) in apps/plugins/prictureflow.c. It will compile, but it will not resize properly within PictureFlow, at least not with the parameters I gave you.
Comment by David Maliniak (major_works) - Wednesday, 02 January 2008, 15:32 GMT
Thanks, Kugel... but in looking through pictureflow.c, I'm not finding a function named read_bmp_file. There is one called create_bmp. I do see the definitions of PREFERRED_IMG_WIDTH and _HEIGHT in lines 88 to 94. Otherwise, I'm unsure as to where in pictureflow.c to add the parameters. Any help would be appreciated.
Comment by Thomas Martitz (kugel.) - Wednesday, 02 January 2008, 15:42 GMT
It's line 607 in the latest revision.
Comment by David Maliniak (major_works) - Wednesday, 02 January 2008, 15:48 GMT
Yep... just found that moments before you posted. Thanks!
Comment by Akio Idehara (idak) - Sunday, 20 January 2008, 11:11 GMT
Here is new albumart resize implementation.
This patch only impacts albumart resize (and not for all bmp function).
Comment by Akio Idehara (idak) - Sunday, 20 January 2008, 11:46 GMT
Add some optimization code.
Comment by Travis Tooke (tdtooke) - Sunday, 20 January 2008, 18:53 GMT
Very nice. Small, precise, can't beat it with a stick! I think this version is very committable.
Comment by Nicolas Pennequin (nicolas_p) - Sunday, 20 January 2008, 20:01 GMT
It does have the problem of a rather big static buffer.
Comment by David Maliniak (major_works) - Monday, 21 January 2008, 06:07 GMT
It also has the problem of not working very well, at least for me. Album art is *not* resized with this patch in my own build, it seems.
Comment by david (dave.d.x) - Monday, 21 January 2008, 06:27 GMT
Not working.Why?Compiling without problem.
Comment by Akio Idehara (idak) - Monday, 21 January 2008, 12:47 GMT
change from static buffer to buffer_alloc() in initialization.
Comment by Akio Idehara (idak) - Monday, 21 January 2008, 13:48 GMT
change from buffer_alloc() to wps_data->img_buf.
This patch does not have any specific big buffer...
Comment by Nicolas Pennequin (nicolas_p) - Monday, 21 January 2008, 13:52 GMT
That's a better approach, but I see no boundary check on the WPS bitmap buffer, so if the resized bitmap is too big there'll be out of bounds access. output_size needs to be checked against data->img_buf_free before any resizing is done.
Comment by Akio Idehara (idak) - Monday, 21 January 2008, 14:34 GMT
Add boundary check for the resized album art size.

@nico: Thank you for these advices.
I can improve my implementation by that.
Comment by Akio Idehara (idak) - Monday, 21 January 2008, 16:17 GMT
add tmp value to access fixed struct member.
No functionality changes.
Comment by David Maliniak (major_works) - Tuesday, 22 January 2008, 04:27 GMT
FYI... still not working, at least not in a Viewports-enabled build.
Comment by Travis Tooke (tdtooke) - Tuesday, 22 January 2008, 07:54 GMT
Interesting, on my iPod 5.5g 30gig it is working.
Comment by Akio Idehara (idak) - Tuesday, 22 January 2008, 12:50 GMT
Album art format: %Cl|x|y|[[l|c|r][d|i|s]mwidth]|[[t|c|b][d|i|s]mheight]|

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
Comment by David Maliniak (major_works) - Thursday, 24 January 2008, 02:10 GMT
I downloaded and installed the version of jClix_night_VP posted on  FS#8385  on 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.
Comment by David Maliniak (major_works) - Thursday, 24 January 2008, 02:37 GMT
I downloaded and installed the version of jClix_night_VP posted on  FS#8385  on 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.
Comment by David Maliniak (major_works) - Thursday, 24 January 2008, 04:05 GMT
OK... figured out why resizing wasn't working. Most AA themes are missing the "s" flag! Thanks again for the pointers.
Comment by Michael Hahn (disorganizer) - Thursday, 24 January 2008, 22:33 GMT
if albumart is more than 176 px wide on my sansa then the aa is not displayed at all.
is this on purpose or a bug?
Comment by Nicolas Pennequin (nicolas_p) - Thursday, 24 January 2008, 22:36 GMT
It's a known limitation that is mentioned in the AA wiki page : picture dimensions must be smaller than the screen's.
Comment by Michael Hahn (disorganizer) - Thursday, 24 January 2008, 23:46 GMT
well, it worked well with the "old" bmp-resize patch method of aa scaling.
so i wonder if its on purpose :-)
Comment by Akio Idehara (idak) - Saturday, 26 January 2008, 03:05 GMT
delete duplicated code.
No functionality changes.
Comment by Akio Idehara (idak) - Saturday, 26 January 2008, 04:57 GMT
And this is smooth_resize_bitmap() version.
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.
Comment by Sasha Khamkov (sanusart) - Saturday, 26 January 2008, 08:28 GMT
This two patches combination works great!
Question/suggestion though: maybe the "s" flag can be applied by default for .wps that lacks the flag.
Comment by Akio Idehara (idak) - Saturday, 26 January 2008, 10:00 GMT
This patch only applies "s" flag by default.
So please also apply above patch...
Comment by Sasha Khamkov (sanusart) - Saturday, 26 January 2008, 10:32 GMT
That was a fast!
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.
Comment by Akio Idehara (idak) - Saturday, 26 January 2008, 10:50 GMT
Sorry, above "apply_s_flag.patch" breaks "d" and "i" flags.
So please apply this one.
#And I found aa align flags bug...
Comment by Sasha Khamkov (sanusart) - Saturday, 26 January 2008, 11:05 GMT
I have no problems except with cabbie2 using "apply_s_flag.patch" (the first).
Comment by Sasha Khamkov (sanusart) - Saturday, 26 January 2008, 11:06 GMT
Both "d" and "i".
Comment by Sasha Khamkov (sanusart) - Saturday, 26 January 2008, 11:07 GMT
The problem with "cabbie2" is no %C size set at all.
Comment by Akio Idehara (idak) - Saturday, 26 January 2008, 13:24 GMT
Fix no %C size set at all.
Comment by Akio Idehara (idak) - Saturday, 26 January 2008, 15:45 GMT
This patch applies "s" flag by default
and add "n" (previous default) flag...

- d: Decreasing
- i: Increasing
- s: Sizing (decrease or increase) (default)
- n: No sizing (previous default)
Comment by Sasha Khamkov (sanusart) - Sunday, 27 January 2008, 07:19 GMT
I noticed some slowness of the WPS, while going to the next song the AA not always displaying and some tags like artist and album sometimes missing. 26.01 there were changes to SVN apps/plugins/resize_test.c - it was renamed to test_resize.c It might be a bit of a problem because the "smooth_resize.patch" -  FS#8308  patching this file (with the old name).
Comment by Sasha Khamkov (sanusart) - Sunday, 27 January 2008, 09:20 GMT
This is small .png to compare with and without smoothing version the AA.
Comment by Akio Idehara (idak) - Thursday, 31 January 2008, 18:39 GMT
This patch changes bitmap width limitation from "LCD_WIDTH" to "320".
If you use sansa e200 or something else, please try that...
Comment by Michael Hahn (disorganizer) - Friday, 01 February 2008, 10:24 GMT
seems to work on e200
Comment by Akio Idehara (idak) - Wednesday, 06 February 2008, 15:18 GMT
This is Metadata on Buffer(MoB) version.
I hope that this patch will resolve the slowness of the WPS...
Comment by Sasha Khamkov (sanusart) - Wednesday, 06 February 2008, 17:22 GMT
Works great: Sansa e200 + latest SVN.
Noticeable improvement - Don't stuck at all anymore, at least for me.
Comment by Akio Idehara (idak) - Thursday, 07 February 2008, 17:25 GMT
This patch fixes that the resized album art is recopied back to the file buffer.
Comment by Travis Tooke (tdtooke) - Saturday, 09 February 2008, 09:57 GMT
It looks like my album art that is sized up is showing noticeably less pixelization, or maybe it's just the particular images I'm seeing at the moment, at any rate it's a good thing. Is this because of smooth resize?
Comment by Sasha Khamkov (sanusart) - Sunday, 10 February 2008, 06:42 GMT
Akio,

If the AA is smaller than 100px:
&bull; 's' flag - no AA at all.
&bull; 'd,i,n' flags - real size AA (cuts the AA if the .wps states smaller) no decreasing.


Comment by Akio Idehara (idak) - Sunday, 10 February 2008, 09:34 GMT
This patch adds mutex_lock to the WPS buffer operation.
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.
Comment by Sasha Khamkov (sanusart) - Sunday, 10 February 2008, 10:06 GMT
No, I mean the size is permanent no matter what flag applied in the WPS.
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
Comment by Akio Idehara (idak) - Sunday, 10 February 2008, 13:14 GMT
forgot to add mutex_init...

@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 flags
http://www.rockbox.org/tracker/task/8517
Comment by Sasha Khamkov (sanusart) - Sunday, 10 February 2008, 13:31 GMT
No, no, just was reporting possible bug in this patch here, thanks.
Comment by Jacob Brooks (jac0b) - Tuesday, 12 February 2008, 23:04 GMT
I get this error when compiling.

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
Comment by Jacob Brooks (jac0b) - Tuesday, 12 February 2008, 23:59 GMT
I deleted my whole folder and d/l'ed the source from the site and I still get the above error when compileing.
Comment by Akio Idehara (idak) - Wednesday, 13 February 2008, 11:29 GMT
delete needless WPS buffer pointer operation.

@jac0b:
Please refer to the above comment
http://www.rockbox.org/tracker/task/5697#comment21000
Comment by Thomas Martitz (kugel.) - Wednesday, 13 February 2008, 21:05 GMT
There's a problem with the buffer and this patch. Loading a high-res album art leads to not display the album art at all, since the wps image buffer is full.

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.
Comment by Jacob Brooks (jac0b) - Wednesday, 13 February 2008, 23:42 GMT
@idak:
Thanks it compiles now.
Comment by Akio Idehara (idak) - Thursday, 14 February 2008, 13:00 GMT
This is the complete MoB (file buffer) version.
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 flags
It's a clear rockbox's bug.
But why no one wants to commit this patch ASAP?
Comment by Sasha Khamkov (sanusart) - Thursday, 14 February 2008, 14:05 GMT
Why no one, I did tested with all 3 patches:
"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.
Comment by Thomas Martitz (kugel.) - Thursday, 14 February 2008, 15:43 GMT
sanusart, firstly, you have a syntax error in the_panel_aa. %Cl|17|86|c90|b90| <-- the b is wrong.

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.
Comment by Thomas Martitz (kugel.) - Thursday, 14 February 2008, 15:48 GMT
Ok, my bad. the "b" isn't wrong (I didn't think of aligning it to center and bottom). But the issue with not applying the s flag when a when a allign flag is given still applies.
Comment by Thomas Martitz (kugel.) - Thursday, 14 February 2008, 16:55 GMT
Ok guys, try this one. It's basically the identical to the previous bmp_resize version, BUT it includes a better/improved version of the apply_s_flag to fix the issue I mentioned above. For me it makes sense anyway to introduce scaling as default with bmp resize.
Comment by Thomas Martitz (kugel.) - Thursday, 14 February 2008, 17:01 GMT
Oh before you wonder, I removed the n flag, since I think "no resizing" isn't needed at all. I can understand, that upscaling isn't wanted (upscaling might be more ugly then no scaling). But downscaling is allways wanted, isn't it? Who wants to crop his album art if he can have it scaled down with a smooth method?
Comment by Sasha Khamkov (sanusart) - Friday, 15 February 2008, 10:01 GMT
Yes, this patch fixed all scaling issues mentioned above (i have no tools to test "d" flag for the moment though) but I think we have buffer overflow now.
Comment by Sasha Khamkov (sanusart) - Friday, 15 February 2008, 10:10 GMT
Sorry I meant "i" - increasing flag at the post above.
Comment by Akio Idehara (idak) - Friday, 15 February 2008, 12:49 GMT
Some cosmetic fix.
And revert "N" flag, it is for debugging.
Comment by David Maliniak (major_works) - Sunday, 17 February 2008, 00:45 GMT
So does the latest version posted on Feb. 15 now include the apply_S_flag patch, making that patch obsolete?
Comment by Thomas Martitz (kugel.) - Sunday, 17 February 2008, 00:57 GMT
It doesn't only include it, it refines it ;)
Comment by David Maliniak (major_works) - Sunday, 17 February 2008, 00:58 GMT
Thanks, Kugel. These tracker threads can be very confusing at times.
Comment by Akio Idehara (idak) - Sunday, 17 February 2008, 12:05 GMT
Some cosmetic fix again.
This patch is no need to apply "apply_s_flag-3.patch" (and aa_align.patch)
Comment by Michael Hahn (disorganizer) - Tuesday, 19 February 2008, 11:20 GMT
just a question and maybe an idea:
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)
Comment by Akio Idehara (idak) - Tuesday, 19 February 2008, 14:09 GMT
@me:
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 ;).
Comment by Michael Hahn (disorganizer) - Tuesday, 19 February 2008, 15:32 GMT
so the albumart-smooth_resize-080217 is already scaling when loading the bmp from disk to the MoB?
(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.

Comment by Akio Idehara (idak) - Thursday, 21 February 2008, 13:21 GMT
> so the albumart-smooth_resize-080217 is already scaling when loading the bmp
> 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.
Comment by Michael Hahn (disorganizer) - Thursday, 21 February 2008, 17:40 GMT
ok. so the question is: why was this restriction implemented?
and thus: can it be left out completely when using bmp-resize?
Comment by Dave Germiquet (enterusername) - Wednesday, 27 February 2008, 08:30 GMT
Just a note,

this doesn't appear to be compiling on current SVN.

Not sure why it patches fine however it errors out during make
Comment by Sasha Khamkov (sanusart) - Wednesday, 27 February 2008, 09:29 GMT
Compiled fine on ubuntu linux, if you're on cygwin - try to update compiler for your target.
Comment by Johannes Schwarz (Ubuntuxer) - Friday, 21 March 2008, 10:25 GMT
I can't compile it, too. Although I'm using Ubuntu!
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
Comment by Thomas Martitz (kugel.) - Friday, 21 March 2008, 13:12 GMT
Have you applied http://www.rockbox.org/tracker/task/8308 ? This patch is required for this patch.
Comment by Akio Idehara (idak) - Friday, 21 March 2008, 13:16 GMT
Some cosmetics fix again.
No functionality changes.

@Ubuntuxer:
Please refer to the above comment
http://www.rockbox.org/tracker/task/5697#comment21554
Comment by Johannes Schwarz (Ubuntuxer) - Saturday, 22 March 2008, 13:28 GMT
Sorry, that I haven't read your comments.
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.
Comment by Thomas Martitz (kugel.) - Monday, 24 March 2008, 20:42 GMT
idak, I doubt that removing (or expanding) the restriction of the bitmap width is a good approach.

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.
Comment by Akio Idehara (idak) - Wednesday, 26 March 2008, 14:04 GMT
> idak, I doubt that removing (or expanding) the restriction of the bitmap width is a good approach.
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
Comment by Michael Hahn (disorganizer) - Sunday, 30 March 2008, 19:54 GMT
what would be needed to get this into a commitable state?
Comment by Michael Hahn (disorganizer) - Monday, 07 April 2008, 15:28 GMT
propably synced to 17018 (smooth-resize lib included in svn) without additional patches
Comment by Keith Perri (perrikwp) - Wednesday, 16 April 2008, 02:42 GMT
Synced to r17136
Comment by Travis Tooke (tdtooke) - Wednesday, 02 July 2008, 05:13 GMT
I too want to know what would be needed to get this commitable. Just reading through it looks like every suggestion put to idak by the devs has been implemented.
Comment by Thomas Martitz (kugel.) - Wednesday, 02 July 2008, 11:23 GMT
The current version wouldn't be committed, because it transfers the smooth scaling algorithm into the core. But it's only for plugins for a reason: The enormous bin size increase with smooth scaling in the core.

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).
Comment by Akio Idehara (idak) - Wednesday, 30 July 2008, 14:37 GMT
First patch moves bmp resize functions
(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.
Comment by Thomas Martitz (kugel.) - Wednesday, 30 July 2008, 15:07 GMT
I'm thinking of dropping the support for everything but resizinh (so drop the [d|i|s|n] part, and scale everytimes). This'll greatly reduce complexity, since we can drop checks and pre-calculations. I also think it's desirable to always resie.

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.
Comment by Dave Chapman (linuxstb) - Wednesday, 30 July 2008, 15:22 GMT
idak,

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?

Comment by Thomas Martitz (kugel.) - Wednesday, 30 July 2008, 19:15 GMT
Attached is a patch, which also moves the resizing into the core, but removes (#if 0's rather) the two special cases (scale down vertically and scale down horizonzally), as they're basically not essential. Doing this, the patch only adds about 2KB to the binsize.

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.
Comment by Thomas Martitz (kugel.) - Wednesday, 30 July 2008, 19:23 GMT
Oops, forgot to svn add resize.[ch].

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.
Comment by Thomas Martitz (kugel.) - Wednesday, 30 July 2008, 20:06 GMT
Less white space changes...
Comment by Thomas Martitz (kugel.) - Thursday, 31 July 2008, 00:17 GMT
Ok, now I got my hands on albumart_smooth_resize. I removed all the scaling options ([d|i|s|n] tags) and cleaned the code a little. This saves about 550 bin size (only for albumart_smooth resize).

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).
Comment by Akio Idehara (idak) - Friday, 01 August 2008, 05:00 GMT
Some cosmetics fix and reduce smooth scaling bin size.

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;
Comment by Dave Chapman (linuxstb) - Sunday, 03 August 2008, 19:47 GMT
idak,

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

Comment by Akio Idehara (idak) - Saturday, 09 August 2008, 09:54 GMT
(1) I wanted to use the smooth_resize_bitmap(), and I also wanted to show what
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.

Comment by Akio Idehara (idak) - Sunday, 24 August 2008, 05:45 GMT
Some cosmetic fix.
Comment by Jacob Brooks (jac0b) - Monday, 22 September 2008, 23:32 GMT
Resync

Loading...