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

[Justin Ruggles]

On 12/11/2010 10:01 PM, Michael Niedermayer wrote:

> On Sat, Dec 11, 2010 at 09:47:04PM -0500, Justin Ruggles wrote:
>> 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.
> 
> predictive, of course but if there is true silence and then something else then
> they will differ by a very large factor, i dont know if that is an issue
> but if it is then this predictiveness is no good


Well, it would catch up after several frames...  Maybe an LPC function
to predict frame size would do better... it is FLAC after all.  ;)

Anyway, anything that's even remotely predictive is better than a fixed
value.  8192 bytes is very certainly a big overestimate for silent frames.

-Justin