Rockbox

Tasklist

FS#11641 - extend and fix png support in imageviewer plugin

Attached to Project: Rockbox
Opened by Marcin Bukat (MarcinBukat) - Friday, 24 September 2010, 07:28 GMT
Last edited by Marcin Bukat (MarcinBukat) - Sunday, 31 October 2010, 12:44 GMT
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

1) Code cleanups - move PNG color types to defines (instead of magic numbers
used directly). Change arithmetics in a few palces to use shifts instead of
muls.
2) Comment out tIMe chunk processing as we do not check nor set timestamp of
png file.
3) Rework LodePNG_convert() to output pixels in rb specific format (greylib or
native).
4) Enable support for greyscale targets.
5) Change optimization to -Os (from -O3)

This was tested in the sim *only* so tests on different real hardware is welcome. The most interesting is mrobe500 (as it has different native pixelformat then any other color target) and greyscale targets
This task depends upon

Closed by  Marcin Bukat (MarcinBukat)
Sunday, 31 October 2010, 12:44 GMT
Reason for closing:  Accepted
Additional comments about closing:  commited as r28413
Comment by Marcin Bukat (MarcinBukat) - Saturday, 25 September 2010, 19:13 GMT
Switch from zlib to tinf as a inflate decompression backend. This saves binsize considerable.
Other minor optimizations.
Comment by Teruaki Kawashima (teru) - Sunday, 26 September 2010, 14:26 GMT
I tried the patch on the simulator for mrobe500. it seems like images are not displayed correctly if it is not squre.
replace info->width to STRIDE(SCREEN_MAIN, info->width, info->height) in for rb->lcd_bitmap_part at line 1496 in png.c (see imageviewer/bmp/bmp.c).
also, smooth_resize_bitmap needs to be fixed for mrobe500.
Comment by Marcin Bukat (MarcinBukat) - Tuesday, 28 September 2010, 22:11 GMT
Changes in this version:
1) Add support for true transparency
2) Add support for simple transparency (tRNS chunk)
3) Add support for background color (bKGD chunk)
4) Masive code refactoring to separate decoder from plugin specific stuff
5) Various cleanups
6) Fix draw_image_part() to use STRIDE macro (proposed by teru).

Missing: image scaling is broken on mrobe500
Comment by Teruaki Kawashima (teru) - Wednesday, 29 September 2010, 14:47 GMT
for bmp_smooth_scale, i tried the patch i attched, and it seems to work on the simulator.
but i don't have a device to test and don't know internals at all, so i don't know if this is proper.

btw, calculation of decoder->native_img_size seems slightly wrong.
IIUC, it should be (decoder->infoPng.width*decoder->infoPng.height*FB_DATA_SZ + decoder->infoPng.width*sizeof(struct uint8_rgb)) for color targets and (decoder->infoPng.width*decoder->infoPng.height + decoder->infoPng.width) for the target using greylib.
Comment by Marcin Bukat (MarcinBukat) - Wednesday, 29 September 2010, 22:18 GMT
fixes in this version:
1) revert to byte fetch and shift processing of fields in png file (pointer magic from v2 makes strange thing on arms which can't do unaligned fetches)
2) correct calculation of native_img_size
3) use fixed bitmap scalers (fixes were commited as r28185 and r28187)
Comment by Marcin Bukat (MarcinBukat) - Thursday, 30 September 2010, 21:59 GMT
git version
Comment by Marcin Bukat (MarcinBukat) - Saturday, 16 October 2010, 20:57 GMT
This version places tables needed by zlib decompressor on the stack (which is placed on iram) in order to speedup decompression. Other minor fixes.
Comment by Marcin Bukat (MarcinBukat) - Monday, 18 October 2010, 14:29 GMT
Attached file loads in ~8s (5-6s zlib decompression, 2-3s pixel format conversion) on coldfire and ~4s (3s decompression, 1s pixel format conversion) on PP5020. Moving static tables onto the stack does not make measurable difference.
Comment by Marcin Bukat (MarcinBukat) - Tuesday, 19 October 2010, 11:28 GMT
~13% of decoding time (measured with stopwatch from the selecting file to the display of the image) is taken by calculation of the adler32 checksum.
Comment by Marcin Bukat (MarcinBukat) - Tuesday, 19 October 2010, 19:53 GMT
* Removed adler32 checksum calculation as this is redundant in case of PNG
* Other minor changes in decompressor
Comment by Marcin Bukat (MarcinBukat) - Tuesday, 19 October 2010, 23:07 GMT
* Removed adler32 checksum calculation as this is redundant in case of PNG
* Other minor changes in decompressor
Comment by Marianne Arnold (pixelma) - Tuesday, 19 October 2010, 23:10 GMT
I tested the v5 version of this on my Ondio and my M5 and already mentioned the results in IRC., but posting it here again:

The patch works on both tested targets, the only thing I'm wondering about is why I sometimes get "file too large" and sometimes "out of memory" on larger files but I could imagine that it has to do with compressed and decompressed size?

Another thing I noticed is that the Archos versionl doesn't do aspect ratio correction like e.g. the bmp viewer does and I noticed when comparing with a square file (200x200) that the bmp player states "decoding 200x160" (so corrected) and the png viewer "decoding 200x200", so to me it looks like the difference is already at the loading step (?). Fixing this is not very important but would be nice :)
Comment by Marcin Bukat (MarcinBukat) - Wednesday, 20 October 2010, 08:05 GMT
'file too large' error is when there is not enough space to load file from storage to memory.'
'out of memory' comes from decompressor or decoder.

There are (at least) two points where 'out of memory' may popup:
1) PNG file has lots of additional chunks and there is no place in buffer to copy raw compressed stream
2)Converting decoded PNG image to desired pixelformat (greylib or native) needs to hold both decoded image, converted image and one line big temp buffor. Later on decoded image and temp buffor are discarded and free space is used as scaler buffor. In theory it is possible to reduce memory needed in this step.
Comment by Marcin Bukat (MarcinBukat) - Wednesday, 20 October 2010, 09:08 GMT
'file too large' error is when there is not enough space to load file from storage to memory.'
'out of memory' comes from decompressor or decoder.

There are (at least) two points where 'out of memory' may popup:
1) PNG file has lots of additional chunks and there is no place in buffer to copy raw compressed stream
2)Converting decoded PNG image to desired pixelformat (greylib or native) needs to hold both decoded image, converted image and one line big temp buffor. Later on decoded image and temp buffor are discarded and free space is used as scaler buffor. In theory it is possible to reduce memory needed in this step.
Comment by Marcin Bukat (MarcinBukat) - Thursday, 21 October 2010, 21:40 GMT
Added aspect ration correction. Since correction is done *after* decoding step, information displayed by the plugin while loading and decoding are related to original image (unlike in bmp viewer). If there is not enough memory to do the correction it is skipped - I think it is better to show something rather than nothing.
Comment by Marcin Bukat (MarcinBukat) - Wednesday, 27 October 2010, 20:11 GMT
v6 patch is broken. This one is correct and includes also optimization to unfilter routine. Test png 'United Kingdom Flag' decodes just below 5s (measured with stopwatch from selecting file to display of the image) on both CF (mpio hd200) and PP (mini 1G).
Comment by Marcin Bukat (MarcinBukat) - Wednesday, 27 October 2010, 20:12 GMT
arrgh wrong file

Loading...