Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Themes
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Nicolas Pennequin - 2007-03-20
Last edited by Nicolas Pennequin - 2007-04-04

FS#6862 - WPS Tokenizer

This is my WPS tokenizer patch. The basic idea is to store the WPS as an array of tokens. That way it is only parsed once and transformed into a form easier to use for displaying.
The concept is somewhat similar to what’s explained here : http://www.rockbox.org/twiki/bin/view/Main/WPSTokenbasedHandling. There are also some explanations on  FS#4826  and  FS#2898 .

To make this patch, I used the code in  FS#4826  as a base and improved the parser (I posted some code on that entry). After that, I started working on integrating it into the current code. This proved to be quite hard, so I decided to rewrite some parts of it, namely gui_wps_refresh() and most of the functions it uses.

It appears to be working nicely with most of the WPS files I tested on most targets.
It still needs to go through extensive testing, so please apply it to your build and test some WPS files :)
Please report any crashes you get (although there shouldn’t be any), and post feedback on performance differences, if you notice any (maybe try to activate heavy CPU features like crossfeed, EQ, replaygain, etc… and compare with an SVN build).
Also I’m not sure at all it will work well on the archos player, so it would be good if someone could try that too.

Closed by  Nicolas Pennequin
2007-04-04 16:11
Reason for closing:  Accepted
Additional comments about closing:  

Comitted to SVN. Please report problems on the forum : http://forums.rockbox.o rg/index.php?topic=9727.0

Nicolas Pennequin commented on 2007-03-21 19:17

New version, with the image preloading code adapted to the parsing code.
Also the debug code has been moved into a separate file.

Nicolas Pennequin commented on 2007-03-22 00:39

A few corrections, especially a very important check I forgot : the number of lines and sublines can’t be allowed to become greater than the max values. This caused serious problems on long WPS files.

Nicolas Pennequin commented on 2007-03-27 23:56

I added a way to hide images within a conditional when they aren’t supposed to be displayed anymore. Also a few other minor improvements and some additional code commenting.

Please test and/or review !

Nicolas Pennequin commented on 2007-03-28 23:21

A few updates. I think this patch is now close to being committable :)

Dan Everton commented on 2007-03-29 04:45

Just some minor comments from a quick review of the patch.

  • There’s an #if 0 block in gwps-common.c. Should that still be there?
  • This code

+ /* Skip the rest of the line */
+ while(*(wps_token + skip) != ‘\\n’)
+ skip++;

  shows up in a few places. Would it be worth factoring that out in to a function? Might save some bytes.
Nicolas Pennequin commented on 2007-03-30 12:51

The #if 0 might become useful again so I’m keeping it for the moment.
Thanks for the other suggestion, it’s done :)
Another update should be arriving shortly.

Nicolas Pennequin commented on 2007-03-30 13:44

So here is the updated version.
Most of the updates are discusses here : http://www.rockbox.org/mail/archive/rockbox-dev-archive-2007-03/0112.shtml There are a few others too, but nothing really important.
The good news is with all the duplicate code removal, binsize seems to have gone down a little. I hope I’ll be able to make other improvements in that area :)

Alexandre Flament commented on 2007-03-31 17:52

I have tested the wps_tokenizer.patch (30 march 2007 version) with my Archos FM Recorder.

It seems to work with the themes I have tested except one : the rockbox_default has some pixels are set randomly (?) on the last line of the screen (just bellow the progress bar). Some pixel are set, some other are clear during the playback, but the line is never filled. The simulator has the same problem. The others themes I have tested : black-white, boxes, iCatcher, relief, Rockboxed (found here : http://www.rockbox.org/twiki/bin/view/Main/WpsArchos )

If HAVE_TAGCACHE is not defined, compilation is done, but the player freezes every time the WPS should appear.

For information : the rombox.ucl is ~1ko bigger with this patch (compiled without tagcache).

Note : I compiled rockbox using the VMPlayer image and I made a dist-upgrade, but sh-elf-gcc is still the version 4.0.3 (rocbox patch #1).

Nicolas Pennequin commented on 2007-03-31 18:16

Thanks for the report :)
I think the pixels under the progressbar on the default WPS are due to the peakmeter… I’ll take a closer look at it.
Also I’ll check why not having tagcache causes a crash.

Nicolas Pennequin commented on 2007-04-01 13:38

Update: Moved the code around and tweaked some things to reduce memory usage.

Nicolas Pennequin commented on 2007-04-02 19:48

Update:
* Fix the issue with the peakmeter described above
* Fix crashing when the WPS had no newline at the end of the file (reported by pixelma)
* Fix some issues with sublines display
* Adapt to amiconn’s archos player fixes. Now both the progressbars are displayed correctly.
* Some other changes I don’t remember :)

Nicolas Pennequin commented on 2007-04-04 00:37

Update:
* Memory improvements, mainly by removing the format_buffer where the WPS source was stored. The parser now uses the plugin buffer as temporary storage for the source.
* BMP loading was rewritten to make the pictures be loaded all at once after the parsing is finished
* Other improvements I can’t remember

Nicolas Pennequin commented on 2007-04-04 14:15

Small update with BMP loading.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing