Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Deprecated: Function create_function() is deprecated in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 104 Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /sites/rockbox.org/flyspray/adodb/adodb.inc.php on line 845 Deprecated: Function create_function() is deprecated in /sites/rockbox.org/flyspray/includes/class.user.php on line 111 FS#5892 : Sample-accurate FLAC seeking (with or without a seektable)

Rockbox

Tasklist

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
Task Type Patches
Category Codecs
Status Closed
Assigned To Adam Boot (rotator)
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

The 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
Comment by Michael Sevakis (MikeS) - Sunday, 27 August 2006, 19:11 GMT
Long seeks (on a 6:29 file) can hang the player or are slow to get started again. This seems to happen if the disk needs to be spun up to buffer more data. The data seems to get buffered but playback never resumes on its own. This prevents playback of suqsequent files when skipping. It seems further action will eventually hang playback but not the backlight thread or probably others.

Hope that help with finding the problem.
Comment by Michael Sevakis (MikeS) - Sunday, 27 August 2006, 19:15 GMT
Oh and sometimes with short seeks the progress bar jumps back a little bit (1 pixel) on some 30s files.
Comment by Adam Boot (rotator) - Sunday, 27 August 2006, 21:39 GMT
I believe I have addressed both issues with this latest patch.

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.
(application/octet-stream)    flac_seeking_v2.patch (17.3 KiB)
Comment by Michael Sevakis (MikeS) - Monday, 28 August 2006, 01:30 GMT
Vastly improved! Still got a hang after seeking long forward then a minute or so back but was able to hit stop and recover playback. No more complete deadlocks.

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.
Comment by Adam Boot (rotator) - Monday, 28 August 2006, 03:19 GMT
Ah hah! That sounds like the same bug I ran into. I thought at first it was my code, but it ended up being something else. Seeking close to the end of a file confuses the playback engine because it has already moved on to decoding the next track (for gapless purposes). Here's the bug report, see lostlogic's comments: http://www.rockbox.org/tracker/task/2687 Basically, any seeking near the end of the track can cause problems.
Comment by Adam Boot (rotator) - Tuesday, 29 August 2006, 01:37 GMT
After some digging into a problem I encountered, I found a major bug in my code. Some of the values I was using turned out not to be what I thought they were. I'm currectly re-writing the entire seeking algorithm, expect a new patch soon.
Comment by Adam Boot (rotator) - Tuesday, 29 August 2006, 04:21 GMT
New patch, it wasn't as much work as I thought it would be. Seeking should now not only be faster but also more reliable. Also, I fixed a bug I overlooked when resuming a file. Resuming still often takes longer than I think it should, but that seems to be an issue with buffering that's out of my control.
(application/octet-stream)    flac_seeking_v3.patch (18.1 KiB)
Comment by Andrew Cupper (snowgoon) - Tuesday, 29 August 2006, 20:13 GMT

Any seeking problems could also be related to a general seeking bug with rb:

http://www.rockbox.org/tracker/task/5879
Comment by Michael Sevakis (MikeS) - Wednesday, 30 August 2006, 05:17 GMT
I don't know...the seeking bug never came up that I recall with other more mature format support. It's too repeatable too easily here and my other nickname being "Flypaper", I probably would have encounter it without any effort. But who knows...I'll do some abusive testing on things now. I'm guessing the problem may be that I'm not using huge lossless files that require rebuffering during rewinds.

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.
Comment by Sander Sweers (infirit) - Wednesday, 30 August 2006, 08:13 GMT
Musepack seeking is in patch: http://www.rockbox.org/tracker/task/5172. It is assigned to preglow but he said to have little time.
Comment by Adam Boot (rotator) - Wednesday, 30 August 2006, 15:09 GMT
I haven't been able to reproduce any bugs except for playback bugs already reported on other tracker entries. However, I have an idea about the problem with the codec failure on the next track after seeking near the end of the current track. When you seek near the end, my code may overshoot the actual seek point, and it will then request the buffer at that point in order to resync the stream. At this point, I believe the playback engine may be triggered to load the codec and metadata for the next track. When my code then jumps backward to find the actual seek point, it triggers the "seeking backwards" bug by requesting the buffer earlier in the stream after the next track has been loaded onto the buffer, overwriting whatever has been loaded for the next track (including the codec). This is just speculation, but if this is indeed the problem, I may be able to come up with a work-around. I'll have some time later this week to look into it in more depth and do some more of that "abusive testing". ;)

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.
Comment by Andrew Cupper (snowgoon) - Wednesday, 30 August 2006, 15:34 GMT

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.
Comment by Adam Boot (rotator) - Wednesday, 30 August 2006, 22:34 GMT
Now that snowgoon's seeking backwards patch is in cvs, I did some quick testing. Seeking backwards from near the end of the track works fine now, no hangs or general playback weirdness like the problems described in MikeS's comment appear to happen anymore. http://www.rockbox.org/tracker/task/5892#comment9759 Still left is the problem of the "Codec failure" on track change after seeking near the end of the track.

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.
Comment by Sander Sweers (infirit) - Monday, 04 September 2006, 19:29 GMT
I found something strange with this patch applied. Usually a quick left would start playback at the beginning of the file. But now it just skips to a specific point in the file but not to the beginning. It works fine in vorbis files. Let me know if you need more info.
Comment by Dave Chapman (linuxstb) - Monday, 06 November 2006, 16:59 GMT
I haven't tried this patch, but you may be interested to know a patch to improve seeking in libFLAC has just been accepted and committed to the FLAC cvs. It could be worth looking at that for ideas.
Comment by Benjamin Woods (woodsb02) - Monday, 06 November 2006, 18:42 GMT
Now that sounds like it could be a solution waiting to happen ;)

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 :) )
Comment by Adam Boot (rotator) - Monday, 06 November 2006, 19:24 GMT
I've been busy this semester and haven't had much time for Rockbox, but I'd like to get back to this patch and finally get it committed once I have the time. I'll take a look at the libFLAC patch and see if it would make sense to integrate it into my patch. The seeking in libFLAC 1.1.2 was fairly unintelligent and performed very poorly on Rockbox targets, and this patch, though based on libFLAC 1.1.2, tried to improve that.

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).
Comment by Benjamin Woods (woodsb02) - Monday, 06 November 2006, 20:31 GMT
Ahh... i see.

Well, I will patiently await your next move! :)
Good luck with any exams you might have... i know i have them.
Comment by Adam Boot (rotator) - Tuesday, 07 November 2006, 22:29 GMT
New patch, updated to use the implementation from libFLAC 1.1.3b2 plus recent patches. This is a pretty big overhaul so there could be bugs, please test. Performance seems to be very good so far.
(application/octet-stream)    flac_seeking_v4.patch (14 KiB)
Comment by Dave Chapman (linuxstb) - Wednesday, 08 November 2006, 08:34 GMT
I haven't tried the patch yet, but I have a couple of minor comments about the code:

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.
Comment by Adam Boot (rotator) - Wednesday, 08 November 2006, 15:44 GMT
linuxstb,

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.
Comment by Benjamin Woods (woodsb02) - Wednesday, 08 November 2006, 15:55 GMT
I have now tested this code extensively (using the latest cvs source code and this patch).

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?
Comment by Dave Chapman (linuxstb) - Wednesday, 08 November 2006, 16:50 GMT
Adam,

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.

Comment by Adam Boot (rotator) - Wednesday, 08 November 2006, 21:58 GMT
Fair enough, I'll remove the const variables, commit it, and then wait for the bug reports to roll in. ;)

Loading...