[FFmpeg-devel] [PATCH] make FLAC parser return frames when it has the required amount (without buffering)

[Justin Ruggles]

On 12/11/2010 09:32 PM, Michael Chinen wrote:

> On Sun, Dec 12, 2010 at 2:00 AM, Justin Ruggles
> <justin.ruggles at gmail.com> wrote:
>> On 12/11/2010 05:30 PM, Michael Chinen wrote:
>>
>>> Hi,
>>>
>>> On Sat, Dec 11, 2010 at 8:58 PM, Justin Ruggles
>>> <justin.ruggles at gmail.com> wrote:
>>>> On 12/11/2010 02:34 PM, Michael Chinen wrote:
>>>>
>>>>> On Sat, Dec 11, 2010 at 4:56 PM, Justin Ruggles
>>>>> <justin.ruggles at gmail.com> wrote:
>>>>>> On 12/10/2010 04:30 PM, Michael Chinen wrote:
>>>>>>
>>>>>>> The FLAC parser wasn't behaving well for large inputs.
>>>>>>> For the problem, see discussion in thread "FLAC parser ineffeciency"
>>>>>>>
>>>>>>> I tested this patch with 16K, 32K, and 10MB input and it seems to work.
>>>>>>> I should note that the parser buffers by copying the entire input so
>>>>>>> for the 10MB case this is using a lot of space.  Not an issue for
>>>>>>> small buffer sizes, but if it is a real use case and should be
>>>>>>> improved let me know and I can submit a separate patch that only takes
>>>>>>> small chunks of the input buffer.
>>>>>>
>>>>>>
>>>>>> If it's not too complicated, it might be better to have the parser guess
>>>>>> how much data it needs to have enough frames to do the analysis and only
>>>>>> read that much.
>>>>>
>>>>> Ok, this should do it.
>>>>
>>>>
>>>> The first patch will tell you how many frames are already in the buffer,
>>>> so instead of adding the full estimate for FLAC_MIN_HEADERS frames, it
>>>> could estimate how much data is needed based on how many frames it
>>>> already has.
>>>
>>> Ok, here it is.
>>> It uses (number of frames needed + 1) because I think an upper-bound is better.
>>
>>
>> Patches applied.
>>
>> I wanted to go ahead and fix the current issue.  But now I think the
>> frame size estimate could be improved.  Rather than using a fixed value
>> of 8192, why not use statistics from the current file?  One simple way
>> might be to use the average size of what is in the buffer, plus some
>> padding.  So maybe if you have 8 headers in the buffer, guess that you
>> have 7 full frames, divide the current fifo size by 7, then multiply by
>> 3/2 for some padding.  That would probably be closer and would avoid the
>> situation where you have, for example, 5.1 24-bit 96kHz FLAC file and
>> the parser always underestimates the amount of data it needs.
> 
> Is the data in the header (max frame size) trustworthy if valid?
> We could use something based on that.
> I'm just thinking of how most my flac files start and end with tiny
> (1-5% of the average) blockfiles.
> I'm guessing even the largest max frame sizes aren't that big that
> it's problematic if we are overshooting all the other frames.


The maximum frame size is in the STREAMINFO, and that field is optional
(0 means the value is not known).  Also, I don't think that extradata is
meant to be available to (and/or used by) the parser.

Even if the first few frames are really small, they're still predictive
of frames that come immediately after them.  So the most recent <10
frames should still be a pretty good predictor of the next frame size.

-Justin