[FFmpeg-devel] [PATCH] FLAC parser
Wed Aug 4 04:07:21 CEST 2010
Michael Chinen wrote:
> I've included everything from yours and Diego's review, but I'll
> respond to the questions below. I feel more confident about it now,
> thanks again to both of you.
> On Sun, Aug 1, 2010 at 10:29 PM, Justin Ruggles
> <justin.ruggles at gmail.com> wrote:
>>> +/* the maximum number of adjacent headers that will compare crcs against each other */
>>> +#define FLAC_MAX_SEQUENTIAL_HEADERS 4
>>> +/* the minimum number of headers that need to be buffered and checked before returning frames */
>>> +#define FLAC_MIN_HEADERS 20
>> Just out of curiosity, have you done testing with fewer headers, both
>> buffered and sequential?
> I tried before with 3 and 10 respectively, and things still seemed to
> work okay, but arbitrarily went with 4 and 20. Maybe it's overkill?
If 3 and 10 works well, then I think it should be used. I'll test the
FLAC files in my lossless music collection (about 450 songs).
Your newest patch is broken. It took me a while to figure out what you
had changed, so I don't have time this evening to write up a new review.
But you need to take a look at flac_parser.c line 238.
I'll send a new review tomorrow evening. Maybe Diego will do another
style/doc review before then? That's where most of the remaining issues
More information about the ffmpeg-devel