[FFmpeg-devel] [PATCH] FLAC parser

[Michael Chinen]

On Thu, Aug 5, 2010 at 12:25 PM, Justin Ruggles
<justin.ruggles at gmail.com> wrote:
> Hi,
>
> Michael Chinen wrote:
>
>> On Thu, Aug 5, 2010 at 2:01 AM, Justin Ruggles <justin.ruggles at gmail.com> wrote:
>>>> + ? ? ? ? ? ?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 tested on a sample I have that is about 30 min long. ?It's basically
> just 5 songs of different genres concatenated together. ?It gave 3 or 4
> log messages about multiple frames in a packet.
>
>> 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.
>
> 10 and 3 worked fine for all the FLAC files in my collection.
Okay, Thanks.  I've changed this, added all the suggestions and fixed a few more
overly long lines.

Also I tested what compute_pkt_fields av_add_index_entry on valid and
the parsers junk packets (which set avctx->frame_size = 0) and it seems
the problem is outside of the parser.
I'll look more into this problem and submit a separate patch as it appears
to be a general seek issue.  This is why when ever you seek with this parser
there is one junk packet, but this shouldn't happen with index seeking if the
proper pos was stored.  The first packet's pos for example is computed to be
0 which is definitely wrong given that it should be offset by the metadata info.
The same issue appears to exist for MP3, which is why I'm more confident
that it is a general seeking issue.

I think this patch gets it a lot closer.  Please let me know what
needs more cleanup.
Thanks,
Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-FLAC-parser.patch
Type: application/octet-stream
Size: 37779 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100806/9b310fb9/attachment.obj>