Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.6
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Marcin Bukat - 2010-09-24
Last edited by Marcin Bukat - 2010-10-31

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

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

Closed by  Marcin Bukat
2010-10-31 12:44
Reason for closing:  Accepted
Additional comments about closing:  

commited as r28413

Marcin Bukat commented on 2010-09-25 19:13

Switch from zlib to tinf as a inflate decompression backend. This saves binsize considerable.
Other minor optimizations.

Teruaki Kawashima commented on 2010-09-26 14:26

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.

Marcin Bukat commented on 2010-09-28 22:11

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

Teruaki Kawashima commented on 2010-09-29 14:47

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.

Marcin Bukat commented on 2010-09-29 22:18

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)

Marcin Bukat commented on 2010-09-30 21:59

git version

Marcin Bukat commented on 2010-10-16 20:57

This version places tables needed by zlib decompressor on the stack (which is placed on iram) in order to speedup decompression. Other minor fixes.

Marcin Bukat commented on 2010-10-18 14:29

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.

Marcin Bukat commented on 2010-10-19 11:28

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

Marcin Bukat commented on 2010-10-19 19:53

* Removed adler32 checksum calculation as this is redundant in case of PNG * Other minor changes in decompressor

Marcin Bukat commented on 2010-10-19 23:07

* Removed adler32 checksum calculation as this is redundant in case of PNG * Other minor changes in decompressor

Marianne Arnold commented on 2010-10-19 23:10

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

Marcin Bukat commented on 2010-10-20 08:05

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

Marcin Bukat commented on 2010-10-20 09:08

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

Marcin Bukat commented on 2010-10-21 21:40

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.

Marcin Bukat commented on 2010-10-27 20:11

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

Marcin Bukat commented on 2010-10-27 20:12

arrgh wrong file

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing