Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Applications
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by BeChris - 2008-10-15
Last edited by fg - 2009-07-25

FS#9493 - Add a png decoder to rockbox

Hi, here’s a work in progress png decoder for rockbox.

I still have some difficulties to do the conversion from 32 bit unsigned char data given by the decoder to the rockbox pixel format.

If anyone want to help, it’d be greatly appreciated.

   diff.txt (74.3 KiB)
Closed by  fg
2009-07-25 21:39
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Committed (version 8) as r22037

Why not add it to the pluginlib, and maybe make the jpegviewer a generic image viewer?

Reading the IRC logs, it was suggested that this task should wait for JPEG inclusion. I disagree.

We should decide if we want PNG support in core a priori, and either reject or accept the feature independent of the JPEG decoder. I tentatively support adding this (I think the feature is worth having), but want to see what the memory hit looks like. The impression I have is that PNG is reasonably complicated compared to JPEG. The format may not be a good fit for rockbox if it cannot be efficiently implemented.

petur commented on 2008-10-18 22:05

anyway, if the jpeg viewer is to become a generic image viewer, this code should become a lib, and the viewer code split up….. It would be a good idea to extract the jpeg decoding from the image viewer, maybe the image decoding can become a core plugin just like the codecs?

Looking at this patch, the 640KB static buffer jumps out and punches me in the face. Do you know how this value (MAX_SCANLINES) was derived?

I think a first step needs to be to investigate the memory usage of this decoder, and see how far it can be reduced. I don’t know anything about the PNG format, so don’t know the answer to that question.

To pack data, you can use the LCD_RGBPACK(r,g,b) macro - where r, g and b are 8-bit values. There is no alpha support in Rockbox. See firmware/export/lcd.h

This MAX_SCANLINES was added by me and is used to only allow decoding of image up to 400×400.
For the moment, I had to use 2 static buffers:
- unsigned char idat[MAX_IDAT_SIZE] (MAX_IDENT_SIZE being set to 300000) : this is the buffer in which content of png IDAT chuncks is written.
- unsigned char scanlines[MAX_SCANLINES] : a placeholder to decompress the zlib stream (which was written into idat just before) which is embedded in all png files.

With a friend we are currently trying to eliminate both buffers and thus reduce the momeory footprint of the decoder.

I’m not sure what this patch does? I patched rockbox with the patch and i don’t see any changes.. Does this patch make it possible to look at .png pics on da ipod?

Not for the moment, this was only a proof of concept for people who may have been interrested.

I will rework it as a plugin.

Any update? Yes? No?

Hello everybody, after several month far from coding on my computer, I’m now glad to announce that I suceed in creating a working version of my png viewer as a plugin.
Current limitations:
- Worked OK on my Sansa E200. Should only work on target with a 16bit LCD.
- The plugin uses the audio buffer. Therefore it will stop plaback when viewing a png file.
- No resizing (someone interrested in giving help ?).
- Lacks some tests on access out of memory buffer boundaries.

How to use it:
Select a png file.

Messages like that should appear:
Available memory : 32866712
blackhole.png:
loading 692 bytes
decoding image
Color conversion requested
Decoder OK

Then, you have to press right button to display the image during 2 seconds.
Any other button will make the plugin exit whithout displaying the image.

Enjoy this first version !

I’m waiting for your feedback.
Thanks in advance.

Cool, I’ll try this out. Thanks for Rescripting!

I tried this on my 5th Gen iPod. Here are the results:

It compiles with Cygwin, but I got quite a few errors (see “png compiling.png”)
But it works great. Some Features I request, Zoom In/Out, Added a custom icon (Or you could just use the jpeg or bmp icon), Layout Like JPEG Viewer, Rotate possibly? (Maybe use: FS#9497) Tomorrow I may try this on my Nano.

Nice Patch.

Opps silly me. I meant to say “Add” instead of “Added”.

Hello, I made some improvements:
- No more annoying compilation warnings.
- Better protection against memory out of bounds accesses.
- Behaviour exactly like JPEG plugin (same button layout, slideshow, progress while decoding, menu) except see below.

Current limitations:
- Resizing not yet implemented :( ⇒ Maybe I’ll have a look at bilinear interpolation.
- It seems decoding some files when a tune is plaing led to corrupted display.

Apply this second patch on a fresh rockbox source tree i.e rollback previous patch before.

Enjoy.

Any comments ?

Resizing not yet implemented :( ⇒ Maybe I’ll have a look at bilinear interpolation.

I presume you’re aware of smooth_resize_bitmap in the plugin lib?

Nice! If this keeps up It will have to be commited.

I’m definitely not able to use resizing (thanks saratoga for smooth_resize_bitmap hint).
My problem is, if I well understood, that output of png decoder is an unsigned char buffer (pixel packed as RGB).
But to use the resizing algorithm, I must convert it to fb_data buffer which I’m not able to do (I tried several approch, I always get scrambled display content).
This is the same problem if I want to include in the core a png loader : I’m not able to produce a valid stuct bitmap (especially the data field) : all is scrambled :(
I’m seriously considering to ask help on IRC.

A final path altough because the former one accidentaly impacted unrelated files:
- jpeg.c (only inserted blank caracters).
- SOURCES (lua plugin was no more compiled)

And it add the possibility to cancel the decoding of a png image by pressing the MENU button.

Oh thanks for that. I had to PNGs in one folder and and couldn’t read them and it kept going on and on trying to read them.

Gman: when you cancel the decoding of a file it will skip to the next one (except, if there is only one file in the folder, the plugin will terminate).
So it won’t solve your problem.
I will consider to terminate the plugin when a decoding is aborted regardless of the number of files in the folder.

You can get this behaviour by changing line:

      if (decoder.error == PLUGIN_ABORT && entries == 1) {

By:

      if (decoder.error == PLUGIN_ABORT) {

Thanks for testing and feedback.

Okay will do.

Yeeeeessss ! It works
I finally got the resizing to work !
After having spent a lot of time tracking a crash on the target (It worked OK in the simulator) I noticed I forgot to align some buffers.

I’m really proud of this plugin :)

What I have to do now is to clean it up a bit (remove encoder part, …) and check whether all king of png files are loaded OK.

Try it and give me your feedback.

(I have incorporated modification above : in case of MENU key pressed while a file is decoded, the plugin will exits)

Yay! Im going to try this right now.

Warning: Undefined array key "useheading" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 1099 Warning: Undefined array key "target" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557 Warning: Trying to access array offset on value of type null in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557

Hi, another major update.
Definitively this plugin is only for player which have a 16bit LCD, sorry no gray display.

Changelog:
- All merged in one file (png.c) + removed unused encoder part.
- Better protection against memory access fault (I hope I found every bugs forgotten before).
- Some image type were not correctly displayed.
- Drastically reduced needed memory to decode/display image.
- Better error messages.
- This plugin displays correctly all images of the PngSuite test suite (http://www.schaik.com/pngsuite/).

I think we are very close to a stable plugin now.

Please let me now if you find a file which crashes the plugin or which is not displayed as expected.

Will this also fix the bug on images bigger than 1000 in width/length? It loads them but nothing happens.

Hey every thing works good! Every thing loads, finally! I think you just need to fix the time to load an image it needs to be quicker but everything is good!

Hi, great work. I’m looking forward to seeing your png viewer in rockbox.
I tested your patch on a e200 and it works fine.
But please avoid the following compiler warnings:
CC apps/plugins/png.c
/home/johannes/Rockbox/rockbox/apps/plugins/png.c: In function ‘LodePNG_decode’:
/home/johannes/Rockbox/rockbox/apps/plugins/png.c:2185: warning: cast from pointer to integer of different size
/home/johannes/Rockbox/rockbox/apps/plugins/png.c:2185: warning: cast to pointer from integer of different size
/home/johannes/Rockbox/rockbox/apps/plugins/png.c: In function ‘get_image’:
/home/johannes/Rockbox/rockbox/apps/plugins/png.c:2722: warning: cast from pointer to integer of different size
/home/johannes/Rockbox/rockbox/apps/plugins/png.c:2722: warning: cast to pointer from integer of different size
LD png.rock

Hi, another update.

Changelog:
- Moved the plugin to its own directory (apps/plugins/png).
- A bit of optimizations on Huffman tree generation (altough rare impact).
- In case of “File too big” error message, the plugin could sometimes loop indefinitely.
- Propose, as made by the JPEG decoder, to stop playback in case of lack of memory.
- Added -O3 for compilation of this plugin : it gives unbelievable performance gain !

The last point is very important and gives incredible results.
Times given are times taken to decode the image, not the last resizing step:

For a 1536×2048 png file:
With -O CFLAGS (the default) : 50.85s
With -O2 : 37.58s
With -O3 : 22.75s

With a 1680×1050 png file:
With -O : 15.95s
With -O2 : 11.19s
With -O3 : 7.8s

Even if -03 adds 2Kb to the size of the .rock file, it’s worth enabling it !

And Ubuntuxer : I don’t get any compilation warnings here, try again with this patch on a fresh rockbox source tree.

As usual, comments ? suggestions ?

Hmm this doesn’t compile.

This patch does some unrelated svn:ignore changes, please remove those.

Could you do that he has gone on vacation till monday.

Hi, for compilation problem did you rollback previous patch and applied the latest on a fresh rockbox source tree ?
Indeed, in patch_png6.txt, png plugin has been moved in its own directory.

And, could you tell me:
- For which platform you are compiling (platform number as given when invoking configure script).
- An extract of the error you get.

Thanks.

Okay so I finally got this to compile and I must say that this is the best version yet. It is so fast and thanks for fixing the memory issue. I think you should send this over to the RockBox Admins to see if they want to commit this.

Warning: Undefined array key "useheading" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 1099 Warning: Undefined array key "target" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557 Warning: Trying to access array offset on value of type null in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557

I think it would be preferable if this plugin and the jpeg viewer could be merged, to achieve a uniform interface and featureset.

An alternative solution could be to use the same interface code for both, and build two plugins from that code, which then include either the jpeg decoder or the png decoder. That way, the interface is the same, and you don’t have to maintain two sets of interface code. The advantage is that the plugin wouldn’t be twice the size it is now (which is a concern with regards to memory usage).

Discussed in IRC: http://www.rockbox.org/irc/log-20090629#01:49:00

Hi, many thanks for those positive comments.
I’d like to do even more optimization before asking for inclusion if I can figure out how Rockbox profiling is working.
When activating it, all the plugins are freezing on my Sansa e260v1.
It seems to come from the -finstrument-functions gcc flag.

Someone out there could have a explanation on it ?
What a pity if I can’t use the rockbox profiling capabilities !

Have you tried doing profiling in the simulator? If that still crashes, you could at least use GDB to analyse the crash..

Hello, I didn’t try profiling under the simulator but put several time measure and found where it would be worth an effort of optimisation.
I already succeed in getting up to 10% performance gain for some image in the scanline unfiltering.
I’m now looking at some other possible coding tricks but don’t expect too much gain.

Stay tuned, I’ll keep you informed.

BeChris - you can read the document in doc/TECH which has a section on profiling right at the end.
Quick summary: you need to
1. ensure the makefile of the thing you’re compiling has -DPROFILE_OPTS in the CCFLAGS somewhere (or you can just add it in the call if it’s not there)
2. put a call to profile_thread() at the start of the thing you want to profile, and a call to profstop() at the end of the thing you want to profile (e.g. the start and end of your main plugin entry function)
3. create a new build directory, and using ../tools/configure set up the Advanced build options to add Profiling

Hello, I denitively can’t do profiling.
When enabled, my plugin randomly crashes and I get strange behaviours.

However, I worked on optimisations and got impressive results.

Changelog:
- A bit of optimizations on scanlines unfiltering (can give up to 10% on certain images).
- Now available for all LCD color bitmap capable players.
- Took inflate algorithm from origianl zlib distribution (image decoding got a boost of 40-50% !).

I only need to taylor a bit error messages which must take into account error coming from the zlib inflate algorithm.

Hello, a small update of previous patch because I forgot png.h in patch_png7.txt.
Now, errors coming from the inflate decompression are correctly displayed.

Give a try at patch_png8.txt (as usual : apply on a fresh source tree).

Benchmark on same files as for patch_png6.txt:
For a 1536×2048 png file: 11.23s
For a 1680×1050 png file: 4.60s

It goes almost twice faster !

For your information, I also worked on the adaptation of the official libpng to see whether I could get even more optimisation from it.
For that, I used dbestfit memory (dis)allocation routines from Daniel Stenberg.

And I was very surprised by the results and was very disappointed to noticed that it was a lot slower than the hybrid implementation LodePng/zlib introduced in patch_png7.

For those interrested, you can find it in patch_png9.txt

nls commented on 2009-07-24 10:04

I would really like to se this merged with the jpeg viewer into an image viewer.
Do you think that is feasible ad if so do you think it would be easier to do after a merge into the rockbox svn or before?

Hi Nils, I would prefer a merge in the svn before working on the unification.
It would be an honor for me to see this patch officially integrated but who decides this ?

Someone like jdgordon

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing