[FFmpeg-devel] [PATCH] FLAC parser

[Michael Chinen]

On Thu, Aug 5, 2010 at 2:01 AM, Justin Ruggles <justin.ruggles at gmail.com> wrote:
>>[...]
>> + ? ? ? ? ? ?if (header->fi.is_var_size) {
>> + ? ? ? ? ? ? ? ?/* check sample numbers */
>> + ? ? ? ? ? ? ? ?if (child->fi.frame_or_sample_num - header->fi.frame_or_sample_num
>> + ? ? ? ? ? ? ? ? ? ?!= child->fi.blocksize) {
>> + ? ? ? ? ? ? ? ? ? ?child_score -= FLAC_HEADER_CHANGED_PENALTY;
>> + ? ? ? ? ? ? ? ?}
>> + ? ? ? ? ? ?} else {
>> + ? ? ? ? ? ? ? ?/* check frame numbers ?*/
>> + ? ? ? ? ? ? ? ?if (child->fi.frame_or_sample_num != header->fi.frame_or_sample_num + 1)
>> + ? ? ? ? ? ? ? ? ? ?child_score -= FLAC_HEADER_CHANGED_PENALTY;
>> + ? ? ? ? ? ?}
>
> Sorry I didn't catch this before, but this check needs to be made a
> little less strict. ?The "blocking strategy bit" in the header only
> became part of the FLAC specification in 2007 in version 1.2.0. ?Before
> that the incremental number was still either frame or sample number, but
> there were other ways of indicating variable frame size. ?My suggestion
> is to check for either +1 or +blocksize, independent of is_var_size.
Okay, thanks, this is good to know.

>> + ? ? ? ? ? ?if (child_score + header->max_score > header->max_score) {
>> + ? ? ? ? ? ? ? ?/* keep the child because the frame scoring is dynamic */
>> + ? ? ? ? ? ? ? ?header->best_child = child;
>> + ? ? ? ? ? ? ? ?header->max_score ?= FLAC_HEADER_BASE_SCORE + child_score;
>> + ? ? ? ? ? ?}
>
>
> This is the part that you changed and is now incorrect. ?Previously, it was:
> if (child_score + FLAC_HEADER_BASE_SCORE > header->max_score)
I have the patch mostly ready but this time I'll look over it a bit
more and test to save embarrassing bugs like this one.  Also I didn't
get any log messages for that as I would expect so I want to make sure
everything is right.  Just out of curiousity how are you testing it to
find this out?

I'll also change to 10 min headers and 3 consecutive after verifying
on the minimal set of FLAC I have with me (not at my home setup.)
Probably will submit something around 12 hours from now, after sanity
testing is done.

Michael