FS#12069 - Playback rewrite - first stages

Attached to Project: Rockbox
Opened by Michael Sevakis (MikeS) - Saturday, 16 April 2011, 08:08 GMT
Last edited by Michael Sevakis (MikeS) - Wednesday, 27 April 2011, 20:19 GMT
Task Type Patches
Category Music playback
Status Closed
Assigned To Michael Sevakis (MikeS)
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.8.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


It's about time to start testing this since it's been working well enough for me now and better than current SVN. It renovates the codec and playback system, addressing whatever bugs are in scope for it and largely confined to directly-related files. It's not all changes I want to make but I almost made my 512KB .patch goal :p. Redoing PCM and some other stuff would be going too far in one felled swoop).

Codec interface is radically changed. NSF codec needs some updating with the metadata parsing if it's to work correctly because it can't poke values into the WPS any longer; the codec's metadata is private to it and playback. I've considered some sort of proper metadata push interface if we do want to do that sort of thing. Anyone who knows about them, give certain codecs a check; however for the most part, the changes are fairly formulaic.

There's a #define in playback.h called 'AUDIO_FAST_SKIP_PREVIEW' which enables fast skip preview to keep the displayed track in time with user actions but does use more RAM, so if it's of little benefit on a target (most likely a flash one), it can be disabled for it.
This task depends upon

Closed by  Michael Sevakis (MikeS)
Wednesday, 27 April 2011, 20:19 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed in r29785 with a few minor tweaks.
Comment by Michael Sevakis (MikeS) - Saturday, 16 April 2011, 19:53 GMT
Resync for alac and aac to r29727
Comment by MichaelGiacomelli (saratoga) - Tuesday, 19 April 2011, 06:05 GMT
Interestingly this patch seems to make  FS#12033  much worse. Playing the same SPC file twice in a row while the WPS screen is updating will crash my e200v1 every time. I have to let SPC run for a couple seconds before I can safely click the same file from the file browser.
Comment by Michael Sevakis (MikeS) - Tuesday, 19 April 2011, 09:12 GMT
Can't duplicate what you're talking about no matter what (varying database/dircache settings) even clicking as fast as I can select the SPC (about 3 times/s) but that bug shouldn't exist anyway.

ETA: it's not precisely the same patch either
Comment by JoshuaChang (JoshuaChang) - Tuesday, 19 April 2011, 11:09 GMT
change the spc songs rapidly in file browser should lead to system freeze, change them in wps/playlist is okay.
Comment by Michael Sevakis (MikeS) - Tuesday, 19 April 2011, 12:10 GMT
That's what I was doing...selecting an SPC in the file browser then clicking select as fast as possible, file->WPS->file->WPS->... etc. which gets about 3starts/second (much slower than my clicking). Nothing went wrong (e200v1). Same results clicking different ones.
Comment by JoshuaChang (JoshuaChang) - Tuesday, 19 April 2011, 13:30 GMT
have you turned off the dircache? on my cowon d2+ with original rockbox, the only way to get rid of this issue is disable the diracache
Comment by Michael Sevakis (MikeS) - Tuesday, 19 April 2011, 13:42 GMT
Dircache on/off, ramcache on/off, database yes/no, voice enabled/diabled, keyclick on/off, etc. etc., I can't get it to mess up. Maybe something with the particular files or some other setting having side effects? Attach .config and maybe the .spc as well, more appropriately over in  FS#12033 , not here.

I use .spc a fair amount and am surprised I never noticed such a problem.
Comment by JoshuaChang (JoshuaChang) - Tuesday, 19 April 2011, 13:48 GMT
i have no idea, for the default configuration can also reproduce the issue, i have many spc, too, and i don't think it's special file related.
Comment by Michael Sevakis (MikeS) - Thursday, 21 April 2011, 03:36 GMT
Updato and synced...satisfied with this one and handles track errors more thoroughtly (that's a messy thing to do right with the curent split loading). I don't want to go deeper now without the changes in SVN afterwhich I'll experiment with more radical ways of simplifying things (there are things I *don't* want to alter just yet, believe it or not :). FYI: Some things *can* be committed separately (like kernel and buffering diffs) but they're really designed to initially support this patch.
Comment by Michael Sevakis (MikeS) - Thursday, 21 April 2011, 08:30 GMT
Frank, here your patch! Hope you not disappoint.

Also, adds a left-out line in apps/codecs/atrac3_rm.c that prevented resume from working there.

ETA: last one is all-in-one of the previous files
Comment by Steve Bavin (pondlife) - Thursday, 21 April 2011, 10:43 GMT
Hi Mike,
I'm attempting to test on a simulator, but build fails (H300 sim, under Cygwin, FWIW):

LD vorbis.codec
/home/Steve/rockbox/buildsim/apps/codecs/libcodec.a(codeclib.o): In function `qsort':
/home/Steve/rockbox/apps/codecs/lib/codeclib.c:149: multiple definition of `_codec_main'
/home/Steve/rockbox/buildsim/apps/codecs/vorbis.o:/home/Steve/rockbox/apps/codecs/vorbis.c:108: first defined here
collect2: ld returned 1 exit status
make: *** [/home/Steve/rockbox/buildsim/apps/codecs/vorbis.codec] Error 1

This is with only the last patch (playback-version-ii-5.patch) applied to SVN r29754.
Comment by Michael Sevakis (MikeS) - Friday, 22 April 2011, 06:32 GMT
hmmm...don't think I changed anything in codeclib since the first patch...will check it out

I mean, it's saying codec_main is inside qsort? :p WTF?
Comment by Michael Sevakis (MikeS) - Friday, 22 April 2011, 07:21 GMT
I just built and ran this as an H300 sim on debian VMWare without trouble besides a few format warnings for DEBUGF (which I corrected).

ETA: Still I have no idea why you were (or may still be) getting that problem.
Comment by Steve Bavin (pondlife) - Sunday, 24 April 2011, 17:50 GMT
A slightly more useful observation; I've noticed that the compiler (gcc 3.4.4) gives an earlier warning:
warning: weak declaration of 'codec_main' not supported
Comment by Michael Sevakis (MikeS) - Monday, 25 April 2011, 15:34 GMT
Maybe it has something to do with not declaring it as such in the headers or I did it kind of wrong ?? We do use the weak declaration in some target code and have for a long time. I have 4.3.2 installed as a native compiler.
Comment by Michael Sevakis (MikeS) - Monday, 25 April 2011, 15:43 GMT
I did a bit of factoring some code out of an operation that really should not be part of starting a codec. Also, this handles short tracks with behavior consistent with longer tracks (I didn't like the way I handled it above by forcing the outstanding skip to complete early even though it works anyway); videogame formats can have many short files in a row (decoding VGM is one) and well it's just better if it all looks good doing it. Later (read: after a commit) I'll see if I can take advantage of limiting decode-ahead to one track into the future.