[FFmpeg-devel] [PATCH] FLAC parser

Michael Chinen mchinen
Wed Jul 21 03:34:11 CEST 2010


On Wed, Jul 21, 2010 at 3:11 AM, Justin Ruggles
<justin.ruggles at gmail.com> wrote:
> Justin Ruggles wrote:
>
>> Hi,
>>
>> Michael Chinen wrote:
>>
>>> Hi,
>>>
>>> This FLAC parser takes the suggestions from a thread from another FLAC
>>> parser patch submitted by Jason Ruggles in March 2009[1].
>>> Currently it stores 20 headers (8 bit crc verified) and finds all
>>> possible (16 bit footer) crc-verified sequences within a neighbor
>>> distance of 4.
>>> It penalizes sequences that have changes in sample rate, bit depth,
>>> and channel arrangement.
>>> The settings probably need some twiddling.
>>>
>>> Seeking seems to work with it (with my av_build_index version in soc
>>> svn as well).
>>>
>>> I used the modifications to flacdec.c (to remove the bytestream and
>>> make a function extern) from Jason's patch, but flac_parser.c is new.
>>>
>>> The second patch is just to make ffplay keep calling av_read_frame
>>> after the ByteIOContext has reach EOF.
>>>
>>> Michael
>>>
>>> [1] http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2009-March/066533.html
>>
>>> From 6134833f75c83beb306936b62e789c62a85ac2a2 Mon Sep 17 00:00:00 2001
>>> From: Michael Chinen <mchinen at gmail.com>
>>> Date: Wed, 14 Jul 2010 02:58:53 +0200
>>> Subject: [PATCH 1/4] Add FLAC parser
>>> ?Based on mailing list discussion from march 09.
>>> ?flac_parser.c verifies all possible chains within a certain header radius and scores them based on changes in sample rate, bit depth, channels.
>>> ?flacdec.c uses Jason Ruggles' changes from a patch in the same discussion.
>>
>> My name is Justin Ruggles. :)
>>
>>> ---
>>> ?libavcodec/Makefile ? ? ?| ? ?1 +
>>> ?libavcodec/allcodecs.c ? | ? ?1 +
>>> ?libavcodec/flac.h ? ? ? ?| ? 12 ++
>>> ?libavcodec/flac_parser.c | ?362 ++++++++++++++++++++++++++++++++++++++++++++++
>>> ?libavcodec/flacdec.c ? ? | ?110 +++-----------
>>> ?5 files changed, 398 insertions(+), 88 deletions(-)
>>> ?create mode 100644 libavcodec/flac_parser.c
>>
>>
>> Some general comments:
>>
>> 1) I haven't gone too far into why this happens, but it doesn't seem to
>> work reliably. ?Using 20 headers there should be no reason why any
>> frames are not detected properly, but in 2 random single-song samples I
>> tested I got instances of multiple frames being joined together (i.e.
>> skipping a valid header). ?The decoder can handle this, but it should
>> not be occuring.
>
> I think the issue is that search_start in find_new_headers() is
> calculated incorrectly.
>
> I changed:
> fpc->buffer_size - (buf_size + (MAX_FRAME_HEADER_SIZE-1) )
> to
> fpc->buffer_size - (buf_size + (MAX_FRAME_HEADER_SIZE+1) )
>
> After that I got no skipped headers and everything worked as expected
> for all samples I tested.
Thanks for looking at it.  I got a slightly different solution.
The problem is an off by one error in find_new_headers.  We don't need
to change the start position, although that does fix the problem as
well.
-    for (i = search_start; i < fpc->buffer_size-MAX_FRAME_HEADER_SIZE; i++) {
+    for (i = search_start; i < fpc->buffer_size -
(MAX_FRAME_HEADER_SIZE - 1); i++) {

Actually I got this by remembering that in your flac parser you had
the "i <= size - 16" (as opposed to "i < size - 16")  in a for loop.
This also makes me realize there that I should also check that the
header found when reaching back into a previous buffer needs to have
its end in the new buffer.  I guess the easiest way is just to make
sure we don't append any headers that start before the last inserted
one.

Probably tomorrow I'll take care of removing the option to disallow
overlaps and to return junk packets.   For junk packets, I'm guessing
"avctx->frame_size = 0" is enough to let av_read_frame_internal know
how to deal with it (e.g. to not add it to the index_entries) If
there's more please let me know.

And sorry about the name.  I'll fix it when I git.  In the meantime
feel free to call me names too :)

Michael



More information about the ffmpeg-devel mailing list