Rockbox

Tasklist

FS#9789 - cuesheet using MoB

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Monday, 12 January 2009, 13:33 GMT
Last edited by Jonathan Gordon (jdgordon) - Monday, 20 July 2009, 05:19 GMT
Task Type Patches
Category Music playback
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Version 3.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

this is a first attempt at moving cuesheet support closer into the playback engine instead of being a tack-on to the WPS...

first version is swcodec only.. hwcodec needs adding.

What this patch does is move the actual cuesheet loading/parsing into playback/buffering. The next step is to fix the wps so the only extra stuff it needs for cuesheets is the progressbar drawing code.
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Monday, 20 July 2009, 05:19 GMT
Reason for closing:  Accepted
Additional comments about closing:  a modified version of this was commited in r21978
Comment by Jonathan Gordon (jdgordon) - Tuesday, 13 January 2009, 00:19 GMT
new version, still only swcodec.. not sure how to do hwcodec just yet...
This one is close to being finished... I've moved everything out of the wps that I wanted to. Playback adds a new event which is called when the cue subtrack changes which the WPS uses to do a full redraw (instead of the mess in update() ). unfortunatly playback doesnt keep the current tracks offset going at the end of the track so we cant use that callback fully just yet
Comment by Jonathan Gordon (jdgordon) - Tuesday, 13 January 2009, 01:44 GMT
first version for hwcodec... I can only test in the sim though which means I have no way of knowing if this works..... PLEASE test.

If it works you should see the cuemarks on the progress bar, be able to "track skip" inside the track, and the id3 info shuold be updated correctly as the song progresses...
Comment by Steve Bavin (pondlife) - Tuesday, 13 January 2009, 07:02 GMT
Silly question from a non-cuesheet user - is there a good reason we treat a cuesheet as a single track with sub-tracks/markers, rather than as a playlist (i.e. multiple indexed tracks)?
Comment by Jonathan Gordon (jdgordon) - Tuesday, 13 January 2009, 07:15 GMT
I'm a non-cue user also... I guess it was easier (and better from a UI perspective) to hack it onto playback instead of playlist. Nico is the person to ask...
Comment by Steve Bavin (pondlife) - Tuesday, 13 January 2009, 17:28 GMT
Seems to work ok on H300. Just one problem spotted so far:

Using the default WPS and album art, play an album, skip to track 5, then repeatedly (fast) skip back/forward between tracks 4 and 5. Playback works fine, but the WPS ends up with junk metadata displayed; heiroglyphs, basically (althogh nicely scrolling heiroglyphs).
Comment by Steve Bavin (pondlife) - Tuesday, 13 January 2009, 17:31 GMT
IMHO, playback shouldn't be handling cuesheets; they should be at a slightly higher level (cuesheet.c?). playback.c should just be told to play file x at offset y - a single interface which should be good for resume/bookmarks/cuesheets.
Comment by Jonathan Gordon (jdgordon) - Friday, 17 July 2009, 00:42 GMT
I plan on picking this up again.... just incase i have forgotten this by the time i get home.. here is my current thinking:
do the patch as it is above with some changes:
- mp3entry.cuesheet will be changed to .cuesheet_handle, if thats >=0 then a call to cuesheet_get_current() will return a cuesheet pointer, doing this because there will be no static buffers anywhere for cuesheets(i.e it will always be on.)
- on file buffer a .cue will be checked for, if found it gets parsed and dumped into the buffer and its hid will get put in the mp3entry struct
- playback will stay how it is in svn, so prev/next track will not know about cuesheets
- the wps will handle actual cue subtracks (hopefully more nicely than svn though)

next steps would be thinking about getting embedded .flac cuesheets working..


oh, there is a comment above that cuesheets should be handled like playlists and I agree 100%... but of course that cant really happen until the track skipping happens through playlist and not playback (wps calls playlist_next(), that then talks to playback.c to change tracks)

I dont really like exposing the handle id in the mp3entry struct, but I dont think a pointer into the buffer is at all safe..... ill think about this a bit... A possible answer is but cuesheet_get_current() inside playback.c so it has access to the internally stored track hid's.... thats sort of clean, but we dont want to add more crap there :/
Comment by Jonathan Gordon (jdgordon) - Friday, 17 July 2009, 06:59 GMT
something along these lines....
note to self, this will probbaly break on targets because the mob pointer is used without making sure it hasnt moved (which it could during a track playtime) the changes to audio_current_track() are bad.. need to put those bufgetdata() calls into something a bit safer

Loading...