Fix two bugs with subframe decoding #7
Conversation
- the linear predictive coding has an overflow error -- with FIR-encoded frames, this often causes the output signal to be noise rather than what it should be. Type-converting to int64 to do the summation, then back to int32 after shifting fixed this for the files that I tested against. - the check if the Rice parameter was the escape code did not handle the 5-bit escape code -- it checked for 0x1f and a 4-bit-long code instead of a 5-bit-long one I don't have any FLAC files that test the latter issue; I have a number of FLAC files that caused the former, all of which have
| @@ -383,7 +383,7 @@ func (subframe *Subframe) decodeRicePart(paramSize uint) error { | |||
| if err != nil { | |||
| return unexpected(err) | |||
| } | |||
| if paramSize == 4 && x == 0xF || paramSize == 4 && x == 0x1F { | |||
| if paramSize == 4 && x == 0xF || paramSize == 5 && x == 0x1F { | |||
There was a problem hiding this comment.
Thanks for spotting this! Do you happen to have a FLAC file which uses Rice parameter escape codes? I would like to implement this functionality but have deferred it since I don't have any FLAC file to verify the implementation against.
There was a problem hiding this comment.
Haha, sorry. I just read your last paragraph where you mentioned that you don't have any file to test this. Making a mental note to read the entire pull request before sending a reply :)
|
Thanks a lot for the pull request! It looks clean and I will merge it as soon as I can verify the overflow issue. If you could share with me one of the FLAC files that triggers the overflow I'd be very happy. Cheers! /u |
|
Thank you very much for your library! Here you go; I included a file that FYI, I discovered while testing that the flac encoder has some unpublished You can also use -l0 to force only fixed predictors. That helped me out (The attachment was too big. Let me know if you have a different preferred Jonathan On Sun, Jan 4, 2015 at 2:23 PM, Jonathan MacMillan <
|
|
Thanks for the upload and the README :) It's the first day of school tomorrow after the holidays, with enough assignments to keep me busy throughout the week ^^ But I'll make sure to play around with the various FLAC files towards the weekend. Sorry for the delay and thanks again for the patch! P.S. If you want to remove the hosted files now (for copyright or other reasons) that is fine. I've downloaded my copy. Cheers /u |
|
Hi Jonathan, A small weekend update. I've been traveling over the weekend and had less time coding than I thought I would. I've started to implement test cases and will continue as time permits. Just to let you know that it may take another week or two. Either way, I hope you are enjoying the first few weeks of the year! Cheers /u |
Fix two bugs with subframe decoding
|
Hi Jonathan! I finally got time to implement the test cases for frame decoding (in rev 4368146). You were right indeed! 1/3 of the original test cases failed the MD5 checksum. After merging your pull request all test cases pass! Thanks a lot :) Cheerful regards |
Fix two bugs with subframe decoding in the FLAC library:
I don't have any FLAC files that test the latter issue; I have a number of FLAC files that caused the former, all of which were very noisy before and are sonically correct now.