Rockbox.org home
Downloads
Release release
Dev builds dev builds
Extras extras
themes themes
Documentation
Manual manual
Wiki wiki
Device Status device status
Support
Forums forums
Mailing lists mailing lists
IRC IRC
Development
Bugs bugs
Patches patches
Dev Guide dev guide
Search



Donate

Rockbox Technical Forums


Login with username, password and session length
Home Help Search Staff List Login Register
News:

Welcome to the Rockbox Technical Forums!

+  Rockbox Technical Forums
|-+  Rockbox General
| |-+  Announcements
| | |-+  WPS tokenizer: REPORT PROBLEMS HERE
« previous next »
  • Print
Pages: 1 [2] 3 4 5

Author Topic: WPS tokenizer: REPORT PROBLEMS HERE  (Read 46162 times)

Offline DukeLuke

  • Member
  • *
  • Posts: 16
    • Homepage
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #15 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 :-\
« Last Edit: April 07, 2007, 03:59:46 AM by DukeLuke »
Logged

Offline Lear

  • Developer
  • Member
  • *
  • Posts: 533
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #16 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.
Logged

Offline NicolasP

  • Developer
  • Member
  • *
  • Posts: 195
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #17 on: April 07, 2007, 08:00:30 AM »
Quote from: 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 :-\
I'm pretty sure I solved the hold switch with r13054. Did you try this version ?
I added the sleep timer tag in r13056.

Quote from: 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.
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 :)
Logged

Offline DukeLuke

  • Member
  • *
  • Posts: 16
    • Homepage
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #18 on: April 07, 2007, 09:14:44 AM »
Quote from: NicolasP on April 07, 2007, 08:00:30 AM
Quote from: 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 :-\
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
Logged

Offline roolku

  • Developer
  • Member
  • *
  • Posts: 350
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #19 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

Logged

Offline Llorean

  • Member
  • *
  • Posts: 12931
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #20 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.
Logged

Offline roolku

  • Developer
  • Member
  • *
  • Posts: 350
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #21 on: April 07, 2007, 04:06:57 PM »
Quote from: 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.

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

Offline NicolasP

  • Developer
  • Member
  • *
  • Posts: 195
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #22 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 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, so this might change. You could try the patch and tell me if it helps. I've posted it on the bug entry.
Logged

Offline roolku

  • Developer
  • Member
  • *
  • Posts: 350
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #23 on: April 08, 2007, 06:25:17 AM »
Quote from: 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.

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

Quote from: NicolasP on April 07, 2007, 09:48:39 PM
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 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. :(

Quote from: NicolasP on April 07, 2007, 09:48:39 PM
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).

Quote from: NicolasP on April 07, 2007, 09:48:39 PM
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;

Quote from: NicolasP on April 07, 2007, 09:48:39 PM
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


Quote from: NicolasP on April 07, 2007, 09:48:39 PM
Also, I'm working on a patch to fix FS#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.
« Last Edit: April 08, 2007, 07:16:58 AM by roolku »
Logged

Offline Lear

  • Developer
  • Member
  • *
  • Posts: 533
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #24 on: April 08, 2007, 01:52:17 PM »
Quote from: 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.

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

Offline stripwax

  • Developer
  • Member
  • *
  • Posts: 84
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #25 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.
« Last Edit: April 09, 2007, 07:11:10 AM by stripwax »
Logged

Offline stripwax

  • Developer
  • Member
  • *
  • Posts: 84
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #26 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.
« Last Edit: April 09, 2007, 07:20:34 AM by stripwax »
Logged

Offline stripwax

  • Developer
  • Member
  • *
  • Posts: 84
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #27 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
« Last Edit: April 09, 2007, 08:36:31 AM by stripwax »
Logged

Offline NicolasP

  • Developer
  • Member
  • *
  • Posts: 195
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #28 on: April 09, 2007, 10:07:42 AM »
I committed your fix, thanks a lot :)
Logged

Offline Lear

  • Developer
  • Member
  • *
  • Posts: 533
Re: WPS tokenizer: REPORT PROBLEMS HERE
« Reply #29 on: April 09, 2007, 10:56:16 AM »
Quote from: 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.

Found and fixed it. A recent change in playback.c was the culprit.
Logged

  • Print
Pages: 1 [2] 3 4 5
« previous next »
+  Rockbox Technical Forums
|-+  Rockbox General
| |-+  Announcements
| | |-+  WPS tokenizer: REPORT PROBLEMS HERE
 

  • SMF 2.0.19 | SMF © 2021, Simple Machines
  • Rockbox Privacy Policy
  • XHTML
  • RSS
  • WAP2

Page created in 0.139 seconds with 21 queries.