Rockbox Technical Forums

Rockbox General => Announcements => Topic started by: NicolasP on April 04, 2007, 11:15:20 AM

Title: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 04, 2007, 11:15:20 AM
The WPS loading and displaying system has been changed. Read the commit message (http://build.rockbox.org/cvsmod/chlog-20070404T144241Z.html) or the tracker entry (http://www.rockbox.org/tracker/task/6862) for more information about the changes. There may be some problems left, although a lot of testing has been done already.

If you encounter a problem like a WPS not being displayed correctly or causing a crash, please report it here after having made sure it hasn't been mentioned before. If a particular WPS causes the problem, please provide a link to download it or attach the relevant files. Also, don't forget to mention which target is affected by the problem.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: donutman25 on April 04, 2007, 02:01:42 PM
Where do i find instruct on how to use this new wps system?
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 04, 2007, 02:16:09 PM
You use it exactly like the old one. It works like the previous system but has improvements. This topic is for problem reporting purposes. By problems I mean broken tags, broken WPSs or unexpected crashes.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Llorean on April 04, 2007, 02:24:11 PM
Basically, everything should be exactly the same. If it's not, let us know.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: robin0800 on April 04, 2007, 03:07:44 PM
I assume this breaks all the WPS patches was it tested with any of them?
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Llorean on April 04, 2007, 03:36:03 PM
It was made available in the tracker. Whether they tested against it or adapted to it is their problem and has no place in this thread. Official builds only.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: pilot000 on April 04, 2007, 04:04:29 PM
I found a "minor" problem. I use my wps on a RTC-modded H140(see http://www.rockbox.org/twiki/bin/view/Main/WpsIriverH100#RemoMartinelli).
This line has a problem with the different delay times:
Code: [Select]
%ac%?Ia<%t3%xdn %Ia |%t0.4%xda.Please Wait.>;%ac%?Ia<%t3%xdo %Id |%t0.4%xda..Please Wait..>;%ac%?Ia<%t3%xdp %It |%t0.4%xda...Please Wait...>With the newest build (13020), the display changes all 0.4 sec., even the next track info is available, but it should change only all 3 seconds.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 04, 2007, 10:45:42 PM
I found a "minor" problem. I use my wps on a RTC-modded H140(see http://www.rockbox.org/twiki/bin/view/Main/WpsIriverH100#RemoMartinelli).
This line has a problem with the different delay times:
Code: [Select]
%ac%?Ia<%t3%xdn %Ia |%t0.4%xda.Please Wait.>;%ac%?Ia<%t3%xdo %Id |%t0.4%xda..Please Wait..>;%ac%?Ia<%t3%xdp %It |%t0.4%xda...Please Wait...>With the newest build (13020), the display changes all 0.4 sec., even the next track info is available, but it should change only all 3 seconds.
This should be fixed with r13026. Thanks for the report :)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: AiZ on April 05, 2007, 08:09:44 AM
Salut Nicolas,

Using r13027, I've some problems with sublines. I'm using a (slightly) modified darkgeek theme on my Nano and for testing purpose, I 've added a line.
If I input A;B;C;D in my wps, ouput is A;B;C;D
If I input %iaA;%itB;%fbC, output is (ID3 artist displayed Ok)A;(ID3 Track Title displayed Ok)B;(File Bitrate displayed Ok)C
If I input %ia;%it;%fb, output is OK.

So, it looks like that what it's just before the semi-colon must be a WPS tag to allow sublines working correctly. If it's ordinary text, cycling is broken.

Or perhaps I'm completely wrong. Can you test this, please ?

A pluche,


        AiZ
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 05, 2007, 10:16:24 AM
Salut Nicolas,

Using r13027, I've some problems with sublines. I'm using a (slightly) modified darkgeek theme on my Nano and for testing purpose, I 've added a line.
If I input A;B;C;D in my wps, ouput is A;B;C;D
If I input %iaA;%itB;%fbC, output is (ID3 artist displayed Ok)A;(ID3 Track Title displayed Ok)B;(File Bitrate displayed Ok)C
If I input %ia;%it;%fb, output is OK.

So, it looks like that what it's just before the semi-colon must be a WPS tag to allow sublines working correctly. If it's ordinary text, cycling is broken.

Or perhaps I'm completely wrong. Can you test this, please ?

A pluche,


        AiZ
You were right. It should be fixed with r13034.
Merci ;)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: elborak on April 05, 2007, 02:15:19 PM
So, if I understand correctly, adding a new WPS tag with the new tokenizer would consist of:

(Note that this ignores any complications for RTC tags and doesn't address where to actually get the data to support the new tag, e.g. reading a previously unsupported ID3 tag.)

Do I have this right?  As an experiment, I updated http://www.rockbox.org/tracker/task/4961 to the current SVN and it seems to work fine.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 05, 2007, 02:30:32 PM
So, if I understand correctly, adding a new WPS tag with the new tokenizer would consist of:
  • Add a new enumeration to wps_token_type in gwps.h.
  • Add whatever entries are appropriate to all_tags in wps_parser.c.
  • Add a case for the new enum to the switch in dump_wps_tokens in wps_debug.c.
  • Add a case for the new enum to the switch in get_tag in gwps-common.c.

(Note that this ignores any complications for RTC tags and doesn't address where to actually get the data to support the new tag, e.g. reading a previously unsupported ID3 tag.)

Do I have this right?  As an experiment, I updated http://www.rockbox.org/tracker/task/4961 to the current SVN and it seems to work fine.
You got it right. I looked at the WPS part of the patch and it seems fine to me :)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: roolku on April 06, 2007, 11:03:22 AM
I don't know if you saw my comment on IRC:

[15:50] Nico_P: what do you mean by "strictly positive tiemout" ? does this mean %t0 is not possible anymore? This is a very useful feature (even documented with an example on CustomWPS) and should definitely stay.

There needs to be a way to turn sublines of conditionally and %t0 is what WPSs are using at the moment.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Llorean on April 06, 2007, 11:06:47 AM
It has been said that the goal is to keep all the old WPS functionality. Have you tried it yet? If it doesn't work, it's a problem, but it sounds like you haven't tested yet.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 06, 2007, 12:45:05 PM
I don't know if you saw my comment on IRC:

[15:50] Nico_P: what do you mean by "strictly positive tiemout" ? does this mean %t0 is not possible anymore? This is a very useful feature (even documented with an example on CustomWPS) and should definitely stay.

There needs to be a way to turn sublines of conditionally and %t0 is what WPSs are using at the moment.
Should be fixed with r13047. This new version should work exactly like the old code.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: DukeLuke on April 07, 2007, 03:53:20 AM
I've got a problem. Since the inclusion of the patch, the sleep timer of my WPS "azure v2" for e200 doesn't show up anymore. The hold switch DOES show up, but it won't disappear when disabling the hold mode :-\
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Lear on April 07, 2007, 05:12:49 AM
I'm not sure if this is related to the tokenizer, but when I play MP3 tracks (with ID3V2 tags) I sometimes get slightly corrupted text in the WPS. E.g., track number is an empty string (but still present), year shows part of a replaygain tag name and similar things. Has anyone else seen that? There has been some minor changes to the ID3 reader, but I doubt they would have effects of this kind.

I don't have access to a development environment at the moment, so I can't make any closer investigation of it right now.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 07, 2007, 08:00:30 AM
I've got a problem. Since the inclusion of the patch, the sleep timer of my WPS "azure v2" for e200 doesn't show up anymore. The hold switch DOES show up, but it won't disappear when disabling the hold mode :-\
I'm pretty sure I solved the hold switch with r13054. Did you try this version ?
I added the sleep timer tag in r13056.

I'm not sure if this is related to the tokenizer, but when I play MP3 tracks (with ID3V2 tags) I sometimes get slightly corrupted text in the WPS. E.g., track number is an empty string (but still present), year shows part of a replaygain tag name and similar things. Has anyone else seen that? There has been some minor changes to the ID3 reader, but I doubt they would have effects of this kind.

I don't have access to a development environment at the moment, so I can't make any closer investigation of it right now.
I saw this once on the sim and thought it could be related to cuesheets. Once I disabled them the bug disappeared. Do yo have cuesheet support activated ? Anyway I'll try to find the cause of this but help is welcome :)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: DukeLuke on April 07, 2007, 09:14:44 AM
I've got a problem. Since the inclusion of the patch, the sleep timer of my WPS "azure v2" for e200 doesn't show up anymore. The hold switch DOES show up, but it won't disappear when disabling the hold mode :-\
I'm pretty sure I solved the hold switch with r13054. Did you try this version ?
I added the sleep timer tag in r13056.
Yay, solved  :D
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: roolku on April 07, 2007, 03:13:37 PM
A few more issues with the tokenizer I am afraid:

1) I can confirm Lear's observation, specifically

Quote
year shows part of a replaygain tag name

It is hard to reproduce, but I managed to get it several times by skipping backwards in quick succession.

2) conditionals don't work properly:

Code: [Select]
%?rp<| %rp>
%?rr<|%rr>

should display playcount/rating respectively if not 0 but nothing is displayed ever.

3) a dynamic tag which is right aligned such as:

Code: [Select]
%ar%pv

will clear the line including part of any graphic that is displayed to the left of it (conditionally if that matters) I suspect the drawing order has changed?

All observed on gigabeat with svn r13060.

Cheers

Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Llorean on April 07, 2007, 03:22:36 PM
I'm pretty sure that updating (or scrolling) aligned right strings cleared images to their left in the past. I'm almost positive I ran into this problem before, since lines are updated as lines.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: roolku on April 07, 2007, 04:06:57 PM
I'm pretty sure that updating (or scrolling) aligned right strings cleared images to their left in the past. I'm almost positive I ran into this problem before, since lines are updated as lines.

All I can say is that I have been using the WPS pre-tokeniser without problems. Since the image is conditional as well, I suspect the drawing order has changed so that the line is drawn after the image now, whereas before it was the other way around.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 07, 2007, 09:48:39 PM
Roolku:
I'm trying to find where the 1) comes from. I managed to reproduce it too but I have absolutly no idea what causes it.

The 2) looks like a non-issue to me. According to my tests, the %rr and %rp tags behave exactly like described in the CustomWPS (http://www.rockbox.org/twiki/bin/view/Main/CustomWPS#Runtime_Database_and_Replaygain) wiki page. The examples you give are true/false constructs, which return true if the tag has a value and false if the tag doesn't. Playcount and rating are numeric so they always have a value, so they'll always return the first case, which is empty in your examples. According to the wiki page, the rating is supposed to be used in an enum with 10 cases and the playcount isn't really meant to be used in a conditional...

About 3), it's possible that some things are refreshed differently than in the old WPS code. This is to try to be more effective. I'd like to see the WPS that has this problem. Also, I'm working on a patch to fix FS#6056 (http://www.rockbox.org/tracker/task/6056), so this might change. You could try the patch and tell me if it helps. I've posted it on the bug entry.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: roolku on April 08, 2007, 06:25:17 AM
Roolku:
I'm trying to find where the 1) comes from. I managed to reproduce it too but I have absolutly no idea what causes it.

It just occurred to me that it might be worth checking the "ID3 View" to see if it is a display only issue or of the year_text pointer is changed (out of bounds access somewhere?).

The 2) looks like a non-issue to me. According to my tests, the %rr and %rp tags behave exactly like described in the CustomWPS (http://www.rockbox.org/twiki/bin/view/Main/CustomWPS#Runtime_Database_and_Replaygain) wiki page. The examples you give are true/false constructs, which return true if the tag has a value and false if the tag doesn't.

Sort of. With the old implementation is was possible to abbreviate enumerations.

%?rr<|A|A|A|A|A|A|A|A|A|A>

to

%?rr<|A>

i.e. if there weren't enough cases in the tag, it would use the last one (documented as ELSE in CustomWPS).

I have also used the same mechanism to only display the audio codec if it wasn't mp3:

%?fc<%t4%fc|%t4%fc|%t0|%t4%fc>;%t4test1;%t4test2

and it still works here (thanks again for fixing the %t0 issue)

EDIT3: I have since found out that the problem comes from your assumption that enumerations must have at least 3 possible cases in gwps-common.c:1319:

Code: [Select]
   if (num_options > 2)  /* enum */
which explains why the codec example works. And indeed

Code: [Select]
%?rr<|A|A>  
works as well. This seems to be an unnecessary limitation to me. :(

Playcount and rating are numeric so they always have a value, so they'll always return the first case, which is empty in your examples.

This mechanism you describe only works/ed if *intval is not used to make the tags enums, but *intval is used here (or at least should be).

According to the wiki page, the rating is supposed to be used in an enum with 10 cases and the playcount isn't really meant to be used in a conditional...

Yes, it was not documented for %rp (hides head in shame), but playcount was an enum in the old implementation. And it still tries to be one if you look at your own code. But it doesn't work with the abreviated notation:

Code: [Select]
       
case WPS_TOKEN_DATABASE_PLAYCOUNT:
            if (intval) {
                *intval = id3->playcount + 1;
            }
            snprintf(buf, buf_size, "%ld", id3->playcount);
            return buf;

About 3), it's possible that some things are refreshed differently than in the old WPS code. This is to try to be more effective. I'd like to see the WPS that has this problem.

I have attached a simplified wps that should demonstrate the issue (has also the examples above).

EDIT: oops, no attachments anymore it seems? I'll try to catch you on irc and dcc it.

EDIT2: you are probably hunting easter eggs instead of bugs right now. :) You can get the wps here: http://roolku.pwp.blueyonder.co.uk/wps.zip


Also, I'm working on a patch to fix FS#6056 (http://www.rockbox.org/tracker/task/6056), so this might change. You could try the patch and tell me if it helps. I've posted it on the bug entry.

I tried it, but it failed to apply. :(

All test with r13069 on gigabeat. Thanks for your efforts.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Lear on April 08, 2007, 01:52:17 PM
Roolku:
I'm trying to find where the 1) comes from. I managed to reproduce it too but I have absolutly no idea what causes it.

I checked the ID3 info screen when I saw the problem, and it showed the same (incorrect) values there. This suggest it isn't the tokenizer, unless it corrupts memory somehow. Whatever the cause, it doesn't always happen for a certain file.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: stripwax on April 09, 2007, 06:24:33 AM
It seems like scrolling lines that contain the next track info tag no longer scroll?  Unfortunately I don't have access to an earlier build so I'm not sure how/when/if this broke.  But the following (for me) doesn't scroll long track titles at all

%s%?It<%It|>

I see the same broken behaviour if I run the Unicatcher wps in the sim  (I'm using Ipod Video sim, if that matters).  Take the %ac out of the "%s%acNext: ..." line to see what I mean.  If I pick a long track title, the WPS displays the truncated track title and the line doesn't scroll.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: stripwax on April 09, 2007, 07:02:29 AM
Hm, why is all the 'next' metadata set to be WPS_REFRESH_DYNAMIC --  apart from %IA (album artist) which is WPS_REFRESH_STATIC - ?  That doesn't make much sense to me.
Setting all the 'next' metadata to be WPS_REFRESH_STATIC seems to fix the above bug.   Am I missing something?

The bug by the way is that the 'next track' text is getting updated continually.  write_line gets called constantly with the (same) next track text. This means that puts_scroll also gets called constantly, so the scroll offset just gets reset to 0 all the time and the scroll_thread doesn't get a chance to actually scroll this text

Edit - amiconn pointed out that the 'next track' metadata needs to be dynamic on hwcodec devices because it's not available until the next track is already in the hw buffer.  Maybe I'll pore over the svn diffs and try and work out how/why this worked before the tokenizer code.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: stripwax on April 09, 2007, 08:33:43 AM
Here, this patch seems to fix.  The new wps code wasn't testing WPS_REFRESH_SCROLL refresh flag, now it is.  If it's a scroll line and refresh_scroll & WPS_REFRESH_SCROLL is not set, do nothing.  I believe this is the original behaviour too.
I've also put all the 'current' track tags to be STATIC and all the 'next' track tages to be DYNAMIC

http://www.rockbox.org/tracker/task/7000
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 09, 2007, 10:07:42 AM
I committed your fix, thanks a lot :)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Lear on April 09, 2007, 10:56:16 AM
Roolku:
I'm trying to find where the 1) comes from. I managed to reproduce it too but I have absolutly no idea what causes it.

Found and fixed it. A recent change in playback.c was the culprit.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: bundy on April 09, 2007, 05:08:42 PM
The newest patch broke few themes with multiple sublines in them.

I made a patch which should solve this: http://www.rockbox.org/tracker/task/7004
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 09, 2007, 09:15:06 PM
I committed your patch, thanks :)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: vimes on April 12, 2007, 04:57:34 AM
I am not 100% sure that the problem comes from this change but I use today's (12th of April) build of Rockbox for Gigabeat and, using any WPS, all the texts element display  are offset to the top of the screen by a constant.
What I should have on screen:
-------------------
|                        |
|                        |
|                        |
|test                   |
|test2                 |
-------------------
what I get :
-------------------
|                        |
|test                   |
|test2                 |
|                        |
|                        |
-------------------
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: PaulJam on April 12, 2007, 06:21:01 AM
Hi,

the tags for Crossfeed (%xd) and Crossfade (%xf) don't work anymore (in the uisimulator).
When i tried them in the simulator, it froze when it should display the wps.
I haven't tried them on the target.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 12, 2007, 09:25:05 AM
The tags for Crossfeed (%xd) and Crossfade (%xf) don't work anymore (in the uisimulator).
When i tried them in the simulator, it froze when it should display the wps.
I haven't tried them on the target.
I'll add them in my next commit.

I am not 100% sure that the problem comes from this change but I use today's (12th of April) build of Rockbox for Gigabeat and, using any WPS, all the texts element display  are offset to the top of the screen by a constant.
Do you use the crossfade or crossfeed tags ?
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: vimes on April 12, 2007, 12:50:12 PM
Both the use of crossfade and crossfeed tags are disabled in the options.
Tough if you meant in the WPS files, I found %xdE, %xdA  etc... tags in the WPS file corresponding to iCatcher and BeatMPGirl  but no straight %xd... but I don't really have any knowledge in WPS syntax.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 12, 2007, 12:56:26 PM
Is the problem still present with the latest version (r13127) ?
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: |2eM!x on April 12, 2007, 03:02:18 PM
Just updated today, I am not sure if this is a bug caused by this but here goes:

Code: [Select]
%al                              %pc%ac%pv                         -%pr  
Using this it seems the center tag is totally ignored.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 12, 2007, 09:39:40 PM
Just updated today, I am not sure if this is a bug caused by this but here goes:

Code: [Select]
%al                              %pc%ac%pv                         -%pr  
Using this it seems the center tag is totally ignored.
I think that's probably because the text on the left is too long to allow the centered text to be centered. The displaying code merges strings that would overlap.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Mmmm on April 14, 2007, 08:18:18 AM
Just noticed this commit....

13 Apr 12:05 Remove the crossfeed WPS tag from the manual.

Does that mean you aren't going to incorporate crossfeed tag? If so why not? ???

EDIT: Just found your chat on IRC about this.... I reckon it should be included because it is useful and a very small change. If I have set crossfeed it is really easy for me to forget that I have done so and very useful to have a reminder on the WPS.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Mmmm on April 14, 2007, 08:39:52 AM
Another thing is that when I did have %?xd<%xdP|%xdQ> in my WPS code, I just got a freeze when the WPS was attempting to load. Shouldn't the line be ignored if it is erroneous?

Took me ages to find the culprit! ;)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 14, 2007, 12:16:44 PM
Mmmm: I'll probably readd the crossfeed tag eventually but it won't be %xd anymore.
I'm about to commit a fix that should prevent crashing with the code you gave.

Bascule: There probably are some glitches with UTF-8 as I don't know much about it. Could you send me one of the files that cause a crash/freeze ?
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: bascule on April 14, 2007, 12:42:17 PM
I'll try again from scratch:
Has this patch stopped the WPS code from accepting UTF-8 .wps files?

I have a couple of UTF-8 without BOM (but with Windows line-endings) .wps files that I have written that worked perfectly before and now cause a unit freeze on my H120 whenever I try to go to the While Playing Screen once the .wps file has been loaded.

They are really simple WPSs, so I don't think it's a tag problem...

I'm on r13142-20070413

EDIT: No, that wasn't it. Converted to ANSI (and UNIX line-endings) - same result

Surely there can't be anything in the attached WPS to cause a problem?

# 'Calmer' WPS to emulate Rio Karma display
# Constructed by bascule
# Designed for nedore-9 font
# Works OK with any font up to nine pixels high
%wd
%al%?mm< [OFF]|[R@]|[R1]|[RS]|[A-B] >   %?ps< [SHF]|[ORD] >%ar%?bc< Batt: [CHG]|Batt: %bl %% >

%ac%s%?it< %it|File: %fm >

%ac%s%?ia< %ia|No Artist Info >

%ac%s%?id< %id|Folder: %d1 >

%pb|12|||

%acNext Track:
%ac%s%?It< %It (%Ia)|No Next Track Info >

%al-%pr   %?mp< #|%>|%|%||%>%>|%|%|%||%>%>|%

%ac%s%?it|%|%||%>%>|%< %< >%ac%pp/%pe%ar%pv db
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 14, 2007, 12:54:09 PM
Bascule: you need to escape the '#' character on the last line.
This made me see a flaw in the parser so thanks :)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: bascule on April 14, 2007, 01:05:06 PM
Oh, yeah, I remember seeing something about the change of behaviour of the hash symbol.

Thanks for that steer, here's hoping I can fix all my homebrews now ;D

EDIT: OK, well, that fixed the simple ones...

However, my WPS up on the Wiki, (TextBox (http://www.rockbox.org/twiki/bin/view/Main/WpsIriverH100#TextBox_by_bascule)), is still causing a freeze, and it contains no hash characters.

It is more complicated, but I don't see why it is misbehaving. Can anyone shed some light?

Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Mmmm on April 14, 2007, 04:59:16 PM
Mmmm: I'll probably readd the crossfeed tag eventually but it won't be %xd anymore.
I'm about to commit a fix that should prevent crashing with the code you gave.

Great stuff.... I'm not worried about the letters used, anything will do: cf cd cr although I think some of those may be taken already..I haven't had a look at that bit of the code for a long long time...
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on April 14, 2007, 11:01:27 PM
However, my WPS up on the Wiki, (TextBox (http://www.rockbox.org/twiki/bin/view/Main/WpsIriverH100#TextBox_by_bascule)), is still causing a freeze, and it contains no hash characters.

It is more complicated, but I don't see why it is misbehaving. Can anyone shed some light?
That WPS made me find another flaw in the parsing code, so thanks again :)
It should work fine with r13162.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: vimes on April 15, 2007, 05:53:41 AM
Is the problem still present with the latest version (r13127) ?
I just downloaded the latest version (couldn't find the r#####) and it still displays the info improperly. I uploaded comparison images in hope it will help you out.
(http://img440.imageshack.us/img440/2784/err1mb9.jpg)
(http://img143.imageshack.us/img143/3628/err2ni0.jpg)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Llorean on April 15, 2007, 05:54:48 AM
Um, that theme requires a custom build. You are aware that this thread is about the OFFICIAL build, right?
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: vimes on April 15, 2007, 05:57:05 AM
Yep, but I never ever used custom builds and there were working great before (i.e. except for the covers of course) so I figured the problem relied in the official build.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Llorean on April 15, 2007, 06:03:46 AM
Also, from the screenshots, it looks like you're not using the same font...
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: vimes on April 15, 2007, 06:11:00 AM
In any case, it also happens with the UniCatcher theme provided with the build while  using the default_rockbox font
(http://img216.imageshack.us/img216/3044/sanstitre1wu1.jpg)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: Llorean on April 15, 2007, 06:15:54 AM
Unicatcher is not meant to be used with rockbox_default.

You cannot expect themes to look right if you haven't installed the fonts they require.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: bascule on April 15, 2007, 07:05:04 AM
However, my WPS up on the Wiki, (TextBox (http://www.rockbox.org/twiki/bin/view/Main/WpsIriverH100#TextBox_by_bascule)), is still causing a freeze, and it contains no hash characters.

It is more complicated, but I don't see why it is misbehaving. Can anyone shed some light?
That WPS made me find another flaw in the parsing code, so thanks again :)
It should work fine with r13162.

Thanks, Nicolas, that works fine, now.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: RedBreva on May 01, 2007, 02:40:28 PM
a few themes have failed to load with %bp - output from debugwps complains that it is invalid...

As far as I can tell from the Wiki it is still valid code...
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on May 01, 2007, 03:23:04 PM
How have they failed ? It's normal that %bp is deemed unknown on th sim, because the sim doesn't have charging.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: RedBreva on May 01, 2007, 03:47:52 PM
The sim 'locks up' after loading - for example:
http://www.rockbox-themes.org/data/160x128x2/Headphones.zip

The debug output is here:
http://www.rockbox-themes.org/temp/headphones.zip
The only error I could see was the unknown for pb :-(

The music plays fine if the default theme is selected.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on May 01, 2007, 05:23:39 PM
According to the debug output, the WPS has too many tokens, and this cuts a conditional before it closes. That's why the player locks.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: stripwax on May 02, 2007, 06:17:09 AM
IMO the player should not lock up ...  no matter what kind of crap wps you throw at it..

If the token buffer is too full to fit a closing conditional, then either that last conditional should be removed, or the token buffer rolled back.

Or even, if the WPS failed to load for whatever reason, how about emptying the entire token buffer and inserting a single string token that describes what the error was?
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: NicolasP on May 02, 2007, 06:28:58 AM
Of course, I was planning on adding an error case for this, but I haven't had much time for rockbox in the last few days. My goal is to prevent all possible lock-ups.
About the message, I like the idea but displaying a splash message and then the default WPS would probably be a better solution than a WPS with a message. The only problem is that the error strings would need to be localized and that's probably not worth the binsize, so I think we'll have to stick to the sim messages.
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: roolku on May 02, 2007, 11:33:00 AM
Not sure if you have seen:

http://forums.rockbox.org/index.php?topic=10241.msg78345#msg78345

not only does it effect id3v1, but any empty text tag (i.e. winamp seems to write empty comment strings in id3v2)
Title: Re: WPS tokenizer: REPORT PROBLEMS HERE
Post by: RedBreva on June 06, 2007, 02:03:15 PM
Suggestion:
Would it be possible to optionally allow for EOL markers in sublines, after the semi-colon, an EOL with no semi-colon would obviously be considered the end of the statement

A very simple example:
Code: [Select]
%t5%alShuffle:%ar%cb %ce;%t5%al Repeat:%ar%cb %ce
Could become:
Code: [Select]
%t5%alShuffle:%ar%cb %ce;
%t5%al Repeat:%ar%cb %ce

With some of the very complex syntax now appearing in wps's (long line lengths!), spotting errors can be difficult, able being able to split them makes them easier to read, and the 'line number' for the error given by the parser would be more specific as to the exact position of the failure.