Rockbox

“album art wps” need

This patch looks great
just played around a bit with it and it seems to work just
BMP viewer is pretty nice too, but it could use a few improvements if it were to become Rockbox’s main bitmap viewer.

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.

bmp resize patch
up source
a few comments.

macro rename BMPVIEWER_*

   bmp_resize_v0.99.patch (30.9 KiB)

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).

* does the change in rockpaint.c mean that smaller than screen bmp.c will be resized when opened
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
if loading the bmp fails in the viewer, you should quit instead of displaying a white
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),
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
it good in this though corrected?

   bmp_resize_v0.99.patch (30.9 KiB)

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).

To the read_bmp_file() function in the upcoming version, ExpansionOnly/ReductionOnly/Automatic select argument
is thought that it is possible to display it without changing if it is smaller than the screen.

fix:Mono bmp some pixels missing

THANKS Ashen.

   bmp_resize_v0.99.1.patch (31.1 KiB)

over
buffer_Xtension.

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

   bmp_resize_v0.99.2.patch (31.8 KiB)

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).

Updated patch to compile against CVS HEAD 12.08.2006

   bmp_resize_v0.99.3.patch (31.9 KiB)

takka, I have also a
you please add two additional parameters to
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

another thing: IMHO it’s not correct to check against LCD_WIDTH/LCD_HEIGHT inside
Because you could (theoretically) load bitmaps to different screens with different
it should be possible to load a greyscale image to
LCD-remote (which definitely has other dimensions than LCD_WIDTH/LCD_HEIGHT
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)

Sorry for
will do it correct next time

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)

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
there is a bitmap size and a size to which the bitmap should
would be able to calculate a ration when I know about both

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
I misinterpreted your idea???

And I still like my idea to differentiate between width resizing and height
enable the possibility to only resize one dimension...

I implemented my suggestion and created a new patch
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?

   bmp_resize_v0.99.4-MM.2006082... (26.9 KiB)

Build failure with current
* No rule to make target `/src/iriver/rockbox/bla/build/apps/plugins/bmpviewer.rock’, needed by `all’.
* [rocks] Error 2

I have - current


album_art_v5.1.0_TEST-MM_20060820

Any ideas?

Ciao Norbert

my failure - I forget to add the new file bmpviewer.c to the patch...

Here is an update version (nothing else changed)

   bmp_resize_v0.99.4-MM.2006082... (34.4 KiB)

Here is another version which fixes a small
either dst_x < 0 or dst_y <
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
this on intend or is it a bug?

Ooops - I forgot to attach the file.

   bmp_resize_v0.99.5-MM.2006082... (34.8 KiB)

Because I needed a completely working bmp_resize
takka did not answer I had a deeper look at the
did not understand takka’s code (I’m no graphics
decided to rewrite the complete resizing code.

So although I call it v0.99.6 it’s my first version with
resize
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
given maxwidth/maxheight are (as the name says) only maximum width/heights which may not be exceeded when
are _not_ the sizes which the resized bitmap will
width/height ratio of the original bitmap will always be kept
normally only one dimension of the resized bitmap will be the given
other will calculated accordingly to the original width/height ratio...

Increasing and Decreasing should both work and the correct bitmap dimensions
given back in the bitmap structure.

I also hope the source code is easy enough to be understandable by other developers
only thing missing is to optimize and hopefully combine some code
that can wait...

Tell me if it’s working for you or not...

   bmp_resize_v0.99.6-MM.2006090... (36.9 KiB)

Here’s the same version as
with today’s CVS and splitted into two
for the resizing code and the other to add the
resizing code is (and was) platform independent, but
code is missing keycode defintions for several
it will currently not compile for several platforms (e.g.
bmpviewer is only example code and not necessary needed so I
put it in an extra patch which has been applied on top of the
if wanted.

IMHO it should somehow be integrated in the
then would extend it into a “pictureviewer” ;)

   bmp_resize_v0.99.6-MM.2006091... (28.4 KiB)
   bmpviewer_20060913.patch (8.5 KiB)

bmp_resize_v0.99.6-MM.20060913.patch do not work with current CVS (20061011) :(

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)

Frederik, pls, how apply it? Thanks.

Just replace the bmp.c file after applying the previous patch

Sorry, but do not work... pls, update...

I get strange colors... brown gets purple... Can someone commit this or is it my fault and the patch is working now again???

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 :))

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)

tested with iPod

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)

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!

   bmp_resize_v0.99.6-MM.2006101... (28.3 KiB)
   bmpviewer_20061015.patch (8.6 KiB)

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

sync

   bmp_resize_v0.99.6-ace214.200... (28.3 KiB)

sorry i was dumb and sloppy and broke/didn't update some stuff in that one- it patches but doesnt compile. this one works.

   bmp_resize_v0.99.6-ace214.200... (28.3 KiB)

Sorr for the wrong statement... bmpr_resize works perfect... the higher processing time was produced by a bug in another patch. Sorry.

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.

@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...

   bmp_resize_v0.99.6-MM.2006102... (28.3 KiB)

bmp_resize_v0.99.6-MM.20061023.patch brakes the h120 build.

Sync to todays CVS and small fix for targets with LCD_DEPTH != 16 (should now be able to build again...)

   bmp_resize_v0.99.6-MM.2006110... (28.8 KiB)

After the complete rework of apps/recorder/bmp.c the patch need to be synced :/

Yep... the little idiot (sitting here <- ME) is not able to get the bmp.c to get properly patched ;-)

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.

In the meantime, it's possible to simply download an older version of bmp.c from cvs and use that instead.

any news about syncing ?

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)

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 :)

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.

   bmpresize_20070201.patch.zip (11.2 KiB)

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

SVN DIFF did not picked up new files. Here is tested version of patch.

   bmpresize_20070202.patch (29.6 KiB)

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.

   bmpresize_20070204-broken.pat... (30.1 KiB)

Fix for album art resize.

   bmpresize_20070204-broken_2.p... (30.2 KiB)

@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...

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.

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

   error.bmp (831 KiB)

I apply 2/8 build.. but apply faild.. um..

   error2.bmp (836 KiB)

@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.

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?

Arggg ..... YOU MUST FIRST APPLY album_art_v5.2_nobmpresize_20061215.patch!!!

Thank you for norbert.... really thank you.. !!! I solve it..

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?

   error3.bmp (0.99 MiB)

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!

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?

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.

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.

   bmpresize_20070306.patch (29.7 KiB)

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.)

   bmpresize_20070306C.patch (28.4 KiB)

The album art patch has changed, this patch needs a sync. error in apps/gui/gui-common.c Some functions are now missing

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)

   bmpresize-20070413.patch (32.6 KiB)

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

This fixed patch works for me.

   bmpresize-20070413-fixed.patch (28.1 KiB)

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)

Not working for me: Hunk #4 FAILER at 250

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!

I re-synced the patch.

   bmpresize-20070417.patch (29.8 KiB)

Thx a lot !

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?

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...

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!

Needs resync, there isn't a apps/recorder/backdrop.c anymore.

apps/recorder/backdrop.c move to apps/gui/backdrop.c

Thanks, feel kinda silly now since that was so simple.

Synced hopefully:) Works for me!

   bmpresize-20070430.patch (29.8 KiB)

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?

This might help, this is my personal copy of this patch. Apply it using -p1.

   bmpresize-20070624.patch (31 KiB)

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

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

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?

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.

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 !

It patched fine for me on SVN 14973

It patched on today's svn r14976, sorry for the wrong alert, I probably did something wrong yesterday !

Fix tab mixed indent that brings on a headache. No functionality changes for the patch...

   bmp_resize-071005.patch (30.7 KiB)

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.

Didn't do a single thing with any algorithms, just synced it:

   bmpresize-20071110.patch (30.3 KiB)

i think this needs to be fixed. Im getting hunk errors.

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.

Quick and dirty sync. I don't like the way I handled this, but it works..

   bmpresize-20071112.patch (30.3 KiB)

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.

   stretch_image_plane.c (1.1 KiB)

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.

I think it looks good too. But I'm so busy and I have no time to try it...

Is there any chance to have this patch working with the current album art implementation?

sync current album art implementation

   bmpresize-1205.patch (30.1 KiB)

@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.

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?

I think pictureflow broke this patch. It will not make with this patch applied.

I think your right. I tried compiling last night with this patch and it wouldn't get past the "make rocks" portion of compiling.

Anyone up to the task of making this work with current SVN? I would if I could...

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.

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.

It's line 607 in the latest revision.

Yep... just found that moments before you posted. Thanks!

Here is new albumart resize implementation. This patch only impacts albumart resize (and not for all bmp function).

   albumart-resize-080120.patch (3.3 KiB)

Add some optimization code.

   albumart-resize-080120-02.pat... (3.4 KiB)

Very nice. Small, precise, can't beat it with a stick! I think this version is very committable.

It does have the problem of a rather big static buffer.

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.

Not working.Why?Compiling without problem.

change from static buffer to buffer_alloc() in initialization.

   albumart-resize-080121.patch (4.7 KiB)

change from buffer_alloc() to wps_data->img_buf. This patch does not have any specific big buffer...

   albumart-resize-080121-02.pat... (3.9 KiB)

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.

Add boundary check for the resized album art size. @nico: Thank you for these advices. I can improve my implementation by that.

   albumart-resize-080121-03.pat... (4.1 KiB)

add tmp value to access fixed struct member. No functionality changes.

   albumart-resize-080122.patch (5.6 KiB)

FYI... still not working, at least not in a Viewports-enabled build.

Interesting, on my iPod 5.5g 30gig it is working.

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

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.

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.

OK... figured out why resizing wasn't working. Most AA themes are missing the "s" flag! Thanks again for the pointers.

if albumart is more than 176 px wide on my sansa then the aa is not displayed at
this on purpose or a bug?

It's a known limitation that is mentioned in the AA wiki page : picture dimensions must be smaller than the screen's.

well, it worked well with the "old" bmp-resize patch method of aa
i wonder if its on purpose

delete duplicated
functionality changes.

   albumart-resize-080126.patch (4.9 KiB)

And this is smooth_resize_bitmap()
you try to use this
also apply the "smooth_resize.patch"
- Port of a imlib2 based smooth scaling algorithm for bitmaps.

   albumart-smooth_resize-080126... (4.9 KiB)

This two patches combination works
though: maybe the "s" flag can be applied by default for .wps that lacks the flag.

This patch only applies "s" flag by
please also apply above patch...

   apply_s_flag.patch (0.9 KiB)

That was a
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.

Sorry, above "apply_s_flag.patch" breaks "d" and "i"
please apply this
I found aa align flags bug...

   apply_s_flag-2.patch (2.3 KiB)

I have no problems except with cabbie2 using "apply_s_flag.patch" (the first).

Both "d" and "i".

The problem with "cabbie2" is no %C size set at all.

Fix no %C size set at all.

   albumart-resize-080126-02.pat... (5.1 KiB)
   albumart-smooth_resize-080126... (5.2 KiB)

This patch applies "s" flag by
add "n" (previous default) flag...

- d:
i:
s: Sizing (decrease or increase)
n: No sizing (previous default)

   apply_s_flag-3.patch (3.2 KiB)

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).

This is small .png to compare with and without smoothing version the AA.

   2008-01-27_111748.png (154.1 KiB)

This patch changes bitmap width limitation from "LCD_WIDTH" to
you use sansa e200 or something else, please try that...

   bmp_wide_width.patch (1.1 KiB)

seems to work on e200

This is Metadata on Buffer(MoB)
hope that this patch will resolve the slowness of the WPS...

   albumart-smooth_resize-080207... (4.3 KiB)

Works great: Sansa e200 + latest SVN. Noticeable improvement - Don't stuck at all anymore, at least for me.

This patch fixes that the resized album art is recopied back to the file buffer.

   albumart-smooth_resize-080208... (6.4 KiB)

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?

Akio,

If the AA is smaller than
's' flag - no AA at
'd,i,n' flags - real size AA (cuts the AA if the .wps states smaller) no decreasing.

This patch adds mutex_lock to the WPS buffer
hope that the accidental buggy pixelization will be gone...

guess that you change WPSes
a new WPS's album art buffer is
previous WPS's album art buffer is
if the previous WPS has no album art tag, no album art is
a new song's album art image is
is the "original" Album Art draw behavior.

   albumart-smooth_resize-080210... (8.5 KiB)

No, I mean the size is permanent no matter what flag applied in the
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

forgot to add mutex_init...

see... That's maybe original aa align flags
I found at coding "apply s flag
you want to use "b/t" or "r/l"
apply the "aa_align.patch"
- Fix Album art align

   albumart-smooth_resize-080210... (8.9 KiB)

No, no, just was reporting possible bug in this patch here, thanks.

I get this error when compiling.

CC
file included from
error: rockboxlogo.h: No such file or
* [/home/jacob/rockbox/build/apps/bookmark.o] Error
* [build] Error 2

I deleted my whole folder and d/l'ed the source from the site and I still get the above error when compileing.

delete needless WPS buffer pointer operation. @jac0b: Please refer to the above comment http://www.rockbox.org/tracker/task/5697#comment21000

   albumart-smooth_resize-080213... (8.9 KiB)

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.

@idak: Thanks it compiles now.

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?

   albumart-smooth_resize-080214... (6.4 KiB)

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.

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.

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.

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.

   albumart-smooth_resize-080214... (9.4 KiB)

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?

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.

Sorry I meant "i" - increasing flag at the post above.

Some cosmetic fix. And revert "N" flag, it is for debugging.

   albumart-smooth_resize-080215... (14.6 KiB)

So does the latest version posted on Feb. 15 now include the apply_S_flag patch, making that patch obsolete?

It doesn't only include it, it refines it ;)

Thanks, Kugel. These tracker threads can be very confusing at times.

Some cosmetic fix again. This patch is no need to apply "apply_s_flag-3.patch" (and aa_align.patch)

   albumart-smooth_resize-080217... (14.4 KiB)

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)

@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 ;).

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.

> 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.

   albumart-smooth_resize-080221... (16.9 KiB)

ok. so the question is: why was this restriction implemented? and thus: can it be left out completely when using bmp-resize?

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

Compiled fine on ubuntu linux, if you're on cygwin - try to update compiler for your target.

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

Have you applied http://www.rockbox.org/tracker/task/8308 ? This patch is required for this patch.

Some cosmetics fix again. No functionality changes. @Ubuntuxer: Please refer to the above comment http://www.rockbox.org/tracker/task/5697#comment21554

   albumart-smooth_resize-080321... (16.8 KiB)

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.

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.

> 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

   albumart-smooth_resize-080325... (14.9 KiB)

what would be needed to get this into a commitable state?

propably synced to 17018 (smooth-resize lib included in svn) without additional patches

   albumart-smooth_resize-080407... (16.9 KiB)

Synced to r17136

   albumart-smooth_resize-080415... (16.1 KiB)

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.

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).

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.

   01_resize_into_core-080730.pa... (46.9 KiB)
   02_albumart-smooth_resize-080... (16.7 KiB)

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.

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?

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.

   resize_into_core_v2.patch (35.8 KiB)

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.

   resize_into_core_v2.patch (57.3 KiB)

Less white space changes...

   resize_into_core_v2c.patch (49.1 KiB)

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).

   albumart_resize_no_options.pa... (14.1 KiB)

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;

   01_resize_into_core-080801.pa... (43 KiB)
   02_albumart_resize_no_options... (15.2 KiB)

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

(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.

Some cosmetic fix.

   01_resize_into_core-080824.pa... (43 KiB)
   02_albumart_resize_no_options... (15.2 KiB)

Resync

   01_resize_into_core-080922.pa... (42.9 KiB)