Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category User Interface → Themes
  • Assigned To No-one
  • Operating System Gigabeat F/X
  • 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 roolku - 2009-09-09
Last edited by jdgordon - 2009-09-18

FS#10596 - extra C displayed under album art and freeze when no backdrop

r22667, observed on target and sim

I noticed an extra C in my WPS when the album art didn’t fill the whole view port. In an attempt to narrow down the cause I removed items until all that remained was:

—-snip—-
### Basic Setup ###
%X|wpsbackdrop.bmp|

### Cover ###
%V|0|0|-|240|1|CECFCE|000000|
%Cl|0|0|c240|b240|
%C
—-snip—-

If there is an album art of say 240×200 a C will show in the top left corner. (not sure how long it has been like this as most of my album art is square)

However, if I also remove the backdrop like here:

—-snip—-
### Cover ###
%V|0|0|-|240|1|CECFCE|000000|
%Cl|0|0|c240|b240|
%C
—-snip—-

then rockbox hangs as soon as you go to the WPS (so parsing seems to be okay).

Closed by  jdgordon
2009-09-18 05:16
Reason for closing:  Fixed

Between s/"parsing seems to be ok"/"parsing didn't fail loudly, but possibly produced bullshit"/ :)

Can you tell the revision it was introduced?

try thsi patch

Now it segfaults in the sim:

Program received signal SIGSEGV, Segmentation fault.
[Switching to thread 43600.0xabe8]
0x004346ac in get_token_value (gwps=0x52fc40, token=0x52fd10,

  buf=0x569fc00 "\006", buf_size=128, intval=0x0)
  at /home/robert/clean/apps/gui/skin_engine/skin_tokens.c:326

326 *intval = 1;
(gdb)

okay, the version with backdrop now works.

When trying the second example without backdrop it either freezes (if I resume playback immediately after starting the simulator) or it segfaults (if I resume playback after (re-) selecting the wps in the theme settings):

Program received signal SIGSEGV, Segmentation fault.
[Switching to thread 44616.0xac14]
0x00431344 in get_line (gwps=0x52fc40, subline=0x55bfa0, align=0x559fd00,

  linebuf=0x559fd10 "", linebuf_size=260)
  at /home/robert/clean/apps/gui/skin_engine/skin_display.c:640

640 while (*value && (buf < linebuf_end))
(gdb)

ok, ill try to investigate this properly in the next few days… if you want you can go back to the svn code and just change the C to a space…. (return " "; instead of return "C";)

That wps works fine for me (tried it in e200 sim).

### Cover ###
%V|0|0|-|220|1|CECFCE|000000|
%Cl|0|0|c220|b220|
%C

I am also seeing this "C" on the e200 widecabbie theme. The "C" is located at the top left corner of the AA viewport. I believe it started with r22646. I have been able to reproduce in the e200 sim. I have not had Rockbox hang in my case.

I'm able to reproduce the freeze/crash with the UnhelpulAA theme (you can find it on the theme site for e200).

Interestingly, it doesn't have any images (except AA and progressbar, but those don't count).

This should fix the crash/freeze.

edit: 2nd try

Pretty sure u want to redo that….. x-x always =0

there's no x-x, it's x - x ? 1:0.

the ? takes precedence, doesn't it?

I also discovered yesterday that this "%C<%t10……" will display album art if true which is confusing to say the least %C should be the only tag to display album art IMHO

Sure it will display it. You need %?C if you want something conditionally on that.

curr_line→curr_subline→last_token_idx = wps_data→num_tokens - wps_data→num_tokens ? 1 : 0;
add brackets
curr_line→curr_subline→last_token_idx = (wps_data→num_tokens - wps_data→num_tokens ? 1 : 0);
curr_line→curr_subline→last_token_idx = ((wps_data→num_tokens - wps_data→num_tokens) ? 1 : 0);
wps_data→num_tokens - wps_data→num_tokens is never going to equal anything other than 0

Kugal my mistake I meant to write %?C<t10…… I.e. the conditional does this hence perhaps a bug?

I'm still seeing extra C's with r22695 on the Plain&Simple2 theme for the c200v1. This wps alternates textual information with album art using a 5 second timer, but the album art is being left on the screen.

teru commented on 2009-09-14 13:04

I think we shuold separate %C tag to two tags, %C and %Cd.
%C indicates whether album art is available while %Cd displays aa.
but this breaks existing wps… What is problem of current implementation is it doesn't care if %C tag is used as conditional or not.
Displaying aa even when %C is used as conditional e.g. %?C<…> is not correct behavior and makes it difficult to use %?C<…> in the viewport where aa isn't intended to be displayed. I'm guessing this is the cause of  FS#10599 .
thus %C should be handled completely differently if it is used as conditional.
this would be almost equivalent to have two tags except about breaking existing wps.

I thought about that too. Breaking all WPSs isn't really nice. Can we fix %?C to not display the AA? Other conditionals don't show this behavior too.

This should fix that AA is drawn due to the conditional (%?C).
i can't tell yet why the AA in  FS#10599  is drawn in the wrong vp.

teru commented on 2009-09-14 14:24

function parse_albumart_display in skin_parser.c sets the viewport for albumart while parsing %C and %?C<..>.
so, album art is drawn in the last viewport having %C or %?C<..>.

arg, that's a tricky one. I don't think that's fixable very cleanly without introducing %Cd

Introducing %Cd is possibly a good idea if we want multiple album arts (different sizes, or for the next track) too (i.e. with an identifier following, such as %Cda).

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing