FS#9186 - flac decoder doesn't handle truncated files nicely

Attached to Project: Rockbox
Opened by PaulJam (PaulJam) - Friday, 11 July 2008, 15:54 GMT
Last edited by Jonas Häggqvist (rasher) - Thursday, 11 December 2008, 18:06 GMT
Task Type Bugs
Category Codecs
Status New
Assigned To No-one
Operating System SW-codec
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


When you play back a broken flac file (in this case a file that was truncated) it can happen that he decoder outputs constant loud noise instead of just skipping to the next track.

I'm not sure if this is really a bug since you are not supposed to play back broken files.

Attached is a file where this happens.

H300 r18009 (happens in the uisim too).
This task depends upon

Comment by Jonas Häggqvist (rasher) - Thursday, 11 December 2008, 18:06 GMT
  • Field changed: Status (Unconfirmed → New)
Confirmed in r19376 - this doesn't happen in mplayer, using ffmpeg, so this is either fixed since Rockbox synced libffmpegFLAC or a strict Rockbox problem.

I think this counts as a bug - Rockbox should be more tolerant of faulty files.
Comment by Saurabh Kumar (Fuhrer) - Friday, 25 March 2011, 23:19 GMT
This patch when applied to "../rockbox/apps/codecs/libffmpegFLAC/decoder.c" file successfully solves the above problem. The creation of this patch was based from the root directory of Rockbox.

I have tested this code on simulators of Apple Ipod 4G Grayscale and Philips GoGear HDD6330 where it worked perfectly.

Earlier , when " Melodisch4.flac " (which is a corrupted file, included above) was played on these players , as soon as the progress of the play reached the broken part of the file , the player started producing constant loud noises and was struck at that point. But now , after applying this patch , as soon as the progress reaches the broken part , execution of that file stops and the next song in the playlist starts playing automatically.
   patch (8.7 KiB)
Comment by MichaelGiacomelli (saratoga) - Saturday, 26 March 2011, 16:30 GMT
Two comments:

1) Theres a lot of unrelated changes in that patch. Things like moving white space, brackets, etc. You should avoid those since they confuse the SVN history and make it difficult to review the patch.

2) calc_power(2,(int)s->bps) is the same as 2<<(s->bps) because a shift is the same as raising to the power of two. I think the calc_power function can be avoided entirely.
Comment by Thom Johansen (preglow) - Saturday, 26 March 2011, 21:18 GMT
This method basically seems to assume the reconstruction filter is unstable (probably thanks to errors in the file), and just checks to see if the decoded samples are out of bounds. Another way to check for an unstable filter would be to compute the reflection coefficients from the LPC filter coefs and see if they are in the -1 to +1 range. If some of them aren't , the filter is unstable and the entire block of samples can be scrapped (even before actually decoding them). This would probably be faster than the per sample check this patch implements on some platforms, and possibly slower on some others (primarily ARM based ones).
Oh, and yeah, this only goes for the LPC decoding, I haven't looked too closely at the fixed decoder parts.
Comment by Saurabh Kumar (Fuhrer) - Sunday, 27 March 2011, 05:02 GMT
Hello saratoga sir,

I am resubmitting the patch appending modifications as per your comments.
Thanks !

Please , review it again....:)
   patch (3.2 KiB)
Comment by Dave Hooper (stripwax) - Tuesday, 29 March 2011, 20:45 GMT
I would be interested in knowing, given that mplayer doesn't have this problem, whether indeed this has been explicitly fixed in upstream libffmpegFLAC and rockbox should pull in those fixes accordingly.