• Status Closed
  • Percent Complete
  • Task Type Patches
  • Category Music playback
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.1
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by jdgordon - 2009-01-12
Last edited by jdgordon - 2009-07-20

FS#9789 - cuesheet using MoB

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.

Closed by  jdgordon
2009-07-20 05:19
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

a modified version of this was commited in r21978

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

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…

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

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…

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

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.

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 :/

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


Available keyboard shortcuts


Task Details

Task Editing