Rockbox mail archiveSubject: Re: WPS playlist viewer exposes a bug in either plaback/buffering or playlist. I'm stumped!
Re: WPS playlist viewer exposes a bug in either plaback/buffering or playlist. I'm stumped!
From: Jonathan Gordon <jdgordy_at_gmail.com>
Date: Sat, 17 Apr 2010 21:27:11 +1000
On 17 April 2010 21:16, Magnus Holmgren <magnushol_at_gmail.com> wrote:
> On 2010-04-16 09:07, Jonathan Gordon wrote:
>> Well I'm stumped on this bug
>> (http://www.rockbox.org/tracker/task/11175 ). Basically what happens
>> is if you use a wps with the playlist viewer (attached should work on
>> every target) the playlist will load in the wrong order and Wierd Shit
>> (TM) happens when jumping around tracks. If you use a theme without it
>> (or even I assume the same theme with it disabled (i.e in a
>> conditional viewport)) the playlist would work fine.
>> The code for the playlist viewer is draw_playlist_viewer_list() in
>> apps/gui/skin_engine/skin_display.c starting around line 182. I'm
>> pretty certain the problem is either in audio_peek_track() being
>> called or playlist_peek(). Nothing else there touchees anything
>> outside of the drawing/skln code.
>> I dont know playlist/buffering/playback anywhere near enough to figure
>> out what's going on, so if someone who does could have a look, that
>> would be great.
> I had a quick look at it, and I noticed two things:
> * It uses quite a bit of stack (about 2700 bytes, on ARM at least). Probably
> not a problem, but checking stack usage could perhaps be an idea.
> * Looks like "buf" (in draw_playlist_viewer_list) can overflow. Consider
> what happens if "buf_used" goes larger than "sizeof(buf)"... Suggestion: use
> strlcat instead. Protects against overflow and simplifies the code. Don't
> know if that's what causing the problems though.
The large stack usage is probably (mostly) the struct mp3entry which
is needed as a buffer for tracks which havnt been buffered yet. We
might be able to avoid this by reading directly of the buffer but that
could be dangerous.
I'll change it to strlcat() when I get some time (unless someone else
wants to :) )
Received on 2010-04-17