FS#5892 - Sample-accurate FLAC seeking (with or without a seektable)
Attached to Project:
Rockbox
Opened by Adam Boot (rotator) - Sunday, 27 August 2006, 18:39 GMT
Last edited by Adam Boot (rotator) - Sunday, 27 August 2006, 21:31 GMT
Opened by Adam Boot (rotator) - Sunday, 27 August 2006, 18:39 GMT
Last edited by Adam Boot (rotator) - Sunday, 27 August 2006, 21:31 GMT
|
DetailsThe seeking routines in this patch were originally adapted from libFLAC 1.1.2. With this patch, seeking in FLAC files is accurate to the sample and resume is accurate to the next frame after the saved offset.
This patch could stand to be tested by those with FLAC libraries larger than my own. Edit: See comments for latest patch. |
This task depends upon
Closed by Adam Boot (rotator)
Wednesday, 08 November 2006, 22:01 GMT
Reason for closing: Accepted
Additional comments about closing: Checked into CVS
Wednesday, 08 November 2006, 22:01 GMT
Reason for closing: Accepted
Additional comments about closing: Checked into CVS
Hope that help with finding the problem.
1) Long seeks are very slow or hang player:
I've made the seeking a bit more aggressive and added some more checks to make sure it eventually stops on the correct frame. Long seeks (outside of the buffer) will always be slow because the disk needs to spin up, but after that seeking shouldn't ever take more than a couple seconds, with it usually being less. There may be some instances where it still takes longer than it should to seek, but I can't reproduce it, even with 20 minute+ seeks on tracks with no seektable.
2) Progress bar jumps back a bit:
This was due to a precision error with the target sample value being passed to the seek routine. If my calculations are correct, it was as much as 882ms off for 44.1kHz tracks.
I'm not sure how to describe this: on a short file (10s) I seeked forward to about 9s and then backwards and it start playing the _next_ file but the wps still showed the the file as playing. Once this happens I can't back up to the previous file; it just starts playing at the beginning of the first no matter how much I hit "back". In fact any backward seek over a second or so after a certain point of playback causes the next file to start playing but no wps update. Letting the next play in full recovers and I can jump back to it fine. This may be confined to short files but I don't know the cutoff.
Get a codec failure nearing the end of the 6 min music file after a seek to almost the end (an mp3 follows it and fails to play). I suspect the new codec on the audio buffer is being corrupted.
Still gets backward jumps and moves. Even saw the progress bar go backwards smoothly. Need more pb precision for short files.
Any seeking problems could also be related to a general seeking bug with rb:
http://www.rockbox.org/tracker/task/5879
If you get this to a point where I punish it to the max with no problems (nothing left but the bugs that were preexisting) I'll commit it since lack of seeking is the main reason I don't use flac much (sheer size is second). How bout MPC seeking now? :)
I'll try the updated patch once I get some good progress on adding mutiple sample rate recording for SWCODEC.
Edit: Actually, this appears to be what snowgoon's patch (mentioned above) fixes, so perhaps this bug will go away once his patch is implemented. I'll do some testing with that as well.
Seeking in mpc is different to other codecs in that it seeks forward from the start of the song, rather than guessing and then trying to resync. For the seeking backward bug to occur a buffer overwrite must occur, in musepack this can happen when seeking forward, in other codecs you must allow the track to play until the buffer refills, or if you seek to near the end of the file then the next track is loaded which also causes the bug. Regarding seeking performance there are a couple of possible things to try: increase the file chunk read size during seeking, ie. CODEC_SET_FILEBUF_CHUNKSIZE, or increase the rebuffer preseek (I've made it configurable in my patch using CODEC_SET_FILEBUF_PRESEEK) - this could improve performance if the flac codec tries to resync backwards more than 32k during the seek.
To reproduce the "Codec failure":
1) Seek close enough to the end of the track to make the playback engine load the next track.
2) If the seeking algorithm happened to overshoot the seek point and then seeked backwards to compensate, simply let the track play to the end. Otherwise, seek backwards then let the track play out in order to definitely cause the failure.
I could try to come up with a work-around for the case when the algorithm overshoots the seekpoint, but I'm not sure if I want to do anything about it since any work-arounds I can come up with will make seeking slower, and it still won't be the "proper" full solution.
I got a few exams in the next few days, but after that I may have a look at that... but I am not a rockbox developer... never even seen the source code before. I imagine it is just a case of updating the FLAC used in rockbox to the latest CVS and testing. I will look into that in a few days.
Any developers able to do that quicker and better than me? (not a hard thing to accomplish :) )
woodsb02: Rockbox doesn't use libFLAC, it uses an ffmpeg implementation, so it's not a simple matter of applying the libFLAC patch to Rockbox. The work would involve applying the concepts of the libFLAC patch to this patch (plus probably removing the optimization I made).
Well, I will patiently await your next move! :)
Good luck with any exams you might have... i know i have them.
1) A comment refers to libFLAC 1.1.2 - should this be updated to 1.1.3?
2) In the flac_seek() function, you declare about 10 const variables and initialise them to various values from the fc struct. Couldn't you just use the fc-> values directly?
I'll test the patch itself on my journey to work this morning and report back.
1) That comment is on the frame sync method which I didn't touch with this latest update, so it is in fact from 1.1.2. It's probably still the same in 1.1.3, so I'll check that and update the comment.
2) All the const variables declarations are copied from the libFLAC code. I kept it this way so that the variable naming scheme is kept the same between this code and libFLAC.
It works perfect as far as I can tell! I have tried to put it through every stress I could... rewinding passed the beginning of a song, fastforwarding past the end, skipping songs quickly, resuming from turned off...
It skips to the right spot every time - you beauty mate!
Adam Boot (rotator): You have done a stand up job! *Applause*
Now... how soon can this be applied to cvs? I suppose you need more than just 1 tester (me) eh?
I've now tested the patch (on my 5g ipod), and all seems well. As Benjamin said - nice job. IIRC, you have a H300, so the patch has now had both big-endian and little-endian testing on target.
Regarding those const variables, I can understand you wanting to use the same variable names as libFLAC, but I still think it looks ugly. Most of the names are exactly the same anyway, so all you're saving is the "fc->" prefix.
But apart from that, I think you should commit it. The easiest way to find out if it has problems with certain FLAC files is to throw it out to the world.
Dave.